From ca2cd05f851659b06c0586409ee7eb2366a72cce Mon Sep 17 00:00:00 2001 From: Alex Tercete Date: Tue, 1 Oct 2024 21:32:07 +0100 Subject: [PATCH 1/4] refactor(executor): pass env vars in generic way BREAKING CHANGE: `state` has been replaced by the more generic `env` `labgrid_config` now takes `env` instead of `state`. As a consequence, `//labgrid/config:state` has been replaced with `//labgrid/config:env` (which is private). --- labgrid/config/BUILD.bazel | 8 ++++---- labgrid/config/EnvironmentInfo.bzl | 15 +++++++++++++++ labgrid/config/{state.bzl => env.bzl} | 10 +++++----- labgrid/config/providers.bzl | 6 ------ labgrid/config/rule.bzl | 12 +++++++----- labgrid/config/toolchain/macro.bzl | 4 +++- labgrid/executor/BUILD.bazel | 2 -- labgrid/executor/args.bzl | 19 +++++++++---------- labgrid/executor/executor.py | 14 ++++++++------ 9 files changed, 51 insertions(+), 39 deletions(-) create mode 100644 labgrid/config/EnvironmentInfo.bzl rename labgrid/config/{state.bzl => env.bzl} (59%) delete mode 100644 labgrid/config/providers.bzl diff --git a/labgrid/config/BUILD.bazel b/labgrid/config/BUILD.bazel index a131e87a..8ec81409 100644 --- a/labgrid/config/BUILD.bazel +++ b/labgrid/config/BUILD.bazel @@ -1,6 +1,6 @@ load(":deps.bzl", "deps") +load(":env.bzl", "env") load(":src.bzl", "src") -load(":state.bzl", "state") src( name = "src", @@ -13,9 +13,9 @@ alias( visibility = ["//visibility:public"], ) -state( - name = "state", - visibility = ["//visibility:public"], +env( + name = "env", + visibility = ["//:__subpackages__"], ) deps( diff --git a/labgrid/config/EnvironmentInfo.bzl b/labgrid/config/EnvironmentInfo.bzl new file mode 100644 index 00000000..f8c10b50 --- /dev/null +++ b/labgrid/config/EnvironmentInfo.bzl @@ -0,0 +1,15 @@ +load("@bazel_skylib//lib:types.bzl", "types") + +visibility("//...") + +def init(environment): + if not types.is_dict(environment): + fail("`EnvironmentInfo.environment` must be a dict: {}".format(environment)) + + return {"environment": environment} + +EnvironmentInfo, _ = provider( + doc = "A map of string keys and values that represent environment variables", + fields = ["environment"], + init = init, +) diff --git a/labgrid/config/state.bzl b/labgrid/config/env.bzl similarity index 59% rename from labgrid/config/state.bzl rename to labgrid/config/env.bzl index 29f75ea6..4b0e6d19 100644 --- a/labgrid/config/state.bzl +++ b/labgrid/config/env.bzl @@ -1,21 +1,21 @@ load("//labgrid/cfg:unstore.bzl", _cfg = "unstore") -load(":providers.bzl", "StateInfo") +load(":EnvironmentInfo.bzl", "EnvironmentInfo") visibility("//labgrid/config/...") -DOC = "Unpacks the `state` from the `//labgrid/toolchain/config:type` toolchain" +DOC = "Unpacks the `env` from the `//labgrid/toolchain/config:type` toolchain" ATTRS = {} def implementation(ctx): toolchain = ctx.toolchains["//labgrid/toolchain/config:type"] - return StateInfo(value = toolchain.state) + return EnvironmentInfo(environment = toolchain.env) -state = rule( +env = rule( doc = DOC, attrs = ATTRS, implementation = implementation, - provides = [StateInfo], + provides = [EnvironmentInfo], toolchains = ["//labgrid/toolchain/config:type"], cfg = _cfg, ) diff --git a/labgrid/config/providers.bzl b/labgrid/config/providers.bzl deleted file mode 100644 index 6b694d3a..00000000 --- a/labgrid/config/providers.bzl +++ /dev/null @@ -1,6 +0,0 @@ -visibility("//...") - -StateInfo = provider( - doc = "A wrapper around the LabGrid state string value.", - fields = ["value"], -) diff --git a/labgrid/config/rule.bzl b/labgrid/config/rule.bzl index b78a34f5..b7e378cd 100644 --- a/labgrid/config/rule.bzl +++ b/labgrid/config/rule.bzl @@ -9,7 +9,9 @@ Binds a LabGrid configuration file to the associated data required to setup, loa labgrid_config( name = "config", src = "docker.yaml", - state = "accessible", + env = { + "LG_STATE": "accessible" + }, deps = ["@pip//:docker"], ) @@ -51,14 +53,14 @@ ATTRS = { doc = "The LabGrid configuration YAML.", allow_single_file = [".yml", ".yaml"], ), - "state": attr.string( - doc = "The state to transition the LabGrid strategy to.", - ), "deps": attr.label_list( doc = "LabGrid configuration Python dependencies.", providers = [[DefaultInfo, PyInfo]], cfg = "exec", ), + "env": attr.string_dict( + doc = "The environment variables to set.", + ), } def _forward(target): @@ -71,8 +73,8 @@ def implementation(ctx): default = DefaultInfo(files = depset([ctx.file.src])) toolchain = platform_common.ToolchainInfo( src = ctx.file.src, - state = ctx.attr.state, deps = _flatten([_forward(d) for d in ctx.attr.deps]), + env = ctx.attr.env, ) return [default, toolchain] diff --git a/labgrid/config/toolchain/macro.bzl b/labgrid/config/toolchain/macro.bzl index 4b622152..cba6cc29 100644 --- a/labgrid/config/toolchain/macro.bzl +++ b/labgrid/config/toolchain/macro.bzl @@ -8,7 +8,9 @@ def labgrid_config_toolchain(*, name, src, state, manager, target_compatible_wit name = "{}-config".format(name), src = native.package_relative_label(src), deps = [native.package_relative_label(d) for d in deps], - state = state, + env = { + "LG_STATE": state, + }, ) labgrid_manager( diff --git a/labgrid/executor/BUILD.bazel b/labgrid/executor/BUILD.bazel index 992d1ac9..25b3c110 100644 --- a/labgrid/executor/BUILD.bazel +++ b/labgrid/executor/BUILD.bazel @@ -34,8 +34,6 @@ py_test( args = [ "--manager", "labgrid.executor.host:manager", - "--state", - "any", "echo", "$${DATA}", ], diff --git a/labgrid/executor/args.bzl b/labgrid/executor/args.bzl index 8cff4535..730f019b 100644 --- a/labgrid/executor/args.bzl +++ b/labgrid/executor/args.bzl @@ -1,4 +1,4 @@ -load("//labgrid/config:providers.bzl", "StateInfo") +load("//labgrid/config:EnvironmentInfo.bzl", "EnvironmentInfo") visibility("//labgrid/executor/...") @@ -11,21 +11,20 @@ ATTRS = { default = "//labgrid/manager:src", cfg = "exec", ), - "_state": attr.label( - doc = "The state to transition the LabGrid strategy to", - default = "//labgrid/config:state", + "_env": attr.label( + doc = "The environment variables to set", + default = "//labgrid/config:env", cfg = "exec", ), } def implementation(ctx): output = ctx.actions.declare_file("executor-args.txt") - lines = [ - "--manager", - ctx.file._manager.short_path, - "--state", - ctx.attr._state[StateInfo].value, - ] + + lines = ["--manager", ctx.file._manager.short_path] + 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])) diff --git a/labgrid/executor/executor.py b/labgrid/executor/executor.py index ba2f1c93..71318597 100644 --- a/labgrid/executor/executor.py +++ b/labgrid/executor/executor.py @@ -54,8 +54,12 @@ def arguments(prsr: ArgumentParser) -> None: type=load, ) prsr.add_argument( - "--state", - help="The state to transition the LabGrid strategy to.", + "--env", + action="append", + metavar=("NAME", "VALUE"), + nargs=2, + help="An environment variable to set when running the binary. Can be provided multiple times.", + default=[], ) @@ -83,10 +87,8 @@ def main(exe: Path, *args: str) -> int: cmd = (which("sh"), *cmd) with parsed.manager() as data: - env = {k: v for k, v in data.env.items() if k != "RUNFILES_DIR"} - # TODO: Move this to a manager - if parsed.state is not None: - env |= {"LG_STATE": parsed.state} + env = {k: v for k, v in parsed.env} + env |= {k: v for k, v in data.env.items() if k != "RUNFILES_DIR"} try: run(cmd, env=env, check=True) except CalledProcessError as e: -- GitLab From 0b5c22ae3d0b44335631f7b496fdd7ce0c4419df Mon Sep 17 00:00:00 2001 From: Alex Tercete Date: Fri, 4 Oct 2024 13:37:57 +0100 Subject: [PATCH 2/4] refactor(executor): pass config as env var The executor now supports passing env vars which have runfile location paths as their values. These can be passed to `labgrid_config` by using `$(rlocationpath)` pointing to files provided under `data`. --- labgrid/config/BUILD.bazel | 6 ++++++ labgrid/config/data.bzl | 20 ++++++++++++++++++++ labgrid/config/rule.bzl | 15 +++++++++++++-- labgrid/config/toolchain/macro.bzl | 4 +++- labgrid/executor/BUILD.bazel | 2 ++ labgrid/executor/executor.py | 9 ++++++++- 6 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 labgrid/config/data.bzl diff --git a/labgrid/config/BUILD.bazel b/labgrid/config/BUILD.bazel index 8ec81409..8d71a115 100644 --- a/labgrid/config/BUILD.bazel +++ b/labgrid/config/BUILD.bazel @@ -1,3 +1,4 @@ +load(":data.bzl", "data") load(":deps.bzl", "deps") load(":env.bzl", "env") load(":src.bzl", "src") @@ -18,6 +19,11 @@ env( visibility = ["//:__subpackages__"], ) +data( + name = "data", + visibility = ["//:__subpackages__"], +) + deps( name = "deps", visibility = ["//visibility:public"], diff --git a/labgrid/config/data.bzl b/labgrid/config/data.bzl new file mode 100644 index 00000000..b51aaf69 --- /dev/null +++ b/labgrid/config/data.bzl @@ -0,0 +1,20 @@ +load("//labgrid/cfg:unstore.bzl", _cfg = "unstore") + +visibility("//labgrid/config/...") + +DOC = "Unpacks the `data` from the `//labgrid/toolchain/config:type` toolchain" + +ATTRS = {} + +def implementation(ctx): + toolchain = ctx.toolchains["//labgrid/toolchain/config:type"] + return DefaultInfo(files = toolchain.data) + +data = rule( + doc = DOC, + attrs = ATTRS, + implementation = implementation, + provides = [DefaultInfo], + toolchains = ["//labgrid/toolchain/config:type"], + cfg = _cfg, +) diff --git a/labgrid/config/rule.bzl b/labgrid/config/rule.bzl index b7e378cd..29ab04d6 100644 --- a/labgrid/config/rule.bzl +++ b/labgrid/config/rule.bzl @@ -11,7 +11,9 @@ labgrid_config( src = "docker.yaml", env = { "LG_STATE": "accessible" + "LG_ENV": "$(location :config.yaml)" }, + data = ["config.yaml"] deps = ["@pip//:docker"], ) @@ -59,7 +61,13 @@ ATTRS = { cfg = "exec", ), "env": attr.string_dict( - doc = "The environment variables to set.", + doc = "The environment variables to set. Subject to `$(location)` substitution.", + ), + "data": attr.label_list( + allow_files = True, + doc = "The data files to use.", + providers = [DefaultInfo], + cfg = "exec", ), } @@ -70,11 +78,14 @@ def _flatten(iterable): return [e for x in iterable for e in x] def implementation(ctx): + data = ctx.attr.data + [ctx.attr.src] + env = {k: ctx.expand_location(v, targets = data) for k, v in ctx.attr.env.items()} default = DefaultInfo(files = depset([ctx.file.src])) toolchain = platform_common.ToolchainInfo( src = ctx.file.src, deps = _flatten([_forward(d) for d in ctx.attr.deps]), - env = ctx.attr.env, + env = env, + data = depset(transitive = [d.files for d in data]), ) return [default, toolchain] diff --git a/labgrid/config/toolchain/macro.bzl b/labgrid/config/toolchain/macro.bzl index cba6cc29..a13cda76 100644 --- a/labgrid/config/toolchain/macro.bzl +++ b/labgrid/config/toolchain/macro.bzl @@ -4,11 +4,13 @@ load("@rules_labgrid//labgrid/manager:defs.bzl", "labgrid_manager") visibility("//...") def labgrid_config_toolchain(*, name, src, state, manager, target_compatible_with, deps = ()): + src = native.package_relative_label(src) labgrid_config( name = "{}-config".format(name), - src = native.package_relative_label(src), + src = src, deps = [native.package_relative_label(d) for d in deps], env = { + "LG_ENV": "$(rlocationpath {})".format(src), "LG_STATE": state, }, ) diff --git a/labgrid/executor/BUILD.bazel b/labgrid/executor/BUILD.bazel index 25b3c110..7fbfa4fa 100644 --- a/labgrid/executor/BUILD.bazel +++ b/labgrid/executor/BUILD.bazel @@ -9,11 +9,13 @@ args( py_binary( name = "executor", srcs = ["executor.py"], + data = ["//labgrid/config:data"], tags = ["manual"], visibility = ["//:__subpackages__"], deps = [ "//bazel/labgrid/executor", "//labgrid/manager", + "@rules_python//python/runfiles", ], ) diff --git a/labgrid/executor/executor.py b/labgrid/executor/executor.py index 71318597..b5a0738b 100644 --- a/labgrid/executor/executor.py +++ b/labgrid/executor/executor.py @@ -13,6 +13,7 @@ from shutil import which from subprocess import CalledProcessError, run from sys import argv, stderr +from python.runfiles import Runfiles from bazel.labgrid.executor.manager import Manager @@ -87,7 +88,7 @@ def main(exe: Path, *args: str) -> int: cmd = (which("sh"), *cmd) with parsed.manager() as data: - env = {k: v for k, v in parsed.env} + env = {k: resolve_runfile(v) for k, v in parsed.env} env |= {k: v for k, v in data.env.items() if k != "RUNFILES_DIR"} try: run(cmd, env=env, check=True) @@ -98,6 +99,12 @@ def main(exe: Path, *args: str) -> int: return 0 +def resolve_runfile(value): + r = Runfiles.Create() + location = r.Rlocation(value) + if Path(location).exists(): + return location + return value def entry(): exit(main(Path(argv[0]), *argv[1:])) -- GitLab From 3963acb1022ede9c8224c0dbca8fcdc7a6a3c056 Mon Sep 17 00:00:00 2001 From: Alex Tercete Date: Fri, 4 Oct 2024 14:28:24 +0100 Subject: [PATCH 3/4] refactor: replace config manager with a passthrough The config manager was responsible for setting `LG_ENV`, but that's now done generically in `labgrid_config_toolchain`. As such, we can now make the `manager` parameter default to a passthrough. BREAKING CHANGE: `//labgrid/manager:config` has been replaced with `//labgrid/manager:passthrough`. --- e2e/docker/BUILD.bazel | 1 - labgrid/config/toolchain/macro.bzl | 2 +- labgrid/manager/BUILD.bazel | 10 +++------- labgrid/manager/config.py | 23 ----------------------- labgrid/manager/passthrough.py | 15 +++++++++++++++ 5 files changed, 19 insertions(+), 32 deletions(-) delete mode 100644 labgrid/manager/config.py create mode 100644 labgrid/manager/passthrough.py diff --git a/e2e/docker/BUILD.bazel b/e2e/docker/BUILD.bazel index 4c490900..802fd8e7 100644 --- a/e2e/docker/BUILD.bazel +++ b/e2e/docker/BUILD.bazel @@ -25,7 +25,6 @@ platform( labgrid_config_toolchain( name = "config", src = "local-ubuntu.16.04-gnu.yaml", - manager = "@rules_labgrid//labgrid/manager:config", state = "accessible", target_compatible_with = [ ":constraint", diff --git a/labgrid/config/toolchain/macro.bzl b/labgrid/config/toolchain/macro.bzl index a13cda76..5eb785bc 100644 --- a/labgrid/config/toolchain/macro.bzl +++ b/labgrid/config/toolchain/macro.bzl @@ -3,7 +3,7 @@ load("@rules_labgrid//labgrid/manager:defs.bzl", "labgrid_manager") visibility("//...") -def labgrid_config_toolchain(*, name, src, state, manager, target_compatible_with, deps = ()): +def labgrid_config_toolchain(*, name, src, state, target_compatible_with, manager = "@rules_labgrid//labgrid/manager:passthrough", deps = ()): src = native.package_relative_label(src) labgrid_config( name = "{}-config".format(name), diff --git a/labgrid/manager/BUILD.bazel b/labgrid/manager/BUILD.bazel index 3be3ebd8..dde0f194 100644 --- a/labgrid/manager/BUILD.bazel +++ b/labgrid/manager/BUILD.bazel @@ -2,15 +2,11 @@ load("@rules_python//python:defs.bzl", "py_library") load(":src.bzl", "src") py_library( - name = "config", - srcs = ["config.py"], - data = ["//labgrid/config"], + name = "passthrough", + srcs = ["passthrough.py"], tags = ["manual"], visibility = ["//visibility:public"], - deps = [ - "@rules_labgrid//bazel/labgrid/executor", - "@rules_python//python/runfiles", - ], + deps = ["@rules_labgrid//bazel/labgrid/executor"], ) src( diff --git a/labgrid/manager/config.py b/labgrid/manager/config.py deleted file mode 100644 index a50560ef..00000000 --- a/labgrid/manager/config.py +++ /dev/null @@ -1,23 +0,0 @@ -from __future__ import annotations - -from contextlib import contextmanager -from pathlib import Path -from typing import Iterator - -from python.runfiles import Runfiles - -from bazel.labgrid.executor.manager import Data - - -@contextmanager -def manager() -> Iterator[Data]: - """ - A LabGrid context manager that loads the resolved configuration into the `LG_ENV` environment variable. - """ - runfiles = Runfiles.Create() - config = runfiles.Rlocation("rules_labgrid/labgrid/config/src.yaml") - path = Path(config) - if not path.exists(): - raise FileNotFoundError(str(path)) - env = {"LG_ENV": str(path)} - yield Data(env=env) diff --git a/labgrid/manager/passthrough.py b/labgrid/manager/passthrough.py new file mode 100644 index 00000000..4f0fe402 --- /dev/null +++ b/labgrid/manager/passthrough.py @@ -0,0 +1,15 @@ +from __future__ import annotations + +from contextlib import contextmanager +from typing import Iterator + + +from bazel.labgrid.executor.manager import Data + + +@contextmanager +def manager() -> Iterator[Data]: + """ + A passthrough LabGrid context manager + """ + yield Data(env = {}) -- GitLab From bd1baa0cb455ee7ed841114ace7f0f6341eb048e Mon Sep 17 00:00:00 2001 From: Alex Tercete Date: Fri, 4 Oct 2024 15:29:43 +0100 Subject: [PATCH 4/4] chore: remove unused label It's no longer used, and probably won't need to be again. BREAKING CHANGE: `//labgrid/config:src` has been removed. --- labgrid/config/BUILD.bazel | 12 ------------ labgrid/config/src.bzl | 26 -------------------------- 2 files changed, 38 deletions(-) delete mode 100644 labgrid/config/src.bzl diff --git a/labgrid/config/BUILD.bazel b/labgrid/config/BUILD.bazel index 8d71a115..e618d630 100644 --- a/labgrid/config/BUILD.bazel +++ b/labgrid/config/BUILD.bazel @@ -1,18 +1,6 @@ load(":data.bzl", "data") load(":deps.bzl", "deps") load(":env.bzl", "env") -load(":src.bzl", "src") - -src( - name = "src", - visibility = ["//visibility:public"], -) - -alias( - name = "config", - actual = "src", - visibility = ["//visibility:public"], -) env( name = "env", diff --git a/labgrid/config/src.bzl b/labgrid/config/src.bzl deleted file mode 100644 index 0e010838..00000000 --- a/labgrid/config/src.bzl +++ /dev/null @@ -1,26 +0,0 @@ -load("//labgrid/cfg:unstore.bzl", _cfg = "unstore") -load("@rules_python//python:defs.bzl", "PyInfo") - -visibility("//labgrid/config/...") - -DOC = "Unpacks the `src` from the `//labgrid/toolchain/config:type` toolchain" - -ATTRS = {} - -def implementation(ctx): - toolchain = ctx.toolchains["//labgrid/toolchain/config:type"] - output = ctx.actions.declare_file("{}.yaml".format(ctx.label.name)) - ctx.actions.symlink( - target_file = toolchain.src, - output = output, - is_executable = False, - ) - return DefaultInfo(files = depset([output])) - -src = rule( - doc = DOC, - attrs = ATTRS, - implementation = implementation, - toolchains = ["//labgrid/toolchain/config:type"], - cfg = _cfg, -) -- GitLab