WIP: Preserve unnormalized unresolved symlinks

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

@@ -267,7 +267,7 @@ public static FileArtifactValue createForUnresolvedSymlink(Path symlink) throws
             .getFileSystem()
             .getDigestFunction()
             .getHashFunction()
-            .hashString(symlink.readSymbolicLink().getPathString(), ISO_8859_1)
+            .hashString(PathFragment.create(symlink.readSymbolicLink()).getPathString(), ISO_8859_1)
             .asBytes();
 
     // We need to be able to tell the difference between a symlink and a file containing the same

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

@@ -89,7 +89,7 @@ public static FileStateValue create(
       case DIRECTORY:
         return DIRECTORY_FILE_STATE_NODE;
       case SYMLINK:
-        return new SymlinkFileStateValue(path.readSymbolicLinkUnchecked());
+        return new SymlinkFileStateValue(PathFragment.create(path.readSymbolicLinkUnchecked()));
       case FILE:
       case UNKNOWN:
         if (stat == null) {
@@ -125,7 +125,7 @@ public static FileStateValue createWithStatNoFollow(
     } else if (statNoFollow.isDirectory()) {
       return DIRECTORY_FILE_STATE_NODE;
     } else if (statNoFollow.isSymbolicLink()) {
-      return new SymlinkFileStateValue(path.readSymbolicLinkUnchecked());
+      return new SymlinkFileStateValue(PathFragment.create(path.readSymbolicLinkUnchecked()));
     }
     throw new InconsistentFilesystemException("according to stat, existing path " + path + " is "
         + "neither a file nor directory nor symlink.");

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

@@ -276,6 +276,7 @@ java_library(
         ":actions/substitution",
         ":actions/symlink_action",
         ":actions/template_expansion_action",
+        ":actions/unresolved_symlink_action",
         ":actions_provider",
         ":aspect_aware_attribute_mapper",
         ":aspect_collection",
@@ -1513,6 +1514,22 @@ java_library(
     ],
 )
 
+java_library(
+    name = "actions/unresolved_symlink_action",
+    srcs = ["actions/UnresolvedSymlinkAction.java"],
+    deps = [
+        "//src/main/java/com/google/devtools/build/lib/actions",
+        "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
+        "//src/main/java/com/google/devtools/build/lib/collect/nestedset",
+        "//src/main/java/com/google/devtools/build/lib/util",
+        "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
+        "//src/main/java/com/google/devtools/build/lib/vfs",
+        "//src/main/protobuf:failure_details_java_proto",
+        "//third_party:guava",
+        "//third_party:jsr305",
+    ],
+)
+
 java_library(
     name = "buildinfo/build_info_key",
     srcs = ["buildinfo/BuildInfoKey.java"],

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

@@ -36,7 +36,13 @@
 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).
+ *
+ * To create a possibly unresolved symlink to a raw path, use {@link UnresolvedSymlinkAction}
+ * instead.
+ */
 public final class SymlinkAction extends AbstractAction {
   private static final String GUID = "7f4fab4d-d0a7-4f0f-8649-1d0337a21fee";
 
@@ -152,13 +158,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/actions/UnresolvedSymlinkAction.java

@@ -0,0 +1,107 @@
+// 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.actions;
+
+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.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 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 {
+      outputPath.createSymbolicLink(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/analysis/starlark/StarlarkActionFactory.java

@@ -48,6 +48,7 @@
 import com.google.devtools.build.lib.analysis.actions.Substitution;
 import com.google.devtools.build.lib.analysis.actions.SymlinkAction;
 import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction;
+import com.google.devtools.build.lib.analysis.actions.UnresolvedSymlinkAction;
 import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
 import com.google.devtools.build.lib.collect.nestedset.Depset;
 import com.google.devtools.build.lib.collect.nestedset.Depset.TypeException;
@@ -273,7 +274,7 @@ public void symlink(
             ? (String) progressMessageUnchecked
             : "Creating symlink " + outputArtifact.getExecPathString();
 
-    SymlinkAction action;
+    Action action;
     if (targetFile != Starlark.NONE) {
       if (outputArtifact.isSymlink()) {
         throw Starlark.errorf(
@@ -315,12 +316,11 @@ public void symlink(
         throw Starlark.errorf("\"is_executable\" cannot be True when using \"target_path\"");
       }
 
-      action =
-          SymlinkAction.createUnresolved(
-              ruleContext.getActionOwner(),
-              outputArtifact,
-              PathFragment.create((String) targetPath),
-              progressMessage);
+      action = new UnresolvedSymlinkAction(
+          ruleContext.getActionOwner(),
+          outputArtifact,
+          (String) targetPath,
+          progressMessage);
     }
     registerAction(action);
   }

src/main/java/com/google/devtools/build/lib/buildtool/OutputDirectoryLinksUtils.java

@@ -222,7 +222,7 @@ private static void removeAllSymlinks(
       try {
         if (entry.isSymbolicLink()
             && entry.relativeTo(workspace).getPathString().startsWith(symlinkPrefix)
-            && entry.readSymbolicLink().startsWith(outputBase.asFragment())) {
+            && PathFragment.create(entry.readSymbolicLink()).startsWith(outputBase.asFragment())) {
           logger.atFinest().log("Removing %s", entry);
           entry.delete();
         }

src/main/java/com/google/devtools/build/lib/buildtool/PathPrettyPrinter.java

@@ -57,7 +57,7 @@ private Map<PathFragment, PathFragment> resolve(
       PathFragment linkFragment = PathFragment.create(linkName);
       Path dir = workspaceDirectory.getRelative(linkFragment);
       try {
-        PathFragment levelOneLinkTarget = dir.readSymbolicLink();
+        PathFragment levelOneLinkTarget = PathFragment.create(dir.readSymbolicLink());
         if (levelOneLinkTarget.isAbsolute()) {
           result.put(linkFragment, dir.getRelative(levelOneLinkTarget).asFragment());
         }

src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java

@@ -39,7 +39,7 @@
  *
  * <p>The implementation mostly delegates to the local file system except for the case where an
  * action input is a remotely stored action output. Most notably {@link
- * #getInputStream(PathFragment)} and {@link #createSymbolicLink(PathFragment, PathFragment)}.
+ * #getInputStream(PathFragment)} and {@link #createSymbolicLink(PathFragment, String)}.
  *
  * <p>This implementation only supports creating local action outputs.
  */
@@ -175,7 +175,7 @@ protected void chmod(PathFragment path, int mode) throws IOException {
   // -------------------- Symlinks --------------------
 
   @Override
-  protected PathFragment readSymbolicLink(PathFragment path) throws IOException {
+  protected String readSymbolicLink(PathFragment path) throws IOException {
     FileArtifactValue m = getRemoteInputMetadata(path);
     if (m != null) {
       // We don't support symlinks as remote action outputs.
@@ -185,14 +185,14 @@ protected PathFragment readSymbolicLink(PathFragment path) throws IOException {
   }
 
   @Override
-  protected void createSymbolicLink(PathFragment linkPath, PathFragment targetFragment)
+  protected void createSymbolicLink(PathFragment linkPath, String rawTarget)
       throws IOException {
     /*
      * TODO(buchgr): Optimize the case where we are creating a symlink to a remote output. This does
      * add a non-trivial amount of complications though (as symlinks tend to do).
      */
-    downloadFileIfRemote(execRoot.getRelative(targetFragment));
-    super.createSymbolicLink(linkPath, targetFragment);
+    downloadFileIfRemote(execRoot.getRelative(rawTarget));
+    super.createSymbolicLink(linkPath, rawTarget);
   }
 
   // -------------------- Implementations based on stat() --------------------

src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java

@@ -186,7 +186,7 @@ void addFiles(Collection<Path> files) throws ExecException, IOException {
         Digest digest = digestUtil.compute(file, stat.getSize());
         addFile(digest, file);
       } else if (stat.isSymbolicLink() && allowSymlinks) {
-        PathFragment target = file.readSymbolicLink();
+        PathFragment target = PathFragment.create(file.readSymbolicLink());
         // Need to resolve the symbolic link to know what to add, file or directory.
         FileStatus statFollow = file.statIfFound(Symlinks.FOLLOW);
         if (statFollow == null) {
@@ -310,7 +310,7 @@ private Directory computeDirectory(Path path, Tree.Builder tree)
         b.addDirectoriesBuilder().setName(name).setDigest(digestUtil.compute(dir));
         tree.addChildren(dir);
       } else if (dirent.getType() == Dirent.Type.SYMLINK && allowSymlinks) {
-        PathFragment target = child.readSymbolicLink();
+        PathFragment target = PathFragment.create(child.readSymbolicLink());
         if (uploadSymlinks && !target.isAbsolute()) {
           // Whether it is dangling or not, we're passing it on.
           b.addSymlinksBuilder().setName(name).setTarget(target.toString());

src/main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java

@@ -141,7 +141,7 @@ void createInputs(Iterable<PathFragment> inputsToCreate, SandboxInputs inputs)
           FileSystemUtils.createEmptyFile(key);
         }
       } else if (inputs.getSymlinks().containsKey(fragment)) {
-        PathFragment symlinkDest = inputs.getSymlinks().get(fragment);
+        String symlinkDest = inputs.getSymlinks().get(fragment);
         if (symlinkDest != null) {
           key.createSymbolicLink(symlinkDest);
         }

src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java

@@ -224,11 +224,10 @@ private static void cleanRecursively(
             LabelConstants.EXPERIMENTAL_EXTERNAL_PATH_PREFIX.getRelative(
                 absPath.relativeTo(execroot));
       }
-      Optional<PathFragment> destination =
-          getExpectedSymlinkDestination(pathRelativeToWorkDir, inputs);
+      Optional<String> destination = getExpectedSymlinkDestination(pathRelativeToWorkDir, inputs);
       if (destination.isPresent()) {
-        if (SYMLINK.equals(dirent.getType())
-            && absPath.readSymbolicLink().equals(destination.get())) {
+        if (SYMLINK.equals(dirent.getType()) && absPath.readSymbolicLink()
+            .equals(destination.get())) {
           inputsToCreate.remove(pathRelativeToWorkDir);
         } else {
           absPath.delete();
@@ -248,14 +247,14 @@ private static void cleanRecursively(
   }
 
   /**
-   * Returns what the destination of the symlink {@code file} should be, according to {@code
-   * inputs}.
+   * Returns what the destination of the symlink {@code file} should be, according to
+   * {@code inputs}.
    */
-  static Optional<PathFragment> getExpectedSymlinkDestination(
+  static Optional<String> getExpectedSymlinkDestination(
       PathFragment fragment, SandboxInputs inputs) {
     Path file = inputs.getFiles().get(fragment);
     if (file != null) {
-      return Optional.of(file.asFragment());
+      return Optional.of(file.asFragment().getPathString());
     }
     return Optional.ofNullable(inputs.getSymlinks().get(fragment));
   }
@@ -369,7 +368,7 @@ public static final class SandboxInputs {
     private final Set<VirtualActionInput> virtualInputsWithDelayedMaterialization;
     // Virtual inputs that are materialized during {@link #processInputFiles}.
     private final Map<VirtualActionInput, byte[]> materializedVirtualInputs;
-    private final Map<PathFragment, PathFragment> symlinks;
+    private final Map<PathFragment, String> symlinks;
 
     private static final SandboxInputs EMPTY_INPUTS =
         new SandboxInputs(
@@ -379,7 +378,7 @@ public SandboxInputs(
         Map<PathFragment, Path> files,
         Set<VirtualActionInput> virtualInputsWithDelayedMaterialization,
         Map<VirtualActionInput, byte[]> materializedVirtualInputs,
-        Map<PathFragment, PathFragment> symlinks) {
+        Map<PathFragment, String> symlinks) {
       checkState(
           virtualInputsWithDelayedMaterialization.isEmpty() || materializedVirtualInputs.isEmpty(),
           "Either virtualInputsWithDelayedMaterialization or materializedVirtualInputs should be"
@@ -393,7 +392,7 @@ public SandboxInputs(
     public SandboxInputs(
         Map<PathFragment, Path> files,
         Set<VirtualActionInput> virtualInputsWithDelayedMaterialization,
-        Map<PathFragment, PathFragment> symlinks) {
+        Map<PathFragment, String> symlinks) {
       this(files, virtualInputsWithDelayedMaterialization, ImmutableMap.of(), symlinks);
     }
 
@@ -405,7 +404,7 @@ public Map<PathFragment, Path> getFiles() {
       return files;
     }
 
-    public Map<PathFragment, PathFragment> getSymlinks() {
+    public Map<PathFragment, String> getSymlinks() {
       return symlinks;
     }
 
@@ -531,7 +530,7 @@ public SandboxInputs processInputFiles(Map<PathFragment, ActionInput> inputMap,
       throws IOException {
     Map<PathFragment, Path> inputFiles = new TreeMap<>();
     Set<VirtualActionInput> virtualInputsWithDelayedMaterialization = new HashSet<>();
-    Map<PathFragment, PathFragment> inputSymlinks = new TreeMap<>();
+    Map<PathFragment, String> inputSymlinks = new TreeMap<>();
     Map<VirtualActionInput, byte[]> materializedVirtualInputs = new HashMap<>();
 
     for (Map.Entry<PathFragment, ActionInput> e : inputMap.entrySet()) {

src/main/java/com/google/devtools/build/lib/sandbox/SandboxfsSandboxedSpawn.java

@@ -264,7 +264,7 @@ private static void computeSymlinkMappings(
       PathFragment path, Path symlink, Map<PathFragment, PathFragment> mappings)
       throws IOException {
     for (; ; ) {
-      PathFragment symlinkTarget = symlink.readSymbolicLinkUnchecked();
+      PathFragment symlinkTarget = PathFragment.create(symlink.readSymbolicLinkUnchecked());
       if (!symlinkTarget.isAbsolute()) {
         PathFragment keyParent = path.getParentDirectory();
         if (keyParent == null) {