bazel-contrib / bazel-contrib/bazel-lib
run_binary does not include executable environment from RunEnvironmentInfo
- Dominant language
- Starlark
- Stars
- 182
- Forks
- 134
- Avg merge
- 1d 46m
- Merged PRs (30d)
- 1
Description
If the `tool` provided to `run_binary` has a `RunEnvironmentInfo` provider, that environment is not included when running the tool. For example, this is how `py_binary` from rules_py [sets provided environment variables](https://github.com/aspect-build/rules_py/blob/340ac788719a3013f08f63c442c986b9f55188b9/py/private/py_binary.bzl#L131-L134).
This basic patch includes the environment and allows envs specified directly to `run_binary` to override the ones provided by the tool:
```diff
diff --git lib/private/run_binary.bzl lib/private/run_binary.bzl
index b480a27..9f12b21 100644
--- lib/private/run_binary.bzl
+++ lib/private/run_binary.bzl
@@ -51,6 +51,8 @@ Possible fixes:
for a in ctx.attr.args:
args.add_all(split_args(expand_variables(ctx, ctx.expand_location(a, targets = ctx.attr.srcs), inputs = ctx.files.srcs, outs = outputs)))
envs = {}
+ if RunEnvironmentInfo in ctx.attr.tool:
+ envs = ctx.attr.tool[RunEnvironmentInfo].environment
for k, v in ctx.attr.env.items():
envs[k] = expand_variables(ctx, ctx.expand_location(v, targets = ctx.attr.srcs), inputs = ctx.files.srcs, outs = outputs, attribute_name = "env")
```
The patch is working for our use cases right now. Happy to submit it as a PR if the implementation seems generally correct.
Contributor guide
Research direction
Start in lib/private/run_binary.bzl at the run_binary implementation and inspect how the tool and direct env attributes are assembled. Done means the tool's RunEnvironmentInfo environment is included when present, while values specified directly on run_binary override it; verify this with a run_binary invocation covering both sources.
Written by the indexing model from the issue text.
Assessment
- Domain
- build-system, tooling
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 55/100