From f73469d241636972f51bb6522d4abd483817bf06 Mon Sep 17 00:00:00 2001 From: Jonathan Gerrish Date: Tue, 19 May 2020 15:51:05 +0000 Subject: [PATCH] Add override_manifest_package option to avoid stamping manifests https://jira.sc-corp.net/browse/APP-56636 (option #5) By default Bazel always performs a manifest merge action for every android_library with resources. This involves overriding the manifests package attribute with either a package inferred from the java{,tests} source root relative to the BUILD file or from the override specified in the custom_package attribute. This option disables the automatic stamping for users who would be happy to accept the package specified in the manifest and do not wish to override it. Saves 270 Actions Before: ManifestMerger 271 398 kb 74 kb 64176 s After: ManifestMerger 1 302 kb 0 b 489 s Will mostly affect clean builds. --- Automatic squash commit from https://github.sc-corp.net/Snapchat/bazel/pull/37 Cooled by jgerrish --- .../lib/rules/android/AndroidBinary.java | 23 ++++++++-------- .../rules/android/AndroidConfiguration.java | 15 +++++++++++ .../lib/rules/android/AndroidLibrary.java | 3 ++- .../lib/rules/android/AndroidManifest.java | 27 ++++++++++++------- 4 files changed, 47 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java index 98b6398d0a24a1..c2c1bac094cec5 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java @@ -225,20 +225,21 @@ private static RuleConfiguredTargetBuilder init( if (isInstrumentation(ruleContext) && dataContext.getAndroidConfig().disableInstrumentationManifestMerging()) { manifest = - AndroidManifest.fromAttributes(ruleContext, dataContext, androidSemantics) + AndroidManifest.fromAttributes(ruleContext, dataContext, androidSemantics, true /* overrideManifestPackage */) .stamp(dataContext); } else { manifest = - AndroidManifest.fromAttributes(ruleContext, dataContext, androidSemantics) - .mergeWithDeps( - dataContext, - androidSemantics, - ruleContext, - resourceDeps, - manifestValues, - ruleContext.getRule().isAttrDefined("manifest_merger", STRING) - ? ruleContext.attributes().get("manifest_merger", STRING) - : null); + AndroidManifest.fromAttributes(ruleContext, dataContext, androidSemantics, + true /* overrideManifestPackage */) + .mergeWithDeps( + dataContext, + androidSemantics, + ruleContext, + resourceDeps, + manifestValues, + ruleContext.getRule().isAttrDefined("manifest_merger", STRING) + ? ruleContext.attributes().get("manifest_merger", STRING) + : null); } boolean shrinkResourceCycles = diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java index 6a170a78113154..bd8db438221abd 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java @@ -671,6 +671,15 @@ public static class Options extends FragmentOptions { help = "The default value of the exports_manifest attribute on android_library.") public boolean exportsManifestDefault; + @Option( + name = "override_manifest_package", + defaultValue = "true", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = {OptionEffectTag.CHANGES_INPUTS}, + help = "If set will override the package set in the manifest with custom_package attribute " + + "if set otherwise will derive package from the path to the rule.") + public boolean overrideManifestPackage; + @Option( name = "experimental_omit_resources_info_provider_from_android_binary", defaultValue = "false", @@ -988,6 +997,7 @@ public ImmutableSet> requiredOptions() { private final boolean useSingleJarApkBuilder; private final boolean compressJavaResources; private final boolean exportsManifestDefault; + private final boolean overrideManifestPackage; private final boolean useParallelDex2Oat; private final boolean breakBuildOnParallelDex2OatFailure; private final boolean omitResourcesInfoProviderFromAndroidBinary; @@ -1041,6 +1051,7 @@ private AndroidConfiguration(Options options) throws InvalidConfigurationExcepti this.useRexToCompressDexFiles = options.useRexToCompressDexFiles; this.compressJavaResources = options.compressJavaResources; this.exportsManifestDefault = options.exportsManifestDefault; + this.overrideManifestPackage = options.overrideManifestPackage; this.useParallelDex2Oat = options.useParallelDex2Oat; this.breakBuildOnParallelDex2OatFailure = options.breakBuildOnParallelDex2OatFailure; this.omitResourcesInfoProviderFromAndroidBinary = @@ -1254,6 +1265,10 @@ public boolean getExportsManifestDefault() { return exportsManifestDefault; } + public boolean getOverrideManifestPackage() { + return overrideManifestPackage; + } + @Override public boolean omitResourcesInfoProviderFromAndroidBinary() { return this.omitResourcesInfoProviderFromAndroidBinary; diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java index 0e2a8e879f8ac3..a79ed1a487397e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java @@ -152,7 +152,8 @@ public ConfiguredTarget create(RuleContext ruleContext) final ResourceApk resourceApk; if (definesLocalResources) { StampedAndroidManifest manifest = - AndroidManifest.fromAttributes(ruleContext, dataContext, androidSemantics) + AndroidManifest.fromAttributes(ruleContext, dataContext, androidSemantics, + androidConfig.getOverrideManifestPackage()) .stamp(dataContext); ValidatedAndroidResources resources = diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidManifest.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidManifest.java index 388a713010db3e..a98c5b60c17e37 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidManifest.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidManifest.java @@ -62,7 +62,8 @@ public static StampedAndroidManifest forAarImport(Artifact manifest) { public static AndroidManifest fromAttributes( RuleContext ruleContext, AndroidDataContext dataContext) throws InterruptedException, RuleErrorException { - return fromAttributes(ruleContext, dataContext, null); + return fromAttributes(ruleContext, dataContext, null, + true /* overrideManifestPackage */); } /** @@ -77,7 +78,7 @@ public static AndroidManifest fromAttributes( public static AndroidManifest fromAttributes( RuleContext ruleContext, AndroidDataContext dataContext, - @Nullable AndroidSemantics androidSemantics) + @Nullable AndroidSemantics androidSemantics, boolean overrideManifestPackage) throws RuleErrorException, InterruptedException { Artifact rawManifest = null; if (AndroidResources.definesAndroidResources(ruleContext.attributes())) { @@ -85,13 +86,21 @@ public static AndroidManifest fromAttributes( rawManifest = ruleContext.getPrerequisiteArtifact("manifest", TransitionMode.TARGET); } - return from( - dataContext, - ruleContext, - rawManifest, - androidSemantics, - getAndroidPackage(ruleContext), - AndroidCommon.getExportsManifest(ruleContext)); + String androidPackage = overrideManifestPackage ? getAndroidPackage(ruleContext) : null; + boolean exportsManifest = AndroidCommon.getExportsManifest(ruleContext); + + if (rawManifest == null) { + // Generate a dummy manifest + return StampedAndroidManifest.createEmpty( + dataContext.getActionConstructionContext(), androidPackage, exportsManifest); + } + + AndroidManifest raw = new AndroidManifest(rawManifest, androidPackage, exportsManifest); + + if (androidSemantics != null) { + return androidSemantics.renameManifest(dataContext, raw); + } + return raw.renameManifestIfNeeded(dataContext); } /**