Add path mapping support for C++ compile action templates

C++ action templates are now properly path mapped.

Since the default Unix toolchain doesn't support object file groups on macOS, `apple_support` is needed in the test. This requires wiring up `LocalEnvProvider` in the remote worker to get `DEVELOPER_DIR` to be set, which in turn requires improving the runfiles handling for the the worker.

Work towards #6526

Closes #22890.

PiperOrigin-RevId: 657733074
Change-Id: I132e338d7a44964a8a1aad3062a5c9a4c8e79e57

src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java

@@ -1404,6 +1404,13 @@ public Builder addArtifactVariable(String name, Artifact value) {
       return this;
     }
 
+    @CanIgnoreReturnValue
+    public Builder overrideArtifactVariable(String name, Artifact value) {
+      Preconditions.checkNotNull(value, "Cannot set null as a value for variable '%s'", name);
+      variablesMap.put(name, value);
+      return this;
+    }
+
     /**
      * Add an artifact or string variable that expands {@code name} to {@code value}.
      *

src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java

@@ -245,27 +245,23 @@ private CppCompileAction createAction(
 
     CcToolchainVariables.Builder buildVariables =
         CcToolchainVariables.builder(cppCompileActionBuilder.getVariables());
-    buildVariables.overrideStringVariable(
-        CompileBuildVariables.SOURCE_FILE.getVariableName(),
-        sourceTreeFileArtifact.getExecPathString());
-    buildVariables.overrideStringVariable(
-        CompileBuildVariables.OUTPUT_FILE.getVariableName(),
-        outputTreeFileArtifact.getExecPathString());
+    buildVariables.overrideArtifactVariable(
+        CompileBuildVariables.SOURCE_FILE.getVariableName(), sourceTreeFileArtifact);
+    buildVariables.overrideArtifactVariable(
+        CompileBuildVariables.OUTPUT_FILE.getVariableName(), outputTreeFileArtifact);
     if (dotdFileArtifact != null) {
-      buildVariables.overrideStringVariable(
-          CompileBuildVariables.DEPENDENCY_FILE.getVariableName(),
-          dotdFileArtifact.getExecPathString());
+      buildVariables.overrideArtifactVariable(
+          CompileBuildVariables.DEPENDENCY_FILE.getVariableName(), dotdFileArtifact);
     }
     if (diagnosticsFileArtifact != null) {
-      buildVariables.overrideStringVariable(
+      buildVariables.overrideArtifactVariable(
           CompileBuildVariables.SERIALIZED_DIAGNOSTICS_FILE.getVariableName(),
-          diagnosticsFileArtifact.getExecPathString());
+          diagnosticsFileArtifact);
     }
 
     if (ltoIndexFileArtifact != null) {
-      buildVariables.overrideStringVariable(
-          CompileBuildVariables.LTO_INDEXING_BITCODE_FILE.getVariableName(),
-          ltoIndexFileArtifact.getExecPathString());
+      buildVariables.overrideArtifactVariable(
+          CompileBuildVariables.LTO_INDEXING_BITCODE_FILE.getVariableName(), ltoIndexFileArtifact);
     }
 
     builder.setVariables(buildVariables.build());

src/test/java/com/google/devtools/build/lib/remote/util/IntegrationTestUtils.java

@@ -18,6 +18,7 @@
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.shell.Subprocess;
 import com.google.devtools.build.lib.shell.SubprocessBuilder;
 import com.google.devtools.build.lib.util.OS;
@@ -130,9 +131,14 @@ private void start() throws IOException, InterruptedException {
       ensureMkdir(stdPath);
       ensureTouchFile(stdoutPath);
       ensureTouchFile(stderrPath);
-      String workerPath = Runfiles.create().rlocation(WORKER_PATH.getSafePathString());
+      Runfiles runfiles = Runfiles.preload().withSourceRepository("");
+      String workerPath = runfiles.rlocation(WORKER_PATH.getSafePathString());
+      ImmutableMap.Builder<String, String> env = ImmutableMap.builder();
+      env.putAll(System.getenv());
+      env.putAll(runfiles.getEnvVars());
       process =
           new SubprocessBuilder()
+              .setEnv(env.buildKeepingLast())
               .setStdout(new File(stdoutPath.getSafePathString()))
               .setStderr(new File(stderrPath.getSafePathString()))
               .setArgv(

src/test/py/bazel/test_base.py

@@ -401,13 +401,6 @@ def StartRemoteWorker(self):
     port = s.getsockname()[1]
     s.close()
 
-    env_add = {}
-    try:
-      env_add['RUNFILES_MANIFEST_FILE'] = TestBase.GetEnv(
-          'RUNFILES_MANIFEST_FILE')
-    except EnvVarUndefinedError:
-      pass
-
     # Tip: To help debug remote build problems, add the --debug flag below.
     self._worker_proc = subprocess.Popen(
         [
@@ -422,7 +415,8 @@ def StartRemoteWorker(self):
         stdout=self._worker_stdout,
         stderr=self._worker_stderr,
         cwd=self._test_cwd,
-        env=self._EnvMap(env_add=env_add))
+        env=self._EnvMap(env_add=self._runfiles.EnvVars()),
+    )
 
     return port
 

src/test/shell/bazel/BUILD

@@ -1359,7 +1359,10 @@ sh_test(
         ":test-deps",
         "//src/tools/remote:worker",
     ],
-    tags = ["no_windows"],
+    tags = [
+        "no_windows",
+        "requires-network",  # for Bzlmod
+    ],
     deps = [
         "//src/test/shell/bazel/remote:remote_utils",
         "@bazel_tools//tools/bash/runfiles",

src/test/shell/bazel/path_mapping_test.sh

@@ -387,19 +387,31 @@ EOF
 function test_path_stripping_cc_remote() {
   local -r pkg="${FUNCNAME[0]}"
 
+  cat > MODULE.bazel <<EOF
+bazel_dep(name = "apple_support", version = "1.15.1")
+EOF
+
   mkdir -p "$pkg"
   cat > "$pkg/BUILD" <<EOF
-load("//$pkg/common/utils:defs.bzl", "transition_wrapper")
+load("//$pkg/common/utils:defs.bzl", "gen_cc", "transition_wrapper")
 
 cc_binary(
     name = "main",
-    srcs = ["main.cc"],
+    srcs = [
+        "main.cc",
+        ":gen",
+    ],
     deps = [
         "//$pkg/lib1",
         "//$pkg/lib2",
     ],
 )
 
+gen_cc(
+    name = "gen",
+    subject = "TreeArtifact",
+)
+
 transition_wrapper(
     name = "transitioned_main",
     greeting = "Hi there",
@@ -408,12 +420,16 @@ transition_wrapper(
 EOF
   cat > "$pkg/main.cc" <<EOF
 #include <iostream>
+#include <string>
 #include "$pkg/lib1/lib1.h"
 #include "lib2.h"
 
+std::string TreeArtifactGreeting();
+
 int main() {
   std::cout << GetLib1Greeting() << std::endl;
   std::cout << GetLib2Greeting() << std::endl;
+  std::cout << TreeArtifactGreeting() << std::endl;
   return 0;
 }
 EOF
@@ -567,6 +583,34 @@ transition_wrapper = rule(
     },
     executable = True,
 )
+
+def _gen_cc_impl(ctx):
+    out = ctx.actions.declare_directory(ctx.label.name)
+    ctx.actions.run_shell(
+        outputs = [out],
+        command = """\
+cat >{out_path}/gen.cc <<EOF2
+#include <string>
+
+std::string TreeArtifactGreeting() {{
+  return "Hello, {subject}!";
+}}
+EOF2
+        """.format(
+            out_path = out.path,
+            subject = ctx.attr.subject,
+        ),
+    )
+    return [
+        DefaultInfo(files = depset([out])),
+    ]
+
+gen_cc = rule(
+    implementation = _gen_cc_impl,
+    attrs = {
+        "subject": attr.string(),
+    },
+)
 EOF
   cat > "$pkg/common/utils/utils.cc.tpl" <<'EOF'
 #include "utils.h"
@@ -586,6 +630,7 @@ EOF
 
   expect_log 'Hello, lib1!'
   expect_log 'Hello, lib2!'
+  expect_log 'Hello, TreeArtifact!'
   expect_not_log 'remote cache hit'
 
   bazel run \
@@ -598,9 +643,10 @@ EOF
 
   expect_log 'Hi there, lib1!'
   expect_log 'Hi there, lib2!'
+  expect_log 'Hello, TreeArtifact!'
   # Compilation actions for lib1, lib2 and main should result in cache hits due
   # to path stripping, utils is legitimately different and should not.
-  expect_log ' 3 remote cache hit'
+  expect_log ' 4 remote cache hit'
 }
 
 function test_path_stripping_action_key_not_stale_for_path_collision() {

src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD

@@ -1,3 +1,4 @@
+load("@bazel_skylib//rules:copy_file.bzl", "copy_file")
 load("@rules_java//java:defs.bzl", "java_library")
 
 package(
@@ -13,15 +14,25 @@ filegroup(
     visibility = ["//src:__subpackages__"],
 )
 
+copy_file(
+    name = "xcode_locator",
+    src = "//tools/osx:xcode-locator",
+    out = "xcode-locator",
+)
+
 java_library(
     name = "worker",
     srcs = glob(["*.java"]),
+    data = [
+        ":xcode_locator",
+    ],
     resources = ["//src/main/tools:linux-sandbox"],
     visibility = ["//src/tools/remote:__subpackages__"],
     deps = [
         "//src/main/java/com/google/devtools/build/lib/actions",
-        "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity",
         "//src/main/java/com/google/devtools/build/lib/events",
+        "//src/main/java/com/google/devtools/build/lib/exec:bin_tools",
+        "//src/main/java/com/google/devtools/build/lib/exec/local",
         "//src/main/java/com/google/devtools/build/lib/remote",
         "//src/main/java/com/google/devtools/build/lib/remote:store",
         "//src/main/java/com/google/devtools/build/lib/remote/common",
@@ -46,6 +57,7 @@ java_library(
         "//third_party/grpc-java:grpc-jar",
         "//third_party/protobuf:protobuf_java",
         "//third_party/protobuf:protobuf_java_util",
+        "//tools/java/runfiles",
         "@googleapis//:google_bytestream_bytestream_java_grpc",
         "@googleapis//:google_bytestream_bytestream_java_proto",
         "@googleapis//:google_longrunning_operations_java_proto",

src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java

@@ -29,13 +29,16 @@
 import build.bazel.remote.execution.v2.RequestMetadata;
 import build.bazel.remote.execution.v2.WaitExecutionRequest;
 import com.google.common.base.Throwables;
+import com.google.common.collect.ImmutableList;
 import com.google.common.flogger.GoogleLogger;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.ListeningExecutorService;
 import com.google.common.util.concurrent.MoreExecutors;
 import com.google.common.util.concurrent.ThreadFactoryBuilder;
 import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.events.NullEventHandler;
+import com.google.devtools.build.lib.exec.BinTools;
+import com.google.devtools.build.lib.exec.local.LocalEnvProvider;
 import com.google.devtools.build.lib.remote.ExecutionStatusException;
 import com.google.devtools.build.lib.remote.UploadManifest;
 import com.google.devtools.build.lib.remote.common.CacheNotFoundException;
@@ -50,6 +53,7 @@
 import com.google.devtools.build.lib.shell.FutureCommandResult;
 import com.google.devtools.build.lib.util.io.FileOutErr;
 import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.runfiles.Runfiles;
 import com.google.longrunning.Operation;
 import com.google.protobuf.Any;
 import com.google.protobuf.ExtensionRegistry;
@@ -102,6 +106,8 @@ final class ExecutionServer extends ExecutionImplBase {
   private final ConcurrentHashMap<String, ListenableFuture<ActionResult>> operationsCache;
   private final ListeningExecutorService executorService;
   private final DigestUtil digestUtil;
+  private final LocalEnvProvider localEnvProvider;
+  private final BinTools binTools;
 
   public ExecutionServer(
       Path workPath,
@@ -135,6 +141,21 @@ public ExecutionServer(
     // Allow the core threads to die.
     realExecutor.allowCoreThreadTimeOut(true);
     this.executorService = MoreExecutors.listeningDecorator(realExecutor);
+    this.localEnvProvider = LocalEnvProvider.forCurrentOs(System.getenv());
+    String xcodeLocator;
+    try {
+      xcodeLocator =
+          Runfiles.preload()
+              .withSourceRepository("")
+              .rlocation(
+                  "io_bazel/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/xcode-locator");
+    } catch (IOException e) {
+      throw new IllegalStateException(e);
+    }
+    this.binTools =
+        BinTools.forEmbeddedBin(
+            workPath.getFileSystem().getPath(xcodeLocator).getParentDirectory(),
+            ImmutableList.of("xcode-locator"));
   }
 
   @Override
@@ -414,12 +435,13 @@ private static boolean wasTimeout(long timeoutMillis, long wallTimeMillis) {
     return timeoutMillis > 0 && wallTimeMillis > timeoutMillis;
   }
 
-  private static Map<String, String> getEnvironmentVariables(Command command) {
+  private Map<String, String> getEnvironmentVariables(Command command)
+      throws IOException, InterruptedException {
     HashMap<String, String> result = new HashMap<>();
     for (EnvironmentVariable v : command.getEnvironmentVariablesList()) {
       result.put(v.getName(), v.getValue());
     }
-    return result;
+    return new HashMap<>(localEnvProvider.rewriteLocalEnv(result, binTools, "/tmp"));
   }
 
   // Gets the uid of the current user. If uid could not be successfully fetched (e.g., on other
@@ -494,7 +516,7 @@ private static String platformAsString(@Nullable Platform platform) {
   // arguments. Otherwise, returns a Command that would run the specified command inside the
   // specified docker container.
   private com.google.devtools.build.lib.shell.Command getCommand(Command cmd, String pathString)
-      throws StatusException, InterruptedException {
+      throws StatusException, InterruptedException, IOException {
     Map<String, String> environmentVariables = getEnvironmentVariables(cmd);
     // This allows Bazel's integration tests to test for the remote platform.
     environmentVariables.put("BAZEL_REMOTE_PLATFORM", platformAsString(cmd.getPlatform()));

tools/java/runfiles/BUILD

@@ -40,6 +40,7 @@ java_library(
         "Util.java",
     ],
     visibility = [
+        "//src/tools/remote/src/main/java/com/google/devtools/build/remote/worker:__pkg__",
         "//tools/java/runfiles/testing:__pkg__",
     ],
 )