Working getInputs override

scripts/bootstrap/compile.sh

@@ -335,8 +335,9 @@ if [ "${PLATFORM}" = "windows" ]; then
   # We don't rely on runfiles trees on Windows
   cat <<'EOF' >${ARCHIVE_DIR}/build-runfiles${EXE_EXT}
 #!/bin/sh
-mkdir -p $2
-cp $1 $2/MANIFEST
+# Skip over --allow_relative.
+mkdir -p $3
+cp $2 $3/MANIFEST
 EOF
 else
   cat <<'EOF' >${ARCHIVE_DIR}/build-runfiles${EXE_EXT}
@@ -350,8 +351,9 @@ else
 # bootstrap version of Bazel, but we'd still need a shell wrapper around it, so
 # it's not clear whether that would be a win over a few lines of Lovecraftian
 # code)
-MANIFEST="$1"
-TREE="$2"
+# Skip over --allow_relative.
+MANIFEST="$2"
+TREE="$3"
 
 rm -fr "$TREE"
 mkdir -p "$TREE"

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

@@ -249,6 +249,7 @@ java_library(
         "starlark/StarlarkRuleContext.java",
         "starlark/StarlarkRuleTransitionProvider.java",
         "starlark/StarlarkTransition.java",
+        "starlark/UnresolvedSymlinkAction.java",
         "test/AnalysisTestActionBuilder.java",
         "test/BaselineCoverageAction.java",
         "test/CoverageCommon.java",

src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java

@@ -13,16 +13,19 @@
 // limitations under the License.
 package com.google.devtools.build.lib.analysis;
 
+import static com.google.common.collect.ImmutableList.toImmutableList;
 import static java.nio.charset.StandardCharsets.ISO_8859_1;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionKeyContext;
 import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction;
 import com.google.devtools.build.lib.analysis.actions.DeterministicWriter;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
@@ -99,6 +102,8 @@ void writeEntry(
 
   private final boolean remotableSourceManifestActions;
 
+  private NestedSet<Artifact> symlinkArtifacts = null;
+
   /**
    * Creates a new AbstractSourceManifestAction instance using latin1 encoding to write the manifest
    * file and with a specified root path for manifest entries.
@@ -135,6 +140,17 @@ public SourceManifestAction(
     this.remotableSourceManifestActions = remotableSourceManifestActions;
   }
 
+  @Override
+  public synchronized NestedSet<Artifact> getInputs() {
+    if (symlinkArtifacts == null) {
+      ImmutableList<Artifact> symlinks = runfiles.getArtifacts().toList().stream()
+          .filter(Artifact::isSymlink)
+          .collect(toImmutableList());
+      symlinkArtifacts = NestedSetBuilder.wrap(Order.STABLE_ORDER, symlinks);
+    }
+    return symlinkArtifacts;
+  }
+
   @VisibleForTesting
   public void writeOutputFile(OutputStream out, EventHandler eventHandler) throws IOException {
     writeFile(out, runfiles.getRunfilesInputs(eventHandler, getOwner().getLocation()));
@@ -227,7 +243,11 @@ public void writeEntry(Writer manifestWriter, PathFragment rootRelativePath, Art
         // This trailing whitespace is REQUIRED to process the single entry line correctly.
         manifestWriter.append(' ');
         if (symlink != null) {
-          manifestWriter.append(symlink.getPath().getPathString());
+          if (symlink.isSymlink()) {
+            manifestWriter.append(symlink.getPath().readSymbolicLink().getPathString());
+          } else {
+            manifestWriter.append(symlink.getPath().getPathString());
+          }
         }
         manifestWriter.append('\n');
       }

src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkAction.java

@@ -36,7 +36,10 @@
 import java.io.IOException;
 import javax.annotation.Nullable;
 
-/** Action to create a symbolic link. */
+/**
+ * Action to create a symlink to a known-to-exist target with alias semantics similar to a true copy
+ * of the input (if any).
+ */
 public final class SymlinkAction extends AbstractAction {
   private static final String GUID = "7f4fab4d-d0a7-4f0f-8649-1d0337a21fee";
 
@@ -152,13 +155,6 @@ public static SymlinkAction toFileset(
         owner, execPath, primaryInput, primaryOutput, progressMessage, TargetType.FILESET);
   }
 
-  public static SymlinkAction createUnresolved(
-      ActionOwner owner, Artifact primaryOutput, PathFragment targetPath, String progressMessage) {
-    Preconditions.checkArgument(primaryOutput.isSymlink());
-    return new SymlinkAction(
-        owner, targetPath, null, primaryOutput, progressMessage, TargetType.OTHER);
-  }
-
   /**
    * Creates a new SymlinkAction instance, where an input artifact is not present. This is useful
    * when dealing with special cases where input paths that are outside the exec root directory

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java

@@ -273,7 +273,7 @@ public void symlink(
             ? (String) progressMessageUnchecked
             : "Creating symlink " + outputArtifact.getExecPathString();
 
-    SymlinkAction action;
+    Action action;
     if (targetFile != Starlark.NONE) {
       Artifact inputArtifact = (Artifact) targetFile;
       if (outputArtifact.isSymlink()) {
@@ -324,10 +324,10 @@ public void symlink(
       }
 
       action =
-          SymlinkAction.createUnresolved(
+          new UnresolvedSymlinkAction(
               ruleContext.getActionOwner(),
               outputArtifact,
-              PathFragment.create((String) targetPath),
+              (String) targetPath,
               progressMessage);
     }
     registerAction(action);

src/main/java/com/google/devtools/build/lib/analysis/starlark/UnresolvedSymlinkAction.java

@@ -0,0 +1,112 @@
+// Copyright 2022 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.analysis.starlark;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.actions.AbstractAction;
+import com.google.devtools.build.lib.actions.ActionExecutionContext;
+import com.google.devtools.build.lib.actions.ActionExecutionException;
+import com.google.devtools.build.lib.actions.ActionKeyContext;
+import com.google.devtools.build.lib.actions.ActionOwner;
+import com.google.devtools.build.lib.actions.ActionResult;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
+import com.google.devtools.build.lib.analysis.actions.SymlinkAction;
+import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.collect.nestedset.Order;
+import com.google.devtools.build.lib.server.FailureDetails;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.server.FailureDetails.SymlinkAction.Code;
+import com.google.devtools.build.lib.util.DetailedExitCode;
+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 java.io.IOException;
+import javax.annotation.Nullable;
+
+/**
+ * Action to create a possibly unresolved symbolic link to a raw path.
+ *
+ * To create a symlink to a known-to-exist target with alias semantics similar to a true copy of the
+ * input, use {@link SymlinkAction} instead.
+ */
+public final class UnresolvedSymlinkAction extends AbstractAction {
+  private static final String GUID = "0f302651-602c-404b-881c-58913193cfe7";
+
+  private final String target;
+  private final String progressMessage;
+
+  public UnresolvedSymlinkAction(
+      ActionOwner owner,
+      Artifact primaryOutput,
+      String target,
+      String progressMessage) {
+    super(
+        owner,
+        NestedSetBuilder.emptySet(Order.STABLE_ORDER),
+        ImmutableSet.of(primaryOutput));
+    this.target = target;
+    this.progressMessage = progressMessage;
+  }
+
+  @Override
+  public ActionResult execute(ActionExecutionContext actionExecutionContext)
+      throws ActionExecutionException {
+
+    Path outputPath = actionExecutionContext.getInputPath(getPrimaryOutput());
+    try {
+      // TODO: PathFragment#create normalizes the symlink target, which may change how it resolves
+      //  when combined with directory symlinks. Ideally, Bazel's file system abstraction would
+      //  offer a way to create symlinks without any preprocessing of the target.
+      outputPath.createSymbolicLink(PathFragment.create(target));
+    } catch (IOException e) {
+      String message =
+          String.format(
+              "failed to create symbolic link '%s' to '%s' due to I/O error: %s",
+              getPrimaryOutput().getExecPathString(), target, e.getMessage());
+      DetailedExitCode code = createDetailedExitCode(message, Code.LINK_CREATION_IO_EXCEPTION);
+      throw new ActionExecutionException(message, e, this, false, code);
+    }
+
+    return ActionResult.EMPTY;
+  }
+
+  @Override
+  protected void computeKey(
+      ActionKeyContext actionKeyContext,
+      @Nullable ArtifactExpander artifactExpander,
+      Fingerprint fp) {
+    fp.addString(GUID);
+    fp.addString(target);
+  }
+
+  @Override
+  public String getMnemonic() {
+    return "UnresolvedSymlink";
+  }
+
+  @Override
+  protected String getRawProgressMessage() {
+    return progressMessage;
+  }
+
+  private static DetailedExitCode createDetailedExitCode(String message, Code detailedCode) {
+    return DetailedExitCode.of(
+        FailureDetail.newBuilder()
+            .setMessage(message)
+            .setSymlinkAction(FailureDetails.SymlinkAction.newBuilder().setCode(detailedCode))
+            .build());
+  }
+}

src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java

@@ -180,8 +180,8 @@ Command createCommand(Path execRoot, BinTools binTools, Map<String, String> shel
     Preconditions.checkNotNull(shellEnvironment);
     List<String> args = Lists.newArrayList();
     args.add(binTools.getEmbeddedPath(BUILD_RUNFILES).asFragment().getPathString());
+    args.add("--allow_relative");
     if (filesetTree) {
-      args.add("--allow_relative");
       args.add("--use_metadata");
     }
     args.add(inputManifest.relativeTo(execRoot).getPathString());

src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeHelperTest.java

@@ -48,10 +48,11 @@ public void checkCreatedSpawn() {
     assertThat(command.getEnvironment()).isEmpty();
     assertThat(command.getWorkingDirectory()).isEqualTo(execRoot.getPathFile());
     ImmutableList<String> commandLine = command.getArguments();
-    assertThat(commandLine).hasSize(3);
+    assertThat(commandLine).hasSize(4);
     assertThat(commandLine.get(0)).endsWith(SymlinkTreeHelper.BUILD_RUNFILES);
-    assertThat(commandLine.get(1)).isEqualTo("input_manifest");
-    assertThat(commandLine.get(2)).isEqualTo("output/MANIFEST");
+    assertThat(commandLine.get(1)).isEqualTo("--allow_relative");
+    assertThat(commandLine.get(2)).isEqualTo("input_manifest");
+    assertThat(commandLine.get(3)).isEqualTo("output/MANIFEST");
   }
 
   @Test