[7.3.0] Path map executable paths (bazelbuild#23118)

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,62 @@ 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().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();
+ }
 }