Skip to content

Commit

Permalink
Only treat "env" and "env_inherit" attrs specially for native rules
Browse files Browse the repository at this point in the history
This avoids unexpected behavior in the case of Skylark rules that happen to use the same attribute names.

Fixes #12758

PiperOrigin-RevId: 349587545
  • Loading branch information
Googler authored and copybara-github committed Dec 30, 2020
1 parent 7b76683 commit c833660
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@
executed by <code>bazel run</code>.
</p>

<p>
This attribute only applies to native rules, like <code>cc_binary</code>, <code>py_binary</code>,
and <code>sh_binary</code>. It does not apply to Starlark-defined executable rules.
</p>

<p>
<em class="harmful">NOTE: The environment variables are not set when you run the target
outside of bazel (for example, by manually executing the binary in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,8 @@
<p>Specifies additional environment variables to set when the test is
executed by <code>bazel test</code>.
</p>

<p>
This attribute only applies to native rules, like <code>cc_test</code>, <code>py_test</code>,
and <code>sh_test</code>. It does not apply to Starlark-defined test rules.
</p>
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,8 @@
<p>Specifies additional environment variables to inherit from the
external environment when the test is executed by <code>bazel test</code>.
</p>

<p>
This attribute only applies to native rules, like <code>cc_test</code>, <code>py_test</code>,
and <code>sh_test</code>. It does not apply to Starlark-defined test rules.
</p>
Original file line number Diff line number Diff line change
Expand Up @@ -464,8 +464,14 @@ private static CommandLine computeArgs(RuleContext ruleContext, CommandLine addi
}

private static ActionEnvironment computeActionEnvironment(RuleContext ruleContext) {
if (!ruleContext.getRule().isAttrDefined("env", Type.STRING_DICT)
&& !ruleContext.getRule().isAttrDefined("env_inherit", Type.STRING_LIST)) {
// Currently, "env" and "env_inherit" are not added to Starlark-defined rules (unlike "args"),
// in order to avoid breaking existing Starlark rules that use those attribute names.
// TODO(brandjon): Support "env" and "env_inherit" for Starlark-defined rules.
boolean isNativeRule =
ruleContext.getRule().getRuleClassObject().getRuleDefinitionEnvironmentLabel() == null;
if (!isNativeRule
|| (!ruleContext.getRule().isAttrDefined("env", Type.STRING_DICT)
&& !ruleContext.getRule().isAttrDefined("env_inherit", Type.STRING_LIST))) {
return ActionEnvironment.EMPTY;
}
TreeMap<String, String> fixedEnv = new TreeMap<>();
Expand Down

0 comments on commit c833660

Please sign in to comment.