From 93d90afde4f02de7475eb29a89c9132d952c71eb Mon Sep 17 00:00:00 2001 From: Xuming Meng Date: Fri, 18 Oct 2024 14:06:09 +0200 Subject: [PATCH 1/3] feat(config): remove executor args Instead of removing args, args is moved to data dependency and use rlocation to locate the fixed arg file. --- labgrid/BUILD.bazel | 2 +- labgrid/config/deps.bzl | 2 +- labgrid/executor/BUILD.bazel | 7 +++++++ labgrid/executor/args.bzl | 16 ++++++++-------- labgrid/executor/executor.py | 4 ++++ labgrid/genrule/rule.bzl | 8 -------- labgrid/run/binary/rule.bzl | 9 +-------- 7 files changed, 22 insertions(+), 26 deletions(-) diff --git a/labgrid/BUILD.bazel b/labgrid/BUILD.bazel index c2f98900..a636ef4e 100644 --- a/labgrid/BUILD.bazel +++ b/labgrid/BUILD.bazel @@ -1,5 +1,5 @@ -load("@rules_python//python:defs.bzl", "py_test") load("@python_versions//3.11:defs.bzl", "compile_pip_requirements") +load("@rules_python//python:defs.bzl", "py_test") # TODO: generate for more Python interpreter versions and platforms # `rules_uv` would be an easier way to do this diff --git a/labgrid/config/deps.bzl b/labgrid/config/deps.bzl index e0e33c55..9fb9d0e6 100644 --- a/labgrid/config/deps.bzl +++ b/labgrid/config/deps.bzl @@ -1,5 +1,5 @@ -load("//labgrid/cfg:unstore.bzl", _cfg = "unstore") load("@rules_python//python:defs.bzl", "PyInfo") +load("//labgrid/cfg:unstore.bzl", _cfg = "unstore") visibility("//labgrid/config/...") diff --git a/labgrid/executor/BUILD.bazel b/labgrid/executor/BUILD.bazel index bc238473..3b16810a 100644 --- a/labgrid/executor/BUILD.bazel +++ b/labgrid/executor/BUILD.bazel @@ -10,6 +10,7 @@ py_binary( name = "executor", srcs = ["executor.py"], data = [ + ":args", "//labgrid/config:data", "//labgrid/config:tools", ], @@ -42,6 +43,9 @@ py_test( "echo", "$${DATA}", ], + data = [ + ":args", + ], main = "executor.py", deps = ["host"], ) @@ -58,6 +62,9 @@ py_test( "echo", "$${DATA}", ], + data = [ + ":args", + ], main = "executor.py", deps = ["host"], ) diff --git a/labgrid/executor/args.bzl b/labgrid/executor/args.bzl index e09419c6..fb98bbd3 100644 --- a/labgrid/executor/args.bzl +++ b/labgrid/executor/args.bzl @@ -24,19 +24,19 @@ def _import_path(short_path): return ".".join(parts) def implementation(ctx): - output = ctx.actions.declare_file("executor-args.txt") - - lines = [] + arguments = ctx.actions.declare_file(ctx.label.name) + args = ctx.actions.args() for m in ctx.files._managers: - lines.extend(["--manager", "{}:{}".format(_import_path(m.short_path), "manager")]) + args.add("--manager", "{}:{}".format(_import_path(m.short_path), "manager")) for name, value in ctx.attr._env[EnvironmentInfo].environment.items(): - lines.extend(["--env", name, value]) - - ctx.actions.write(output, "\n".join(lines)) - return DefaultInfo(files = depset([output])) + args.add_all("--env", [name, value]) + ctx.actions.write(arguments, content = args) + files = depset([arguments]) + return DefaultInfo(files = files) args = rule( doc = DOC, attrs = ATTRS, implementation = implementation, + provides = [DefaultInfo], ) diff --git a/labgrid/executor/executor.py b/labgrid/executor/executor.py index ba7d34c7..49d5e075 100644 --- a/labgrid/executor/executor.py +++ b/labgrid/executor/executor.py @@ -72,6 +72,10 @@ def main(exe: Path, *args: str) -> int: arguments(prsr) + runfiles = Runfiles.Create() + baked = Path(runfiles.Rlocation("rules_labgrid/labgrid/executor/args")) + args = (f"@{baked}", *args) + parsed = prsr.parse_args(args) cmd = (parsed.program, *parsed.arguments) diff --git a/labgrid/genrule/rule.bzl b/labgrid/genrule/rule.bzl index 50c27211..0713fa58 100644 --- a/labgrid/genrule/rule.bzl +++ b/labgrid/genrule/rule.bzl @@ -40,12 +40,6 @@ ATTRS = { executable = True, cfg = "exec", ), - "_executor_args": attr.label( - allow_single_file = True, - doc = "CLI args for the Labgrid environment executor", - default = "//labgrid/executor:args", - cfg = "exec", - ), "_sh": attr.label( doc = "The Shell interpreter", default = "@ape//:bash", @@ -85,7 +79,6 @@ def implementation(ctx): cmd = cmd.replace(key, value) args = ctx.actions.args() - args.add(ctx.file._executor_args, format = "@%s") args.add("--") args.add(ctx.executable._sh) args.add("-c") @@ -94,7 +87,6 @@ def implementation(ctx): ctx.actions.run( executable = ctx.executable._executor, arguments = [args], - inputs = [ctx.file._executor_args], outputs = ctx.outputs.outs, tools = [t.files_to_run for t in (ctx.attr.tools + ctx.attr.srcs)] + [ctx.executable._sh], use_default_shell_env = False, diff --git a/labgrid/run/binary/rule.bzl b/labgrid/run/binary/rule.bzl index f46d0292..28ea2726 100644 --- a/labgrid/run/binary/rule.bzl +++ b/labgrid/run/binary/rule.bzl @@ -46,12 +46,6 @@ ATTRS = { executable = True, cfg = "exec", ), - "_executor_args": attr.label( - allow_single_file = True, - doc = "CLI args for the Labgrid environment executor", - default = "//labgrid/executor:args", - cfg = "exec", - ), } def implementation(ctx): @@ -77,14 +71,13 @@ def implementation(ctx): } args = ctx.actions.args() - args.add(ctx.file._executor_args, format = "@%s") args.add("--") args.add(ctx.executable.tool) args.add_all(expanded_args) ctx.actions.run( outputs = ctx.outputs.outs, - inputs = ctx.files.srcs + [ctx.file._executor_args], + inputs = ctx.files.srcs, tools = [ctx.executable.tool], executable = ctx.executable._executor, arguments = [args], -- GitLab From fd3b6510164147176a1f8f2c3e342e3d3f6d332d Mon Sep 17 00:00:00 2001 From: Matt Clarkson Date: Mon, 28 Oct 2024 16:42:15 +0000 Subject: [PATCH 2/3] refactor(labgrid/executor): use `args.add_all` with visitor Loop over the collections lazily. --- labgrid/executor/args.bzl | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/labgrid/executor/args.bzl b/labgrid/executor/args.bzl index fb98bbd3..a7ee8eb9 100644 --- a/labgrid/executor/args.bzl +++ b/labgrid/executor/args.bzl @@ -17,19 +17,24 @@ ATTRS = { ), } -def _import_path(short_path): +def _import(short_path): path = short_path.removesuffix(".py") parts = path.split("/") parts = parts[2:] if parts[0] == ".." else parts return ".".join(parts) +def _managers(manager): + return ("--manager", "{}:manager".format(_import(manager.short_path))) + +def _env(pair): + key, value = pair + return ["--env", key, value] + def implementation(ctx): arguments = ctx.actions.declare_file(ctx.label.name) args = ctx.actions.args() - for m in ctx.files._managers: - args.add("--manager", "{}:{}".format(_import_path(m.short_path), "manager")) - for name, value in ctx.attr._env[EnvironmentInfo].environment.items(): - args.add_all("--env", [name, value]) + args.add_all(ctx.files._managers, map_each = _managers) + args.add_all(ctx.attr._env[EnvironmentInfo].environment.items(), map_each = _env) ctx.actions.write(arguments, content = args) files = depset([arguments]) return DefaultInfo(files = files) -- GitLab From 882ce5bbd9d4758cc4d94e08e93d6efe8c0dc41c Mon Sep 17 00:00:00 2001 From: Matt Clarkson Date: Mon, 28 Oct 2024 16:50:55 +0000 Subject: [PATCH 3/3] test: skip `//labgrid/executor` tests on `arm64-linux` targets We do not, at this time, have a registred toolchain for the LabGrid configuation against `arm64-linux`. This means that `:args` does not resolve the correct files for when doing toolchain resolved. For now, skip the tests. --- labgrid/executor/BUILD.bazel | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/labgrid/executor/BUILD.bazel b/labgrid/executor/BUILD.bazel index 3b16810a..f4e357d4 100644 --- a/labgrid/executor/BUILD.bazel +++ b/labgrid/executor/BUILD.bazel @@ -47,6 +47,11 @@ py_test( ":args", ], main = "executor.py", + # TODO: remove this when we have a default LabGrid configuration toolchain for `arm64-linux` + target_compatible_with = [ + "@platforms//cpu:x86_64", + "@platforms//os:linux", + ], deps = ["host"], ) @@ -66,5 +71,10 @@ py_test( ":args", ], main = "executor.py", + # TODO: remove this when we have a default LabGrid configuration toolchain for `arm64-linux` + target_compatible_with = [ + "@platforms//cpu:x86_64", + "@platforms//os:linux", + ], deps = ["host"], ) -- GitLab