Handle ManifestProcessingException in ResourceProcessorBusyBox

bazelbuild · Jan 10, 2022 · ab51ff1 · ab51ff1
1 parent 85a5384
commit ab51ff1
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 65 deletions.
diff --git a/src/tools/android/java/com/google/devtools/build/android/ManifestMergerAction.java b/src/tools/android/java/com/google/devtools/build/android/ManifestMergerAction.java
@@ -68,89 +68,82 @@ public class ManifestMergerAction {
   /** Flag specifications for this action. */
   public static final class Options extends OptionsBase {
     @Option(
-      name = "manifest",
-      defaultValue = "null",
-      converter = ExistingPathConverter.class,
-      category = "input",
-      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
-      effectTags = {OptionEffectTag.UNKNOWN},
-      help =
-          "Path of primary manifest. If not passed, a dummy manifest will be generated and used as"
-              + " the primary."
-    )
+        name = "manifest",
+        defaultValue = "null",
+        converter = ExistingPathConverter.class,
+        category = "input",
+        documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+        effectTags = {OptionEffectTag.UNKNOWN},
+        help =
+            "Path of primary manifest. If not passed, a dummy manifest will be generated and used as"
+                + " the primary.")
     public Path manifest;
 
     @Option(
-      name = "mergeeManifests",
-      defaultValue = "",
-      converter = ExistingPathStringDictionaryConverter.class,
-      category = "input",
-      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
-      effectTags = {OptionEffectTag.UNKNOWN},
-      help = "A dictionary of manifests, and originating target, to be merged into manifest."
-    )
+        name = "mergeeManifests",
+        defaultValue = "",
+        converter = ExistingPathStringDictionaryConverter.class,
+        category = "input",
+        documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+        effectTags = {OptionEffectTag.UNKNOWN},
+        help = "A dictionary of manifests, and originating target, to be merged into manifest.")
     public Map<Path, String> mergeeManifests;
 
     @Option(
-      name = "mergeType",
-      defaultValue = "APPLICATION",
-      converter = MergeTypeConverter.class,
-      category = "config",
-      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
-      effectTags = {OptionEffectTag.UNKNOWN},
-      help = "The type of merging to perform."
-    )
+        name = "mergeType",
+        defaultValue = "APPLICATION",
+        converter = MergeTypeConverter.class,
+        category = "config",
+        documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+        effectTags = {OptionEffectTag.UNKNOWN},
+        help = "The type of merging to perform.")
     public MergeType mergeType;
 
     @Option(
-      name = "manifestValues",
-      defaultValue = "",
-      converter = StringDictionaryConverter.class,
-      category = "config",
-      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
-      effectTags = {OptionEffectTag.UNKNOWN},
-      help =
-          "A dictionary string of values to be overridden in the manifest. Any instance of "
-              + "${name} in the manifest will be replaced with the value corresponding to name in "
-              + "this dictionary. applicationId, versionCode, versionName, minSdkVersion, "
-              + "targetSdkVersion and maxSdkVersion have a dual behavior of also overriding the "
-              + "corresponding attributes of the manifest and uses-sdk tags. packageName will be "
-              + "ignored and will be set from either applicationId or the package in manifest. The "
-              + "expected format of this string is: key:value[,key:value]*. The keys and values "
-              + "may contain colons and commas as long as they are escaped with a backslash."
-    )
+        name = "manifestValues",
+        defaultValue = "",
+        converter = StringDictionaryConverter.class,
+        category = "config",
+        documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+        effectTags = {OptionEffectTag.UNKNOWN},
+        help =
+            "A dictionary string of values to be overridden in the manifest. Any instance of "
+                + "${name} in the manifest will be replaced with the value corresponding to name in "
+                + "this dictionary. applicationId, versionCode, versionName, minSdkVersion, "
+                + "targetSdkVersion and maxSdkVersion have a dual behavior of also overriding the "
+                + "corresponding attributes of the manifest and uses-sdk tags. packageName will be "
+                + "ignored and will be set from either applicationId or the package in manifest. The "
+                + "expected format of this string is: key:value[,key:value]*. The keys and values "
+                + "may contain colons and commas as long as they are escaped with a backslash.")
     public Map<String, String> manifestValues;
 
     @Option(
-      name = "customPackage",
-      defaultValue = "null",
-      category = "config",
-      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
-      effectTags = {OptionEffectTag.UNKNOWN},
-      help = "Custom java package to insert in the package attribute of the manifest tag."
-    )
+        name = "customPackage",
+        defaultValue = "null",
+        category = "config",
+        documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+        effectTags = {OptionEffectTag.UNKNOWN},
+        help = "Custom java package to insert in the package attribute of the manifest tag.")
     public String customPackage;
 
     @Option(
-      name = "manifestOutput",
-      defaultValue = "null",
-      converter = PathConverter.class,
-      category = "output",
-      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
-      effectTags = {OptionEffectTag.UNKNOWN},
-      help = "Path for the merged manifest."
-    )
+        name = "manifestOutput",
+        defaultValue = "null",
+        converter = PathConverter.class,
+        category = "output",
+        documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+        effectTags = {OptionEffectTag.UNKNOWN},
+        help = "Path for the merged manifest.")
     public Path manifestOutput;
 
     @Option(
-      name = "log",
-      defaultValue = "null",
-      category = "output",
-      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
-      effectTags = {OptionEffectTag.UNKNOWN},
-      converter = PathConverter.class,
-      help = "Path to where the merger log should be written."
-    )
+        name = "log",
+        defaultValue = "null",
+        category = "output",
+        documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+        effectTags = {OptionEffectTag.UNKNOWN},
+        converter = PathConverter.class,
+        help = "Path to where the merger log should be written.")
     public Path log;
   }
 
@@ -231,6 +224,11 @@ public static void main(String[] args) throws Exception {
         // to the expected location of the output.
         Files.copy(manifest, options.manifestOutput, StandardCopyOption.REPLACE_EXISTING);
       }
+    } catch (AndroidManifestProcessor.ManifestProcessingException e) {
+      // ManifestProcessingExceptions represent build errors that should be delivered directly to
+      // ResourceProcessorBusyBox where the exception can be delivered with a non-zero status code
+      // to the worker/process
+      throw e;
     } catch (Exception e) {
       logger.log(SEVERE, "Error during merging manifests", e);
       throw e; // This is a proper internal exception, so we bubble it up.

diff --git a/src/tools/android/java/com/google/devtools/build/android/ResourceProcessorBusyBox.java b/src/tools/android/java/com/google/devtools/build/android/ResourceProcessorBusyBox.java
@@ -37,6 +37,8 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import static java.util.logging.Level.SEVERE;
+
 /**
  * Provides an entry point for the resource processing stages.
  *
@@ -237,6 +239,11 @@ private static int processRequest(List<String> args) throws Exception {
       // AndroidDataMerger.MergeConflictException.
       logger.log(Level.SEVERE, e.getMessage());
       return 1;
+    } catch (AndroidManifestProcessor.ManifestProcessingException e) {
+      // We special case ManifestProcessingExceptions here to indicate that this is
+      // caused by a build error, not an Bazel-internal error.
+      logger.log(SEVERE, "Error during merging manifests", e);
+      return 1;
     } catch (OptionsParsingException | IOException | Aapt2Exception | InvalidJavaIdentifier e) {
       logSuppressed(e);
       throw e;