From e0e456d715fdf14ca5be68a935e2d1e0bca11199 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 30a3238c26ffd3..38a4abed6c3c93 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 @@ -224,20 +224,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 b943c5a1052d24..622f94ad2bdbc3 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 @@ -683,6 +683,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", @@ -1014,6 +1023,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; @@ -1068,6 +1078,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 = @@ -1287,6 +1298,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 af07e82ae76d7a..17a7d84e52c8f8 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 @@ -151,7 +151,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 ed9d097fa7a56d..40518278309803 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 @@ -61,7 +61,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 */); } /** @@ -76,7 +77,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())) { @@ -84,13 +85,21 @@ public static AndroidManifest fromAttributes( rawManifest = ruleContext.getPrerequisiteArtifact("manifest"); } - 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); } /**