Add common envs attribute for *_binary and *_test rules #7624
Conversation
|
What does |
|
Hmm I don't see anything controversial here. @lberki wdyt? I can then proceed with the code review (quickly glancing over I'd use ImmutableMap, look into documenting the attribute (so people don't assume it's the env under which the target is built or smth), add more tests (e.g. for |
|
Friendly ping @lberki |
| @@ -83,6 +84,7 @@ | |||
| private final Artifact owningExecutable; | |||
| private final boolean createSymlinks; | |||
| private final CommandLine args; | |||
| private final Map<String, String > envs; | |||
There was a problem hiding this comment.
Make this an ImmutableMap; otherwise, RunfilesSupport would not be immutable.
| private static Map<String, String> computeEnvs( | ||
| RuleContext ruleContext) { | ||
| if (!ruleContext.getRule().isAttrDefined("envs", Type.STRING_DICT)) { | ||
| return new HashMap<>(); |
|
@lberki Without this PR, what's the recommended way to run a binary that relies on environment variables with
|
|
Friendly ping |
|
Looks like activity has stalled on this PR - please address the review comments and rebase this branch if you'd like to continue working on it. |
|
@blico Should we continue working on this PR? |
Addresses #7364
envsis astring_dictattribute common for all *_binary and *_test rules. Bazel will set the environment variables specified inenvsin the run/test environment before executing the given target. Variables specified in--test_envand--action_envwill take priority over theenvsattribute, and theenvsattribute does not support bringing in environment values from the invocation environment.Given the large impact of this change, I expect a lot of feedback. I thought it would be good to get this PR out early to get the discussion going.