Skip to content

Commit

Permalink
Remove a usage of aapt1 when --incompatible_prohibit_aapt1 is specified
Browse files Browse the repository at this point in the history
Bazel previously invoked both aapt1 and aapt2 separately to validate resources
from an android_library, but the former is unnecessary if we don't support
aapt1.  Since we can't remove any plumbing yet, we just label the same
triplet of rTxt/sourceJar/apk artifacts as both "aapt2" and "aapt1".

PiperOrigin-RevId: 281624756
  • Loading branch information
donaldchai authored and copybara-github committed Nov 21, 2019
1 parent 52e607b commit dbe76ce
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1406,4 +1406,8 @@ public boolean filterLibraryJarWithProgramJar() {
boolean useRTxtFromMergedResources() {
return useRTxtFromMergedResources;
}

boolean incompatibleProhibitAapt1() {
return incompatibleProhibitAapt1;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,39 @@ public class ValidatedAndroidResources extends MergedAndroidResources
public static ValidatedAndroidResources validateFrom(
AndroidDataContext dataContext, MergedAndroidResources merged, AndroidAaptVersion aaptVersion)
throws InterruptedException {
if (dataContext.getAndroidConfig().incompatibleProhibitAapt1()) {
Artifact rTxtOut = dataContext.createOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT);
Artifact sourceJarOut =
dataContext.createOutputArtifact(AndroidRuleClasses.ANDROID_JAVA_SOURCE_JAR);
Artifact apkOut = dataContext.createOutputArtifact(AndroidRuleClasses.ANDROID_LIBRARY_APK);

BusyBoxActionBuilder.create(dataContext, "LINK_STATIC_LIBRARY")
.addAapt(AndroidAaptVersion.AAPT2)
.addInput("--libraries", dataContext.getSdk().getAndroidJar())
.addInput("--compiled", merged.getCompiledSymbols())
.addInput("--manifest", merged.getManifest())
// Sets an alternative java package for the generated R.java
// this allows android rules to generate resources outside of the java{,tests} tree.
.maybeAddFlag("--packageForR", merged.getJavaPackage())
.addTransitiveVectoredInput(
"--compiledDep", merged.getResourceDependencies().getTransitiveCompiledSymbols())
.addOutput("--sourceJarOut", sourceJarOut)
.addOutput("--rTxtOut", rTxtOut)
.addOutput("--staticLibraryOut", apkOut)
.buildAndRegister("Linking static android resource library", "AndroidResourceLink");

return of(
merged,
rTxtOut,
sourceJarOut,
apkOut,
// TODO: remove below three when incompatibleProhibitAapt1 is on by default.
rTxtOut,
sourceJarOut,
apkOut,
dataContext.getAndroidConfig().useRTxtFromMergedResources());
}

AndroidResourceValidatorActionBuilder builder =
new AndroidResourceValidatorActionBuilder()
.setJavaPackage(merged.getJavaPackage())
Expand Down Expand Up @@ -150,6 +183,8 @@ public Artifact getJavaSourceJar() {
return sourceJar;
}

// TODO(b/30307842,b/119560471): remove this; it was added for no reason, but persists because
// the Starlark API is not noneable.
@Override
public Artifact getApk() {
return apk;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.RuleContext;
Expand All @@ -30,6 +31,7 @@
import com.google.devtools.build.lib.testutil.MoreAsserts;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.Optional;
import java.util.Set;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -375,6 +377,10 @@ public void testValidateAapt() throws Exception {
MergedAndroidResources merged = makeMergedResources(ruleContext, AndroidAaptVersion.AAPT);
ValidatedAndroidResources validated =
merged.validate(AndroidDataContext.forNative(ruleContext), AndroidAaptVersion.AAPT);
Set<String> actionMnemonics =
ruleContext.getAnalysisEnvironment().getRegisteredActions().stream()
.map(ActionAnalysisMetadata::getMnemonic)
.collect(ImmutableSet.toImmutableSet());

// Inherited values should be equal
assertThat(merged).isEqualTo(new MergedAndroidResources(validated));
Expand All @@ -385,12 +391,14 @@ public void testValidateAapt() throws Exception {
/* inputs = */ ImmutableList.of(validated.getMergedResources(), validated.getManifest()),
/* outputs = */ ImmutableList.of(
validated.getRTxt(), validated.getJavaSourceJar(), validated.getApk()));
assertThat(actionMnemonics).contains("AndroidResourceValidator"); // aapt1 validation

// aapt2 artifacts should not be generated
assertThat(validated.getCompiledSymbols()).isNull();
assertThat(validated.getAapt2RTxt()).isNull();
assertThat(validated.getAapt2SourceJar()).isNull();
assertThat(validated.getStaticLibrary()).isNull();
assertThat(actionMnemonics).doesNotContain("AndroidResourceLink"); // aapt2 validation
}

@Test
Expand Down Expand Up @@ -431,6 +439,24 @@ public void testValidateAapt2() throws Exception {
validated.getAapt2RTxt(), validated.getAapt2SourceJar(), validated.getStaticLibrary()));
}

@Test
public void testValidateNoAapt1() throws Exception {
useConfiguration("--incompatible_prohibit_aapt1");
RuleContext ruleContext = getRuleContext();

makeMergedResources(ruleContext, AndroidAaptVersion.AAPT2)
.validate(AndroidDataContext.forNative(ruleContext), AndroidAaptVersion.AAPT2);

Set<String> actionMnemonics =
ruleContext.getAnalysisEnvironment().getRegisteredActions().stream()
.map(ActionAnalysisMetadata::getMnemonic)
.collect(ImmutableSet.toImmutableSet());
// These are unfortunately the mnemonics used in Bazel; these should be changed once aapt1 is
// removed.
assertThat(actionMnemonics).contains("AndroidResourceLink"); // aapt2 validation
assertThat(actionMnemonics).doesNotContain("AndroidResourceValidator"); // aapt1 validation
}

@Test
public void testGenerateRClass() throws Exception {
RuleContext ruleContext = getRuleContext();
Expand Down

0 comments on commit dbe76ce

Please sign in to comment.