From e8de041906cf63989c74930e4e6cb97895c259af Mon Sep 17 00:00:00 2001
From: brandjon <brandjon@google.com>
Date: Tue, 11 Jun 2019 14:16:18 -0700
Subject: [PATCH] Add /usr/local/bin to default PATH under strict action env

With this change, when --incompatible_strict_action_env is on but no override for PATH is available, the default PATH will include /usr/local/bin. This allows Python 3 to be on PATH on mac, which is important for the autodetecting Python toolchain.

Without this change, users can still use --action_env=PATH[=...], but that doesn't work in all cases (e.g. genrules that are themselves built in the host config).

Fixes #8536. Confirmed that this fixes the failure seen in https://bugs.chromium.org/p/gerrit/issues/detail?id=10953.

RELNOTES: When `--incompatible_strict_action_env` is enabled, the default `PATH` now includes `/usr/local/bin`.
PiperOrigin-RevId: 252696581
---
 .../build/lib/bazel/rules/BazelRuleClassProvider.java  | 10 +++++++++-
 .../lib/bazel/rules/BazelRuleClassProviderTest.java    |  6 +++---
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
index 60dad99dbb86ca..d1de9db6ec7a93 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
@@ -450,7 +450,15 @@ public static String pathOrDefault(OS os, @Nullable String path, @Nullable PathF
     // from the local machine. For now, this can be overridden with --action_env=PATH=<value>, so
     // at least there's a workaround.
     if (os != OS.WINDOWS) {
-      return "/bin:/usr/bin";
+      // The default used to be "/bin:/usr/bin". However, on Mac the Python 3 interpreter, if it is
+      // installed at all, tends to be under /usr/local/bin. The autodetecting Python toolchain
+      // searches PATH for "python3", so if we don't include this directory then we can't run PY3
+      // targets with this toolchain if strict action environment is on.
+      //
+      // Note that --action_env does not propagate to the host config, so it is not a viable
+      // workaround when a genrule is itself built in the host config (e.g. nested genrules). See
+      // #8536.
+      return "/bin:/usr/bin:/usr/local/bin";
     }
 
     String newPath = "";
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProviderTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProviderTest.java
index 43a181e5680826..4bf633b66a044c 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProviderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProviderTest.java
@@ -177,14 +177,14 @@ public void strictActionEnv() throws Exception {
             "--action_env=FOO=bar");
 
     ActionEnvironment env = BazelRuleClassProvider.SHELL_ACTION_ENV.getActionEnvironment(options);
-    assertThat(env.getFixedEnv().toMap()).containsEntry("PATH", "/bin:/usr/bin");
+    assertThat(env.getFixedEnv().toMap()).containsEntry("PATH", "/bin:/usr/bin:/usr/local/bin");
     assertThat(env.getFixedEnv().toMap()).containsEntry("FOO", "bar");
   }
 
   @Test
   public void pathOrDefaultOnLinux() {
-    assertThat(pathOrDefault(OS.LINUX, null, null)).isEqualTo("/bin:/usr/bin");
-    assertThat(pathOrDefault(OS.LINUX, "/not/bin", null)).isEqualTo("/bin:/usr/bin");
+    assertThat(pathOrDefault(OS.LINUX, null, null)).isEqualTo("/bin:/usr/bin:/usr/local/bin");
+    assertThat(pathOrDefault(OS.LINUX, "/not/bin", null)).isEqualTo("/bin:/usr/bin:/usr/local/bin");
   }
 
   @Test