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

[SNAP FORK][UPSTREAMED] enable support multiplex workers in busybox actions #64

Closed
wants to merge 8 commits into from
Closed

[SNAP FORK][UPSTREAMED] enable support multiplex workers in busybox actions #64

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1508984
[SNAP FORK][UPSTREAMED] enable support multiplex workers in busybox a…
Sep 19, 2022
981477e
Add flag to enable resource processr worker mode
Apr 24, 2023
e243693
only disable warning of worker mode is enabled
Apr 24, 2023
6f9998f
Fix issues
Apr 24, 2023
55662a8
Add flag for multiplex workers
Apr 24, 2023
6c0f0cf
Merge branch 'pre-alpha' into mgalindo-busybox-workers
ted-xie May 12, 2023
69a010c
Use persistent android workers from fragment config
May 23, 2023
761207e
Remove unused args
May 23, 2023
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
88 changes: 69 additions & 19 deletions rules/busybox.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,12 @@ def _make_resources_flag(
],
)

def _disable_warnings(ctx, args):
if (ctx.fragments.android.persistent_android_resource_processor or
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed these fragment flags to ctx.fragments.android.persistent_busybox_tools and ctx.fragments.android.persistent_multiplex_busybox_tools internally and tests are passing. Those flags also seem more appropriate than the resource_processor ones; what do you think?

Unfortunately the fragments.android data structure can't accept Void expansion flags such as persistent_android_resource_processor, and changing the return type of that flag handler to boolean causes some other kind of sanity check error. I think in Bazel/Blaze, expansion flags must return Void, so there's somewhat of an impedance mismatch.

ctx.fragments.android.persistent_multiplex_android_resource_processor):
# Disable warnings - this are output to stdin/stderr which breaks worker mode
args.add("--logWarnings=false")

def _path(f):
return f.path

Expand Down Expand Up @@ -283,7 +289,8 @@ def _package(
transitive_input_files = []

args = ctx.actions.args()
args.use_param_file("@%s")
args.use_param_file("@%s", use_always=True)
args.set_param_file_format("multiline")
args.add("--tool", "AAPT2_PACKAGE")
args.add("--")
args.add("--aapt2", aapt.executable)
Expand Down Expand Up @@ -395,13 +402,15 @@ def _package(
resource_apks,
join_with = ":",
)
transitive_input_files.append(resource_apks)

transitive_input_files.append(resource_apks)
_disable_warnings(ctx, args)

_java.run(
_java_run(
ctx = ctx,
host_javabase = host_javabase,
executable = busybox,
tools = [aapt],
tools = [aapt, busybox],
arguments = [args],
inputs = depset(input_files, transitive = transitive_input_files),
outputs = output_files,
Expand All @@ -427,7 +436,8 @@ def _parse(
host_javabase: Target. The host javabase.
"""
args = ctx.actions.args()
args.use_param_file("@%s")
args.use_param_file("@%s", use_always=True)
args.set_param_file_format("multiline")
args.add("--tool", "PARSE")
args.add("--")
args.add(
Expand All @@ -439,7 +449,9 @@ def _parse(
)
args.add("--output", out_symbols)

_java.run(
_disable_warnings(ctx, args)

_java_run(
ctx = ctx,
host_javabase = host_javabase,
executable = busybox,
Expand Down Expand Up @@ -493,7 +505,8 @@ def _merge_assets(
host_javabase: Target. The host javabase.
"""
args = ctx.actions.args()
args.use_param_file("@%s")
args.use_param_file("@%s", use_always=True)
args.set_param_file_format("multiline")
args.add("--tool", "MERGE_ASSETS")
args.add("--")
args.add("--assetsOutput", out_assets_zip)
Expand All @@ -519,10 +532,13 @@ def _merge_assets(
join_with = "&",
)

_java.run(
_disable_warnings(ctx, args)

_java_run(
ctx = ctx,
host_javabase = host_javabase,
executable = busybox,
tools = [busybox],
arguments = [args],
inputs = depset(
assets + [symbols],
Expand Down Expand Up @@ -573,7 +589,8 @@ def _validate_and_link(

# Retrieves the list of files at runtime when a directory is passed.
args = ctx.actions.args()
args.use_param_file("@%s")
args.use_param_file("@%s", use_always=True)
args.set_param_file_format("multiline")
args.add("--tool", "LINK_STATIC_LIBRARY")
args.add("--")
args.add("--aapt2", aapt.executable)
Expand Down Expand Up @@ -604,7 +621,9 @@ def _validate_and_link(
)
input_files.extend(resource_apks)

_java.run(
_disable_warnings(ctx, args)

_java_run(
ctx = ctx,
host_javabase = host_javabase,
executable = busybox,
Expand Down Expand Up @@ -644,7 +663,8 @@ def _compile(

# Retrieves the list of files at runtime when a directory is passed.
args = ctx.actions.args()
args.use_param_file("@%s")
args.use_param_file("@%s", use_always=True)
args.set_param_file_format("multiline")
args.add("--tool", "COMPILE_LIBRARY_RESOURCES")
args.add("--")
args.add("--aapt2", aapt.executable)
Expand All @@ -658,7 +678,9 @@ def _compile(
)
args.add("--output", out_file)

_java.run(
_disable_warnings(ctx, args)

_java_run(
ctx = ctx,
host_javabase = host_javabase,
executable = busybox,
Expand Down Expand Up @@ -720,7 +742,8 @@ def _merge_compiled(
transitive_input_files = []

args = ctx.actions.args()
args.use_param_file("@%s")
args.use_param_file("@%s", use_always=True)
args.set_param_file_format("multiline")
args.add("--tool", "MERGE_COMPILED")
args.add("--")
args.add("--classJarOutput", out_class_jar)
Expand Down Expand Up @@ -759,10 +782,13 @@ def _merge_compiled(
)
transitive_input_files.append(transitive_compiled_resources)

_java.run(
_disable_warnings(ctx, args)

_java_run(
ctx = ctx,
host_javabase = host_javabase,
executable = busybox,
tools = [busybox],
arguments = [args],
inputs = depset(input_files, transitive = transitive_input_files),
outputs = output_files,
Expand All @@ -771,6 +797,13 @@ def _merge_compiled(
"Merging compiled Android Resources in " + out_class_jar.short_path,
)

def _java_run(ctx, *args, **kwargs):
enable_workers = ctx.fragments.android.persistent_android_resource_processor
multiplex_workers = ctx.fragments.android.persistent_multiplex_android_resource_processor
kwargs["supports_workers"] = enable_workers or multiplex_workers
kwargs["supports_multiplex_workers"] = multiplex_workers
_java.run(ctx, *args, **kwargs)

def _escape_mv(s):
"""Escapes `:` and `,` in manifest values so they can be used as a busybox flag."""
return s.replace(":", "\\:").replace(",", "\\,")
Expand Down Expand Up @@ -829,6 +862,7 @@ def _merge_manifests(
# Args for busybox
args = ctx.actions.args()
args.use_param_file("@%s", use_always = True)
args.set_param_file_format("multiline")
args.add("--tool", "MERGE_MANIFEST")
args.add("--")
if manifest:
Expand All @@ -851,10 +885,13 @@ def _merge_manifests(
args.add("--log", out_log_file)
outputs.append(out_log_file)

_java.run(
_disable_warnings(ctx, args)

_java_run(
ctx = ctx,
host_javabase = host_javabase,
executable = busybox,
tools = [busybox],
arguments = [args],
inputs = depset(directs, transitive = transitives),
outputs = outputs,
Expand Down Expand Up @@ -894,14 +931,18 @@ def _process_databinding(
res_dirs = _get_unique_res_dirs(resource_files)

args = ctx.actions.args()
args.use_param_file("@%s", use_always=True)
args.set_param_file_format("multiline")
args.add("--tool", "PROCESS_DATABINDING")
args.add("--")
args.add("--output_resource_directory", databinding_resources_dirname)
args.add_all(res_dirs, before_each = "--resource_root")
args.add("--dataBindingInfoOut", out_databinding_info)
args.add("--appId", java_package)

_java.run(
_disable_warnings(ctx, args)

_java_run(
ctx = ctx,
host_javabase = host_javabase,
executable = busybox,
Expand Down Expand Up @@ -948,6 +989,8 @@ def _generate_binary_r(
host_javabase: A Target. The host javabase.
"""
args = ctx.actions.args()
args.use_param_file("@%s", use_always=True)
args.set_param_file_format("multiline")
args.add("--tool", "GENERATE_BINARY_R")
args.add("--")
args.add("--primaryRTxt", r_txt)
Expand All @@ -967,12 +1010,14 @@ def _generate_binary_r(
# TODO(b/154003916): support transitive "--library transitive_r_txt_path,transitive_manifest_path" flags
args.add("--classJarOutput", out_class_jar)
args.add("--targetLabel", str(ctx.label))
args.use_param_file("@%s")

_java.run(
_disable_warnings(ctx, args)

_java_run(
ctx = ctx,
host_javabase = host_javabase,
executable = busybox,
tools = [busybox],
arguments = [args],
inputs = depset([r_txt, manifest], transitive = transitive_r_txts + transitive_manifests),
outputs = [out_class_jar],
Expand Down Expand Up @@ -1012,6 +1057,8 @@ def _make_aar(
when a resource conflict occurs.
"""
args = ctx.actions.args()
args.use_param_file("@%s", use_always=True)
args.set_param_file_format("multiline")
args.add("--tool", "GENERATE_AAR")
args.add("--")
args.add(
Expand All @@ -1031,10 +1078,13 @@ def _make_aar(
if should_throw_on_conflict:
args.add("--throwOnResourceConflict")

_java.run(
_disable_warnings(ctx, args)

_java_run(
ctx = ctx,
host_javabase = host_javabase,
executable = busybox,
tools = [busybox],
arguments = [args],
inputs = (
resource_files +
Expand Down
12 changes: 11 additions & 1 deletion rules/java.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,17 @@ def _run(

args["arguments"] = jvm_flags + [jar_args] + args.get("arguments", default = [])

ctx.actions.run(**args)
mnemonic = args.get("mnemonic")
supports_workers = args.pop("supports_workers", False)
supports_multiplex_workers = args.pop("supports_multiplex_workers", False)
execution_requirements = args.pop("execution_requirements", dict())
if supports_workers:
execution_requirements["worker-key-mnemonic"] = mnemonic
execution_requirements["supports-workers"] = "1"
if supports_multiplex_workers:
execution_requirements["supports-multiplex-workers"] = "1"

ctx.actions.run(execution_requirements = execution_requirements, **args)

java = struct(
compile = _compile,
Expand Down