Skip to content

Commit

Permalink
BEGIN_PUBLIC
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Googler authored and copybara-github committed Apr 22, 2021
1 parent 0d89740 commit 705b419
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, String> executionInfo;

public static AndroidDataContext forNative(RuleContext ruleContext) {
Expand All @@ -97,6 +98,7 @@ public static AndroidDataContext makeContext(RuleContext ruleContext) {
!hasExemption(ruleContext, "allow_resource_conflicts", true),
androidConfig.useDataBindingV2(),
androidConfig.useDataBindingAndroidX(),
androidConfig.includeProguardLocationReferences(),
executionInfo);
}

Expand All @@ -120,6 +122,7 @@ protected AndroidDataContext(
boolean throwOnResourceConflict,
boolean useDataBindingV2,
boolean useDataBindingAndroidX,
boolean includeProguardLocationReferences,
ImmutableMap<String, String> executionInfo) {
this.persistentBusyboxToolsEnabled = persistentBusyboxToolsEnabled;
this.ruleContext = ruleContext;
Expand All @@ -133,6 +136,7 @@ protected AndroidDataContext(
this.throwOnResourceConflict = throwOnResourceConflict;
this.useDataBindingV2 = useDataBindingV2;
this.useDataBindingAndroidX = useDataBindingAndroidX;
this.includeProguardLocationReferences = includeProguardLocationReferences;
this.executionInfo = executionInfo;
}

Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -314,7 +321,7 @@ private void createAapt2ApkAction(
}

builder.maybeAddFlag("--conditionalKeepRules", conditionalKeepRules);

builder.maybeAddFlag("--includeProguardLocationReferences", includeProguardLocationReferences);
configureCommonFlags(
dataContext,
primaryResources,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -340,6 +339,7 @@ public static ResourceApk processFromTransitiveLibraryData(
.withAssetDependencies(assetDeps)
.setDebug(dataContext.useDebug())
.setThrowOnResourceConflict(dataContext.throwOnResourceConflict())
.setIncludeProguardLocationReferences(dataContext.includeProguardLocationReferences())
.buildWithoutLocalResources(dataContext, manifest, dataBindingContext);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ public static LinkError of(Throwable e) {
private List<CompiledResources> include = ImmutableList.of();
private List<Path> assetDirs = ImmutableList.of();
private boolean conditionalKeepRules = false;
private boolean includeProguardLocationReferences = false;

private ResourceLinker(
Path aapt2, ListeningExecutorService executorService, Path workingDirectory) {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -605,6 +611,12 @@ public ResourceLinker includeOnlyConfigs(List<String> resourceConfigs) {
return this;
}

public ResourceLinker includeProguardLocationReferences(
boolean includeProguardLocationReferences) {
this.includeProguardLocationReferences = includeProguardLocationReferences;
return this;
}

@Override
public String toString() {
return MoreObjects.toStringHelper(this)
Expand All @@ -615,6 +627,7 @@ public String toString() {
.add("densities", densities)
.add("uncompressedExtensions", uncompressedExtensions)
.add("resourceConfigs", resourceConfigs)
.add("includeProguardLocationReferences", includeProguardLocationReferences)
.toString();
}
}

0 comments on commit 705b419

Please sign in to comment.