From f25ead68e872c35c68a210ca32b74b0efb0e4218 Mon Sep 17 00:00:00 2001 From: Matt Clarkson Date: Wed, 27 Nov 2024 09:55:58 +0000 Subject: [PATCH 1/3] fix(genrule): remove quoting of `$(OUTS)`/`$(SRCS)` As per the documentation for the [predefined variables]: > If the filenames corresponding to the input labels or the output filenames contain spaces, `'`, or other special characters...then `$(SRCS)` and `$(OUTS)` are not suitable for interpolation into a command line, as they do not have the semantics that `"${@}"` would in Bash. [predefined variables]: https://bazel.build/reference/be/make-variables#predefined_genrule_variables --- labgrid/genrule/rule.bzl | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/labgrid/genrule/rule.bzl b/labgrid/genrule/rule.bzl index 6edde1d6..296291a8 100644 --- a/labgrid/genrule/rule.bzl +++ b/labgrid/genrule/rule.bzl @@ -1,4 +1,3 @@ -load("@bazel_skylib//lib:shell.bzl", "shell") load("//labgrid/cfg:store.bzl", _cfg = "store") visibility("//...") @@ -49,8 +48,8 @@ ATTRS = { } def _substitutions(ctx): - outs = [shell.quote(o.path) for o in ctx.outputs.outs] - srcs = [shell.quote(o.path) for o in ctx.files.srcs] + outs = [o.path for o in ctx.outputs.outs] + srcs = [o.path for o in ctx.files.srcs] # https://bazel.build/reference/be/make-variables#predefined_genrule_variables substitutions = { -- GitLab From 0a5a495448165b1528e0dae7db73184a6020b238 Mon Sep 17 00:00:00 2001 From: Matt Clarkson Date: Wed, 27 Nov 2024 09:58:32 +0000 Subject: [PATCH 2/3] fix(run): remove `BAZEL_GEN_DIR` base for `--get` The ideal was that this would use the generation directory as the base for the local path to ease the definition of `--get`. However, `BAZEL_GEN_DIR` does not exist...and never has. It was confused with `$(RULEDIR)` which is only a Make variable and never enters the environment. Removing as this could lead to Hyrum's law and start being depended on. --- labgrid/run/run.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/labgrid/run/run.py b/labgrid/run/run.py index 78ccee71..aed99901 100644 --- a/labgrid/run/run.py +++ b/labgrid/run/run.py @@ -247,7 +247,7 @@ class Get: def resolve(self, remote: PurePath) -> Get: return Get( remote=self.join(remote, self.remote), - local=self.join(environ.get("BAZEL_GEN_DIR", Path.cwd()), self.local), + local=self.join(Path.cwd(), self.local), optional=self.optional, ) -- GitLab From a60fec2e654e34a89cb943d7feeb226464f8f4e7 Mon Sep 17 00:00:00 2001 From: Matt Clarkson Date: Tue, 5 Nov 2024 17:05:44 +0000 Subject: [PATCH 3/3] feat(genrule): add `$(RULEDIR)`/`$D` Make variable To be compatible with built-in `genrule`[1] [1]: https://bazel.build/reference/be/make-variables#predefined_genrule_variables --- e2e/localhost/BUILD.bazel | 38 ++++++++++++++++++++++++++++++++++++++ labgrid/genrule/rule.bzl | 5 +++++ 2 files changed, 43 insertions(+) diff --git a/e2e/localhost/BUILD.bazel b/e2e/localhost/BUILD.bazel index 5a261932..f739ea8c 100644 --- a/e2e/localhost/BUILD.bazel +++ b/e2e/localhost/BUILD.bazel @@ -37,6 +37,41 @@ labgrid_genrule( tools = ["@rules_labgrid//labgrid/run"], ) +labgrid_genrule( + name = "hello-and-transfer-to-ruledir", + # We need Bash to redirect to a file on the target + srcs = ["@ape//ape:bash"], + outs = ["run-and-transfer-to-ruledir.actual"], + cmd = "$(location @rules_labgrid//labgrid/run) --get output.txt:$(RULEDIR)/run-and-transfer-to-ruledir.actual $(location @ape//ape:bash) -c 'echo world > output.txt'", + platform = "@rules_labgrid//platform:localhost", + tools = ["@rules_labgrid//labgrid/run"], +) + +labgrid_genrule( + name = "hello-and-transfer-to-d", + # We need Bash to redirect to a file on the target + srcs = ["@ape//ape:bash"], + outs = ["run-and-transfer-to-d.actual"], + cmd = "$(location @rules_labgrid//labgrid/run) --get output.txt:$D/run-and-transfer-to-d.actual $(location @ape//ape:bash) -c 'echo world > output.txt'", + platform = "@rules_labgrid//platform:localhost", + tools = ["@rules_labgrid//labgrid/run"], +) + +labgrid_genrule( + name = "hello-and-transfer-to-nested-d", + # We need Bash to redirect to a file on the target + srcs = ["@ape//ape:bash"], + outs = ["nested/run-and-transfer-to-d.actual"], + cmd = "$(location @rules_labgrid//labgrid/run) --get output.txt:$D/run-and-transfer-to-d.actual $(location @ape//ape:bash) -c 'echo world > output.txt'", + platform = "@rules_labgrid//platform:localhost", + tools = ["@rules_labgrid//labgrid/run"], +) + +alias( + name = "run-and-transfer-to-nested-d.actual", + actual = "nested/run-and-transfer-to-d.actual", +) + labgrid_genrule( name = "hello-with-env", srcs = ["@ape//ape:printenv"], @@ -72,6 +107,9 @@ labgrid_run_binary( "run", "run-with-runfiles", "run-and-transfer-file-back", + "run-and-transfer-to-ruledir", + "run-and-transfer-to-d", + "run-and-transfer-to-nested-d", "run-with-env", "run-binary", ) diff --git a/labgrid/genrule/rule.bzl b/labgrid/genrule/rule.bzl index 296291a8..827ffcfd 100644 --- a/labgrid/genrule/rule.bzl +++ b/labgrid/genrule/rule.bzl @@ -1,3 +1,4 @@ +load("@bazel_skylib//lib:paths.bzl", "paths") load("//labgrid/cfg:store.bzl", _cfg = "store") visibility("//...") @@ -55,6 +56,7 @@ def _substitutions(ctx): substitutions = { "$(OUTS)": " ".join(outs), "$(SRCS)": " ".join(srcs), + "$(RULEDIR)": paths.join(ctx.bin_dir.path, ctx.label.package), } if len(srcs) == 1: @@ -64,8 +66,11 @@ def _substitutions(ctx): if len(outs) == 1: substitutions["$@"] = outs[0] + substitutions["$D"] = paths.dirname(outs[0]) elif "$@" in ctx.attr.cmd: fail("Cannot specify `$@` Make variable with zero or multiple `outs`") + else: + substitutions["$D"] = paths.join(ctx.bin_dir.path, ctx.label.package) for key, value in ctx.var.items(): substitutions["$({})".format(key)] = value -- GitLab