Skip to content

Commit

Permalink
[Bazel] Implement namespaced R classes + avoid resource merging
Browse files Browse the repository at this point in the history
Enable rudimentary R class namespacing where each library only contains
references to the resources it declares instead of declarations plus all
transitive dependency references.

This saves a total of 215MB outputs + 145 Actions for a clean build.

* No resource merging for android_library targets
* Smaller R.{class,txt}
* Less desugaring + dexing
  • Loading branch information
Jonathan Gerrish authored and chancila committed Nov 4, 2020
1 parent 4eff003 commit b7a4308
Show file tree
Hide file tree
Showing 11 changed files with 216 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ public ConfiguredTarget create(RuleContext ruleContext)
manifest,
DataBinding.contextFrom(ruleContext, dataContext.getAndroidConfig()),
neverlink,
dataContext.getAndroidConfig().linkLibraryResources());
dataContext.getAndroidConfig().linkLibraryResources(),
/* namespacedRClass = */ false);

MergedAndroidAssets mergedAssets =
AndroidAssets.forAarImport(assets)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,15 @@ public static class Options extends FragmentOptions {
help = "If disabled does not run aapt2 link for android_library targets")
public boolean linkLibraryResources;

@Option(
name = "namespaced_r_class",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.CHANGES_INPUTS},
help = "If enabled each library only contains references to the resources it declares"
+ " instead of declarations plus all dependency references.")
public boolean namespacedRClass;

@Option(
name = "output_library_linked_resources",
defaultValue = "true",
Expand Down Expand Up @@ -1021,6 +1030,7 @@ public ImmutableSet<Class<? extends FragmentOptions>> requiredOptions() {
private final boolean useRTxtFromMergedResources;
private final boolean outputLibraryMergedAssets;
private final boolean linkLibraryResources;
private final boolean namespacedRClass;
private final boolean outputLibraryLinkedResources;
private final Label legacyMainDexListGenerator;
private final boolean disableInstrumentationManifestMerging;
Expand Down Expand Up @@ -1079,6 +1089,7 @@ private AndroidConfiguration(Options options) throws InvalidConfigurationExcepti
this.filterLibraryJarWithProgramJar = options.filterLibraryJarWithProgramJar;
this.useRTxtFromMergedResources = options.useRTxtFromMergedResources;
this.linkLibraryResources = options.linkLibraryResources;
this.namespacedRClass = options.namespacedRClass;
this.outputLibraryLinkedResources = options.outputLibraryLinkedResources;
this.outputLibraryMergedAssets = options.outputLibraryMergedAssets;
this.legacyMainDexListGenerator = options.legacyMainDexListGenerator;
Expand Down Expand Up @@ -1345,6 +1356,10 @@ boolean linkLibraryResources() {
return linkLibraryResources;
}

boolean namespacedRClass() {
return namespacedRClass;
}

boolean outputLibraryLinkedResources() {
return outputLibraryLinkedResources;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,8 @@ public ConfiguredTarget create(RuleContext ruleContext)
manifest,
DataBinding.contextFrom(ruleContext, dataContext.getAndroidConfig()),
isNeverLink,
androidConfig.linkLibraryResources());
androidConfig.linkLibraryResources(),
androidConfig.namespacedRClass());

MergedAndroidAssets assets = AndroidAssets.from(ruleContext).process(dataContext, assetDeps);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ public class AndroidResourceParsingActionBuilder {
// Optional outputs
@Nullable private Artifact compiledSymbols;
@Nullable private Artifact dataBindingInfoZip;
@Nullable private Artifact classJarOut;
@Nullable private Artifact rTxtOut;

/** Set the artifact location for the output protobuf. */
public AndroidResourceParsingActionBuilder setOutput(Artifact output) {
Expand Down Expand Up @@ -73,6 +75,16 @@ public AndroidResourceParsingActionBuilder setCompiledSymbolsOutput(
return this;
}

public AndroidResourceParsingActionBuilder setClassJarOut(Artifact classJarOut) {
this.classJarOut = classJarOut;
return this;
}

public AndroidResourceParsingActionBuilder setRTxtOut(Artifact rTxtOut) {
this.rTxtOut = rTxtOut;
return this;
}

public AndroidResourceParsingActionBuilder setDataBindingInfoZip(Artifact dataBindingInfoZip) {
this.dataBindingInfoZip = dataBindingInfoZip;
return this;
Expand Down Expand Up @@ -109,7 +121,18 @@ private void build(AndroidDataContext dataContext) {
.addOutput("--dataBindingInfoOut", dataBindingInfoZip);
}

compiledBuilder.buildAndRegister("Compiling Android resources", "AndroidResourceCompiler");
if (classJarOut != null) {
compiledBuilder.maybeAddOutput("--classJarOutput", classJarOut);
}

if (rTxtOut != null) {
compiledBuilder.maybeAddOutput("--rTxtOut", rTxtOut);
}

compiledBuilder.addAndroidJar()
.addLabelFlag("--targetLabel")
.maybeAddInput("--manifest", manifest)
.buildAndRegister("Compiling Android resources", "AndroidResourceCompiler");
}
}

Expand All @@ -135,6 +158,8 @@ public ParsedAndroidResources build(
androidResources,
output,
compiledSymbols,
classJarOut,
rTxtOut,
dataContext.getLabel(),
manifest,
dataBindingContext);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -395,9 +395,11 @@ public Optional<? extends AndroidResources> maybeFilter(
public ParsedAndroidResources parse(
AndroidDataContext dataContext,
StampedAndroidManifest manifest,
DataBindingContext dataBindingContext)
DataBindingContext dataBindingContext,
boolean generateNamespacedRFiles)
throws InterruptedException {
return ParsedAndroidResources.parseFrom(dataContext, this, manifest, dataBindingContext);
return ParsedAndroidResources.parseFrom(dataContext, this, manifest, dataBindingContext,
generateNamespacedRFiles);
}

/**
Expand All @@ -410,25 +412,33 @@ public ValidatedAndroidResources process(
StampedAndroidManifest manifest,
DataBindingContext dataBindingContext,
boolean neverlink,
boolean linkResources)
boolean linkResources,
boolean namespacedRClass)
throws RuleErrorException, InterruptedException {
return process(
dataContext,
manifest,
ResourceDependencies.fromRuleDeps(ruleContext, neverlink),
dataBindingContext,
linkResources);
linkResources,
namespacedRClass);
}

ValidatedAndroidResources process(
AndroidDataContext dataContext,
StampedAndroidManifest manifest,
ResourceDependencies resourceDeps,
DataBindingContext dataBindingContext,
boolean linkResources)
boolean linkResources,
boolean namespacedRClass)
throws InterruptedException {
MergedAndroidResources merge = parse(dataContext, manifest, dataBindingContext)
.merge(dataContext, resourceDeps);
ParsedAndroidResources parse =
parse(dataContext, manifest, dataBindingContext, namespacedRClass);

MergedAndroidResources merge = namespacedRClass ?
parse.noMerge(dataContext, resourceDeps) :
parse.merge(dataContext, resourceDeps);

return linkResources ? merge.validate(dataContext) : merge.validateNoLink(dataContext);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
package com.google.devtools.build.lib.rules.android;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.rules.android.AndroidDataConverter.JoinerType;
Expand Down Expand Up @@ -215,6 +214,8 @@ public ProcessedAndroidData build(
databindingProcessedResources,
symbols,
/* compiledSymbols = */ null,
/* classJarOut = */ null,
/* rTxtOut = */ null,
dataContext.getLabel(),
processedManifest,
dataBindingContext);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,8 @@ public ValidatedAndroidResources mergeRes(
Sequence.cast(deps, AndroidResourcesInfo.class, "deps"), neverlink),
DataBinding.contextFrom(
enableDataBinding, ctx.getActionConstructionContext(), ctx.getAndroidConfig()),
ctx.getAndroidConfig().linkLibraryResources());
ctx.getAndroidConfig().linkLibraryResources(),
ctx.getAndroidConfig().namespacedRClass());
} catch (RuleErrorException e) {
throw handleRuleException(errorReporter, e);
}
Expand Down Expand Up @@ -273,7 +274,8 @@ public Dict<Provider, NativeInfo> processAarImportData(
getProviders(depsTargets, AndroidResourcesInfo.PROVIDER),
/* neverlink = */ false),
DataBinding.getDisabledDataBindingContext(ctx),
ctx.getAndroidConfig().linkLibraryResources());
ctx.getAndroidConfig().linkLibraryResources(),
ctx.getAndroidConfig().namespacedRClass());

MergedAndroidAssets mergedAssets =
AndroidAssets.forAarImport(assets)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@
public class ParsedAndroidResources extends AndroidResources {
@Nullable private final Artifact symbols;
@Nullable private final Artifact compiledSymbols;
@Nullable
private final Artifact classJarOut;
@Nullable
private final Artifact rTxtOut;
private final Label label;
private final StampedAndroidManifest manifest;
private final DataBindingContext dataBindingContext;
Expand All @@ -37,7 +41,8 @@ public static ParsedAndroidResources parseFrom(
AndroidDataContext dataContext,
AndroidResources resources,
StampedAndroidManifest manifest,
DataBindingContext dataBindingContext)
DataBindingContext dataBindingContext,
boolean generateNamespacedRFiles)
throws InterruptedException {
AndroidResourceParsingActionBuilder builder = new AndroidResourceParsingActionBuilder();

Expand All @@ -47,6 +52,14 @@ public static ParsedAndroidResources parseFrom(
AndroidResources databindingProcessedResources =
dataBindingContext.processResources(dataContext, resources, manifest.getPackage());

if (generateNamespacedRFiles) {
builder
.setClassJarOut(
dataContext.createOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_CLASS_JAR))
.setRTxtOut(dataContext.createOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT))
.setManifest(manifest.getManifest());
}

return builder
.setCompiledSymbolsOutput(
dataContext.createOutputArtifact(AndroidRuleClasses.ANDROID_COMPILED_SYMBOLS))
Expand All @@ -62,18 +75,22 @@ public static ParsedAndroidResources of(
AndroidResources resources,
@Nullable Artifact symbols,
@Nullable Artifact compiledSymbols,
@Nullable Artifact classJarOut,
@Nullable Artifact rTxtOut,
Label label,
StampedAndroidManifest manifest,
DataBindingContext dataBindingContext) {
return new ParsedAndroidResources(
resources, symbols, compiledSymbols, label, manifest, dataBindingContext);
resources, symbols, compiledSymbols, classJarOut, rTxtOut, label, manifest, dataBindingContext);
}

ParsedAndroidResources(ParsedAndroidResources other, StampedAndroidManifest manifest) {
this(
other,
other.symbols,
other.compiledSymbols,
/* classJarOut= */ null,
/* rTxtOut= */ null,
other.label,
manifest,
other.dataBindingContext);
Expand All @@ -83,12 +100,16 @@ protected ParsedAndroidResources(
AndroidResources resources,
Artifact symbols,
@Nullable Artifact compiledSymbols,
@Nullable Artifact classJarOut,
@Nullable Artifact rTxtOut,
Label label,
StampedAndroidManifest manifest,
DataBindingContext dataBindingContext) {
super(resources);
this.symbols = symbols;
this.compiledSymbols = compiledSymbols;
this.classJarOut = classJarOut;
this.rTxtOut = rTxtOut;
this.label = label;
this.manifest = manifest;
this.dataBindingContext = dataBindingContext;
Expand All @@ -104,6 +125,16 @@ public Artifact getCompiledSymbols() {
return compiledSymbols;
}

@Nullable
Artifact getAapt2RTxt() {
return rTxtOut;
}

@Nullable
public Artifact getClassJar() {
return classJarOut;
}

public Iterable<Artifact> getArtifacts() {
return getResources();
}
Expand Down Expand Up @@ -134,6 +165,13 @@ MergedAndroidResources merge(AndroidDataContext dataContext, ResourceDependencie
return MergedAndroidResources.mergeFrom(dataContext, this, resourceDeps);
}

/** Skips merging this target's resources instead using artifacts produced at compilation. */
public MergedAndroidResources noMerge(AndroidDataContext dataContext,
ResourceDependencies resourceDeps) {
return MergedAndroidResources.of(this, null, this.classJarOut, this.rTxtOut,
null, resourceDeps, this.manifest.withProcessedManifest(this.manifest.getManifest()));
}

@Override
public Optional<? extends ParsedAndroidResources> maybeFilter(
RuleErrorConsumer errorConsumer, ResourceFilter resourceFilter, boolean isDependency)
Expand All @@ -142,7 +180,7 @@ public Optional<? extends ParsedAndroidResources> maybeFilter(
.map(
resources ->
ParsedAndroidResources.of(
resources, symbols, compiledSymbols, label, manifest, dataBindingContext));
resources, symbols, compiledSymbols, null, null, label, manifest, dataBindingContext));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,8 @@ public ValidatedAndroidResources export() {
new AndroidResources(getResources(), getResourceRoots()),
getSymbols(),
getCompiledSymbols(),
null,
null,
getLabel(),
getStampedManifest(),
// Null out databinding to avoid accidentally propagating ActionCreationContext
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,8 @@ private void assertFilterValidated(
getManifest(),
DataBinding.contextFrom(ruleContext, dataContext.getAndroidConfig()),
/* neverlink = */ false,
/* linkResources */ true);
/* linkResources */ true,
AndroidDataContext.forNative(ruleContext).getAndroidConfig().namespacedRClass());
Optional<? extends AndroidResources> maybeFiltered =
assertFilter(unfiltered, filteredResources, /* isDependency = */ true);

Expand Down Expand Up @@ -423,7 +424,8 @@ private ParsedAndroidResources assertParse(
raw.parse(
AndroidDataContext.forNative(ruleContext),
manifest,
dataBindingContext);
dataBindingContext,
AndroidDataContext.forNative(ruleContext).getAndroidConfig().namespacedRClass());

// Inherited values should be equal
assertThat(raw).isEqualTo(new AndroidResources(parsed));
Expand Down Expand Up @@ -459,7 +461,8 @@ private ParsedAndroidResources makeParsedResources(
.parse(
AndroidDataContext.forNative(ruleContext),
getManifest(),
dataBindingContext);
dataBindingContext,
AndroidDataContext.forNative(ruleContext).getAndroidConfig().namespacedRClass());
}

private ProcessedAndroidManifest getManifest() {
Expand Down
Loading

0 comments on commit b7a4308

Please sign in to comment.