Skip to content

Commit

Permalink
Introduce --incompatible_prohibit_aapt1 flag.
Browse files Browse the repository at this point in the history 
If this flag is active, AAPT2 is the only Android resource packaging tool in use and we prohibit methods of configuring the AAPT tool version.

#10000

PiperOrigin-RevId: 274703802
  • Loading branch information
@bchang @copybara-github
bchang authored and copybara-github committed Oct 15, 2019
1 parent dd69a70 commit f610fd6
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 29 deletions.
68 changes: 44 additions & 24 deletions src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,16 +213,19 @@ public static AndroidAaptVersion fromString(String value) {
public static AndroidAaptVersion chooseTargetAaptVersion(RuleContext ruleContext)
throws RuleErrorException {
if (ruleContext.isLegalFragment(AndroidConfiguration.class)) {
AndroidDataContext dataContext = AndroidDataContext.makeContext(ruleContext);

if (ruleContext.getRule().isAttrDefined("aapt_version", STRING)) {
// On rules that can choose a version, verify attribute and flag.
return chooseTargetAaptVersion(
AndroidDataContext.makeContext(ruleContext),
ruleContext,
ruleContext.attributes().get("aapt_version", STRING));
dataContext, ruleContext, ruleContext.attributes().get("aapt_version", STRING));
} else {
// On rules that can't choose, assume aapt2 if aapt2 is present in the sdk.
// This ensures that non-leaf nodes (e.g. android_library) will generate aapt2 actions.
if (dataContext.getAndroidConfig().incompatibleProhibitAapt1) {
verifySdkHasAapt2(dataContext, ruleContext);
return AAPT2;
}
return AndroidSdkProvider.fromRuleContext(ruleContext).getAapt2() != null ? AAPT2 : AAPT;
}
}
Expand All @@ -232,7 +235,10 @@ public static AndroidAaptVersion chooseTargetAaptVersion(RuleContext ruleContext
/**
* Select the aapt version for resource processing actions.
*
* <p>Order of precedence:
* <p>If --incompatible_prohibit_aapt1 is active, and the rule specifies the {@code
* aapt_version} attribute, this will fail.
*
* <p>Otherwise, order of precedence:
* <li>1. --android_aapt flag
* <li>2. 'aapt_version' attribute on target
*
Expand All @@ -250,6 +256,16 @@ public static AndroidAaptVersion chooseTargetAaptVersion(
@Nullable String attributeString)
throws RuleErrorException {

if (dataContext.getAndroidConfig().incompatibleProhibitAapt1) {
verifySdkHasAapt2(dataContext, errorConsumer);

if (!"auto".equals(attributeString)) {
throw errorConsumer.throwWithAttributeError(
"aapt_version", "Attribute is no longer supported");
}
return AAPT2;
}

boolean hasAapt2 = dataContext.getSdk().getAapt2() != null;
AndroidAaptVersion flag = dataContext.getAndroidConfig().getAndroidAaptVersion();
AndroidAaptVersion attribute = AndroidAaptVersion.fromString(attributeString);
Expand All @@ -267,6 +283,13 @@ public static AndroidAaptVersion chooseTargetAaptVersion(

return version;
}

private static void verifySdkHasAapt2(
AndroidDataContext dataContext, RuleErrorConsumer errorConsumer) throws RuleErrorException {
if (dataContext.getSdk().getAapt2() == null) {
throw errorConsumer.throwWithRuleError("aapt2 not available from the android_sdk");
}
}
}

/** Android configuration options. */
Expand Down Expand Up @@ -907,19 +930,19 @@ public static class Options extends FragmentOptions {
public boolean persistentBusyboxTools;

@Option(
name = "incompatible_use_aapt2_by_default",
name = "incompatible_prohibit_aapt1",
documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
effectTags = {OptionEffectTag.LOSES_INCREMENTAL_STATE, OptionEffectTag.AFFECTS_OUTPUTS},
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
defaultValue = "true",
defaultValue = "false",
help =
"Switch the Android rules to use aapt2 by default for resource processing. "
"End support for aapt in Android rules. "
+ "To resolve issues when migrating your app to build with aapt2, see "
+ "https://developer.android.com/studio/command-line/aapt2#aapt2_changes")
public boolean incompatibleUseAapt2ByDefault;
public boolean incompatibleProhibitAapt1;

@Option(
name = "experimental_remove_r_classes_from_instrumentation_test_jar",
Expand Down Expand Up @@ -1060,7 +1083,7 @@ public ImmutableSet<Class<? extends FragmentOptions>> requiredOptions() {
private final boolean useRTxtFromMergedResources;

// Incompatible changes
private final boolean incompatibleUseAapt2ByDefault;
private final boolean incompatibleProhibitAapt1;

private AndroidConfiguration(Options options) throws InvalidConfigurationException {
this.sdk = options.sdk;
Expand Down Expand Up @@ -1108,27 +1131,28 @@ private AndroidConfiguration(Options options) throws InvalidConfigurationExcepti
this.dataBindingUpdatedArgs = options.dataBindingUpdatedArgs;
this.persistentBusyboxTools = options.persistentBusyboxTools;
this.filterRJarsFromAndroidTest = options.filterRJarsFromAndroidTest;
this.incompatibleUseAapt2ByDefault = options.incompatibleUseAapt2ByDefault;
this.incompatibleProhibitAapt1 = options.incompatibleProhibitAapt1;
this.removeRClassesFromInstrumentationTestJar =
options.removeRClassesFromInstrumentationTestJar;
this.alwaysFilterDuplicateClassesFromAndroidTest =
options.alwaysFilterDuplicateClassesFromAndroidTest;
this.filterLibraryJarWithProgramJar = options.filterLibraryJarWithProgramJar;
this.useRTxtFromMergedResources = options.useRTxtFromMergedResources;

// Make the value of --android_aapt aapt2 if --incompatible_use_aapt2_by_default is enabled
// and --android_aapt = AUTO
//
// We use the --incompatible_use_aapt2_by_default flag to signal a breaking change in Bazel.
// We use the --incompatible_prohibit_aapt1 flag to signal a breaking change in Bazel.
// This is required by the Bazel Incompatible Changes policy.
//
// TODO(jingwen): We can remove the incompatible change flag only when the depot migration is
// complete and the default value of --android_aapt is switched from `auto` to `aapt2`.
if (options.incompatibleUseAapt2ByDefault
&& options.androidAaptVersion == AndroidAaptVersion.AUTO) {
if (incompatibleProhibitAapt1) {
if (options.androidAaptVersion != AndroidAaptVersion.AAPT2) {
throw new InvalidConfigurationException(
"--android_aapt is no longer available for setting aapt version to aapt");
}
this.androidAaptVersion = AndroidAaptVersion.AAPT2;
} else {
this.androidAaptVersion = options.androidAaptVersion;
if (options.androidAaptVersion == AndroidAaptVersion.AUTO) {
this.androidAaptVersion = AndroidAaptVersion.AAPT2;
} else {
this.androidAaptVersion = options.androidAaptVersion;
}
}

if (incrementalDexingShardsAfterProguard < 0) {
Expand Down Expand Up @@ -1356,10 +1380,6 @@ public boolean persistentBusyboxTools() {
return persistentBusyboxTools;
}

public boolean incompatibleChangeUseAapt2ByDefault() {
return incompatibleUseAapt2ByDefault;
}

@Override
public String getOutputDirectoryName() {
return configurationDistinguisher.suffix;
Expand Down
37 changes: 32 additions & 5 deletions src/test/java/com/google/devtools/build/lib/rules/android/AndroidResourcesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
import static org.junit.Assert.fail;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
Expand Down Expand Up @@ -495,15 +496,41 @@ public void testProcessBinaryDataGeneratesProguardOutput() throws Exception {
}

@Test
public void test_incompatibleUseAapt2ByDefaultEnabled_targetsAapt2() throws Exception {
useConfiguration("--incompatible_use_aapt2_by_default");
RuleContext ruleContext =
getRuleContext(
"android_binary", "aapt_version = 'auto',", "manifest = 'AndroidManifest.xml',");
public void test_incompatibleProhibitAapt1_targetsAapt2() throws Exception {
useConfiguration("--incompatible_prohibit_aapt1");
RuleContext ruleContext = getRuleContext("android_binary", "manifest = 'AndroidManifest.xml',");
assertThat(AndroidAaptVersion.chooseTargetAaptVersion(ruleContext))
.isEqualTo(AndroidAaptVersion.AAPT2);
}

@Test
public void test_incompatibleProhibitAapt1_aaptVersionAapt_throwsAttributeError()
throws Exception {
useConfiguration("--incompatible_prohibit_aapt1");
try {
getRuleContext(
"android_binary", "aapt_version = 'aapt',", "manifest = 'AndroidManifest.xml',");
fail("Expected AssertionError");
} catch (AssertionError e) {
assertThat(e).hasMessageThat().contains("aapt_version");
assertThat(e).hasMessageThat().contains("Attribute is no longer supported");
}
}

@Test
public void test_incompatibleProhibitAapt1_aaptVersionAapt2_throwsAttributeError()
throws Exception {
useConfiguration("--incompatible_prohibit_aapt1");
try {
getRuleContext(
"android_binary", "aapt_version = 'aapt2',", "manifest = 'AndroidManifest.xml',");
fail("Expected AssertionError");
} catch (AssertionError e) {
assertThat(e).hasMessageThat().contains("aapt_version");
assertThat(e).hasMessageThat().contains("Attribute is no longer supported");
}
}

/**
* Validates that a parse action was invoked correctly. Returns the {@link ParsedAndroidResources}
* for further validation.
Expand Down

0 comments on commit f610fd6

Please sign in to comment.