Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

support persistent worker for aar extractors #18496

Closed
wants to merge 1 commit into from
Closed

support persistent worker for aar extractors #18496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 65 additions & 10 deletions src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,14 @@
package com.google.devtools.build.lib.rules.android;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.ExecutionRequirements;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.actions.ParamFileInfo;
import com.google.devtools.build.lib.actions.ParameterFile;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.FileProvider;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
Expand Down Expand Up @@ -65,6 +69,11 @@ public class AarImport implements RuleConfiguredTargetFactory {
private static final String MERGED_JAR = "classes_and_libs_merged.jar";
private static final String PROGUARD_SPEC = "proguard.txt";

private static final ParamFileInfo WORKERS_FORCED_PARAM_FILE_INFO =
ParamFileInfo.builder(ParameterFile.ParameterFileType.UNQUOTED)
.setUseAlways(true)
SupSaiYaJin marked this conversation as resolved.
Show resolved Hide resolved
.build();

private final JavaSemantics javaSemantics;
private final AndroidSemantics androidSemantics;

Expand Down Expand Up @@ -272,13 +281,29 @@ private NestedSet<Artifact> extractProguardSpecs(RuleContext ruleContext, Artifa
return builder.addTransitive(proguardSpecs).add(proguardSpecArtifact).build();
}

private static boolean isPersistentAarExtractor(RuleContext ruleContext) {
AndroidConfiguration androidConfig =
ruleContext.getConfiguration().getFragment(AndroidConfiguration.class);
return androidConfig.persistentAarExtractor();
}

/**
* Creates action to extract embedded Proguard.txt from an AAR. If the file is not found, an empty
* file will be created
*/
private static SpawnAction createAarEmbeddedProguardExtractorActions(
RuleContext ruleContext, Artifact aar, Artifact proguardSpecArtifact) {
return new SpawnAction.Builder()
SpawnAction.Builder actionBuilder = new SpawnAction.Builder();
boolean isPersistentAarExtractor = isPersistentAarExtractor(ruleContext);
ParamFileInfo paramFileInfo = null;
if (isPersistentAarExtractor) {
ImmutableMap.Builder<String, String> executionInfo = ImmutableMap.builder();
executionInfo.putAll(ExecutionRequirements.WORKER_MODE_ENABLED);
executionInfo.put(ExecutionRequirements.REQUIRES_WORKER_PROTOCOL, "json");
actionBuilder.setExecutionInfo(executionInfo.build());
paramFileInfo = WORKERS_FORCED_PARAM_FILE_INFO;
}
return actionBuilder
.useDefaultShellEnvironment()
.setExecutable(
ruleContext.getExecutablePrerequisite(AarImportBaseRule.AAR_EMBEDDED_PROGUARD_EXTACTOR))
Expand All @@ -290,7 +315,7 @@ private static SpawnAction createAarEmbeddedProguardExtractorActions(
CustomCommandLine.builder()
.addExecPath("--input_aar", aar)
.addExecPath("--output_proguard_file", proguardSpecArtifact)
.build())
.build(), paramFileInfo)
.build(ruleContext);
}

Expand Down Expand Up @@ -352,7 +377,17 @@ private static SpawnAction createAarResourcesExtractorActions(
Artifact databindingBrFiles,
Artifact databindingSetterStoreFiles) {

return new SpawnAction.Builder()
SpawnAction.Builder actionBuilder = new SpawnAction.Builder();
boolean isPersistentAarExtractor = isPersistentAarExtractor(ruleContext);
ParamFileInfo paramFileInfo = null;
if (isPersistentAarExtractor) {
ImmutableMap.Builder<String, String> executionInfo = ImmutableMap.builder();
executionInfo.putAll(ExecutionRequirements.WORKER_MODE_ENABLED);
executionInfo.put(ExecutionRequirements.REQUIRES_WORKER_PROTOCOL, "json");
actionBuilder.setExecutionInfo(executionInfo.build());
paramFileInfo = WORKERS_FORCED_PARAM_FILE_INFO;
}
return actionBuilder
.useDefaultShellEnvironment()
.setExecutable(
ruleContext.getExecutablePrerequisite(AarImportBaseRule.AAR_RESOURCES_EXTRACTOR))
Expand All @@ -370,7 +405,7 @@ private static SpawnAction createAarResourcesExtractorActions(
.addExecPath("--output_assets_dir", assetsDir)
.addExecPath("--output_databinding_br_dir", databindingBrFiles)
.addExecPath("--output_databinding_setter_store_dir", databindingSetterStoreFiles)
.build())
.build(), paramFileInfo)
.build(ruleContext);
}

Expand All @@ -379,7 +414,17 @@ private static SpawnAction createAarEmbeddedJarsExtractorActions(
Artifact aar,
Artifact jarsTreeArtifact,
Artifact singleJarParamFile) {
return new SpawnAction.Builder()
SpawnAction.Builder actionBuilder = new SpawnAction.Builder();
boolean isPersistentAarExtractor = isPersistentAarExtractor(ruleContext);
ParamFileInfo paramFileInfo = null;
if (isPersistentAarExtractor) {
ImmutableMap.Builder<String, String> executionInfo = ImmutableMap.builder();
executionInfo.putAll(ExecutionRequirements.WORKER_MODE_ENABLED);
executionInfo.put(ExecutionRequirements.REQUIRES_WORKER_PROTOCOL, "json");
actionBuilder.setExecutionInfo(executionInfo.build());
paramFileInfo = WORKERS_FORCED_PARAM_FILE_INFO;
}
return actionBuilder
.useDefaultShellEnvironment()
.setExecutable(
ruleContext.getExecutablePrerequisite(AarImportBaseRule.AAR_EMBEDDED_JARS_EXTACTOR))
Expand All @@ -393,7 +438,7 @@ private static SpawnAction createAarEmbeddedJarsExtractorActions(
.addExecPath("--input_aar", aar)
.addExecPath("--output_dir", jarsTreeArtifact)
.addExecPath("--output_singlejar_param_file", singleJarParamFile)
.build())
.build(), paramFileInfo)
.build(ruleContext);
}

Expand All @@ -420,8 +465,18 @@ private static SpawnAction createAarJarsMergingActions(

private static SpawnAction createAarNativeLibsFilterActions(
RuleContext ruleContext, Artifact aar, Artifact outputZip) {
SpawnAction.Builder actionBuilder =
new SpawnAction.Builder()
SpawnAction.Builder actionBuilder = new SpawnAction.Builder();
boolean isPersistentAarExtractor = isPersistentAarExtractor(ruleContext);
ParamFileInfo paramFileInfo = null;
if (isPersistentAarExtractor) {
ImmutableMap.Builder<String, String> executionInfo = ImmutableMap.builder();
executionInfo.putAll(ExecutionRequirements.WORKER_MODE_ENABLED);
executionInfo.put(ExecutionRequirements.REQUIRES_WORKER_PROTOCOL, "json");
actionBuilder.setExecutionInfo(executionInfo.build());
paramFileInfo = WORKERS_FORCED_PARAM_FILE_INFO;
}
return
actionBuilder
.useDefaultShellEnvironment()
.setExecutable(
ruleContext.getExecutablePrerequisite(
Expand All @@ -435,8 +490,8 @@ private static SpawnAction createAarNativeLibsFilterActions(
.addExecPath("--input_aar", aar)
.add("--cpu", ruleContext.getConfiguration().getCpu())
.addExecPath("--output_zip", outputZip)
.build());
return actionBuilder.build(ruleContext);
.build(), paramFileInfo)
.build(ruleContext);
}

private static DataBindingV2Provider createDatabindingProvider(
Expand Down
18 changes: 18 additions & 0 deletions src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -824,6 +824,16 @@ public static class Options extends FragmentOptions {
+ "transitive classpath, otherwise it uses the deploy jar")
public boolean oneVersionEnforcementUseTransitiveJarsForBinaryUnderTest;

@Option(
name = "experimental_persistent_aar_extractor",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY,
effectTags = {
OptionEffectTag.EXECUTION,
},
help = "Enable persistent aar extractor by using workers.")
public boolean persistentAarExtractor;

@Option(
name = "persistent_android_resource_processor",
defaultValue = "null",
Expand Down Expand Up @@ -1129,6 +1139,7 @@ public FragmentOptions getExec() {
exec.manifestMergerOrder = manifestMergerOrder;
exec.oneVersionEnforcementUseTransitiveJarsForBinaryUnderTest =
oneVersionEnforcementUseTransitiveJarsForBinaryUnderTest;
exec.persistentAarExtractor = persistentAarExtractor;
exec.persistentBusyboxTools = persistentBusyboxTools;
exec.persistentMultiplexBusyboxTools = persistentMultiplexBusyboxTools;
exec.disableNativeAndroidRules = disableNativeAndroidRules;
Expand Down Expand Up @@ -1174,6 +1185,7 @@ public FragmentOptions getExec() {
private final boolean dataBindingV2;
private final boolean dataBindingUpdatedArgs;
private final boolean dataBindingAndroidX;
private final boolean persistentAarExtractor;
private final boolean persistentBusyboxTools;
private final boolean persistentMultiplexBusyboxTools;
private final boolean persistentDexDesugar;
Expand Down Expand Up @@ -1236,6 +1248,7 @@ public AndroidConfiguration(BuildOptions buildOptions) throws InvalidConfigurati
this.dataBindingV2 = options.dataBindingV2;
this.dataBindingUpdatedArgs = options.dataBindingUpdatedArgs;
this.dataBindingAndroidX = options.dataBindingAndroidX;
this.persistentAarExtractor = options.persistentAarExtractor;
this.persistentBusyboxTools = options.persistentBusyboxTools;
this.persistentMultiplexBusyboxTools = options.persistentMultiplexBusyboxTools;
this.persistentDexDesugar = options.persistentDexDesugar;
Expand Down Expand Up @@ -1478,6 +1491,11 @@ public boolean useDataBindingAndroidX() {
return dataBindingAndroidX;
}

@Override
public boolean persistentAarExtractor() {
return persistentAarExtractor;
}

@Override
public boolean persistentBusyboxTools() {
return persistentBusyboxTools;
Expand Down
7 changes: 7 additions & 0 deletions .../java/com/google/devtools/build/lib/starlarkbuildapi/android/AndroidConfigurationApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,13 @@ public interface AndroidConfigurationApi extends StarlarkValue {
documented = false)
boolean useDataBindingAndroidX();

@StarlarkMethod(
name = "persistent_aar_extractor",
structField = true,
doc = "",
documented = false)
boolean persistentAarExtractor();

@StarlarkMethod(
name = "persistent_busybox_tools",
structField = true,
Expand Down
34 changes: 34 additions & 0 deletions src/test/shell/bazel/android/aar_integration_test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,4 +147,38 @@ EOF
assert_one_of $apk_contents "lib/armeabi-v7a/libapp.so"
}

function test_aar_extractor_worker() {
create_new_workspace
setup_android_sdk_support
cat > AndroidManifest.xml <<EOF
<manifest package="com.example"/>
EOF
mkdir -p res/layout
cat > res/layout/mylayout.xml <<EOF
<?xml version="1.0" encoding="utf-8"?>
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android" />
EOF
mkdir assets
echo "some asset" > assets/a
zip example.aar AndroidManifest.xml res/layout/mylayout.xml assets/a
cat > BUILD <<EOF
aar_import(
name = "example",
aar = "example.aar",
)
android_binary(
name = "app",
custom_package = "com.example",
manifest = "AndroidManifest.xml",
deps = [":example"],
)
EOF

bazel clean
bazel build --experimental_persistent_aar_extractor :app || fail "build failed"
apk_contents="$(zipinfo -1 bazel-bin/app.apk)"
assert_one_of $apk_contents "assets/a"
assert_one_of $apk_contents "res/layout/mylayout.xml"
}

run_suite "aar_import integration tests"
11 changes: 11 additions & 0 deletions tools/android/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ py_binary(
deps = [
":junction_lib",
"//third_party/py/abseil",
":json_worker_wrapper",
],
)

Expand Down Expand Up @@ -123,6 +124,7 @@ py_binary(
deps = [
":junction_lib",
"//third_party/py/abseil",
":json_worker_wrapper",
],
)

Expand All @@ -138,6 +140,7 @@ py_binary(
deps = [
":junction_lib",
"//third_party/py/abseil",
":json_worker_wrapper",
],
)

Expand All @@ -153,6 +156,7 @@ py_binary(
deps = [
":junction_lib",
"//third_party/py/abseil",
":json_worker_wrapper",
],
)

Expand Down Expand Up @@ -193,6 +197,13 @@ py_library(
visibility = ["//visibility:private"],
)

py_library(
name = "json_worker_wrapper",
srcs = ["json_worker_wrapper.py"],
visibility = ["//visibility:private"],
srcs_version = "PY3",
)
SupSaiYaJin marked this conversation as resolved.
Show resolved Hide resolved

py_test(
name = "junction_test",
srcs = select({
Expand Down
11 changes: 11 additions & 0 deletions tools/android/BUILD.tools
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,7 @@ py_binary(
deps = [
":junction_lib",
"//third_party/py/abseil",
":json_worker_wrapper",
],
)

Expand All @@ -371,6 +372,7 @@ py_binary(
deps = [
":junction_lib",
"//third_party/py/abseil",
":json_worker_wrapper",
],
)

Expand All @@ -381,6 +383,7 @@ py_binary(
deps = [
":junction_lib",
"//third_party/py/abseil",
":json_worker_wrapper",
],
)

Expand All @@ -391,6 +394,7 @@ py_binary(
deps = [
":junction_lib",
"//third_party/py/abseil",
":json_worker_wrapper",
],
)

Expand All @@ -408,6 +412,13 @@ py_library(
visibility = ["//visibility:private"],
)

py_library(
name = "json_worker_wrapper",
srcs = ["json_worker_wrapper.py"],
visibility = ["//visibility:private"],
srcs_version = "PY3",
)
SupSaiYaJin marked this conversation as resolved.
Show resolved Hide resolved

alias(
name = "android_runtest",
actual = "fail.sh",
Expand Down
4 changes: 2 additions & 2 deletions tools/android/aar_embedded_jars_extractor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
from absl import flags

from tools.android import junction
from tools.android import json_worker_wrapper

FLAGS = flags.FLAGS

Expand Down Expand Up @@ -100,5 +101,4 @@ def main(unused_argv):


if __name__ == "__main__":
FLAGS(sys.argv)
app.run(main)
json_worker_wrapper.wrap_worker(FLAGS, main, app.run)
4 changes: 2 additions & 2 deletions tools/android/aar_embedded_proguard_extractor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
from absl import flags

from tools.android import junction
from tools.android import json_worker_wrapper

FLAGS = flags.FLAGS

Expand Down Expand Up @@ -71,5 +72,4 @@ def main(unused_argv):


if __name__ == "__main__":
FLAGS(sys.argv)
app.run(main)
json_worker_wrapper.wrap_worker(FLAGS, main, app.run)
4 changes: 2 additions & 2 deletions tools/android/aar_native_libs_zip_creator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
from absl import flags

from tools.android import junction
from tools.android import json_worker_wrapper

FLAGS = flags.FLAGS

Expand Down Expand Up @@ -104,5 +105,4 @@ def main(unused_argv):


if __name__ == "__main__":
FLAGS(sys.argv)
app.run(main)
json_worker_wrapper.wrap_worker(FLAGS, main, app.run)
Loading
Loading