Merge "Improve resource merging error handling"

4 files changed, 153 insertions, 102 deletions

diff --git a/gradle/src/main/groovy/com/android/build/gradle/internal/LoggerWrapper.groovy b/gradle/src/main/groovy/com/android/build/gradle/internal/LoggerWrapper.groovy
index c57f1e8..575c07e 100644
--- a/gradle/src/main/groovy/com/android/build/gradle/internal/LoggerWrapper.groovy
+++ b/gradle/src/main/groovy/com/android/build/gradle/internal/LoggerWrapper.groovy
@@ -15,6 +15,7 @@
  */
 package com.android.build.gradle.internal
 
+import com.android.ide.common.res2.MergingException
 import com.android.utils.ILogger
 import org.gradle.api.logging.LogLevel
 import org.gradle.api.logging.Logger
@@ -32,6 +33,17 @@ class LoggerWrapper implements ILogger {
 
     @Override
     void error(Throwable throwable, String s, Object... objects) {
+        if (throwable instanceof MergingException) {
+            // MergingExceptions have a known cause: they aren't internal errors, they
+            // are errors in the user's code, so a full exception is not helpful (and
+            // these exceptions should include a pointer to the user's error right in
+            // the message).
+            //
+            // Furthermore, these exceptions are already caught by the MergeResources
+            // and MergeAsset tasks, so don't duplicate the output
+            return
+        }
+
         if (objects != null && objects.length > 0) {
             s = String.format(s, objects)
         }
diff --git a/gradle/src/main/groovy/com/android/build/gradle/tasks/MergeAssets.groovy b/gradle/src/main/groovy/com/android/build/gradle/tasks/MergeAssets.groovy
index b86fc8b..b40a87a 100644
--- a/gradle/src/main/groovy/com/android/build/gradle/tasks/MergeAssets.groovy
+++ b/gradle/src/main/groovy/com/android/build/gradle/tasks/MergeAssets.groovy
@@ -19,8 +19,8 @@ import com.android.ide.common.res2.AssetMerger
 import com.android.ide.common.res2.AssetSet
 import com.android.ide.common.res2.FileStatus
 import com.android.ide.common.res2.FileValidity
-import com.android.ide.common.res2.MergeConsumer
 import com.android.ide.common.res2.MergedAssetWriter
+import com.android.ide.common.res2.MergingException
 import org.gradle.api.tasks.InputFiles
 import org.gradle.api.tasks.OutputDirectory
 
@@ -65,78 +65,81 @@ public class MergeAssets extends IncrementalTask {
         // create a new merger and populate it with the sets.
         AssetMerger merger = new AssetMerger()
 
-        for (AssetSet assetSet : assetSets) {
-            // set needs to be loaded.
-            assetSet.loadFromFiles(plugin.logger)
-            merger.addDataSet(assetSet)
-        }
+        try {
+            for (AssetSet assetSet : assetSets) {
+                // set needs to be loaded.
+                assetSet.loadFromFiles(plugin.logger)
+                merger.addDataSet(assetSet)
+            }
 
-        // get the merged set and write it down.
-        MergedAssetWriter writer = new MergedAssetWriter(destinationDir)
+            // get the merged set and write it down.
+            MergedAssetWriter writer = new MergedAssetWriter(destinationDir)
 
-        try {
             merger.mergeData(writer, false /*doCleanUp*/)
-        } catch (MergeConsumer.ConsumerException e) {
+
+            // No exception? Write the known state.
+            merger.writeBlobTo(getIncrementalFolder(), writer)
+        } catch (MergingException e) {
+            println e.getMessage()
             merger.cleanBlob(getIncrementalFolder())
-            throw e.getCause()
+            throw new ResourceException(e.getMessage(), e)
         }
-
-        // No exception? Write the known state.
-        merger.writeBlobTo(getIncrementalFolder(), writer)
     }
 
     @Override
     protected void doIncrementalTaskAction(Map<File, FileStatus> changedInputs) {
         // create a merger and load the known state.
         AssetMerger merger = new AssetMerger()
-        if (!merger.loadFromBlob(getIncrementalFolder(), true /*incrementalState*/)) {
-            doFullTaskAction()
-            return
-        }
-
-        // compare the known state to the current sets to detect incompatibility.
-        // This is in case there's a change that's too hard to do incrementally. In this case
-        // we'll simply revert to full build.
-        List<AssetSet> assetSets = getInputAssetSets()
-
-        if (!merger.checkValidUpdate(assetSets)) {
-            project.logger.info("Changed Asset sets: full task run!")
-            doFullTaskAction()
-            return
-        }
+        try {
+            if (!merger.loadFromBlob(getIncrementalFolder(), true /*incrementalState*/)) {
+                doFullTaskAction()
+                return
+            }
 
-        // The incremental process is the following:
-        // Loop on all the changed files, find which ResourceSet it belongs to, then ask
-        // the resource set to update itself with the new file.
-        for (Map.Entry<File, FileStatus> entry : changedInputs.entrySet()) {
-            File changedFile = entry.getKey()
+            // compare the known state to the current sets to detect incompatibility.
+            // This is in case there's a change that's too hard to do incrementally. In this case
+            // we'll simply revert to full build.
+            List<AssetSet> assetSets = getInputAssetSets()
 
-            merger.findDataSetContaining(changedFile, fileValidity)
-            if (fileValidity.status == FileValidity.FileStatus.UNKNOWN_FILE) {
+            if (!merger.checkValidUpdate(assetSets)) {
+                project.logger.info("Changed Asset sets: full task run!")
                 doFullTaskAction()
                 return
-            } else if (fileValidity.status == FileValidity.FileStatus.VALID_FILE) {
-                if (!fileValidity.dataSet.updateWith(
-                        fileValidity.sourceFile, changedFile, entry.getValue(), plugin.logger)) {
-                    project.logger.info(
-                            String.format("Failed to process %s event! Full task run",
-                                    entry.getValue()))
+            }
+
+            // The incremental process is the following:
+            // Loop on all the changed files, find which ResourceSet it belongs to, then ask
+            // the resource set to update itself with the new file.
+            for (Map.Entry<File, FileStatus> entry : changedInputs.entrySet()) {
+                File changedFile = entry.getKey()
+
+                merger.findDataSetContaining(changedFile, fileValidity)
+                if (fileValidity.status == FileValidity.FileStatus.UNKNOWN_FILE) {
                     doFullTaskAction()
                     return
+                } else if (fileValidity.status == FileValidity.FileStatus.VALID_FILE) {
+                    if (!fileValidity.dataSet.updateWith(
+                            fileValidity.sourceFile, changedFile, entry.getValue(),
+                            plugin.logger)) {
+                        project.logger.info(
+                                String.format("Failed to process %s event! Full task run",
+                                        entry.getValue()))
+                        doFullTaskAction()
+                        return
+                    }
                 }
             }
-        }
 
-        MergedAssetWriter writer = new MergedAssetWriter(getOutputDir())
+            MergedAssetWriter writer = new MergedAssetWriter(getOutputDir())
 
-        try {
             merger.mergeData(writer, false /*doCleanUp*/)
-        } catch (MergeConsumer.ConsumerException e) {
+
+            // No exception? Write the known state.
+            merger.writeBlobTo(getIncrementalFolder(), writer)
+        } catch (MergingException e) {
+            println e.getMessage()
             merger.cleanBlob(getIncrementalFolder())
-            throw e.getCause()
+            throw new ResourceException(e.getMessage(), e)
         }
-
-        // No exception? Write the known state.
-        merger.writeBlobTo(getIncrementalFolder(), writer)
     }
 }
diff --git a/gradle/src/main/groovy/com/android/build/gradle/tasks/MergeResources.groovy b/gradle/src/main/groovy/com/android/build/gradle/tasks/MergeResources.groovy
index df1986c..b7d4c56 100644
--- a/gradle/src/main/groovy/com/android/build/gradle/tasks/MergeResources.groovy
+++ b/gradle/src/main/groovy/com/android/build/gradle/tasks/MergeResources.groovy
@@ -17,8 +17,8 @@ package com.android.build.gradle.tasks
 import com.android.build.gradle.internal.tasks.IncrementalTask
 import com.android.ide.common.res2.FileStatus
 import com.android.ide.common.res2.FileValidity
-import com.android.ide.common.res2.MergeConsumer
 import com.android.ide.common.res2.MergedResourceWriter
+import com.android.ide.common.res2.MergingException
 import com.android.ide.common.res2.ResourceMerger
 import com.android.ide.common.res2.ResourceSet
 import org.gradle.api.tasks.Input
@@ -69,79 +69,81 @@ public class MergeResources extends IncrementalTask {
         // create a new merger and populate it with the sets.
         ResourceMerger merger = new ResourceMerger()
 
-        for (ResourceSet resourceSet : resourceSets) {
-            // set needs to be loaded.
-            resourceSet.loadFromFiles(plugin.logger)
-            merger.addDataSet(resourceSet)
-        }
+        try {
+            for (ResourceSet resourceSet : resourceSets) {
+                // set needs to be loaded.
+                resourceSet.loadFromFiles(plugin.logger)
+                merger.addDataSet(resourceSet)
+            }
 
-        // get the merged set and write it down.
-        MergedResourceWriter writer = new MergedResourceWriter(
-                destinationDir, getProcess9Patch() ? builder.aaptRunner : null)
+            // get the merged set and write it down.
+            MergedResourceWriter writer = new MergedResourceWriter(
+                    destinationDir, getProcess9Patch() ? builder.aaptRunner : null)
 
-        try {
             merger.mergeData(writer, false /*doCleanUp*/)
-        } catch (MergeConsumer.ConsumerException e) {
+
+            // No exception? Write the known state.
+            merger.writeBlobTo(getIncrementalFolder(), writer)
+        } catch (MergingException e) {
+            println e.getMessage()
             merger.cleanBlob(getIncrementalFolder())
-            throw e.getCause()
+            throw new ResourceException(e.getMessage(), e)
         }
-
-        // No exception? Write the known state.
-        merger.writeBlobTo(getIncrementalFolder(), writer)
     }
 
     @Override
     protected void doIncrementalTaskAction(Map<File, FileStatus> changedInputs) {
         // create a merger and load the known state.
         ResourceMerger merger = new ResourceMerger()
-        if (!merger.loadFromBlob(getIncrementalFolder(), true /*incrementalState*/)) {
-            doFullTaskAction()
-            return
-        }
-
-        // compare the known state to the current sets to detect incompatibility.
-        // This is in case there's a change that's too hard to do incrementally. In this case
-        // we'll simply revert to full build.
-        List<ResourceSet> resourceSets = getInputResourceSets()
-
-        if (!merger.checkValidUpdate(resourceSets)) {
-            project.logger.info("Changed Resource sets: full task run!")
-            doFullTaskAction()
-            return
-        }
+        try {
+            if (!merger.loadFromBlob(getIncrementalFolder(), true /*incrementalState*/)) {
+                doFullTaskAction()
+                return
+            }
 
-        // The incremental process is the following:
-        // Loop on all the changed files, find which ResourceSet it belongs to, then ask
-        // the resource set to update itself with the new file.
-        for (Map.Entry<File, FileStatus> entry : changedInputs.entrySet()) {
-            File changedFile = entry.getKey()
+            // compare the known state to the current sets to detect incompatibility.
+            // This is in case there's a change that's too hard to do incrementally. In this case
+            // we'll simply revert to full build.
+            List<ResourceSet> resourceSets = getInputResourceSets()
 
-            merger.findDataSetContaining(changedFile, fileValidity)
-            if (fileValidity.status == FileValidity.FileStatus.UNKNOWN_FILE) {
+            if (!merger.checkValidUpdate(resourceSets)) {
+                project.logger.info("Changed Resource sets: full task run!")
                 doFullTaskAction()
                 return
-            } else if (fileValidity.status == FileValidity.FileStatus.VALID_FILE) {
-                if (!fileValidity.dataSet.updateWith(
-                        fileValidity.sourceFile, changedFile, entry.getValue(), plugin.logger)) {
-                    project.logger.info(
-                            String.format("Failed to process %s event! Full task run",
-                                    entry.getValue()))
+            }
+
+            // The incremental process is the following:
+            // Loop on all the changed files, find which ResourceSet it belongs to, then ask
+            // the resource set to update itself with the new file.
+            for (Map.Entry<File, FileStatus> entry : changedInputs.entrySet()) {
+                File changedFile = entry.getKey()
+
+                merger.findDataSetContaining(changedFile, fileValidity)
+                if (fileValidity.status == FileValidity.FileStatus.UNKNOWN_FILE) {
                     doFullTaskAction()
                     return
+                } else if (fileValidity.status == FileValidity.FileStatus.VALID_FILE) {
+                    if (!fileValidity.dataSet.updateWith(
+                            fileValidity.sourceFile, changedFile, entry.getValue(),
+                            plugin.logger)) {
+                        project.logger.info(
+                                String.format("Failed to process %s event! Full task run",
+                                        entry.getValue()))
+                        doFullTaskAction()
+                        return
+                    }
                 }
             }
-        }
 
-        MergedResourceWriter writer = new MergedResourceWriter(
-                getOutputDir(), getProcess9Patch() ? builder.aaptRunner : null)
-        try {
+            MergedResourceWriter writer = new MergedResourceWriter(
+                    getOutputDir(), getProcess9Patch() ? builder.aaptRunner : null)
             merger.mergeData(writer, false /*doCleanUp*/)
-        } catch (MergeConsumer.ConsumerException e) {
+            // No exception? Write the known state.
+            merger.writeBlobTo(getIncrementalFolder(), writer)
+        } catch (MergingException e) {
+            println e.getMessage()
             merger.cleanBlob(getIncrementalFolder())
-            throw e.getCause()
+            throw new ResourceException(e.getMessage(), e)
         }
-
-        // No exception? Write the known state.
-        merger.writeBlobTo(getIncrementalFolder(), writer)
     }
 }
diff --git a/gradle/src/main/groovy/com/android/build/gradle/tasks/ResourceException.java b/gradle/src/main/groovy/com/android/build/gradle/tasks/ResourceException.java
new file mode 100644
index 0000000..7d1b142
--- /dev/null
+++ b/gradle/src/main/groovy/com/android/build/gradle/tasks/ResourceException.java
@@ -0,0 +1,34 @@
+/*
+ * Copyright (C) 2012 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.android.build.gradle.tasks;
+
+import com.android.ide.common.res2.MergingException;
+
+/**
+ * Exception used for resource merging errors, thrown when
+ * a {@link MergingException} is thrown by the resource merging code.
+ * We can't just rethrow the {@linkplain MergingException} because
+ * gradle 1.8 seems to want a RuntimeException; without it you get
+ * the error message
+ * {@code
+ *     > Could not call IncrementalTask.taskAction() on task ':MyPrj:mergeDebugResources'
+ * }
+ */
+public class ResourceException extends RuntimeException {
+    public ResourceException(String message, Throwable throwable) {
+        super(message, throwable);
+    }
+}