Emit unresolved symlinks correctly in the BEP.

Fixes #11942.

PiperOrigin-RevId: 539589072
Change-Id: I760b842b145d7f88013024ff0dffee414261520f

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

@@ -170,9 +170,13 @@ public Collection<LocalFile> referencedLocalFiles() {
               /*artifactMetadata=*/ null));
     }
     if (exception == null) {
-      localFiles.add(
-          new LocalFile(
-              primaryOutput, LocalFileType.OUTPUT, outputArtifact, primaryOutputMetadata));
+      LocalFileType type = LocalFileType.OUTPUT_FILE;
+      if (outputArtifact.isDirectory()) {
+        type = LocalFileType.OUTPUT_DIRECTORY;
+      } else if (outputArtifact.isSymlink()) {
+        type = LocalFileType.OUTPUT_SYMLINK;
+      }
+      localFiles.add(new LocalFile(primaryOutput, type, outputArtifact, primaryOutputMetadata));
     }
     return localFiles.build();
   }

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

@@ -114,13 +114,6 @@ public FileArtifactValue getFileArtifactValue(Artifact artifact) {
     return importantInputMap.getInputMetadata(artifact);
   }
 
-  /** Returns true if the given artifact is guaranteed to be a file (and not a directory). */
-  public static boolean isGuaranteedToBeOutputFile(FileStateType type) {
-    return type == FileStateType.REGULAR_FILE
-        || type == FileStateType.SPECIAL_FILE
-        || type == FileStateType.NONEXISTENT;
-  }
-
   public void visitArtifacts(Iterable<Artifact> artifacts, ArtifactReceiver receiver) {
     for (Artifact artifact : artifacts) {
       if (artifact.isMiddlemanArtifact()) {

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

@@ -29,6 +29,7 @@
 import com.google.devtools.build.lib.actions.CompletionContext.ArtifactReceiver;
 import com.google.devtools.build.lib.actions.EventReportingArtifacts;
 import com.google.devtools.build.lib.actions.FileArtifactValue;
+import com.google.devtools.build.lib.actions.FileArtifactValue.UnresolvedSymlinkArtifactValue;
 import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper.ArtifactsInOutputGroup;
 import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
 import com.google.devtools.build.lib.analysis.test.InstrumentedFilesInfo;
@@ -324,9 +325,11 @@ public void accept(Artifact artifact) {
             String name = artifactNameFunction.apply(artifact);
             String uri =
                 converters.pathConverter().apply(completionContext.pathResolver().toPath(artifact));
-            if (uri != null) {
-              builder.addImportantOutput(
-                  newFileFromArtifact(name, artifact, completionContext).setUri(uri).build());
+            BuildEventStreamProtos.File file =
+                newFileFromArtifact(name, artifact, completionContext, uri);
+            // Omit files with unknown contents (e.g. if uploading failed).
+            if (file.getFileCase() != BuildEventStreamProtos.File.FileCase.FILE_NOT_SET) {
+              builder.addImportantOutput(file);
             }
           }
 
@@ -342,13 +345,12 @@ private static Iterable<Artifact> filterFilesets(Iterable<Artifact> artifacts) {
     return Iterables.filter(artifacts, artifact -> !artifact.isFileset());
   }
 
-  public static BuildEventStreamProtos.File.Builder newFileFromArtifact(
-      String name, Artifact artifact, CompletionContext completionContext) {
-    return newFileFromArtifact(name, artifact, PathFragment.EMPTY_FRAGMENT, completionContext);
-  }
-
-  public static BuildEventStreamProtos.File.Builder newFileFromArtifact(
-      String name, Artifact artifact, PathFragment relPath, CompletionContext completionContext) {
+  public static BuildEventStreamProtos.File newFileFromArtifact(
+      @Nullable String name,
+      Artifact artifact,
+      PathFragment relPath,
+      CompletionContext completionContext,
+      @Nullable String uri) {
     if (name == null) {
       name = artifact.getRootRelativePath().getRelative(relPath).getPathString();
       if (OS.getCurrent() != OS.WINDOWS) {
@@ -367,19 +369,31 @@ public static BuildEventStreamProtos.File.Builder newFileFromArtifact(
             .setName(name)
             .addAllPathPrefix(artifact.getRoot().getExecPath().segments());
     FileArtifactValue fileArtifactValue = completionContext.getFileArtifactValue(artifact);
-    if (fileArtifactValue != null && fileArtifactValue.getType().exists()) {
+    if (fileArtifactValue instanceof UnresolvedSymlinkArtifactValue) {
+      file.setSymlinkTargetPath(
+          ((UnresolvedSymlinkArtifactValue) fileArtifactValue).getSymlinkTarget());
+    } else if (fileArtifactValue != null && fileArtifactValue.getType().exists()) {
       byte[] digest = fileArtifactValue.getDigest();
       if (digest != null) {
         file.setDigest(LOWERCASE_HEX_ENCODING.encode(digest));
       }
       file.setLength(fileArtifactValue.getSize());
     }
-    return file;
+    if (uri != null) {
+      file.setUri(uri);
+    }
+    return file.build();
+  }
+
+  public static BuildEventStreamProtos.File newFileFromArtifact(
+      String name, Artifact artifact, CompletionContext completionContext, @Nullable String uri) {
+    return newFileFromArtifact(name, artifact, PathFragment.EMPTY_FRAGMENT, completionContext, uri);
   }
 
-  public static BuildEventStreamProtos.File.Builder newFileFromArtifact(
-      Artifact artifact, CompletionContext completionContext) {
-    return newFileFromArtifact(/* name= */ null, artifact, completionContext);
+  public static BuildEventStreamProtos.File newFileFromArtifact(
+      Artifact artifact, CompletionContext completionContext, @Nullable String uri) {
+    return newFileFromArtifact(
+        /* name= */ null, artifact, PathFragment.EMPTY_FRAGMENT, completionContext, uri);
   }
 
   @Override
@@ -450,7 +464,8 @@ public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventContext convert
     Iterable<Artifact> filteredImportantArtifacts = getLegacyFilteredImportantArtifacts();
     for (Artifact artifact : filteredImportantArtifacts) {
       if (artifact.isDirectory()) {
-        builder.addDirectoryOutput(newFileFromArtifact(artifact, completionContext).build());
+        builder.addDirectoryOutput(
+            newFileFromArtifact(artifact, completionContext, /* uri= */ null));
       }
     }
     // TODO(aehlig): remove direct reporting of artifacts as soon as clients no longer need it.

src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEvent.java

@@ -45,9 +45,10 @@ final class LocalFile {
      * associated files for.
      */
     public enum LocalFileType {
-      OUTPUT,
-      /** An OUTPUT_FILE is an OUTPUT that is known to be a file, not a directory. */
+      OUTPUT, /* of uncertain file type */
       OUTPUT_FILE,
+      OUTPUT_DIRECTORY,
+      OUTPUT_SYMLINK,
       SUCCESSFUL_TEST_OUTPUT,
       FAILED_TEST_OUTPUT,
       COVERAGE_OUTPUT,
@@ -56,14 +57,12 @@ public enum LocalFileType {
       LOG,
       PERFORMANCE_LOG;
 
-      /** Returns true if the LocalFile is a generic output from an Action. */
+      /** Returns whether the LocalFile is a declared action output. */
       public boolean isOutput() {
-        return this == OUTPUT || this == OUTPUT_FILE;
-      }
-
-      /** Returns true if the LocalFile is known by construction to be a file, not a directory. */
-      public boolean isGuaranteedFile() {
-        return this != OUTPUT;
+        return this == OUTPUT
+            || this == OUTPUT_FILE
+            || this == OUTPUT_DIRECTORY
+            || this == OUTPUT_SYMLINK;
       }
     }
 

src/main/java/com/google/devtools/build/lib/buildeventstream/LocalFilesArtifactUploader.java

@@ -17,6 +17,7 @@
 
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.devtools.build.lib.buildeventstream.BuildEvent.LocalFile;
+import com.google.devtools.build.lib.buildeventstream.BuildEvent.LocalFile.LocalFileType;
 import com.google.devtools.build.lib.buildeventstream.PathConverter.FileUriPathConverter;
 import com.google.devtools.build.lib.vfs.Path;
 import io.netty.util.AbstractReferenceCounted;
@@ -66,7 +67,12 @@ public String apply(Path path) {
         // We should throw here, the file wasn't declared in BuildEvent#referencedLocalFiles
         return null;
       }
-      if (!localFile.type.isGuaranteedFile()
+      LocalFileType type = localFile.type;
+      if (type.equals(LocalFileType.OUTPUT_DIRECTORY)
+          || type.equals(LocalFileType.OUTPUT_SYMLINK)) {
+        return null;
+      }
+      if (type.equals(LocalFileType.OUTPUT)
           && fileIsDirectory.computeIfAbsent(path, Path::isDirectory)) {
         return null;
       }

src/main/java/com/google/devtools/build/lib/buildeventstream/PathConverter.java

@@ -29,8 +29,8 @@ public interface PathConverter {
       path -> {
         throw new IllegalStateException(
             String.format(
-                "Can't convert '%s', as it has not been"
-                    + "declared as a referenced artifact of a build event",
+                "Can't convert '%s', as it has not been declared as a referenced artifact of a"
+                    + " build event",
                 path.getPathString()));
       };
 

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

@@ -27,6 +27,7 @@
 import com.google.common.eventbus.Subscribe;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.devtools.build.lib.buildeventstream.BuildEvent.LocalFile;
+import com.google.devtools.build.lib.buildeventstream.BuildEvent.LocalFile.LocalFileType;
 import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader;
 import com.google.devtools.build.lib.buildeventstream.PathConverter;
 import com.google.devtools.build.lib.buildtool.buildevent.ProfilerStartedEvent;
@@ -131,13 +132,21 @@ private static final class PathMetadata {
     private final Path path;
     private final Digest digest;
     private final boolean directory;
+    private final boolean symlink;
     private final boolean remote;
     private final boolean omitted;
 
-    PathMetadata(Path path, Digest digest, boolean directory, boolean remote, boolean omitted) {
+    PathMetadata(
+        Path path,
+        Digest digest,
+        boolean directory,
+        boolean symlink,
+        boolean remote,
+        boolean omitted) {
       this.path = path;
       this.digest = digest;
       this.directory = directory;
+      this.symlink = symlink;
       this.remote = remote;
       this.omitted = omitted;
     }
@@ -154,6 +163,10 @@ public boolean isDirectory() {
       return directory;
     }
 
+    public boolean isSymlink() {
+      return symlink;
+    }
+
     public boolean isRemote() {
       return remote;
     }
@@ -167,32 +180,44 @@ public boolean isOmitted() {
    * Collects metadata for {@code file}. Depending on the underlying filesystem used this method
    * might do I/O.
    */
-  private PathMetadata readPathMetadata(Path file) throws IOException {
-    if (file.isDirectory()) {
+  private PathMetadata readPathMetadata(Path path, LocalFile file) throws IOException {
+    if (file.type == LocalFileType.OUTPUT_DIRECTORY
+        || (file.type == LocalFileType.OUTPUT && path.isDirectory())) {
       return new PathMetadata(
-          file,
+          path,
           /* digest= */ null,
           /* directory= */ true,
+          /* symlink= */ false,
+          /* remote= */ false,
+          /* omitted= */ false);
+    }
+    if (file.type == LocalFileType.OUTPUT_SYMLINK) {
+      return new PathMetadata(
+          path,
+          /* digest= */ null,
+          /* directory= */ false,
+          /* symlink= */ true,
           /* remote= */ false,
           /* omitted= */ false);
     }
 
-    PathFragment filePathFragment = file.asFragment();
+    PathFragment filePathFragment = path.asFragment();
     boolean omitted = false;
     if (omittedFiles.contains(filePathFragment)) {
       omitted = true;
     } else {
       for (PathFragment treeRoot : omittedTreeRoots) {
-        if (file.startsWith(treeRoot)) {
+        if (path.startsWith(treeRoot)) {
           omittedFiles.add(filePathFragment);
           omitted = true;
         }
       }
     }
 
-    DigestUtil digestUtil = new DigestUtil(xattrProvider, file.getFileSystem().getDigestFunction());
-    Digest digest = digestUtil.compute(file);
-    return new PathMetadata(file, digest, /* directory= */ false, isRemoteFile(file), omitted);
+    DigestUtil digestUtil = new DigestUtil(xattrProvider, path.getFileSystem().getDigestFunction());
+    Digest digest = digestUtil.compute(path);
+    return new PathMetadata(
+        path, digest, /* directory= */ false, /* symlink= */ false, isRemoteFile(path), omitted);
   }
 
   private static void processQueryResult(
@@ -208,6 +233,7 @@ private static void processQueryResult(
                 file.getPath(),
                 file.getDigest(),
                 file.isDirectory(),
+                file.isSymlink(),
                 /* remote= */ true,
                 file.isOmitted());
         knownRemotePaths.add(remotePathMetadata);
@@ -217,7 +243,11 @@ private static void processQueryResult(
 
   private boolean shouldUpload(PathMetadata path) {
     boolean result =
-        path.getDigest() != null && !path.isRemote() && !path.isDirectory() && !path.isOmitted();
+        path.getDigest() != null
+            && !path.isRemote()
+            && !path.isDirectory()
+            && !path.isSymlink()
+            && !path.isOmitted();
 
     if (remoteBuildEventUploadMode == RemoteBuildEventUploadMode.MINIMAL) {
       result = result && (isLog(path) || isProfile(path));
@@ -302,6 +332,7 @@ private Single<List<PathMetadata>> uploadLocalFiles(
                               path.getPath(),
                               path.getDigest(),
                               path.isDirectory(),
+                              path.isSymlink(),
                               // set remote to true so the PathConverter will use bytestream://
                               // scheme to convert the URI for this file
                               /* remote= */ true,
@@ -315,7 +346,7 @@ private Single<List<PathMetadata>> uploadLocalFiles(
         .collect(Collectors.toList());
   }
 
-  private Single<PathConverter> upload(Set<Path> files) {
+  private Single<PathConverter> doUpload(Map<Path, LocalFile> files) {
     if (files.isEmpty()) {
       return Single.just(PathConverter.NO_CONVERSION);
     }
@@ -329,17 +360,20 @@ private Single<PathConverter> upload(Set<Path> files) {
     return Single.using(
         remoteCache::retain,
         remoteCache ->
-            Flowable.fromIterable(files)
+            Flowable.fromIterable(files.entrySet())
                 .map(
-                    file -> {
+                    entry -> {
+                      Path path = entry.getKey();
+                      LocalFile file = entry.getValue();
                       try {
-                        return readPathMetadata(file);
+                        return readPathMetadata(path, file);
                       } catch (IOException e) {
-                        reportUploadError(e, file, null);
+                        reportUploadError(e, path, null);
                         return new PathMetadata(
-                            file,
+                            path,
                             /* digest= */ null,
                             /* directory= */ false,
+                            /* symlink= */ false,
                             /* remote= */ false,
                             /* omitted= */ false);
                       }
@@ -361,7 +395,7 @@ public void onProfilerStartedEvent(ProfilerStartedEvent event) {
 
   @Override
   public ListenableFuture<PathConverter> upload(Map<Path, LocalFile> files) {
-    return toListenableFuture(upload(files.keySet()).subscribeOn(scheduler));
+    return toListenableFuture(doUpload(files).subscribeOn(scheduler));
   }
 
   @Override