Skip to content

Commit

Permalink
Automated rollback of commit f064184.
Browse files Browse the repository at this point in the history
*** Reason for rollback ***

Causes a significant performance regression in Blaze; see b/300864946 .

*** Original change description ***

Add an is_android option to facilitate exec toolchain selection

RELNOTES:None.
PiperOrigin-RevId: 566535845
Change-Id: Iaeab16348d5ea7c0e85e7e95aa4b6ceafe6a4b00
  • Loading branch information
lberki authored and copybara-github committed Sep 19, 2023
1 parent 657e01c commit 19f5e93
Show file tree
Hide file tree
Showing 4 changed files with 4 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1108,19 +1108,6 @@ public static class Options extends FragmentOptions {
+ " transition` with changed options to avoid potential action conflicts.")
public boolean androidPlatformsTransitionsUpdateAffected;

@Option(
name = "is_android",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION,
help =
"This option exists for the purposes of enabling the toolchain resolution mechanism"
+ " to select a different `exec` toolchain when targeting Android. An example use"
+ " case is Rust: The Rust toolchain has a requirement that certain types of"
+ " libraries (proc-macro) that are built in `exec` mode *have* to be compiled with"
+ " the same toolchain as the libraries built in `target` mode.")
public boolean isAndroid;

@Override
public FragmentOptions getExec() {
Options exec = (Options) super.getExec();
Expand Down Expand Up @@ -1151,7 +1138,6 @@ public FragmentOptions getExec() {
exec.persistentBusyboxTools = persistentBusyboxTools;
exec.persistentMultiplexBusyboxTools = persistentMultiplexBusyboxTools;
exec.disableNativeAndroidRules = disableNativeAndroidRules;
exec.isAndroid = isAndroid;

// Unless the build was started from an Android device, exec means MAIN.
exec.configurationDistinguisher = ConfigurationDistinguisher.MAIN;
Expand Down Expand Up @@ -1208,7 +1194,6 @@ public FragmentOptions getExec() {
private final boolean hwasan;
private final boolean getJavaResourcesFromOptimizedJar;
private final boolean includeProguardLocationReferences;
private final boolean isAndroid;

public AndroidConfiguration(BuildOptions buildOptions) throws InvalidConfigurationException {
Options options = buildOptions.get(Options.class);
Expand Down Expand Up @@ -1270,7 +1255,6 @@ public AndroidConfiguration(BuildOptions buildOptions) throws InvalidConfigurati
this.hwasan = options.hwasan;
this.getJavaResourcesFromOptimizedJar = options.getJavaResourcesFromOptimizedJar;
this.includeProguardLocationReferences = options.includeProguardLocationReferences;
this.isAndroid = options.isAndroid;

if (incrementalDexingShardsAfterProguard < 0) {
throw new InvalidConfigurationException(
Expand Down Expand Up @@ -1561,10 +1545,6 @@ boolean outputLibraryMergedAssets() {
return outputLibraryMergedAssets;
}

boolean isAndroid() {
return isAndroid;
}

/** 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 @@ -74,7 +74,7 @@ public BuildOptions patch(BuildOptionsView options, EventHandler eventHandler) {
// 2. Otherwise, leave --platforms alone (this will probably lead to build errors).
if (!androidOptions.androidPlatforms.isEmpty()) {
// If the current value of --platforms is not one of the values of --android_platforms, change
// it to be the first one. If the current --platforms is part of --android_platforms, leave it
// it to be the first one. If the curent --platforms is part of --android_platforms, leave it
// as-is.
// NOTE: This does not handle aliases at all, so if someone is using aliases with platform
// definitions this check will break.
Expand All @@ -90,8 +90,6 @@ public BuildOptions patch(BuildOptionsView options, EventHandler eventHandler) {
newOptions.get(CppOptions.class).enableCcToolchainResolution = true;
}

newOptions.get(AndroidConfiguration.Options.class).isAndroid = true;

if (androidOptions.androidPlatformsTransitionsUpdateAffected) {
ImmutableSet.Builder<String> affected = ImmutableSet.builder();
if (!options
Expand All @@ -104,10 +102,6 @@ public BuildOptions patch(BuildOptionsView options, EventHandler eventHandler) {
!= newOptions.get(CppOptions.class).enableCcToolchainResolution) {
affected.add("//command_line_option:incompatible_enable_cc_toolchain_resolution");
}
if (options.get(AndroidConfiguration.Options.class).isAndroid
!= newOptions.get(AndroidConfiguration.Options.class).isAndroid) {
affected.add("//command_line_option:is_android");
}
FunctionTransitionUtil.updateAffectedByStarlarkTransition(
newOptions.get(CoreOptions.class), affected.build());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ public void dataBindingAnnotationProcessorFlags_v3_4() throws Exception {
(JavaCompileAction)
getGeneratingAction(getFirstArtifactEndingWith(allArtifacts, "app.jar"));
String dataBindingFilesDir =
getConfiguration(ctapp)
targetConfig
.getBinDirectory(RepositoryName.MAIN)
.getExecPath()
.getRelative("java/android/binary/databinding/app")
Expand Down Expand Up @@ -1214,7 +1214,7 @@ public void dataBinding_androidLocalTest_dataBindingEnabled_usesDataBindingFlags
getGeneratingAction(
getFirstArtifactEndingWith(allArtifacts, "databinding_enabled_test-class.jar"));
String dataBindingFilesDir =
getConfiguration(testTarget)
targetConfig
.getBinDirectory(RepositoryName.MAIN)
.getExecPath()
.getRelative("javatests/android/test/databinding/databinding_enabled_test")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,10 +190,7 @@ public void testTestExecutableRunfiles() throws Exception {
.toList());
assertThat(runfiles.stream().map(Artifact::toString).collect(toImmutableList()))
.containsAtLeast(
getDeviceFixtureScript(
getDirectPrerequisite(
androidInstrumentationTest.getConfiguredTarget(),
"//javatests/com/app:device_fixture"))
getDeviceFixtureScript(getConfiguredTarget("//javatests/com/app:device_fixture"))
.toString(),
getInstrumentationApk(getConfiguredTarget("//javatests/com/app:instrumentation_app"))
.toString(),
Expand Down

0 comments on commit 19f5e93

Please sign in to comment.