From fd67506b0c73eed53089de5df339e6f4e2d810de Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 1 Oct 2024 12:14:17 -0700 Subject: [PATCH] Introduce `FilesetOutputTree`. This is a mechanical refactoring to replace `ImmutableList` with an extra abstraction layer called `FilesetOutputTree`. In this change, `FilesetOutputTree` only contains `ImmutableList`. In a subsequent change, we will also store a `boolean` indicating whether the fileset has any relative symlinks. If the fileset does not have any relative symlinks (expected to be a common case), we can make some significant performance optimizations. PiperOrigin-RevId: 681116734 Change-Id: If49f8acf15122f6287cd426cb36a081952424b84 --- .../lib/actions/ActionExecutionContext.java | 28 ++++---- .../build/lib/actions/ArtifactExpander.java | 4 +- .../google/devtools/build/lib/actions/BUILD | 12 +++- .../devtools/build/lib/actions/BaseSpawn.java | 2 +- .../build/lib/actions/CompletionContext.java | 21 +++--- .../build/lib/actions/FilesetOutputTree.java | 68 +++++++++++++++++++ .../build/lib/actions/SimpleSpawn.java | 14 ++-- .../devtools/build/lib/actions/Spawn.java | 2 +- .../google/devtools/build/lib/analysis/BUILD | 2 +- .../lib/analysis/actions/SpawnAction.java | 10 +-- .../starlark/StarlarkCustomCommandLine.java | 2 +- .../com/google/devtools/build/lib/exec/BUILD | 2 +- .../build/lib/exec/SpawnInputExpander.java | 21 +++--- .../build/lib/exec/SymlinkTreeStrategy.java | 3 +- .../google/devtools/build/lib/remote/BUILD | 2 +- .../build/lib/remote/RemoteOutputService.java | 6 +- .../lib/skyframe/ActionExecutionFunction.java | 54 +++++++-------- .../lib/skyframe/ActionExecutionValue.java | 35 +++++----- .../lib/skyframe/ActionInputMapHelper.java | 34 +++++----- .../skyframe/ActionInputMetadataProvider.java | 10 +-- .../google/devtools/build/lib/skyframe/BUILD | 13 ++-- .../lib/skyframe/CompletionFunction.java | 6 +- .../skyframe/MetadataConsumerForMetrics.java | 11 ++- .../lib/skyframe/SkyframeActionExecutor.java | 33 +++++---- .../build/lib/skyframe/SkyframeExecutor.java | 4 +- .../com/google/devtools/build/lib/vfs/BUILD | 2 +- .../devtools/build/lib/vfs/OutputService.java | 6 +- .../google/devtools/build/lib/actions/BUILD | 1 + .../lib/actions/CompletionContextTest.java | 11 +-- .../lib/actions/util/ActionsTestUtil.java | 3 +- .../devtools/build/lib/actions/util/BUILD | 1 + .../build/lib/analysis/starlark/BUILD | 1 + .../StarlarkCustomCommandLineTest.java | 18 ++--- .../com/google/devtools/build/lib/exec/BUILD | 1 + .../lib/exec/SpawnInputExpanderTest.java | 65 ++++++++++-------- .../lib/exec/SpawnLogContextTestBase.java | 3 +- .../google/devtools/build/lib/exec/util/BUILD | 2 +- .../build/lib/exec/util/SpawnBuilder.java | 10 ++- .../skyframe/ActionExecutionValueTest.java | 39 ++++++----- ...ValueTransformSharedTreeArtifactsTest.java | 4 +- .../ActionOutputMetadataStoreTest.java | 7 +- .../google/devtools/build/lib/skyframe/BUILD | 3 + .../LostImportantOutputHandlerModule.java | 2 +- .../google/devtools/build/lib/testutil/BUILD | 1 + .../build/lib/testutil/SpawnInputUtils.java | 8 +-- 45 files changed, 340 insertions(+), 247 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/actions/FilesetOutputTree.java diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java index aa238a589deb19..02ecff77a1e9c0 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java @@ -211,14 +211,14 @@ public enum ShowSubcommands { private final FileOutErr fileOutErr; private final ExtendedEventHandler eventHandler; private final ImmutableMap clientEnv; - private final ImmutableMap> topLevelFilesets; + private final ImmutableMap topLevelFilesets; @Nullable private final ArtifactExpander artifactExpander; @Nullable private final Environment env; @Nullable private final FileSystem actionFileSystem; @Nullable private final Object skyframeDepsResult; - private ImmutableList outputSymlinks = ImmutableList.of(); + private FilesetOutputTree filesetOutput = FilesetOutputTree.EMPTY; private final ArtifactPathResolver pathResolver; private final DiscoveredModulesPruner discoveredModulesPruner; @@ -236,7 +236,7 @@ private ActionExecutionContext( FileOutErr fileOutErr, ExtendedEventHandler eventHandler, Map clientEnv, - ImmutableMap> topLevelFilesets, + ImmutableMap topLevelFilesets, @Nullable ArtifactExpander artifactExpander, @Nullable Environment env, @Nullable FileSystem actionFileSystem, @@ -278,7 +278,7 @@ public ActionExecutionContext( FileOutErr fileOutErr, ExtendedEventHandler eventHandler, Map clientEnv, - ImmutableMap> topLevelFilesets, + ImmutableMap topLevelFilesets, ArtifactExpander artifactExpander, @Nullable FileSystem actionFileSystem, @Nullable Object skyframeDepsResult, @@ -425,21 +425,21 @@ public ExtendedEventHandler getEventHandler() { return eventHandler; } - public ImmutableMap> getTopLevelFilesets() { + public ImmutableMap getTopLevelFilesets() { return topLevelFilesets; } - public ImmutableList getOutputSymlinks() { - return outputSymlinks; + public FilesetOutputTree getFilesetOutput() { + return filesetOutput; } - public void setOutputSymlinks(ImmutableList outputSymlinks) { + public void setFilesetOutput(FilesetOutputTree filesetOutput) { checkState( - this.outputSymlinks.isEmpty(), - "Unexpected reassignment of the outputSymlinks of a Fileset from\n:%s to:\n%s", - this.outputSymlinks, - outputSymlinks); - this.outputSymlinks = checkNotNull(outputSymlinks); + this.filesetOutput.isEmpty(), + "Unexpected reassignment of Fileset output from\n:%s to:\n%s", + this.filesetOutput, + filesetOutput); + this.filesetOutput = checkNotNull(filesetOutput); } @Override @@ -586,7 +586,7 @@ public ActionExecutionContext withOutputsAsInputs( ImmutableMap.builder(); for (ActionInput input : additionalInputs) { - additionalInputMap.put(input, getOutputMetadataStore().getOutputMetadata(input)); + additionalInputMap.put(input, outputMetadataStore.getOutputMetadata(input)); } StaticInputMetadataProvider additionalInputMetadata = diff --git a/src/main/java/com/google/devtools/build/lib/actions/ArtifactExpander.java b/src/main/java/com/google/devtools/build/lib/actions/ArtifactExpander.java index c7dadf232de972..02c0e795dc787d 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ArtifactExpander.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ArtifactExpander.java @@ -14,7 +14,6 @@ package com.google.devtools.build.lib.actions; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSortedSet; import com.google.devtools.build.lib.actions.Artifact.ArchivedTreeArtifact; import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType; @@ -52,8 +51,7 @@ default ImmutableSortedSet tryExpandTreeArtifact(Artifact tree * @throws MissingExpansionException if the expander is missing data needed to expand provided * fileset. */ - default ImmutableList expandFileset(Artifact fileset) - throws MissingExpansionException { + default FilesetOutputTree expandFileset(Artifact fileset) throws MissingExpansionException { throw new MissingExpansionException("Cannot expand fileset " + fileset); } diff --git a/src/main/java/com/google/devtools/build/lib/actions/BUILD b/src/main/java/com/google/devtools/build/lib/actions/BUILD index 9d563607b6db3e..9dc1bca45b00f4 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BUILD +++ b/src/main/java/com/google/devtools/build/lib/actions/BUILD @@ -155,6 +155,7 @@ java_library( ":execution_requirements", ":file_metadata", ":fileset_output_symlink", + ":fileset_output_tree", ":localhost_capacity", ":middleman_type", ":package_roots", @@ -359,7 +360,7 @@ java_library( srcs = ["ArtifactExpander.java"], deps = [ ":artifacts", - ":fileset_output_symlink", + ":fileset_output_tree", "//third_party:guava", "//third_party:jsr305", ], @@ -398,6 +399,15 @@ java_library( deps = ["//src/main/protobuf:failure_details_java_proto"], ) +java_library( + name = "fileset_output_tree", + srcs = ["FilesetOutputTree.java"], + deps = [ + ":fileset_output_symlink", + "//third_party:guava", + ], +) + java_library( name = "fileset_output_symlink", srcs = ["FilesetOutputSymlink.java"], diff --git a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java index d11f81c6184dd9..3e42a730566b39 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java @@ -61,7 +61,7 @@ public ImmutableList getArguments() { } @Override - public ImmutableMap> getFilesetMappings() { + public ImmutableMap getFilesetMappings() { return ImmutableMap.of(); } diff --git a/src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java b/src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java index a3a8e641ffcd6e..39c2bd21d59dde 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java +++ b/src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java @@ -56,7 +56,7 @@ public final class CompletionContext implements ArtifactExpander { private final Path execRoot; private final ArtifactPathResolver pathResolver; private final Map treeArtifacts; - private final Map> filesets; + private final Map filesets; @Nullable private final FileArtifactValue baselineCoverageValue; // Only contains the metadata for 'important' artifacts of the Target/Aspect that completed. Any // 'unimportant' artifacts produced by internal output groups (most importantly, _validation) will @@ -70,7 +70,7 @@ public final class CompletionContext implements ArtifactExpander { public CompletionContext( Path execRoot, Map treeArtifacts, - Map> filesets, + Map filesets, @Nullable FileArtifactValue baselineCoverageValue, ArtifactPathResolver pathResolver, ActionInputMap importantInputMap, @@ -88,7 +88,7 @@ public CompletionContext( public static CompletionContext create( Map treeArtifacts, - Map> filesets, + Map filesets, @Nullable FileArtifactValue baselineCoverageValue, boolean expandFilesets, boolean fullyResolveFilesetSymlinks, @@ -124,7 +124,7 @@ public ActionInputMap getImportantInputMap() { return importantInputMap; } - public Map> getExpandedFilesets() { + public Map getExpandedFilesets() { return filesets; } @@ -174,7 +174,7 @@ public void visitArtifacts(Iterable artifacts, ArtifactReceiver receiv } private void visitFileset(Artifact filesetArtifact, ArtifactReceiver receiver) { - ImmutableList links = filesets.get(filesetArtifact); + ImmutableList links = filesets.get(filesetArtifact).symlinks(); FilesetManifest filesetManifest = FilesetManifest.constructFilesetManifestWithoutError( links, @@ -207,15 +207,14 @@ public ArchivedTreeArtifact getArchivedTreeArtifact(Artifact treeArtifact) { } @Override - public ImmutableList expandFileset(Artifact fileset) - throws MissingExpansionException { + public FilesetOutputTree expandFileset(Artifact fileset) throws MissingExpansionException { checkArgument(fileset.isFileset(), fileset); checkState(expandFilesets, "Fileset expansion disabled, cannot expand %s", fileset); - ImmutableList links = filesets.get(fileset); - if (links == null) { + FilesetOutputTree filesetOutput = filesets.get(fileset); + if (filesetOutput == null) { throw new MissingExpansionException("Missing expansion for fileset: " + fileset); } - return links; + return filesetOutput; } /** A function that accepts an {@link Artifact}. */ @@ -230,7 +229,7 @@ public interface PathResolverFactory { ArtifactPathResolver createPathResolverForArtifactValues( ActionInputMap actionInputMap, Map> treeArtifacts, - Map> filesets, + Map filesets, String workspaceName); boolean shouldCreatePathResolverForArtifactValues(); diff --git a/src/main/java/com/google/devtools/build/lib/actions/FilesetOutputTree.java b/src/main/java/com/google/devtools/build/lib/actions/FilesetOutputTree.java new file mode 100644 index 00000000000000..9cdffebbfcf451 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/actions/FilesetOutputTree.java @@ -0,0 +1,68 @@ +// Copyright 2024 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.actions; + +import static com.google.common.base.Preconditions.checkNotNull; + +import com.google.common.base.MoreObjects; +import com.google.common.collect.ImmutableList; + +/** A collection of {@link FilesetOutputSymlink}s comprising the output tree of a fileset. */ +public final class FilesetOutputTree { + + public static final FilesetOutputTree EMPTY = new FilesetOutputTree(ImmutableList.of()); + + public static FilesetOutputTree create(ImmutableList symlinks) { + return symlinks.isEmpty() ? EMPTY : new FilesetOutputTree(symlinks); + } + + private final ImmutableList symlinks; + + private FilesetOutputTree(ImmutableList symlinks) { + this.symlinks = checkNotNull(symlinks); + } + + public ImmutableList symlinks() { + return symlinks; + } + + public int size() { + return symlinks.size(); + } + + public boolean isEmpty() { + return symlinks.isEmpty(); + } + + @Override + public int hashCode() { + return symlinks.hashCode(); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof FilesetOutputTree that)) { + return false; + } + return symlinks.equals(that.symlinks); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this).add("symlinks", symlinks).toString(); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java index 88d9cc1cbf62b1..9e91b27f134f8f 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java @@ -37,7 +37,7 @@ public final class SimpleSpawn implements Spawn { private final ImmutableMap executionInfo; private final NestedSet inputs; private final NestedSet tools; - private final ImmutableMap> filesetMappings; + private final ImmutableMap filesetMappings; private final ImmutableList outputs; // If null, all outputs are mandatory. @Nullable private final Set mandatoryOutputs; @@ -50,7 +50,7 @@ private SimpleSpawn( ImmutableList arguments, ImmutableMap environment, ImmutableMap executionInfo, - ImmutableMap> filesetMappings, + ImmutableMap filesetMappings, NestedSet inputs, NestedSet tools, Collection outputs, @@ -87,7 +87,7 @@ public SimpleSpawn( ImmutableList arguments, ImmutableMap environment, ImmutableMap executionInfo, - ImmutableMap> filesetMappings, + ImmutableMap filesetMappings, NestedSet inputs, NestedSet tools, Collection outputs, @@ -114,7 +114,7 @@ public SimpleSpawn( ImmutableList arguments, ImmutableMap environment, ImmutableMap executionInfo, - ImmutableMap> filesetMappings, + ImmutableMap filesetMappings, NestedSet inputs, NestedSet tools, Collection outputs, @@ -140,7 +140,7 @@ public SimpleSpawn( ImmutableList arguments, ImmutableMap environment, ImmutableMap executionInfo, - ImmutableMap> filesetMappings, + ImmutableMap filesetMappings, NestedSet inputs, NestedSet tools, Collection outputs, @@ -167,7 +167,7 @@ public SimpleSpawn( ImmutableList arguments, ImmutableMap environment, ImmutableMap executionInfo, - ImmutableMap> filesetMappings, + ImmutableMap filesetMappings, NestedSet inputs, NestedSet tools, Collection outputs, @@ -247,7 +247,7 @@ public ImmutableMap getEnvironment() { } @Override - public ImmutableMap> getFilesetMappings() { + public ImmutableMap getFilesetMappings() { return filesetMappings; } diff --git a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java index f85e1fc7330c58..5bec0638438f5e 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java @@ -65,7 +65,7 @@ public interface Spawn extends DescribableExecutionUnit { * Map of the execpath at which we expect the Fileset symlink trees, to a list of * FilesetOutputSymlinks which contains the details of the Symlink trees. */ - ImmutableMap> getFilesetMappings(); + ImmutableMap getFilesetMappings(); /** * Returns the list of files that are required to execute this spawn (e.g. the compiler binary), diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD index 6f331ad2c83b17..aed3d9a30245e5 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD @@ -371,7 +371,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:commandline_item", "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", - "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink", + "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_tree", "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity", "//src/main/java/com/google/devtools/build/lib/actions:package_roots", "//src/main/java/com/google/devtools/build/lib/actions:runfiles_tree", diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java index a917c9539f8438..1d1a24605ba1bc 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java @@ -51,7 +51,7 @@ import com.google.devtools.build.lib.actions.CommandLines.CommandLineAndParamFileInfo; import com.google.devtools.build.lib.actions.CommandLines.ExpandedCommandLines; import com.google.devtools.build.lib.actions.ExecException; -import com.google.devtools.build.lib.actions.FilesetOutputSymlink; +import com.google.devtools.build.lib.actions.FilesetOutputTree; import com.google.devtools.build.lib.actions.ParamFileInfo; import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.actions.ResourceSetOrBuilder; @@ -495,7 +495,7 @@ public ImmutableMap getExecutionInfo() { /** A spawn instance that is tied to a specific SpawnAction. */ private static final class ActionSpawn extends BaseSpawn { private final NestedSet inputs; - private final Map> filesetMappings; + private final Map filesetMappings; private final ImmutableMap effectiveEnvironment; private final boolean reportOutputs; private final PathMapper pathMapper; @@ -513,7 +513,7 @@ private ActionSpawn( boolean envResolved, NestedSet inputs, Iterable additionalInputs, - Map> filesetMappings, + Map filesetMappings, boolean reportOutputs, PathMapper pathMapper) throws CommandLineExpansionException { @@ -545,7 +545,7 @@ private ActionSpawn( private static void addNonFilesetInputs( NestedSetBuilder builder, NestedSet inputs, - Map> filesetMappings) { + Map filesetMappings) { if (filesetMappings.isEmpty()) { // Keep the original nested set intact. This aids callers that exploit the nested set // structure to perform optimizations (see SpawnInputExpander#walkInputs and its callers). @@ -570,7 +570,7 @@ public ImmutableMap getEnvironment() { } @Override - public ImmutableMap> getFilesetMappings() { + public ImmutableMap getFilesetMappings() { return ImmutableMap.copyOf(filesetMappings); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLine.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLine.java index c318d95a8e246c..f1d7ee71f5bcc8 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLine.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLine.java @@ -471,7 +471,7 @@ private static void expandFileset( throws CommandLineExpansionException { ImmutableList expandedFileSet; try { - expandedFileSet = artifactExpander.expandFileset(fileset); + expandedFileSet = artifactExpander.expandFileset(fileset).symlinks(); } catch (MissingExpansionException e) { throw new CommandLineExpansionException( String.format( diff --git a/src/main/java/com/google/devtools/build/lib/exec/BUILD b/src/main/java/com/google/devtools/build/lib/exec/BUILD index 346d4e7a7281e6..ac6bc13fb9fbec 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/BUILD +++ b/src/main/java/com/google/devtools/build/lib/exec/BUILD @@ -254,7 +254,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:action_input_helper", "//src/main/java/com/google/devtools/build/lib/actions:artifact_expander", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", - "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink", + "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_tree", "//src/main/java/com/google/devtools/build/lib/actions:runfiles_tree", "//src/main/java/com/google/devtools/build/lib/collect/nestedset", "//src/main/java/com/google/devtools/build/lib/vfs", diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java index c873fa9c496db2..9f145a4a234942 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java @@ -18,6 +18,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputHelper; import com.google.devtools.build.lib.actions.Artifact; @@ -29,7 +30,7 @@ import com.google.devtools.build.lib.actions.FilesetManifest; import com.google.devtools.build.lib.actions.FilesetManifest.ForbiddenRelativeSymlinkException; import com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehavior; -import com.google.devtools.build.lib.actions.FilesetOutputSymlink; +import com.google.devtools.build.lib.actions.FilesetOutputTree; import com.google.devtools.build.lib.actions.ForbiddenActionInputException; import com.google.devtools.build.lib.actions.InputMetadataProvider; import com.google.devtools.build.lib.actions.PathMapper; @@ -151,14 +152,14 @@ public void addSingleRunfilesTreeToInputs( } } } else if (artifact.isFileset()) { - ImmutableList filesetLinks; + FilesetOutputTree filesetOutput; try { - filesetLinks = artifactExpander.expandFileset(artifact); + filesetOutput = artifactExpander.expandFileset(artifact); } catch (MissingExpansionException e) { throw new IllegalStateException(e); } // TODO(bazel-team): Add path mapping support for filesets. - addFilesetManifest(location, artifact, filesetLinks, inputMap, baseDirectory); + addFilesetManifest(location, artifact, filesetOutput, inputMap, baseDirectory); } else { // TODO: b/7075837 - If we want to prohibit directory inputs, we can check if // localArtifact is a directory and, if so, throw a ForbiddenActionInputException. @@ -169,12 +170,11 @@ public void addSingleRunfilesTreeToInputs( @VisibleForTesting void addFilesetManifests( - Map> filesetMappings, + Map filesetMappings, Map inputMap, PathFragment baseDirectory) throws ForbiddenRelativeSymlinkException { - for (Map.Entry> entry : - filesetMappings.entrySet()) { + for (Map.Entry entry : filesetMappings.entrySet()) { Artifact fileset = entry.getKey(); addFilesetManifest(fileset.getExecPath(), fileset, entry.getValue(), inputMap, baseDirectory); } @@ -183,13 +183,14 @@ void addFilesetManifests( private void addFilesetManifest( PathFragment location, Artifact filesetArtifact, - ImmutableList filesetLinks, + FilesetOutputTree filesetOutput, Map inputMap, PathFragment baseDirectory) throws ForbiddenRelativeSymlinkException { Preconditions.checkArgument(filesetArtifact.isFileset(), filesetArtifact); FilesetManifest filesetManifest = - FilesetManifest.constructFilesetManifest(filesetLinks, location, relSymlinkBehavior); + FilesetManifest.constructFilesetManifest( + filesetOutput.symlinks(), location, relSymlinkBehavior); for (Map.Entry mapping : filesetManifest.getEntries().entrySet()) { String value = mapping.getValue(); @@ -333,7 +334,7 @@ public void walkInputs( spawn.getPathMapper(), visitor); - Map> filesetMappings = spawn.getFilesetMappings(); + ImmutableMap filesetMappings = spawn.getFilesetMappings(); // filesetMappings is assumed to be very small, so no need to implement visitNonLeaves() for // improved runtime. visitor.visit( diff --git a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java index efff9886a959e4..39b45494e0e3fc 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java @@ -85,7 +85,8 @@ public void createSymlinks( filesetLinks = actionExecutionContext .getArtifactExpander() - .expandFileset(action.getInputManifest()); + .expandFileset(action.getInputManifest()) + .symlinks(); } catch (MissingExpansionException e) { throw new IllegalStateException(e); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/BUILD b/src/main/java/com/google/devtools/build/lib/remote/BUILD index 50025f6b2a557e..af5338dbc413e6 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD @@ -65,7 +65,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", - "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink", + "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_tree", "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster", "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories", "//src/main/java/com/google/devtools/build/lib/analysis:config/build_options", diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java index ca2b4687cb4bcd..22f4c0138cc6e4 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java @@ -26,7 +26,7 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.ArtifactPathResolver; -import com.google.devtools.build.lib.actions.FilesetOutputSymlink; +import com.google.devtools.build.lib.actions.FilesetOutputTree; import com.google.devtools.build.lib.actions.InputMetadataProvider; import com.google.devtools.build.lib.actions.RemoteArtifactChecker; import com.google.devtools.build.lib.actions.cache.MetadataInjector; @@ -117,7 +117,7 @@ public void updateActionFileSystemContext( FileSystem actionFileSystem, Environment env, MetadataInjector injector, - ImmutableMap> filesets) { + ImmutableMap filesets) { ((RemoteActionFileSystem) actionFileSystem).updateContext(action); } @@ -229,7 +229,7 @@ public ArtifactPathResolver createPathResolverForArtifactValues( ImmutableList pathEntries, ActionInputMap actionInputMap, Map> treeArtifacts, - Map> filesets) { + Map filesets) { FileSystem remoteFileSystem = new RemoteActionFileSystem( fileSystem, diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java index c20d33314093dd..0e6efd357f7c07 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java @@ -57,7 +57,7 @@ import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.DiscoveredInputsEvent; import com.google.devtools.build.lib.actions.FileArtifactValue; -import com.google.devtools.build.lib.actions.FilesetOutputSymlink; +import com.google.devtools.build.lib.actions.FilesetOutputTree; import com.google.devtools.build.lib.actions.InputMetadataProvider; import com.google.devtools.build.lib.actions.LostInputsActionExecutionException; import com.google.devtools.build.lib.actions.PackageRootResolver; @@ -823,8 +823,7 @@ private ActionExecutionValue checkCacheAndExecuteIfNeeded( env.getListener(), state.discoveredInputs != null, action, actionLookupData)); } - ImmutableMap> expandedFilesets = - state.getExpandedFilesets(); + ImmutableMap expandedFilesets = state.getExpandedFilesets(); ArtifactExpander artifactExpander = new ActionInputArtifactExpander(state.inputArtifactData, expandedFilesets); @@ -872,7 +871,7 @@ private ActionExecutionValue checkCacheAndExecuteIfNeeded( + "SkyframeAwareAction which should be re-executed unconditionally. Action: %s", action); return ActionExecutionValue.createFromOutputMetadataStore( - outputMetadataStore, /* outputSymlinks= */ ImmutableList.of(), action); + outputMetadataStore, /* filesetOutput= */ FilesetOutputTree.EMPTY, action); } outputMetadataStore.prepareForActionExecution(); @@ -961,8 +960,7 @@ public void run( // TODO(b/160603797): For the sake of action key computation, we should not need // state.filesetsInsideRunfiles. In fact, for the outputMetadataStore, we are guaranteed to // not expand any filesets since we request metadata for input/output Artifacts only. - ImmutableMap> expandedFilesets = - state.getExpandedFilesets(); + ImmutableMap expandedFilesets = state.getExpandedFilesets(); if (action.discoversInputs()) { state.discoveredInputs = action.getInputs(); addDiscoveredInputs(state, env, action); @@ -987,11 +985,10 @@ public void run( */ private static final class ActionInputArtifactExpander implements ArtifactExpander { private final ActionInputMap inputArtifactData; - private final Map> expandedFilesets; + private final Map expandedFilesets; ActionInputArtifactExpander( - ActionInputMap inputArtifactData, - Map> expandedFilesets) { + ActionInputMap inputArtifactData, Map expandedFilesets) { this.inputArtifactData = inputArtifactData; this.expandedFilesets = expandedFilesets; } @@ -1008,14 +1005,13 @@ public ImmutableSortedSet expandTreeArtifact(Artifact treeArti } @Override - public ImmutableList expandFileset(Artifact fileset) - throws MissingExpansionException { + public FilesetOutputTree expandFileset(Artifact fileset) throws MissingExpansionException { checkArgument(fileset.isFileset(), fileset); - ImmutableList filesetLinks = expandedFilesets.get(fileset); - if (filesetLinks == null) { + FilesetOutputTree filesetOutput = expandedFilesets.get(fileset); + if (filesetOutput == null) { throw new MissingExpansionException("Missing expansion for fileset: " + fileset); } - return filesetLinks; + return filesetOutput; } @Override @@ -1136,16 +1132,17 @@ private static Object establishSkyframeDependencies(Environment env, Action acti private static class CheckInputResults { /** Metadata about Artifacts consumed by this Action. */ private final ActionInputMap actionInputMap; + /** Artifact expansion mapping for Filesets embedded in Runfiles. */ - private final ImmutableMap> - filesetsInsideRunfiles; + private final ImmutableMap filesetsInsideRunfiles; + /** Artifact expansion mapping for top level filesets. */ - private final ImmutableMap> topLevelFilesets; + private final ImmutableMap topLevelFilesets; CheckInputResults( ActionInputMap actionInputMap, - Map> filesetsInsideRunfiles, - Map> topLevelFilesets) { + Map filesetsInsideRunfiles, + Map topLevelFilesets) { this.actionInputMap = actionInputMap; this.filesetsInsideRunfiles = ImmutableMap.copyOf(filesetsInsideRunfiles); this.topLevelFilesets = ImmutableMap.copyOf(topLevelFilesets); @@ -1155,8 +1152,8 @@ private static class CheckInputResults { private interface AccumulateInputResultsFactory { R create( S actionInputMapSink, - Map> filesetsInsideRunfiles, - Map> topLevelFilesets); + Map filesetsInsideRunfiles, + Map topLevelFilesets); } /** @@ -1304,10 +1301,8 @@ private R accumulateInputs( // When there are no missing values or there was an error, we can start checking individual // files. We don't bother to optimize the error-ful case since it's rare. - Map> filesetsInsideRunfiles = - Maps.newHashMapWithExpectedSize(0); - Map> topLevelFilesets = - Maps.newHashMapWithExpectedSize(0); + Map filesetsInsideRunfiles = Maps.newHashMapWithExpectedSize(0); + Map topLevelFilesets = Maps.newHashMapWithExpectedSize(0); S inputArtifactData = actionInputMapSinkFactory.apply(allInputsList.size()); List undoneInputs = new ArrayList<>(0); @@ -1512,11 +1507,12 @@ public void setSerializationState(SerializationState state) { */ static class InputDiscoveryState implements SkyKeyComputeState { AllInputs allInputs; + /** Mutable map containing metadata for known artifacts. */ ActionInputMap inputArtifactData = null; - ImmutableMap> filesetsInsideRunfiles = null; - ImmutableMap> topLevelFilesets = null; + ImmutableMap filesetsInsideRunfiles = null; + ImmutableMap topLevelFilesets = null; Token token = null; NestedSet discoveredInputs = null; FileSystem actionFileSystem = null; @@ -1539,12 +1535,12 @@ boolean hasCheckedActionCache() { return token != null; } - ImmutableMap> getExpandedFilesets() { + ImmutableMap getExpandedFilesets() { if (topLevelFilesets == null || topLevelFilesets.isEmpty()) { return filesetsInsideRunfiles; } - Map> filesetsMap = + Map filesetsMap = Maps.newHashMapWithExpectedSize(filesetsInsideRunfiles.size() + topLevelFilesets.size()); filesetsMap.putAll(filesetsInsideRunfiles); filesetsMap.putAll(topLevelFilesets); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java index 87c99019c934c8..9a596d5a810338 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java @@ -19,7 +19,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.common.collect.Maps; @@ -31,7 +30,7 @@ import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.FileArtifactValue; -import com.google.devtools.build.lib.actions.FilesetOutputSymlink; +import com.google.devtools.build.lib.actions.FilesetOutputTree; 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; @@ -47,7 +46,6 @@ import java.util.Collection; import java.util.Map; import java.util.NoSuchElementException; -import java.util.Objects; import java.util.function.BiFunction; import javax.annotation.Nullable; @@ -62,7 +60,7 @@ private ActionExecutionValue() {} public static ActionExecutionValue createFromOutputMetadataStore( ImmutableMap artifactData, ImmutableMap treeArtifactData, - ImmutableList outputSymlinks, + FilesetOutputTree filesetOutput, NestedSet discoveredModules) { // Use forEach instead of entrySet to avoid instantiating an EntrySet in ImmutableMap. artifactData.forEach( @@ -93,7 +91,7 @@ public static ActionExecutionValue createFromOutputMetadataStore( tree)); }); - if (!outputSymlinks.isEmpty()) { + if (!filesetOutput.isEmpty()) { checkArgument( artifactData.size() == 1, "Fileset actions should have a single output file (the manifest): %s", @@ -106,7 +104,7 @@ public static ActionExecutionValue createFromOutputMetadataStore( discoveredModules.isEmpty(), "Fileset actions do not discover modules: %s", discoveredModules); - return new Fileset(artifactData, outputSymlinks); + return new Fileset(artifactData, filesetOutput); } if (!discoveredModules.isEmpty()) { @@ -135,12 +133,12 @@ public static ActionExecutionValue createFromOutputMetadataStore( static ActionExecutionValue createFromOutputMetadataStore( ActionOutputMetadataStore actionOutputMetadataStore, - ImmutableList outputSymlinks, + FilesetOutputTree filesetOutput, Action action) { return createFromOutputMetadataStore( actionOutputMetadataStore.getAllArtifactData(), actionOutputMetadataStore.getAllTreeArtifactData(), - outputSymlinks, + filesetOutput, action instanceof IncludeScannable includeScannable ? includeScannable.getDiscoveredModules() : NestedSetBuilder.emptySet(Order.STABLE_ORDER)); @@ -203,8 +201,8 @@ public ImmutableMap getAllTreeArtifactValues() { return ImmutableMap.of(); } - public ImmutableList getOutputSymlinks() { - return ImmutableList.of(); + public FilesetOutputTree getFilesetOutput() { + return FilesetOutputTree.EMPTY; } public NestedSet getDiscoveredModules() { @@ -229,7 +227,7 @@ public final boolean equals(Object obj) { } return getAllFileValues().equals(o.getAllFileValues()) && getAllTreeArtifactValues().equals(o.getAllTreeArtifactValues()) - && Objects.equals(getOutputSymlinks(), o.getOutputSymlinks()) + && getFilesetOutput().equals(o.getFilesetOutput()) // We use shallowEquals to avoid materializing the nested sets just for change-pruning. This // makes change-pruning potentially less effective, but never incorrect. && getDiscoveredModules().shallowEquals(o.getDiscoveredModules()); @@ -239,7 +237,7 @@ && getAllTreeArtifactValues().equals(o.getAllTreeArtifactValues()) public final int hashCode() { return 31 * HashCodes.hashObjects( - getAllFileValues(), getAllTreeArtifactValues(), getOutputSymlinks()) + getAllFileValues(), getAllTreeArtifactValues(), getFilesetOutput()) + getDiscoveredModules().shallowHashCode(); } @@ -316,7 +314,7 @@ public final ActionExecutionValue transformForSharedAction(Action action) transformMap(artifactData, newArtifactMap, action, (newArtifact, value) -> value), transformMap( treeArtifactData, newArtifactMap, action, ActionExecutionValue::transformSharedTree), - getOutputSymlinks(), + getFilesetOutput(), // Discovered modules come from the action's inputs, and so don't need to be transformed. getDiscoveredModules()); } @@ -368,18 +366,17 @@ public final ImmutableMap getAllFileValues() { * com.google.devtools.build.lib.view.fileset.SkyframeFilesetManifestAction}. */ private static final class Fileset extends SingleOutputFile { - private final ImmutableList outputSymlinks; + private final FilesetOutputTree filesetOutput; Fileset( - ImmutableMap artifactData, - ImmutableList outputSymlinks) { + ImmutableMap artifactData, FilesetOutputTree filesetOutput) { super(artifactData); - this.outputSymlinks = outputSymlinks; + this.filesetOutput = filesetOutput; } @Override - public ImmutableList getOutputSymlinks() { - return outputSymlinks; + public FilesetOutputTree getFilesetOutput() { + return filesetOutput; } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java index 8feecef76f466b..50cc96ac1339b9 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java @@ -16,7 +16,6 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; -import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; import com.google.devtools.build.lib.actions.ActionInputMapSink; import com.google.devtools.build.lib.actions.ActionLookupData; @@ -26,7 +25,7 @@ import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact; import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; import com.google.devtools.build.lib.actions.FileArtifactValue; -import com.google.devtools.build.lib.actions.FilesetOutputSymlink; +import com.google.devtools.build.lib.actions.FilesetOutputTree; import com.google.devtools.build.lib.actions.RunfilesArtifactValue; import com.google.devtools.build.lib.analysis.actions.SymlinkAction; import com.google.devtools.build.lib.skyframe.TreeArtifactValue.ArchivedRepresentation; @@ -44,8 +43,8 @@ private ActionInputMapHelper() {} static void addToMap( ActionInputMapSink inputMap, BiConsumer treeArtifactConsumer, - Map> filesetsInsideRunfiles, - Map> topLevelFilesets, + Map filesetsInsideRunfiles, + Map topLevelFilesets, Artifact key, SkyValue value, Environment env) @@ -68,8 +67,8 @@ static void addToMap( static void addToMap( ActionInputMapSink inputMap, BiConsumer treeArtifactConsumer, - Map> filesetsInsideRunfiles, - Map> topLevelFilesets, + Map filesetsInsideRunfiles, + Map topLevelFilesets, Artifact key, SkyValue value, Environment env, @@ -84,11 +83,10 @@ static void addToMap( (artifact, metadata) -> { inputMap.put(artifact, metadata, /* depOwner= */ key); if (artifact.isFileset()) { - ImmutableList expandedFileset = - getFilesets(env, (SpecialArtifact) artifact); - if (expandedFileset != null) { - filesetsInsideRunfiles.put(artifact, expandedFileset); - consumer.accumulate(expandedFileset); + FilesetOutputTree filesetOutput = getFilesetOutput(env, (SpecialArtifact) artifact); + if (filesetOutput != null) { + filesetsInsideRunfiles.put(artifact, filesetOutput); + consumer.accumulate(filesetOutput); } } else { consumer.accumulate(metadata); @@ -123,10 +121,10 @@ static void addToMap( FileArtifactValue metadata = ((ActionExecutionValue) value).getExistingFileArtifactValue(key); inputMap.put(key, metadata, key); if (key.isFileset()) { - ImmutableList filesets = getFilesets(env, (SpecialArtifact) key); - if (filesets != null) { - topLevelFilesets.put(key, filesets); - consumer.accumulate(filesets); + FilesetOutputTree filesetOutput = getFilesetOutput(env, (SpecialArtifact) key); + if (filesetOutput != null) { + topLevelFilesets.put(key, filesetOutput); + consumer.accumulate(filesetOutput); } } else { consumer.accumulate(metadata); @@ -140,8 +138,8 @@ static void addToMap( } @Nullable - private static ImmutableList getFilesets( - Environment env, SpecialArtifact actionInput) throws InterruptedException { + private static FilesetOutputTree getFilesetOutput(Environment env, SpecialArtifact actionInput) + throws InterruptedException { checkState(actionInput.isFileset(), actionInput); ActionLookupData generatingActionKey = actionInput.getGeneratingActionKey(); ActionLookupKey filesetActionLookupKey = generatingActionKey.getActionLookupKey(); @@ -190,7 +188,7 @@ private static ImmutableList getFilesets( // SkyframeFilesetManifestAction whose ActionExecutionValue (filesetValue) is needed here. return null; } - return filesetValue.getOutputSymlinks(); + return filesetValue.getFilesetOutput(); } private static void expandTreeArtifactAndPopulateArtifactData( diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMetadataProvider.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMetadataProvider.java index 81dd22abe9bcea..2edc4660e42f55 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMetadataProvider.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMetadataProvider.java @@ -25,7 +25,7 @@ import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FilesetManifest; import com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehaviorWithoutError; -import com.google.devtools.build.lib.actions.FilesetOutputSymlink; +import com.google.devtools.build.lib.actions.FilesetOutputTree; import com.google.devtools.build.lib.actions.InputMetadataProvider; import com.google.devtools.build.lib.actions.RunfilesArtifactValue; import com.google.devtools.build.lib.actions.RunfilesTree; @@ -61,19 +61,19 @@ final class ActionInputMetadataProvider implements InputMetadataProvider { ActionInputMetadataProvider( PathFragment execRoot, ActionInputMap inputArtifactData, - Map> filesets) { + Map filesets) { this.execRoot = execRoot; this.inputArtifactData = inputArtifactData; this.filesetMapping = Suppliers.memoize(() -> createFilesetMapping(filesets, execRoot)); } private static ImmutableMap createFilesetMapping( - Map> filesets, PathFragment execRoot) { + Map filesets, PathFragment execRoot) { Map filesetMap = new HashMap<>(); - for (ImmutableList links : filesets.values()) { + for (FilesetOutputTree filesetOutput : filesets.values()) { FilesetManifest manifest = FilesetManifest.constructFilesetManifestWithoutError( - links, execRoot, RelativeSymlinkBehaviorWithoutError.RESOLVE); + filesetOutput.symlinks(), execRoot, RelativeSymlinkBehaviorWithoutError.RESOLVE); manifest .getArtifactValues() .forEach( diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD index 6d7fb97b3f24e7..f468abb07375cf 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD @@ -233,7 +233,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:commandline_item", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", - "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink", + "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_tree", "//src/main/java/com/google/devtools/build/lib/actions:important_output_handler", "//src/main/java/com/google/devtools/build/lib/actions:package_roots", "//src/main/java/com/google/devtools/build/lib/actions:resource_manager", @@ -482,7 +482,7 @@ java_library( "//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/actions:file_metadata", - "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink", + "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_tree", "//src/main/java/com/google/devtools/build/lib/collect/nestedset", "//src/main/java/com/google/devtools/build/lib/concurrent", "//src/main/java/com/google/devtools/build/lib/rules/cpp:cpp_interface", @@ -507,7 +507,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", - "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink", + "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_tree", "//src/main/java/com/google/devtools/build/lib/actions:runfiles_metadata", "//src/main/java/com/google/devtools/build/lib/analysis:actions/symlink_action", "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", @@ -538,7 +538,7 @@ java_library( "//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/actions:file_metadata", - "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink", + "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_tree", "//src/main/java/com/google/devtools/build/lib/actions:runfiles_metadata", "//src/main/java/com/google/devtools/build/lib/actions:runfiles_tree", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", @@ -1776,10 +1776,9 @@ java_library( deps = [ ":tree_artifact_value", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", - "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink", + "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_tree", "//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto", "//src/main/java/com/google/devtools/build/lib/concurrent", - "//third_party:guava", ], ) @@ -2452,7 +2451,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:artifact_expander", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", - "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink", + "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_tree", "//src/main/java/com/google/devtools/build/lib/actions:package_roots", "//src/main/java/com/google/devtools/build/lib/actions:thread_state_receiver", "//src/main/java/com/google/devtools/build/lib/analysis:config/core_options", diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java index f4d6623f171d6f..79db0fbbf01afe 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java @@ -36,7 +36,7 @@ import com.google.devtools.build.lib.actions.EventReportingArtifacts; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue; -import com.google.devtools.build.lib.actions.FilesetOutputSymlink; +import com.google.devtools.build.lib.actions.FilesetOutputTree; import com.google.devtools.build.lib.actions.ImportantOutputHandler; import com.google.devtools.build.lib.actions.ImportantOutputHandler.ImportantOutputException; import com.google.devtools.build.lib.actions.ImportantOutputHandler.LostArtifacts; @@ -222,8 +222,8 @@ public SkyValue compute(SkyKey skyKey, Environment env) // TODO: b/239184359 - Can we just get the tree artifacts from the ActionInputMap? Map treeArtifacts = new HashMap<>(); - Map> expandedFilesets = new HashMap<>(); - Map> topLevelFilesets = new HashMap<>(); + Map expandedFilesets = new HashMap<>(); + Map topLevelFilesets = new HashMap<>(); ActionExecutionException firstActionExecutionException = null; NestedSetBuilder rootCausesBuilder = NestedSetBuilder.stableOrder(); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/MetadataConsumerForMetrics.java b/src/main/java/com/google/devtools/build/lib/skyframe/MetadataConsumerForMetrics.java index 9a679f0a7a92c3..6fbf143a72f545 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/MetadataConsumerForMetrics.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/MetadataConsumerForMetrics.java @@ -13,10 +13,9 @@ // limitations under the License. package com.google.devtools.build.lib.skyframe; -import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FileStateType; -import com.google.devtools.build.lib.actions.FilesetOutputSymlink; +import com.google.devtools.build.lib.actions.FilesetOutputTree; import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildMetrics.ArtifactMetrics; import com.google.devtools.build.lib.concurrent.ThreadSafety; import java.util.concurrent.atomic.AtomicInteger; @@ -34,14 +33,14 @@ public void accumulate(FileArtifactValue metadata) {} public void accumulate(TreeArtifactValue treeArtifactValue) {} @Override - public void accumulate(ImmutableList filesetOutputSymlinks) {} + public void accumulate(FilesetOutputTree filesetOutput) {} }; void accumulate(FileArtifactValue metadata); void accumulate(TreeArtifactValue treeArtifactValue); - void accumulate(ImmutableList filesetOutputSymlinks); + void accumulate(FilesetOutputTree filesetOutput); /** Accumulates file metadata for later export to a {@link ArtifactMetrics.FilesMetric} object. */ class FilesMetricConsumer implements MetadataConsumerForMetrics { @@ -69,11 +68,11 @@ public void accumulate(TreeArtifactValue treeArtifactValue) { } @Override - public void accumulate(ImmutableList filesetOutputSymlinks) { + public void accumulate(FilesetOutputTree filesetOutput) { // This is a bit of a fudge: we include the symlinks as a count, but don't count their // targets' sizes, because (a) plumbing the data is hard, (b) it would double-count symlinks // to output files, and (c) it's not even uniquely generated content for input files. - count.addAndGet(filesetOutputSymlinks.size()); + count.addAndGet(filesetOutput.size()); } @ThreadSafety.ThreadSafe diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java index 600dd904cd76ba..2295e87cc2a18a 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java @@ -63,7 +63,7 @@ import com.google.devtools.build.lib.actions.EnvironmentalExecException; import com.google.devtools.build.lib.actions.Executor; import com.google.devtools.build.lib.actions.FileArtifactValue; -import com.google.devtools.build.lib.actions.FilesetOutputSymlink; +import com.google.devtools.build.lib.actions.FilesetOutputTree; import com.google.devtools.build.lib.actions.InputMetadataProvider; import com.google.devtools.build.lib.actions.LostInputsActionExecutionException; import com.google.devtools.build.lib.actions.NotifyOnActionCacheHit; @@ -396,7 +396,7 @@ private void updateActionFileSystemContext( FileSystem actionFileSystem, Environment env, MetadataInjector metadataInjector, - ImmutableMap> filesets) { + ImmutableMap filesets) { outputService.updateActionFileSystemContext( action, actionFileSystem, env, metadataInjector, filesets); } @@ -497,8 +497,8 @@ ActionExecutionValue executeAction( long actionStartTime, ActionLookupData actionLookupData, ArtifactExpander artifactExpander, - ImmutableMap> expandedFilesets, - ImmutableMap> topLevelFilesets, + ImmutableMap expandedFilesets, + ImmutableMap topLevelFilesets, @Nullable FileSystem actionFileSystem, @Nullable Object skyframeDepsResult, ActionPostprocessing postprocessing, @@ -592,7 +592,7 @@ private ActionExecutionContext getContext( InputMetadataProvider inputMetadataProvider, OutputMetadataStore outputMetadataStore, ArtifactExpander artifactExpander, - ImmutableMap> topLevelFilesets, + ImmutableMap topLevelFilesets, @Nullable FileSystem actionFileSystem, @Nullable Object skyframeDepsResult, ActionLookupData actionLookupData) { @@ -743,7 +743,7 @@ public T getContext(Class type) { checkOutputs( action, outputMetadataStore, - /* filesetOutputSymlinksForMetrics= */ null, + /* filesetOutputForMetrics= */ null, /* isActionCacheHitForMetrics= */ true); if (!eventPosted) { eventHandler.post( @@ -1235,7 +1235,7 @@ private ActionExecutionValue actuallyCompleteAction( if (!checkOutputs( action, outputMetadataStore, - actionExecutionContext.getOutputSymlinks(), + actionExecutionContext.getFilesetOutput(), /* isActionCacheHitForMetrics= */ false)) { throw toActionExecutionException( "not all outputs were created or valid", @@ -1309,17 +1309,16 @@ private ActionExecutionValue actuallyCompleteAction( fileOutErr, ErrorTiming.NO_ERROR); - ImmutableList outputSymlinks = - actionExecutionContext.getOutputSymlinks(); + FilesetOutputTree filesetOutput = actionExecutionContext.getFilesetOutput(); checkState( - outputSymlinks.isEmpty() || action instanceof SkyframeAwareAction, + filesetOutput.isEmpty() || action instanceof SkyframeAwareAction, "Unexpected to find outputSymlinks set in an action which is not a SkyframeAwareAction." + "\nAction: %s" + "\nSymlinks: %s", action, - outputSymlinks); + filesetOutput); return ActionExecutionValue.createFromOutputMetadataStore( - this.outputMetadataStore, outputSymlinks, action); + this.outputMetadataStore, filesetOutput, action); } /** @@ -1542,7 +1541,7 @@ private static LostInputsCheck lostInputsCheck( private boolean checkOutputs( Action action, OutputMetadataStore outputMetadataStore, - @Nullable ImmutableList filesetOutputSymlinksForMetrics, + @Nullable FilesetOutputTree filesetOutputForMetrics, boolean isActionCacheHitForMetrics) throws InterruptedException { boolean success = true; @@ -1563,7 +1562,7 @@ private boolean checkOutputs( output, metadata, outputMetadataStore, - filesetOutputSymlinksForMetrics, + filesetOutputForMetrics, isActionCacheHitForMetrics, action); } catch (IOException e) { @@ -1592,7 +1591,7 @@ private void addOutputToMetrics( Artifact output, FileArtifactValue metadata, OutputMetadataStore outputMetadataStore, - @Nullable ImmutableList filesetOutputSymlinks, + @Nullable FilesetOutputTree filesetOutput, boolean isActionCacheHit, Action actionForDebugging) throws IOException, InterruptedException { @@ -1604,8 +1603,8 @@ private void addOutputToMetrics( output, outputMetadataStore, actionForDebugging))); return; } - if (output.isFileset() && filesetOutputSymlinks != null) { - outputArtifactsSeen.accumulate(filesetOutputSymlinks); + if (output.isFileset() && filesetOutput != null) { + outputArtifactsSeen.accumulate(filesetOutput); } else if (!output.isTreeArtifact()) { outputArtifactsSeen.accumulate(metadata); if (isActionCacheHit) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index 4169358a0f3d21..639a812badbec5 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -79,7 +79,7 @@ import com.google.devtools.build.lib.actions.Executor; import com.google.devtools.build.lib.actions.FileStateValue; import com.google.devtools.build.lib.actions.FileValue; -import com.google.devtools.build.lib.actions.FilesetOutputSymlink; +import com.google.devtools.build.lib.actions.FilesetOutputTree; import com.google.devtools.build.lib.actions.InputMetadataProvider; import com.google.devtools.build.lib.actions.MapBasedActionGraph; import com.google.devtools.build.lib.actions.PackageRoots; @@ -527,7 +527,7 @@ public boolean shouldCreatePathResolverForArtifactValues() { public ArtifactPathResolver createPathResolverForArtifactValues( ActionInputMap actionInputMap, Map> treeArtifacts, - Map> filesets, + Map filesets, String workspaceName) { checkState(shouldCreatePathResolverForArtifactValues()); return outputService.createPathResolverForArtifactValues( diff --git a/src/main/java/com/google/devtools/build/lib/vfs/BUILD b/src/main/java/com/google/devtools/build/lib/vfs/BUILD index 00be35d0017e04..538257903d8e0d 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/BUILD +++ b/src/main/java/com/google/devtools/build/lib/vfs/BUILD @@ -91,7 +91,7 @@ java_library( ":vfs", "//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/actions:fileset_output_symlink", + "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_tree", "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories", "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception", diff --git a/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java b/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java index 6e79e5cbd816d7..1102cf34ea1343 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java @@ -26,7 +26,7 @@ import com.google.devtools.build.lib.actions.BuildFailedException; import com.google.devtools.build.lib.actions.EnvironmentalExecException; import com.google.devtools.build.lib.actions.ExecException; -import com.google.devtools.build.lib.actions.FilesetOutputSymlink; +import com.google.devtools.build.lib.actions.FilesetOutputTree; import com.google.devtools.build.lib.actions.LostInputsActionExecutionException; import com.google.devtools.build.lib.actions.RemoteArtifactChecker; import com.google.devtools.build.lib.actions.cache.MetadataInjector; @@ -215,7 +215,7 @@ default void updateActionFileSystemContext( FileSystem actionFileSystem, Environment env, MetadataInjector injector, - ImmutableMap> filesets) {} + ImmutableMap filesets) {} /** * Checks the filesystem returned by {@link #createActionFileSystem} for errors attributable to @@ -235,7 +235,7 @@ default ArtifactPathResolver createPathResolverForArtifactValues( ImmutableList pathEntries, ActionInputMap actionInputMap, Map> treeArtifacts, - Map> filesets) { + Map filesets) { throw new IllegalStateException("Path resolver not supported by this class"); } diff --git a/src/test/java/com/google/devtools/build/lib/actions/BUILD b/src/test/java/com/google/devtools/build/lib/actions/BUILD index e4f119dd83fbfe..701de12b1c3ff6 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/BUILD +++ b/src/test/java/com/google/devtools/build/lib/actions/BUILD @@ -41,6 +41,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink", + "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_tree", "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity", "//src/main/java/com/google/devtools/build/lib/actions:middleman_type", "//src/main/java/com/google/devtools/build/lib/actions:package_roots", diff --git a/src/test/java/com/google/devtools/build/lib/actions/CompletionContextTest.java b/src/test/java/com/google/devtools/build/lib/actions/CompletionContextTest.java index b4b40e3c846b97..4d9c0d06b288bb 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/CompletionContextTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/CompletionContextTest.java @@ -55,8 +55,7 @@ public final class CompletionContextTest { private final ActionInputMap inputMap = new ActionInputMap(BugReporter.defaultInstance(), 0); private final Map treeExpansions = new HashMap<>(); - private final Map> filesetExpansions = - new HashMap<>(); + private final Map filesetExpansions = new HashMap<>(); private Path execRoot; private ArtifactRoot outputRoot; @@ -109,7 +108,9 @@ public void fileset_noExpansion() { SpecialArtifact fileset = createFileset("fs"); inputMap.put(fileset, DUMMY_METADATA, /* depOwner= */ null); filesetExpansions.put( - fileset, ImmutableList.of(filesetLink("a1", "b1"), filesetLink("a2", "b2"))); + fileset, + FilesetOutputTree.create( + ImmutableList.of(filesetLink("a1", "b1"), filesetLink("a2", "b2")))); CompletionContext ctx = createCompletionContext(/* expandFilesets= */ false); ArtifactReceiver receiver = mock(ArtifactReceiver.class); @@ -126,7 +127,7 @@ public void fileset_withExpansion() throws Exception { inputMap.put(fileset, DUMMY_METADATA, /* depOwner= */ null); ImmutableList links = ImmutableList.of(filesetLink("a1", "b1"), filesetLink("a2", "b2")); - filesetExpansions.put(fileset, links); + filesetExpansions.put(fileset, FilesetOutputTree.create(links)); CompletionContext ctx = createCompletionContext(/* expandFilesets= */ true); ArtifactReceiver receiver = mock(ArtifactReceiver.class); @@ -139,7 +140,7 @@ public void fileset_withExpansion() throws Exception { .verify(receiver) .acceptFilesetMapping(fileset, PathFragment.create("a2"), execRoot.getRelative("b2")); - assertThat(ctx.expandFileset(fileset)).isEqualTo(links); + assertThat(ctx.expandFileset(fileset).symlinks()).isEqualTo(links); } private static List visit(CompletionContext ctx, Artifact artifact) { diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java index 61609e0431ba38..8b382b9f3d18b5 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java +++ b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java @@ -57,6 +57,7 @@ import com.google.devtools.build.lib.actions.DiscoveredModulesPruner; import com.google.devtools.build.lib.actions.Executor; import com.google.devtools.build.lib.actions.FileArtifactValue; +import com.google.devtools.build.lib.actions.FilesetOutputTree; import com.google.devtools.build.lib.actions.InputMetadataProvider; import com.google.devtools.build.lib.actions.MiddlemanType; import com.google.devtools.build.lib.actions.PackageRootResolver; @@ -241,7 +242,7 @@ public static ActionExecutionValue createActionExecutionValue( return ActionExecutionValue.createFromOutputMetadataStore( artifactData, treeArtifactData, - /* outputSymlinks= */ ImmutableList.of(), + FilesetOutputTree.EMPTY, /* discoveredModules= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER)); } diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/BUILD b/src/test/java/com/google/devtools/build/lib/actions/util/BUILD index 2eb82a0fc2a29e..a50de42ae7fce1 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/util/BUILD +++ b/src/test/java/com/google/devtools/build/lib/actions/util/BUILD @@ -24,6 +24,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:artifact_owner", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", + "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_tree", "//src/main/java/com/google/devtools/build/lib/actions:middleman_type", "//src/main/java/com/google/devtools/build/lib/actions:package_roots", "//src/main/java/com/google/devtools/build/lib/actions:runfiles_metadata", diff --git a/src/test/java/com/google/devtools/build/lib/analysis/starlark/BUILD b/src/test/java/com/google/devtools/build/lib/analysis/starlark/BUILD index 4bdc3da47ee98c..c2b04684d76ef7 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/starlark/BUILD +++ b/src/test/java/com/google/devtools/build/lib/analysis/starlark/BUILD @@ -85,6 +85,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:commandline_item", "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink", + "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_tree", "//src/main/java/com/google/devtools/build/lib/analysis:config/core_options", "//src/main/java/com/google/devtools/build/lib/analysis:starlark/starlark_custom_command_line", "//src/main/java/com/google/devtools/build/lib/cmdline", diff --git a/src/test/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLineTest.java b/src/test/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLineTest.java index cdfecb145ffbf6..002403d9afcfd7 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLineTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLineTest.java @@ -33,6 +33,7 @@ import com.google.devtools.build.lib.actions.CommandLine.SimpleArgChunk; import com.google.devtools.build.lib.actions.CommandLineExpansionException; import com.google.devtools.build.lib.actions.FilesetOutputSymlink; +import com.google.devtools.build.lib.actions.FilesetOutputTree; import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.analysis.config.CoreOptions; @@ -236,7 +237,8 @@ public void vectorArgAddToFingerprint_expandFileset_includesInDigest() throws Ex ArtifactExpander artifactExpander = createArtifactExpander( /* treeExpansions= */ ImmutableMap.of(), - ImmutableMap.of(fileset, ImmutableList.of(symlink1, symlink2))); + ImmutableMap.of( + fileset, FilesetOutputTree.create(ImmutableList.of(symlink1, symlink2)))); commandLine.addToFingerprint( actionKeyContext, artifactExpander, CoreOptions.OutputPathsMode.OFF, fingerprint); @@ -313,7 +315,8 @@ public void vectorArgArguments_expandsFileset() throws Exception { ArtifactExpander artifactExpander = createArtifactExpander( /* treeExpansions= */ ImmutableMap.of(), - ImmutableMap.of(fileset, ImmutableList.of(symlink1, symlink2))); + ImmutableMap.of( + fileset, FilesetOutputTree.create(ImmutableList.of(symlink1, symlink2)))); Iterable arguments = commandLine.arguments(artifactExpander, PathMapper.NOOP); @@ -413,7 +416,7 @@ private SpecialArtifact createSpecialArtifact(String relativePath, SpecialArtifa private static ArtifactExpander createArtifactExpander( ImmutableMap> treeExpansions, - ImmutableMap> filesetExpansions) { + ImmutableMap filesetExpansions) { return new ArtifactExpander() { @Override public ImmutableSortedSet expandTreeArtifact(Artifact treeArtifact) @@ -427,14 +430,13 @@ public ImmutableSortedSet expandTreeArtifact(Artifact treeArti } @Override - public ImmutableList expandFileset(Artifact artifact) - throws MissingExpansionException { + public FilesetOutputTree expandFileset(Artifact artifact) throws MissingExpansionException { //noinspection SuspiciousMethodCalls - ImmutableList filesetLinks = filesetExpansions.get(artifact); - if (filesetLinks == null) { + FilesetOutputTree filesetOutput = filesetExpansions.get(artifact); + if (filesetOutput == null) { throw new MissingExpansionException("Cannot expand " + artifact); } - return filesetLinks; + return filesetOutput; } }; } diff --git a/src/test/java/com/google/devtools/build/lib/exec/BUILD b/src/test/java/com/google/devtools/build/lib/exec/BUILD index 6921f415dab1b8..7c84892a5c1d04 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/BUILD +++ b/src/test/java/com/google/devtools/build/lib/exec/BUILD @@ -29,6 +29,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink", + "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_tree", "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity", "//src/main/java/com/google/devtools/build/lib/actions:runfiles_tree", "//src/main/java/com/google/devtools/build/lib/actions:thread_state_receiver", diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java index 2d9d07ddba610e..931a504333cf4c 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java @@ -35,6 +35,7 @@ import com.google.devtools.build.lib.actions.ArtifactRoot.RootType; import com.google.devtools.build.lib.actions.FilesetManifest; import com.google.devtools.build.lib.actions.FilesetOutputSymlink; +import com.google.devtools.build.lib.actions.FilesetOutputTree; import com.google.devtools.build.lib.actions.InputMetadataProvider; import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.actions.RunfilesTree; @@ -108,12 +109,13 @@ public ImmutableSortedSet expandTreeArtifact(Artifact treeArti } @Override - public ImmutableList expandFileset(Artifact artifact) { - return ImmutableList.of( - FilesetOutputSymlink.createForTesting( - PathFragment.create("zizz"), - PathFragment.create("/foo/fake_exec/xyz/zizz"), - PathFragment.create("/foo/fake_exec/"))); + public FilesetOutputTree expandFileset(Artifact artifact) { + return FilesetOutputTree.create( + ImmutableList.of( + FilesetOutputSymlink.createForTesting( + PathFragment.create("zizz"), + PathFragment.create("/foo/fake_exec/xyz/zizz"), + PathFragment.create("/foo/fake_exec/")))); } }; @@ -425,8 +427,8 @@ private SpecialArtifact createSpecialArtifact(String relPath, SpecialArtifactTyp @Test public void testEmptyManifest() throws Exception { - Map> filesetMappings = - ImmutableMap.of(createFileset("out"), ImmutableList.of()); + ImmutableMap filesetMappings = + ImmutableMap.of(createFileset("out"), FilesetOutputTree.EMPTY); expander.addFilesetManifests(filesetMappings, inputMap, PathFragment.EMPTY_FRAGMENT); @@ -436,8 +438,10 @@ public void testEmptyManifest() throws Exception { @Test public void testManifestWithSingleFile() throws Exception { Artifact fileset = createFileset("out"); - Map> filesetMappings = - ImmutableMap.of(fileset, ImmutableList.of(filesetSymlink("foo/bar", "/dir/file"))); + ImmutableMap filesetMappings = + ImmutableMap.of( + fileset, + FilesetOutputTree.create(ImmutableList.of(filesetSymlink("foo/bar", "/dir/file")))); expander.addFilesetManifests(filesetMappings, inputMap, PathFragment.EMPTY_FRAGMENT); @@ -449,11 +453,13 @@ public void testManifestWithSingleFile() throws Exception { @Test public void testManifestWithTwoFiles() throws Exception { Artifact fileset = createFileset("out"); - Map> filesetMappings = + ImmutableMap filesetMappings = ImmutableMap.of( fileset, - ImmutableList.of( - filesetSymlink("foo/bar", "/dir/file"), filesetSymlink("foo/baz", "/dir/file"))); + FilesetOutputTree.create( + ImmutableList.of( + filesetSymlink("foo/bar", "/dir/file"), + filesetSymlink("foo/baz", "/dir/file")))); expander.addFilesetManifests(filesetMappings, inputMap, PathFragment.EMPTY_FRAGMENT); @@ -466,8 +472,10 @@ public void testManifestWithTwoFiles() throws Exception { @Test public void testManifestWithDirectory() throws Exception { Artifact fileset = createFileset("out"); - Map> filesetMappings = - ImmutableMap.of(fileset, ImmutableList.of(filesetSymlink("foo/bar", "/some"))); + ImmutableMap filesetMappings = + ImmutableMap.of( + fileset, + FilesetOutputTree.create(ImmutableList.of(filesetSymlink("foo/bar", "/some")))); expander.addFilesetManifests(filesetMappings, inputMap, PathFragment.EMPTY_FRAGMENT); @@ -492,12 +500,13 @@ private SpecialArtifact createFileset(String execPath) { public void testManifestWithErrorOnRelativeSymlink() { expander = new SpawnInputExpander(execRoot, ERROR); Artifact fileset = createFileset("out"); - ImmutableMap> filesetMappings = + ImmutableMap filesetMappings = ImmutableMap.of( fileset, - ImmutableList.of( - filesetSymlink("workspace/bar", "foo"), - filesetSymlink("workspace/foo", "/root/bar"))); + FilesetOutputTree.create( + ImmutableList.of( + filesetSymlink("workspace/bar", "foo"), + filesetSymlink("workspace/foo", "/root/bar")))); FilesetManifest.ForbiddenRelativeSymlinkException e = assertThrows( @@ -513,12 +522,13 @@ public void testManifestWithErrorOnRelativeSymlink() { public void testManifestWithIgnoredRelativeSymlink() throws Exception { expander = new SpawnInputExpander(execRoot, IGNORE); Artifact fileset = createFileset("out"); - ImmutableMap> filesetMappings = + ImmutableMap filesetMappings = ImmutableMap.of( fileset, - ImmutableList.of( - filesetSymlink("workspace/bar", "foo"), - filesetSymlink("workspace/foo", "/root/bar"))); + FilesetOutputTree.create( + ImmutableList.of( + filesetSymlink("workspace/bar", "foo"), + filesetSymlink("workspace/foo", "/root/bar")))); expander.addFilesetManifests(filesetMappings, inputMap, PathFragment.EMPTY_FRAGMENT); @@ -531,12 +541,13 @@ public void testManifestWithIgnoredRelativeSymlink() throws Exception { public void testManifestWithResolvedRelativeSymlink() throws Exception { expander = new SpawnInputExpander(execRoot, RESOLVE); Artifact fileset = createFileset("out"); - ImmutableMap> filesetMappings = + ImmutableMap filesetMappings = ImmutableMap.of( fileset, - ImmutableList.of( - filesetSymlink("workspace/bar", "foo"), - filesetSymlink("workspace/foo", "/root/bar"))); + FilesetOutputTree.create( + ImmutableList.of( + filesetSymlink("workspace/bar", "foo"), + filesetSymlink("workspace/foo", "/root/bar")))); expander.addFilesetManifests(filesetMappings, inputMap, PathFragment.EMPTY_FRAGMENT); diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java index 8c89e6515d9cc2..76b68d70b44b38 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java +++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java @@ -37,6 +37,7 @@ import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.actions.CommandLines.ParamFileActionInput; import com.google.devtools.build.lib.actions.FileArtifactValue; +import com.google.devtools.build.lib.actions.FilesetOutputTree; import com.google.devtools.build.lib.actions.InputMetadataProvider; import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; import com.google.devtools.build.lib.actions.PathMapper; @@ -1478,7 +1479,7 @@ public void testFilesetInput(@TestParameter DirContents dirContents) throws Exce defaultSpawnBuilder() .withInput(filesetInput) // The implementation only relies on the map keys, so the value can be empty. - .withFilesetMapping(filesetInput, ImmutableList.of()) + .withFilesetMapping(filesetInput, FilesetOutputTree.EMPTY) .build(); SpawnLogContext context = createSpawnLogContext(); diff --git a/src/test/java/com/google/devtools/build/lib/exec/util/BUILD b/src/test/java/com/google/devtools/build/lib/exec/util/BUILD index 17ceb7a1f62e97..0d87becdc05cfb 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/util/BUILD +++ b/src/test/java/com/google/devtools/build/lib/exec/util/BUILD @@ -25,7 +25,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:artifact_expander", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", - "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink", + "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_tree", "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity", "//src/main/java/com/google/devtools/build/lib/actions:middleman_type", "//src/main/java/com/google/devtools/build/lib/actions:runfiles_metadata", diff --git a/src/test/java/com/google/devtools/build/lib/exec/util/SpawnBuilder.java b/src/test/java/com/google/devtools/build/lib/exec/util/SpawnBuilder.java index 80f4afc61f5e6b..ba77a5744509c8 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/util/SpawnBuilder.java +++ b/src/test/java/com/google/devtools/build/lib/exec/util/SpawnBuilder.java @@ -23,7 +23,7 @@ import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputHelper; import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.actions.FilesetOutputSymlink; +import com.google.devtools.build.lib.actions.FilesetOutputTree; import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.actions.SimpleSpawn; @@ -54,8 +54,7 @@ public final class SpawnBuilder { private final NestedSetBuilder inputs = NestedSetBuilder.stableOrder(); private final List outputs = new ArrayList<>(); @Nullable private Set mandatoryOutputs; - private final Map> filesetMappings = - new HashMap<>(); + private final Map filesetMappings = new HashMap<>(); private final NestedSetBuilder tools = NestedSetBuilder.stableOrder(); private ResourceSet resourceSet = ResourceSet.ZERO; @@ -212,10 +211,9 @@ public SpawnBuilder withMandatoryOutputs(@Nullable Set ma } @CanIgnoreReturnValue - public SpawnBuilder withFilesetMapping( - Artifact fileset, ImmutableList mappings) { + public SpawnBuilder withFilesetMapping(Artifact fileset, FilesetOutputTree filesetOutput) { Preconditions.checkArgument(fileset.isFileset(), "Artifact %s is not fileset", fileset); - filesetMappings.put(fileset, mappings); + filesetMappings.put(fileset, filesetOutput); return this; } diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionExecutionValueTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionExecutionValueTest.java index a63ab58e7e721f..ba724e9961a9f7 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionExecutionValueTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionExecutionValueTest.java @@ -29,6 +29,7 @@ import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue; import com.google.devtools.build.lib.actions.FilesetOutputSymlink; +import com.google.devtools.build.lib.actions.FilesetOutputTree; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; @@ -108,10 +109,14 @@ public void equality() { .addEqualityGroup(ImmutableMap.of(tree1, tree1Value1)) .addEqualityGroup(ImmutableMap.of(tree1, tree1Value2)) // outputSymlinks - .addEqualityGroup(createWithOutputSymlinks(ImmutableList.of(symlink1))) - .addEqualityGroup(createWithOutputSymlinks(ImmutableList.of(symlink2))) - .addEqualityGroup(createWithOutputSymlinks(ImmutableList.of(symlink1, symlink2))) - .addEqualityGroup(createWithOutputSymlinks(ImmutableList.of(symlink2, symlink1))) + .addEqualityGroup( + createWithFilesetOutput(FilesetOutputTree.create(ImmutableList.of(symlink1)))) + .addEqualityGroup( + createWithFilesetOutput(FilesetOutputTree.create(ImmutableList.of(symlink2)))) + .addEqualityGroup( + createWithFilesetOutput(FilesetOutputTree.create(ImmutableList.of(symlink1, symlink2)))) + .addEqualityGroup( + createWithFilesetOutput(FilesetOutputTree.create(ImmutableList.of(symlink2, symlink1)))) // discoveredModules .addEqualityGroup( createWithDiscoveredModules( @@ -142,12 +147,13 @@ public void serialization() throws Exception { // Single output file createWithArtifactData(ImmutableMap.of(output("output1"), VALUE_1_REMOTE)), // Fileset - createWithOutputSymlinks( - ImmutableList.of( - FilesetOutputSymlink.createForTesting( - PathFragment.create("name"), - PathFragment.create("target"), - PathFragment.create("execPath")))), + createWithFilesetOutput( + FilesetOutputTree.create( + ImmutableList.of( + FilesetOutputSymlink.createForTesting( + PathFragment.create("name"), + PathFragment.create("target"), + PathFragment.create("execPath"))))), // Module discovering createWithDiscoveredModules( NestedSetBuilder.create(Order.STABLE_ORDER, output("module"))), @@ -168,7 +174,7 @@ public void serialization() throws Exception { ActionExecutionValue.createFromOutputMetadataStore( ImmutableMap.of(output("file"), VALUE_1_REMOTE), ImmutableMap.of(tree("tree"), TreeArtifactValue.empty()), - /* outputSymlinks= */ ImmutableList.of(), + FilesetOutputTree.EMPTY, /* discoveredModules= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER))) .addDependency(FileSystem.class, OUTPUT_ROOT.getRoot().getFileSystem()) .addDependency( @@ -182,7 +188,7 @@ private static ActionExecutionValue createWithArtifactData( return ActionExecutionValue.createFromOutputMetadataStore( /* artifactData= */ artifactData, /* treeArtifactData= */ ImmutableMap.of(), - /* outputSymlinks= */ ImmutableList.of(), + FilesetOutputTree.EMPTY, /* discoveredModules= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER)); } @@ -191,16 +197,15 @@ private static ActionExecutionValue createWithTreeArtifactData( return ActionExecutionValue.createFromOutputMetadataStore( /* artifactData= */ ImmutableMap.of(), treeArtifactData, - /* outputSymlinks= */ ImmutableList.of(), + FilesetOutputTree.EMPTY, /* discoveredModules= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER)); } - private static ActionExecutionValue createWithOutputSymlinks( - ImmutableList outputSymlinks) { + private static ActionExecutionValue createWithFilesetOutput(FilesetOutputTree filesetOutput) { return ActionExecutionValue.createFromOutputMetadataStore( ImmutableMap.of(output("fileset.manifest"), VALUE_1_REMOTE), /* treeArtifactData= */ ImmutableMap.of(), - outputSymlinks, + filesetOutput, /* discoveredModules= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER)); } @@ -214,7 +219,7 @@ private static ActionExecutionValue createWithDiscoveredModules( return ActionExecutionValue.createFromOutputMetadataStore( /* artifactData= */ ImmutableMap.of(output("modules.pcm"), VALUE_1_REMOTE), /* treeArtifactData= */ ImmutableMap.of(), - /* outputSymlinks= */ ImmutableList.of(), + FilesetOutputTree.EMPTY, discoveredModules); } diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionExecutionValueTransformSharedTreeArtifactsTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionExecutionValueTransformSharedTreeArtifactsTest.java index a829095f0f6a9a..0dbc7e1fbfbcba 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionExecutionValueTransformSharedTreeArtifactsTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionExecutionValueTransformSharedTreeArtifactsTest.java @@ -16,7 +16,6 @@ import static com.google.common.truth.Truth.assertThat; import static org.mockito.Mockito.mock; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.ActionLookupData; import com.google.devtools.build.lib.actions.ActionLookupKey; @@ -30,6 +29,7 @@ import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.actions.ArtifactRoot.RootType; import com.google.devtools.build.lib.actions.FileArtifactValue; +import com.google.devtools.build.lib.actions.FilesetOutputTree; import com.google.devtools.build.lib.actions.util.ActionsTestUtil.NullAction; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; @@ -262,7 +262,7 @@ private static ActionExecutionValue createActionExecutionValue( return ActionExecutionValue.createFromOutputMetadataStore( fileArtifacts, treeArtifacts, - /* outputSymlinks= */ ImmutableList.of(), + FilesetOutputTree.EMPTY, /* discoveredModules= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER)); } diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStoreTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStoreTest.java index 279ea4bbed2fff..04b0dc0153f0a0 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStoreTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStoreTest.java @@ -34,6 +34,7 @@ import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue; import com.google.devtools.build.lib.actions.FilesetOutputSymlink; +import com.google.devtools.build.lib.actions.FilesetOutputTree; import com.google.devtools.build.lib.actions.HasDigest; import com.google.devtools.build.lib.actions.HasDigest.ByteStringDigest; import com.google.devtools.build.lib.actions.StaticInputMetadataProvider; @@ -395,7 +396,7 @@ public void injectRemoteTreeArtifactMetadata() throws Exception { // child is missing, getExistingFileArtifactValue will throw. ActionExecutionValue actionExecutionValue = ActionExecutionValue.createFromOutputMetadataStore( - store, /* outputSymlinks= */ ImmutableList.of(), new NullAction()); + store, FilesetOutputTree.EMPTY, new NullAction()); tree.getChildren().forEach(actionExecutionValue::getExistingFileArtifactValue); } @@ -566,8 +567,8 @@ public void getMetadataFromFilesetMapping() throws Exception { Artifact artifact = ActionsTestUtil.createArtifactWithRootRelativePath( outputRoot, PathFragment.create("foo/bar")); - ImmutableMap> expandedFilesets = - ImmutableMap.of(artifact, symlinks); + ImmutableMap expandedFilesets = + ImmutableMap.of(artifact, FilesetOutputTree.create(symlinks)); ActionInputMetadataProvider inputMetadataProvider = new ActionInputMetadataProvider( diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD index 65420c16ca5802..298f81d80ef4f6 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD @@ -711,6 +711,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", + "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_tree", "//src/main/java/com/google/devtools/build/lib/collect/nestedset", "//src/main/java/com/google/devtools/build/lib/skyframe:action_execution_value", "//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value", @@ -1506,6 +1507,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink", + "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_tree", "//src/main/java/com/google/devtools/build/lib/collect/nestedset", "//src/main/java/com/google/devtools/build/lib/skyframe:action_execution_value", "//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value", @@ -2043,6 +2045,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink", + "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_tree", "//src/main/java/com/google/devtools/build/lib/remote", "//src/main/java/com/google/devtools/build/lib/skyframe:action_execution_value", "//src/main/java/com/google/devtools/build/lib/skyframe:action_input_metadata_provider", diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/rewinding/LostImportantOutputHandlerModule.java b/src/test/java/com/google/devtools/build/lib/skyframe/rewinding/LostImportantOutputHandlerModule.java index abf44edb7935a7..879989780245b0 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/rewinding/LostImportantOutputHandlerModule.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/rewinding/LostImportantOutputHandlerModule.java @@ -174,7 +174,7 @@ private Stream expand(Artifact output, ArtifactExpander expander if (output.isFileset()) { ImmutableList links; try { - links = expander.expandFileset(output); + links = expander.expandFileset(output).symlinks(); } catch (MissingExpansionException e) { throw new IllegalStateException(e); } diff --git a/src/test/java/com/google/devtools/build/lib/testutil/BUILD b/src/test/java/com/google/devtools/build/lib/testutil/BUILD index bdf9e30ce7b839..597e67bbfb8f5e 100644 --- a/src/test/java/com/google/devtools/build/lib/testutil/BUILD +++ b/src/test/java/com/google/devtools/build/lib/testutil/BUILD @@ -154,6 +154,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:artifact_expander", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink", + "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_tree", "//src/main/java/com/google/devtools/build/lib/vfs", "//third_party:guava", ], diff --git a/src/test/java/com/google/devtools/build/lib/testutil/SpawnInputUtils.java b/src/test/java/com/google/devtools/build/lib/testutil/SpawnInputUtils.java index ca82f1f86f3626..5b46b1cd387250 100644 --- a/src/test/java/com/google/devtools/build/lib/testutil/SpawnInputUtils.java +++ b/src/test/java/com/google/devtools/build/lib/testutil/SpawnInputUtils.java @@ -25,6 +25,7 @@ import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; import com.google.devtools.build.lib.actions.ArtifactExpander.MissingExpansionException; import com.google.devtools.build.lib.actions.FilesetOutputSymlink; +import com.google.devtools.build.lib.actions.FilesetOutputTree; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.vfs.Path; import java.util.Map; @@ -50,13 +51,12 @@ public static Artifact getFilesetArtifactWithName(Spawn spawn, String name) { public static ActionInput getFilesetInputWithName( Spawn spawn, ActionExecutionContext context, String artifactName, String inputName) { Path execRoot = context.getExecRoot(); - for (Map.Entry> entry : - spawn.getFilesetMappings().entrySet()) { + for (Map.Entry entry : spawn.getFilesetMappings().entrySet()) { Artifact filesetArtifact = entry.getKey(); if (!filesetArtifact.getExecPathString().contains(artifactName)) { continue; } - for (FilesetOutputSymlink filesetOutputSymlink : entry.getValue()) { + for (FilesetOutputSymlink filesetOutputSymlink : entry.getValue().symlinks()) { if (filesetOutputSymlink.getTargetPath().toString().contains(inputName)) { Path inputPath = execRoot.getRelative(filesetOutputSymlink.getTargetPath()); return ActionInputHelper.fromPath(inputPath.asFragment()); @@ -73,7 +73,7 @@ public static ActionInput getRunfilesFilesetInputWithName( ImmutableList filesetLinks; try { - filesetLinks = context.getArtifactExpander().expandFileset(filesetArtifact); + filesetLinks = context.getArtifactExpander().expandFileset(filesetArtifact).symlinks(); } catch (MissingExpansionException e) { throw new IllegalStateException(e); }