[7.3.0] Path map executable paths

In `CommandLines`, the very first argument of the first command line is always a path to an executable. As such, it should be path mapped, even when it is a string. This wasn't the case for `SpawnAction`'s created via `ctx.actions.run(executable = <some string>)`.

Work towards bazelbuild#6526
Work towards bazelbuild#22366

Closes bazelbuild#22844.

PiperOrigin-RevId: 656258007
Change-Id: Ia046a7cc66aae51aec764e2f1c49e1d4f69e4b37

Closes bazelbuild#23040

src/main/java/com/google/devtools/build/lib/actions/Artifact.java

@@ -49,6 +49,7 @@
 import com.google.devtools.build.lib.util.HashCodes;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.vfs.PathStrippable;
 import com.google.devtools.build.lib.vfs.Root;
 import com.google.devtools.build.skyframe.ExecutionPhaseSkyKey;
 import com.google.devtools.build.skyframe.SkyFunctionName;

src/main/java/com/google/devtools/build/lib/actions/BUILD

@@ -114,7 +114,6 @@ java_library(
         "NotifyOnActionCacheHit.java",
         "ParamFileInfo.java",
         "ParameterFile.java",
-        "PathStrippable.java",
         "RemoteArtifactChecker.java",
         "ResourceEstimator.java",
         "RunningActionEvent.java",
@@ -315,7 +314,6 @@ java_library(
         ":commandline_item",
         ":fileset_output_symlink",
         ":package_roots",
-        ":path_strippable",
         "//src/main/java/com/google/devtools/build/docgen/annot",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/collect",
@@ -472,15 +470,6 @@ java_library(
     ],
 )
 
-java_library(
-    name = "path_strippable",
-    srcs = ["PathStrippable.java"],
-    deps = [
-        ":commandline_item",
-        "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
-    ],
-)
-
 java_library(
     name = "resource_manager",
     srcs = [

src/main/java/com/google/devtools/build/lib/actions/CommandLines.java

@@ -25,6 +25,7 @@
 import com.google.devtools.build.lib.util.Fingerprint;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.vfs.PathStrippable;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import com.google.protobuf.ByteString;
 import java.io.IOException;
@@ -541,12 +542,15 @@ public Iterable<String> arguments() throws CommandLineExpansionException, Interr
 
     @Override
     public Iterable<String> arguments(
-        @Nullable ArtifactExpander artifactExpander, PathMapper pathMapper)
-        throws CommandLineExpansionException, InterruptedException {
-      if (arg instanceof PathStrippable) {
-        return ImmutableList.of(((PathStrippable) arg).expand(pathMapper::map));
-      }
-      return ImmutableList.of(CommandLineItem.expandToCommandLine(arg));
+        @Nullable ArtifactExpander artifactExpander, PathMapper pathMapper) {
+      return ImmutableList.of(
+          switch (arg) {
+            case PathStrippable ps -> ps.expand(pathMapper::map);
+            // StarlarkAction stores the executable path as a string to save memory, but it should
+            // still be mapped just like a PathFragment.
+            case String s -> pathMapper.map(PathFragment.create(s)).getPathString();
+            default -> CommandLineItem.expandToCommandLine(arg);
+          });
     }
   }
 }

src/main/java/com/google/devtools/build/lib/vfs/BUILD

@@ -20,6 +20,7 @@ OS_PATH_POLICY_SOURCES = [
 PATH_FRAGMENT_SOURCES = [
     "PathFragment.java",
     "PathSegmentIterator.java",
+    "PathStrippable.java",
 ]
 
 OUTPUT_SERVICE_SOURCES = [

src/main/java/com/google/devtools/build/lib/vfs/PathFragment.java

@@ -15,7 +15,6 @@
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.actions.CommandLineItem;
 import com.google.devtools.build.lib.skyframe.serialization.DeserializationContext;
 import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec;
 import com.google.devtools.build.lib.skyframe.serialization.SerializationContext;
@@ -26,6 +25,7 @@
 import com.google.protobuf.CodedInputStream;
 import com.google.protobuf.CodedOutputStream;
 import java.io.IOException;
+import java.util.function.UnaryOperator;
 import javax.annotation.Nullable;
 
 /**
@@ -50,7 +50,7 @@
  */
 @Immutable
 public abstract class PathFragment
-    implements Comparable<PathFragment>, FileType.HasFileType, CommandLineItem {
+    implements Comparable<PathFragment>, FileType.HasFileType, PathStrippable {
   private static final OsPathPolicy OS = OsPathPolicy.getFilePathOs();
 
   @SerializationConstant
@@ -802,8 +802,8 @@ public String filePathForFileTypeMatcher() {
   }
 
   @Override
-  public String expandToCommandLine() {
-    return normalizedPath;
+  public String expand(UnaryOperator<PathFragment> stripPaths) {
+    return stripPaths.apply(this).normalizedPath;
   }
 
   private static void checkBaseName(String baseName) {

src/main/java/com/google/devtools/build/lib/vfs/PathStrippable.java

@@ -0,0 +1,30 @@
+// Copyright 2023 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.vfs;
+
+import com.google.devtools.build.lib.actions.CommandLineItem;
+import java.util.function.UnaryOperator;
+
+/**
+ * A {@link CommandLineItem} that can apply the {@code stripPaths} map to optionally strip config
+ * prefixes before returning output artifact exec paths.
+ */
+public interface PathStrippable extends CommandLineItem {
+  String expand(UnaryOperator<PathFragment> stripPaths);
+
+  @Override
+  default String expandToCommandLine() {
+    return expand(UnaryOperator.identity());
+  }
+}

src/test/java/com/google/devtools/build/lib/analysis/actions/StrippingPathMapperTest.java

@@ -210,4 +210,66 @@ public void starlarkRule_optedInViaModifyExecutionInfo() throws Exception {
             "<pkg/source.txt:pkg/source.txt::pkg>")
         .inOrder();
   }
+
+  @Test
+  public void starlarkRule_stringExecutablePath() throws Exception {
+    scratch.file("defs/BUILD");
+    scratch.file(
+        "defs/defs.bzl",
+        """
+        def my_rule_impl(ctx):
+            out = ctx.actions.declare_file(ctx.label.name)
+            ctx.actions.run(
+                executable = ctx.executable.tool.path,
+                arguments = [ctx.actions.args().add(out)],
+                outputs = [out],
+                tools = [ctx.executable.tool],
+                execution_requirements = {"supports-path-mapping": "1"},
+            )
+            return DefaultInfo(files = depset([out]))
+
+        my_rule = rule(
+            implementation = my_rule_impl,
+            attrs = {
+                "tool": attr.label(
+                    default = "//foo:script",
+                    cfg = "exec",
+                    executable = True,
+                ),
+            },
+        )
+        """);
+    scratch.file(
+        "foo/BUILD",
+        """
+        sh_binary(
+            name = 'script',
+            srcs = ['script.sh'],
+            visibility = ['//visibility:public'],
+        )
+        """);
+    scratch.file(
+        "BUILD",
+        """
+        load("//defs:defs.bzl", "my_rule")
+        my_rule(name = "my_rule")
+        """);
+
+    ConfiguredTarget configuredTarget = getConfiguredTarget("//:my_rule");
+    Artifact outputArtifact =
+        configuredTarget.getProvider(FileProvider.class).getFilesToBuild().toList().get(0);
+    SpawnAction action = (SpawnAction) getGeneratingAction(outputArtifact);
+    Spawn spawn =
+        action.getSpawn(
+            new ActionExecutionContextBuilder()
+                .setMetadataProvider(new FakeActionInputFileCache())
+                .build());
+
+    assertThat(spawn.getPathMapper().isNoop()).isFalse();
+    String outDir = analysisMock.getProductName() + "-out";
+    assertThat(spawn.getArguments())
+        .containsExactly(
+            "%s/cfg/bin/foo/script".formatted(outDir), "%s/cfg/bin/my_rule".formatted(outDir))
+        .inOrder();
+  }
 }