From c83366064621d5a265eba14d93a03deff58fe6d8 Mon Sep 17 00:00:00 2001
From: Googler <noreply@google.com>
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 <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
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 @@
 <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>
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 @@
 <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>
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<String, String> fixedEnv = new TreeMap<>();