From c83366064621d5a265eba14d93a03deff58fe6d8 Mon Sep 17 00:00:00 2001 From: Googler Date: Wed, 30 Dec 2020 12:18:32 -0800 Subject: [PATCH] Only treat "env" and "env_inherit" attrs specially for native rules This avoids unexpected behavior in the case of Skylark rules that happen to use the same attribute names. Fixes #12758 PiperOrigin-RevId: 349587545 --- .../build/docgen/templates/attributes/binary/env.html | 5 +++++ .../build/docgen/templates/attributes/test/env.html | 5 +++++ .../docgen/templates/attributes/test/env_inherit.html | 5 +++++ .../devtools/build/lib/analysis/RunfilesSupport.java | 10 ++++++++-- 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/google/devtools/build/docgen/templates/attributes/binary/env.html b/src/main/java/com/google/devtools/build/docgen/templates/attributes/binary/env.html index 5dd2232fbe7edf..80cb86bcbbfbf5 100644 --- a/src/main/java/com/google/devtools/build/docgen/templates/attributes/binary/env.html +++ b/src/main/java/com/google/devtools/build/docgen/templates/attributes/binary/env.html @@ -6,6 +6,11 @@ executed by bazel run.

+

+ This attribute only applies to native rules, like cc_binary, py_binary, + and sh_binary. It does not apply to Starlark-defined executable rules. +

+

NOTE: The environment variables are not set when you run the target outside of bazel (for example, by manually executing the binary in diff --git a/src/main/java/com/google/devtools/build/docgen/templates/attributes/test/env.html b/src/main/java/com/google/devtools/build/docgen/templates/attributes/test/env.html index 5aed643fbbb4f0..3db26d13a88b5b 100644 --- a/src/main/java/com/google/devtools/build/docgen/templates/attributes/test/env.html +++ b/src/main/java/com/google/devtools/build/docgen/templates/attributes/test/env.html @@ -6,3 +6,8 @@

Specifies additional environment variables to set when the test is executed by bazel test.

+ +

+ This attribute only applies to native rules, like cc_test, py_test, + and sh_test. It does not apply to Starlark-defined test rules. +

diff --git a/src/main/java/com/google/devtools/build/docgen/templates/attributes/test/env_inherit.html b/src/main/java/com/google/devtools/build/docgen/templates/attributes/test/env_inherit.html index c7e6500c3f048c..a2c8366696087d 100644 --- a/src/main/java/com/google/devtools/build/docgen/templates/attributes/test/env_inherit.html +++ b/src/main/java/com/google/devtools/build/docgen/templates/attributes/test/env_inherit.html @@ -3,3 +3,8 @@

Specifies additional environment variables to inherit from the external environment when the test is executed by bazel test.

+ +

+ This attribute only applies to native rules, like cc_test, py_test, + and sh_test. It does not apply to Starlark-defined test rules. +

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java index d867ec1c3b295b..ef74b411cb9d87 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java @@ -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 fixedEnv = new TreeMap<>();