diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutedEvent.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutedEvent.java index 0f5d9688ea38b9..f643093b4ebbee 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutedEvent.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutedEvent.java @@ -186,7 +186,7 @@ public Collection referencedLocalFiles() { localFiles.add( new LocalFile( primaryOutput, - LocalFileType.forArtifact(outputArtifact), + LocalFileType.forArtifact(outputArtifact, primaryOutputMetadata), outputArtifact, primaryOutputMetadata)); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java b/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java index 3adf040fc35bd0..5b3ee7871a0147 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java @@ -406,12 +406,13 @@ public ImmutableList referencedLocalFiles() { new ArtifactReceiver() { @Override public void accept(Artifact artifact) { + FileArtifactValue metadata = completionContext.getFileArtifactValue(artifact); builder.add( new LocalFile( completionContext.pathResolver().toPath(artifact), - LocalFileType.forArtifact(artifact), + LocalFileType.forArtifact(artifact, metadata), artifact, - completionContext.getFileArtifactValue(artifact))); + metadata)); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEvent.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEvent.java index 89dcdb36304903..9f5d029d05eef4 100644 --- a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEvent.java +++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEvent.java @@ -21,6 +21,7 @@ import com.google.common.util.concurrent.ListenableFuture; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.FileArtifactValue; +import com.google.devtools.build.lib.buildeventstream.BuildEvent.LocalFile.LocalFileType; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.vfs.Path; import java.util.Collection; @@ -58,7 +59,9 @@ public enum LocalFileType { LOG, PERFORMANCE_LOG; - /** Returns whether the LocalFile is a declared action output. */ + /** + * Returns whether the LocalFile is a declared action output. + */ public boolean isOutput() { return this == OUTPUT || this == OUTPUT_FILE @@ -66,7 +69,23 @@ public boolean isOutput() { || this == OUTPUT_SYMLINK; } - public static LocalFileType forArtifact(Artifact artifact) { + /** + * Returns the {@link LocalFileType} inferred from a + * {@link FileArtifactValue, or from the associated {@link Artifact} if the former is not + * available. + */ + public static LocalFileType forArtifact(Artifact artifact, + @Nullable FileArtifactValue metadata) { + if (metadata != null) { + switch (metadata.getType()) { + case DIRECTORY: + return LocalFileType.OUTPUT_DIRECTORY; + case SYMLINK: + return LocalFileType.OUTPUT_SYMLINK; + default: + return LocalFileType.OUTPUT_FILE; + } + } if (artifact.isDirectory()) { return LocalFileType.OUTPUT_DIRECTORY; } else if (artifact.isSymlink()) { diff --git a/src/main/java/com/google/devtools/build/lib/runtime/NamedArtifactGroup.java b/src/main/java/com/google/devtools/build/lib/runtime/NamedArtifactGroup.java index 66ba8919af86e5..ea2f29758dd558 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/NamedArtifactGroup.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/NamedArtifactGroup.java @@ -75,14 +75,14 @@ public Collection referencedLocalFiles() { for (Object elem : set.getLeaves()) { ExpandedArtifact expandedArtifact = (ExpandedArtifact) elem; if (expandedArtifact.relPath == null) { - FileArtifactValue fileMetadata = + FileArtifactValue metadata = completionContext.getFileArtifactValue(expandedArtifact.artifact); artifacts.add( new LocalFile( completionContext.pathResolver().toPath(expandedArtifact.artifact), - getOutputType(fileMetadata), - fileMetadata == null ? null : expandedArtifact.artifact, - fileMetadata)); + LocalFileType.forArtifact(expandedArtifact.artifact, metadata), + metadata == null ? null : expandedArtifact.artifact, + metadata)); } else { // TODO(b/199940216): Can fileset metadata be properly handled here? artifacts.add( @@ -96,20 +96,6 @@ public Collection referencedLocalFiles() { return artifacts.build(); } - private static LocalFileType getOutputType(@Nullable FileArtifactValue fileMetadata) { - if (fileMetadata == null) { - return LocalFileType.OUTPUT; - } - switch (fileMetadata.getType()) { - case DIRECTORY: - return LocalFileType.OUTPUT_DIRECTORY; - case SYMLINK: - return LocalFileType.OUTPUT_SYMLINK; - default: - return LocalFileType.OUTPUT_FILE; - } - } - @Override public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventContext converters) { PathConverter pathConverter = converters.pathConverter(); diff --git a/src/test/java/com/google/devtools/build/lib/analysis/TargetCompleteEventTest.java b/src/test/java/com/google/devtools/build/lib/analysis/TargetCompleteEventTest.java index 53defdf74df67f..4c636ff8f24e3b 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/TargetCompleteEventTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/TargetCompleteEventTest.java @@ -73,10 +73,8 @@ public void testReferencedSourceFile() throws Exception { artifactsToBuild.getAllArtifactsByOutputGroup(), /* announceTargetSummary= */ false); - assertThat(event.referencedLocalFiles()).hasSize(1); - LocalFile localFile = event.referencedLocalFiles().get(0); - assertThat(localFile.path).isEqualTo(artifact.getPath()); - assertThat(localFile.type).isEqualTo(LocalFileType.OUTPUT_FILE); + assertThat(event.referencedLocalFiles()).containsExactly( + new LocalFile(artifact.getPath(), LocalFileType.OUTPUT_FILE, artifact, metadata)); } @Test @@ -97,11 +95,8 @@ public void testReferencedSourceDirectory() throws Exception { artifactsToBuild.getAllArtifactsByOutputGroup(), /* announceTargetSummary= */ false); - assertThat(event.referencedLocalFiles()).hasSize(1); - LocalFile localFile = event.referencedLocalFiles().get(0); - assertThat(localFile.path).isEqualTo(artifact.getPath()); - // TODO(tjgq): This should be reported as a directory. - assertThat(localFile.type).isEqualTo(LocalFileType.OUTPUT_FILE); + assertThat(event.referencedLocalFiles()).containsExactly( + new LocalFile(artifact.getPath(), LocalFileType.OUTPUT_DIRECTORY, artifact, metadata)); } @Test @@ -112,7 +107,7 @@ public void testReferencedTreeArtifact() throws Exception { " d = ctx.actions.declare_directory(ctx.label.name)", " ctx.actions.run_shell(", " outputs = [d],", - " command = 'touch %s/file.txt' % d.path,", + " command = 'cd %s && mkdir dir && touch dir/file.txt && ln -s dir sym' % d.path,", " )", " return DefaultInfo(files = depset([d]))", "dir = rule(_impl)"); @@ -123,16 +118,21 @@ public void testReferencedTreeArtifact() throws Exception { "filegroup(name = 'files', srcs = ['dir'])"); ConfiguredTargetAndData ctAndData = getCtAndData("//:files"); ArtifactsToBuild artifactsToBuild = getArtifactsToBuild(ctAndData); - SpecialArtifact artifact = + SpecialArtifact tree = (SpecialArtifact) Iterables.getOnlyElement(artifactsToBuild.getAllArtifacts().toList()); + TreeFileArtifact fileChild = TreeFileArtifact.createTreeOutput(tree, + PathFragment.create("dir/file.txt")); + FileArtifactValue fileMetadata = FileArtifactValue.createForNormalFile(new byte[]{1, 2, 3}, + null, 10); + TreeFileArtifact dirChild = TreeFileArtifact.createTreeOutput(tree, PathFragment.create("sym")); + FileArtifactValue dirMetadata = FileArtifactValue.createForDirectoryWithMtime(123456789); TreeArtifactValue metadata = - TreeArtifactValue.newBuilder(artifact) - .putChild( - TreeFileArtifact.createTreeOutput(artifact, PathFragment.create("file")), - FileArtifactValue.createForNormalFile(new byte[] {1, 2, 3}, null, 10)) + TreeArtifactValue.newBuilder(tree) + .putChild(fileChild, fileMetadata) + .putChild(dirChild, dirMetadata) .build(); CompletionContext completionContext = - getCompletionContext(ImmutableMap.of(), ImmutableMap.of(artifact, metadata)); + getCompletionContext(ImmutableMap.of(), ImmutableMap.of(tree, metadata)); TargetCompleteEvent event = TargetCompleteEvent.successfulBuild( @@ -141,11 +141,9 @@ public void testReferencedTreeArtifact() throws Exception { artifactsToBuild.getAllArtifactsByOutputGroup(), /* announceTargetSummary= */ false); - assertThat(event.referencedLocalFiles()).hasSize(1); - LocalFile localFile = event.referencedLocalFiles().get(0); - assertThat(localFile.path) - .isEqualTo(Iterables.getOnlyElement(metadata.getChildren()).getPath()); - assertThat(localFile.type).isEqualTo(LocalFileType.OUTPUT_FILE); + assertThat(event.referencedLocalFiles()).containsExactly( + new LocalFile(fileChild.getPath(), LocalFileType.OUTPUT_FILE, fileChild, fileMetadata), + new LocalFile(dirChild.getPath(), LocalFileType.OUTPUT_DIRECTORY, dirChild, dirMetadata)); } @Test @@ -181,10 +179,8 @@ public void testReferencedUnresolvedSymlink() throws Exception { artifactsToBuild.getAllArtifactsByOutputGroup(), /* announceTargetSummary= */ false); - assertThat(event.referencedLocalFiles()).hasSize(1); - LocalFile localFile = event.referencedLocalFiles().get(0); - assertThat(localFile.path).isEqualTo(artifact.getPath()); - assertThat(localFile.type).isEqualTo(LocalFileType.OUTPUT_SYMLINK); + assertThat(event.referencedLocalFiles()).containsExactly( + new LocalFile(artifact.getPath(), LocalFileType.OUTPUT_SYMLINK, artifact, metadata)); } /** Regression test for b/165671166. */