Skip to content

Commit

Permalink
Simplify --incompatible_use_aapt2_by_default by making --android_aapt…
Browse files Browse the repository at this point in the history
… depend on it at analysis time.

Now, the incompatible change flag is in effect for the parts of the Android rules which select the version based on the --android_aapt config value.

If the version is still `auto` after resolution of the --android_aapt flag and aapt_version attribute, default to aapt.

bazelbuild#6907

RELNOTES: None.
PiperOrigin-RevId: 254259050
  • Loading branch information
jin authored and siberex committed Jul 4, 2019
1 parent db304aa commit 0bc0b65
Showing 1 changed file with 20 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -259,14 +259,11 @@ public static AndroidAaptVersion chooseTargetAaptVersion(RuleContext ruleContext
}

/**
* Select the aapt version for resource processing actions, based on the combination of
* --android_aapt flag, aapt_version target attribute, and --incompatible_use_aapt2_by_default
* flag.
* Select the aapt version for resource processing actions.
*
* <p>Order of precedence:
* <li>1. --android_aapt flag
* <li>2. 'aapt_version' attribute on target
* <li>3. --incompatible_use_aapt2_by_default flag
*
* @param dataContext the Android data context for detecting aapt2 and fetching Android configs
* @param errorConsumer the rule context for reporting errors during version selection
Expand All @@ -286,27 +283,18 @@ public static AndroidAaptVersion chooseTargetAaptVersion(
AndroidAaptVersion flag = dataContext.getAndroidConfig().getAndroidAaptVersion();
AndroidAaptVersion attribute = AndroidAaptVersion.fromString(attributeString);

AndroidAaptVersion version = flag == AndroidAaptVersion.AUTO ? attribute : flag;
AndroidAaptVersion version = flag == AUTO ? attribute : flag;

if (version == AAPT2 && !hasAapt2) {
throw errorConsumer.throwWithRuleError(
"aapt2 processing requested but not available on the android_sdk");
}

if (version != AndroidAaptVersion.AUTO) {
return version;
if (version == AUTO) {
return AAPT;
}

// At this point, the version is still auto. If the user passes
// --incompatible_use_aapt2_by_default explicitly or implicitly via
// --all_incompatible_changes, use aapt2 by default.
//
// We use the --incompatible_use_aapt2_by_default 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`.
return dataContext.getAndroidConfig().incompatibleChangeUseAapt2ByDefault() ? AAPT2 : AAPT;
return version;
}
}

Expand Down Expand Up @@ -1127,7 +1115,6 @@ private AndroidConfiguration(Options options) throws InvalidConfigurationExcepti
this.useRexToCompressDexFiles = options.useRexToCompressDexFiles;
this.compressJavaResources = options.compressJavaResources;
this.exportsManifestDefault = options.exportsManifestDefault;
this.androidAaptVersion = options.androidAaptVersion;
this.useAapt2ForRobolectric = options.useAapt2ForRobolectric;
this.throwOnResourceConflict = options.throwOnResourceConflict;
this.useParallelDex2Oat = options.useParallelDex2Oat;
Expand All @@ -1151,6 +1138,21 @@ private AndroidConfiguration(Options options) throws InvalidConfigurationExcepti
this.alwaysFilterDuplicateClassesFromAndroidTest =
options.alwaysFilterDuplicateClassesFromAndroidTest;

// 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.
// 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) {
this.androidAaptVersion = AndroidAaptVersion.AAPT2;
} else {
this.androidAaptVersion = options.androidAaptVersion;
}

if (incrementalDexingShardsAfterProguard < 0) {
throw new InvalidConfigurationException(
"--experimental_incremental_dexing_after_proguard must be a positive number");
Expand Down

0 comments on commit 0bc0b65

Please sign in to comment.