From 84c1ed430405b154b6e9eb2c28281f450e250eff Mon Sep 17 00:00:00 2001 From: Ted Date: Tue, 7 Feb 2023 17:18:30 -0500 Subject: [PATCH] Multiplex worker fixes and tests for Android busybox tools (#17371) * Fix multiplexed workers for Android busybox tools Unblocks merging for PR #15952. RELNOTES: PiperOrigin-RevId: 505149043 Change-Id: I1cb5ccb4c162f26fea51e0cdef8c8553f3a3d5df * Add worker assertions resource_processing_integration_test Adding a few assertions to the worker and multiplex worker tests to ensure that the expected workers are in fact coming up. Closes #15952. PiperOrigin-RevId: 505690186 Change-Id: If732c9f040ec144395b5525a48d3a7d367fe244b --------- Co-authored-by: Benjamin Lee Co-authored-by: kshyanashree <109167932+kshyanashree@users.noreply.github.com> --- .../rules/android/AndroidConfiguration.java | 20 +++++++++++++++++++ .../lib/rules/android/AndroidDataContext.java | 8 ++++++++ .../rules/android/BusyBoxActionBuilder.java | 4 ++++ .../android/AndroidConfigurationApi.java | 7 +++++++ .../resource_processing_integration_test.sh | 16 +++++++++++++-- 5 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java index 810a5c0b3b9b67..a3fb4898797c46 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java @@ -938,6 +938,7 @@ public static class Options extends FragmentOptions { "Enable persistent and multiplexed Android tools (dexing, desugaring, resource " + "processing).", expansion = { + "--internal_persistent_multiplex_busybox_tools", "--persistent_multiplex_android_resource_processor", "--persistent_multiplex_android_dex_desugar", }) @@ -962,6 +963,17 @@ public static class Options extends FragmentOptions { help = "Tracking flag for when busybox workers are enabled.") public boolean persistentBusyboxTools; + @Option( + name = "internal_persistent_multiplex_busybox_tools", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = { + OptionEffectTag.HOST_MACHINE_RESOURCE_OPTIMIZATIONS, + OptionEffectTag.EXECUTION, + }, + defaultValue = "false", + help = "Tracking flag for when multiplexed busybox workers are enabled.") + public boolean persistentMultiplexBusyboxTools; + @Option( name = "experimental_remove_r_classes_from_instrumentation_test_jar", defaultValue = "true", @@ -1096,6 +1108,7 @@ public FragmentOptions getHost() { oneVersionEnforcementUseTransitiveJarsForBinaryUnderTest; host.persistentBusyboxTools = persistentBusyboxTools; host.disableNativeAndroidRules = disableNativeAndroidRules; + host.persistentMultiplexBusyboxTools = persistentMultiplexBusyboxTools; // Unless the build was started from an Android device, host means MAIN. host.configurationDistinguisher = ConfigurationDistinguisher.MAIN; @@ -1141,6 +1154,7 @@ public FragmentOptions getHost() { private final boolean dataBindingUpdatedArgs; private final boolean dataBindingAndroidX; private final boolean persistentBusyboxTools; + private final boolean persistentMultiplexBusyboxTools; private final boolean filterRJarsFromAndroidTest; private final boolean removeRClassesFromInstrumentationTestJar; private final boolean alwaysFilterDuplicateClassesFromAndroidTest; @@ -1201,6 +1215,7 @@ public AndroidConfiguration(BuildOptions buildOptions) throws InvalidConfigurati this.dataBindingUpdatedArgs = options.dataBindingUpdatedArgs; this.dataBindingAndroidX = options.dataBindingAndroidX; this.persistentBusyboxTools = options.persistentBusyboxTools; + this.persistentMultiplexBusyboxTools = options.persistentMultiplexBusyboxTools; this.filterRJarsFromAndroidTest = options.filterRJarsFromAndroidTest; this.removeRClassesFromInstrumentationTestJar = options.removeRClassesFromInstrumentationTestJar; @@ -1453,6 +1468,11 @@ public boolean persistentBusyboxTools() { return persistentBusyboxTools; } + @Override + public boolean persistentMultiplexBusyboxTools() { + return persistentMultiplexBusyboxTools; + } + @Override public boolean incompatibleUseToolchainResolution() { return incompatibleUseToolchainResolution; diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidDataContext.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidDataContext.java index 1ae5b9d7ed2899..ad8c12b937724c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidDataContext.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidDataContext.java @@ -64,6 +64,7 @@ public class AndroidDataContext implements AndroidDataContextApi { private final FilesToRunProvider busybox; private final AndroidSdkProvider sdk; private final boolean persistentBusyboxToolsEnabled; + private final boolean persistentMultiplexBusyboxToolsEnabled; private final boolean optOutOfResourcePathShortening; private final boolean optOutOfResourceNameObfuscation; private final boolean throwOnShrinkResources; @@ -90,6 +91,7 @@ public static AndroidDataContext makeContext(RuleContext ruleContext) { ruleContext, ruleContext.getExecutablePrerequisite("$android_resources_busybox"), androidConfig.persistentBusyboxTools(), + androidConfig.persistentMultiplexBusyboxTools(), AndroidSdkProvider.fromRuleContext(ruleContext), hasExemption(ruleContext, "allow_raw_access_to_resource_paths", false), hasExemption(ruleContext, "allow_resource_name_obfuscation_opt_out", false), @@ -114,6 +116,7 @@ protected AndroidDataContext( RuleContext ruleContext, FilesToRunProvider busybox, boolean persistentBusyboxToolsEnabled, + boolean persistentMultiplexBusyboxToolsEnabled, AndroidSdkProvider sdk, boolean optOutOfResourcePathShortening, boolean optOutOfResourceNameObfuscation, @@ -126,6 +129,7 @@ protected AndroidDataContext( boolean includeProguardLocationReferences, ImmutableMap executionInfo) { this.persistentBusyboxToolsEnabled = persistentBusyboxToolsEnabled; + this.persistentMultiplexBusyboxToolsEnabled = persistentMultiplexBusyboxToolsEnabled; this.ruleContext = ruleContext; this.busybox = busybox; this.sdk = sdk; @@ -222,6 +226,10 @@ public boolean isPersistentBusyboxToolsEnabled() { return persistentBusyboxToolsEnabled; } + public boolean isPersistentMultiplexBusyboxToolsEnabled() { + return persistentMultiplexBusyboxToolsEnabled; + } + public boolean optOutOfResourcePathShortening() { return optOutOfResourcePathShortening; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/BusyBoxActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/BusyBoxActionBuilder.java index af0762384cd873..24fcfe93190265 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/BusyBoxActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/BusyBoxActionBuilder.java @@ -369,6 +369,10 @@ public void buildAndRegister(String message, String mnemonic) { commandLine.add("--logWarnings=false"); spawnActionBuilder.addCommandLine(commandLine.build(), WORKERS_FORCED_PARAM_FILE_INFO); executionInfo.putAll(ExecutionRequirements.WORKER_MODE_ENABLED); + + if (dataContext.isPersistentMultiplexBusyboxToolsEnabled()) { + executionInfo.putAll(ExecutionRequirements.WORKER_MULTIPLEX_MODE_ENABLED); + } } else { spawnActionBuilder.addCommandLine(commandLine.build(), FORCED_PARAM_FILE_INFO); } diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/android/AndroidConfigurationApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/android/AndroidConfigurationApi.java index 197d3c16a4e5a9..c29241be6f5bc7 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/android/AndroidConfigurationApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/android/AndroidConfigurationApi.java @@ -231,6 +231,13 @@ public interface AndroidConfigurationApi extends StarlarkValue { documented = false) boolean persistentBusyboxTools(); + @StarlarkMethod( + name = "persistent_multiplex_busybox_tools", + structField = true, + doc = "", + documented = false) + boolean persistentMultiplexBusyboxTools(); + @StarlarkMethod( name = "get_output_directory_name", structField = true, diff --git a/src/test/shell/bazel/android/resource_processing_integration_test.sh b/src/test/shell/bazel/android/resource_processing_integration_test.sh index 1c3bd1f858d541..718bad9c0111bf 100755 --- a/src/test/shell/bazel/android/resource_processing_integration_test.sh +++ b/src/test/shell/bazel/android/resource_processing_integration_test.sh @@ -111,7 +111,13 @@ function test_persistent_resource_processor() { create_android_binary setup_font_resources - assert_build //java/bazel:bin --persistent_android_resource_processor + assert_build //java/bazel:bin --persistent_android_resource_processor \ + --worker_verbose &> $TEST_log + expect_log "Created new non-sandboxed AndroidResourceParser worker (id [0-9]\+)" + expect_log "Created new non-sandboxed AndroidResourceCompiler worker (id [0-9]\+)" + expect_log "Created new non-sandboxed AndroidCompiledResourceMerger worker (id [0-9]\+)" + expect_log "Created new non-sandboxed AndroidAapt2 worker (id [0-9]\+)" + expect_log "Created new non-sandboxed ManifestMerger worker (id [0-9]\+)" } function test_persistent_multiplex_resource_processor() { @@ -121,7 +127,13 @@ function test_persistent_multiplex_resource_processor() { setup_font_resources assert_build //java/bazel:bin --experimental_worker_multiplex \ - --persistent_multiplex_android_tools + --persistent_multiplex_android_tools \ + --worker_verbose &> $TEST_log + expect_log "Created new non-sandboxed AndroidResourceParser multiplex-worker (id [0-9]\+)" + expect_log "Created new non-sandboxed AndroidResourceCompiler multiplex-worker (id [0-9]\+)" + expect_log "Created new non-sandboxed AndroidCompiledResourceMerger multiplex-worker (id [0-9]\+)" + expect_log "Created new non-sandboxed AndroidAapt2 multiplex-worker (id [0-9]\+)" + expect_log "Created new non-sandboxed ManifestMerger multiplex-worker (id [0-9]\+)" } run_suite "Resource processing integration tests"