From 705b419f95060305de93e7861544bd73bb663f19 Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 22 Apr 2021 11:35:56 -0700 Subject: [PATCH] BEGIN_PUBLIC When using aapt2 to link and generate proguard configurations, pass it the flag --no-proguard-location-reference. These location references are comments which include temporary directory names, which cause build output to be nondeterministic. In case location references are needed for debugging, create a build flag --android_include_proguard_location_references that suppresses passing --no-proguard-location-reference to aapt2. END_PUBLIC RELNOTES: Generate proguard configurations deterministically. PiperOrigin-RevId: 369915362 --- .../lib/rules/android/AndroidConfiguration.java | 16 ++++++++++++++++ .../lib/rules/android/AndroidDataContext.java | 8 ++++++++ .../AndroidResourcesProcessorBuilder.java | 9 ++++++++- .../lib/rules/android/ProcessedAndroidData.java | 5 ++--- .../build/lib/rules/android/ResourceApk.java | 2 +- .../android/Aapt2ResourcePackagingAction.java | 9 +++++++++ .../build/android/aapt2/ResourceLinker.java | 13 +++++++++++++ 7 files changed, 57 insertions(+), 5 deletions(-) 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 af522a5db37928..98029debf10fea 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 @@ -978,6 +978,16 @@ public static class Options extends FragmentOptions { + "bundling the final APK.") public boolean getJavaResourcesFromOptimizedJar; + @Option( + name = "android_include_proguard_location_references", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = {OptionEffectTag.AFFECTS_OUTPUTS}, + help = + "When using aapt2 to generate proguard configurations, include location references." + + " This will make the build nondeterministic.") + public boolean includeProguardLocationReferences; + @Override public FragmentOptions getHost() { Options host = (Options) super.getHost(); @@ -1063,6 +1073,7 @@ public FragmentOptions getHost() { private final boolean incompatibleUseToolchainResolution; private final boolean hwasan; private final boolean getJavaResourcesFromOptimizedJar; + private final boolean includeProguardLocationReferences; public AndroidConfiguration(BuildOptions buildOptions) throws InvalidConfigurationException { Options options = buildOptions.get(Options.class); @@ -1123,6 +1134,7 @@ public AndroidConfiguration(BuildOptions buildOptions) throws InvalidConfigurati this.incompatibleUseToolchainResolution = options.incompatibleUseToolchainResolution; this.hwasan = options.hwasan; this.getJavaResourcesFromOptimizedJar = options.getJavaResourcesFromOptimizedJar; + this.includeProguardLocationReferences = options.includeProguardLocationReferences; if (incrementalDexingShardsAfterProguard < 0) { throw new InvalidConfigurationException( @@ -1403,6 +1415,10 @@ public boolean getJavaResourcesFromOptimizedJar() { return getJavaResourcesFromOptimizedJar; } + public boolean includeProguardLocationReferences() { + return includeProguardLocationReferences; + } + /** Returns the label provided with --legacy_main_dex_list_generator, if any. */ // TODO(b/147692286): Move R8's main dex list tool into tool repository. @StarlarkConfigurationField( diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidDataContext.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidDataContext.java index dc55f49f287130..630c1fdb7d169f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidDataContext.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidDataContext.java @@ -71,6 +71,7 @@ public class AndroidDataContext implements AndroidDataContextApi { private final boolean throwOnResourceConflict; private final boolean useDataBindingV2; private final boolean useDataBindingAndroidX; + private final boolean includeProguardLocationReferences; private final ImmutableMap executionInfo; public static AndroidDataContext forNative(RuleContext ruleContext) { @@ -97,6 +98,7 @@ public static AndroidDataContext makeContext(RuleContext ruleContext) { !hasExemption(ruleContext, "allow_resource_conflicts", true), androidConfig.useDataBindingV2(), androidConfig.useDataBindingAndroidX(), + androidConfig.includeProguardLocationReferences(), executionInfo); } @@ -120,6 +122,7 @@ protected AndroidDataContext( boolean throwOnResourceConflict, boolean useDataBindingV2, boolean useDataBindingAndroidX, + boolean includeProguardLocationReferences, ImmutableMap executionInfo) { this.persistentBusyboxToolsEnabled = persistentBusyboxToolsEnabled; this.ruleContext = ruleContext; @@ -133,6 +136,7 @@ protected AndroidDataContext( this.throwOnResourceConflict = throwOnResourceConflict; this.useDataBindingV2 = useDataBindingV2; this.useDataBindingAndroidX = useDataBindingAndroidX; + this.includeProguardLocationReferences = includeProguardLocationReferences; this.executionInfo = executionInfo; } @@ -244,6 +248,10 @@ public boolean useDataBindingAndroidX() { return useDataBindingAndroidX; } + public boolean includeProguardLocationReferences() { + return includeProguardLocationReferences; + } + public boolean annotateRFieldsFromTransitiveDeps() { return ruleContext.getFeatures().contains(ANNOTATE_R_FIELDS_FROM_TRANSITIVE_DEPS); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java index 1fccf0614cc759..6886c35c335c61 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java @@ -62,6 +62,7 @@ public class AndroidResourcesProcessorBuilder { private boolean throwOnResourceConflict; private String packageUnderTest; private boolean isTestWithResources = false; + private boolean includeProguardLocationReferences = false; /** * The output zip for resource-processed data binding expressions (i.e. a zip of .xml files). @@ -122,6 +123,12 @@ public AndroidResourcesProcessorBuilder setMainDexProguardOut(Artifact mainDexPr return this; } + public AndroidResourcesProcessorBuilder setIncludeProguardLocationReferences( + boolean includeProguardLocationReferences) { + this.includeProguardLocationReferences = includeProguardLocationReferences; + return this; + } + public AndroidResourcesProcessorBuilder setRTxtOut(Artifact rTxtOut) { this.rTxtOut = rTxtOut; return this; @@ -314,7 +321,7 @@ private void createAapt2ApkAction( } builder.maybeAddFlag("--conditionalKeepRules", conditionalKeepRules); - + builder.maybeAddFlag("--includeProguardLocationReferences", includeProguardLocationReferences); configureCommonFlags( dataContext, primaryResources, diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ProcessedAndroidData.java b/src/main/java/com/google/devtools/build/lib/rules/android/ProcessedAndroidData.java index 80ab6a24bef2ed..438e42b53f90ad 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ProcessedAndroidData.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ProcessedAndroidData.java @@ -251,9 +251,8 @@ private static AndroidResourcesProcessorBuilder builderForTopLevelTarget( // Output .setProguardOut( ProguardHelper.getProguardConfigArtifact( - dataContext.getLabel(), - dataContext.getActionConstructionContext(), - proguardPrefix)); + dataContext.getLabel(), dataContext.getActionConstructionContext(), proguardPrefix)) + .setIncludeProguardLocationReferences(dataContext.includeProguardLocationReferences()); } static ProcessedAndroidData of( diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java index 2d4ee41a2ba80c..2c9d44a530fe05 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java @@ -327,7 +327,6 @@ public static ResourceApk processFromTransitiveLibraryData( AssetDependencies assetDeps, StampedAndroidManifest manifest) throws InterruptedException { - return new AndroidResourcesProcessorBuilder() .setLibrary(true) .setRTxtOut(dataContext.createOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT)) @@ -340,6 +339,7 @@ public static ResourceApk processFromTransitiveLibraryData( .withAssetDependencies(assetDeps) .setDebug(dataContext.useDebug()) .setThrowOnResourceConflict(dataContext.throwOnResourceConflict()) + .setIncludeProguardLocationReferences(dataContext.includeProguardLocationReferences()) .buildWithoutLocalResources(dataContext, manifest, dataBindingContext); } } diff --git a/src/tools/android/java/com/google/devtools/build/android/Aapt2ResourcePackagingAction.java b/src/tools/android/java/com/google/devtools/build/android/Aapt2ResourcePackagingAction.java index 864276afeeca89..25103f6788a87d 100644 --- a/src/tools/android/java/com/google/devtools/build/android/Aapt2ResourcePackagingAction.java +++ b/src/tools/android/java/com/google/devtools/build/android/Aapt2ResourcePackagingAction.java @@ -328,6 +328,14 @@ public static final class Options extends OptionsBase { effectTags = {OptionEffectTag.NO_OP}, help = "Unused/deprecated option.") public boolean isTestWithResources; + + @Option( + name = "includeProguardLocationReferences", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = {OptionEffectTag.AFFECTS_OUTPUTS}, + help = "When generating proguard configurations, include location references.") + public boolean includeProguardLocationReferences; } public static void main(String[] args) throws Exception { @@ -468,6 +476,7 @@ public static void main(String[] args) throws Exception { .debug(aaptConfigOptions.debug) .includeGeneratedLocales(aaptConfigOptions.generatePseudoLocale) .includeOnlyConfigs(aaptConfigOptions.resourceConfigs) + .includeProguardLocationReferences(options.includeProguardLocationReferences) .link(compiled); profiler.recordEndOf("link").startTask("validate"); diff --git a/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java b/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java index f2e4317d14e3dd..f934ff525c0f2d 100644 --- a/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java +++ b/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java @@ -139,6 +139,7 @@ public static LinkError of(Throwable e) { private List include = ImmutableList.of(); private List assetDirs = ImmutableList.of(); private boolean conditionalKeepRules = false; + private boolean includeProguardLocationReferences = false; private ResourceLinker( Path aapt2, ListeningExecutorService executorService, Path workingDirectory) { @@ -426,6 +427,11 @@ private ProtoApk linkProtoApk( .add("--java", javaSourceDirectory) .add("--proguard", proguardConfig) .add("--proguard-main-dex", mainDexProguard) + // By default, exclude the file path location comments, since the paths + // include temporary directory names, which otherwise cause + // nondeterministic build output. + .when(!includeProguardLocationReferences) + .thenAdd("--no-proguard-location-reference") .when(conditionalKeepRules) .thenAdd("--proguard-conditional-keep-rules") .add("-o", linked) @@ -605,6 +611,12 @@ public ResourceLinker includeOnlyConfigs(List resourceConfigs) { return this; } + public ResourceLinker includeProguardLocationReferences( + boolean includeProguardLocationReferences) { + this.includeProguardLocationReferences = includeProguardLocationReferences; + return this; + } + @Override public String toString() { return MoreObjects.toStringHelper(this) @@ -615,6 +627,7 @@ public String toString() { .add("densities", densities) .add("uncompressedExtensions", uncompressedExtensions) .add("resourceConfigs", resourceConfigs) + .add("includeProguardLocationReferences", includeProguardLocationReferences) .toString(); } }