diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java index c7cf5261c6a6ca..ed2bef95195f49 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java @@ -373,7 +373,7 @@ private static Artifact createRunfilesAction( inputManifest, runfiles, outputManifest, - /*filesetTree=*/ false)); + /*filesetRoot=*/ null)); return outputManifest; } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeAction.java index 475ce4ddaaf6c7..ce05c090af9f03 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeAction.java @@ -30,6 +30,7 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.util.Fingerprint; +import com.google.devtools.build.lib.vfs.PathFragment; import javax.annotation.Nullable; /** @@ -40,12 +41,12 @@ @AutoCodec public final class SymlinkTreeAction extends AbstractAction { - private static final String GUID = "63412bda-4026-4c8e-a3ad-7deb397728d4"; + private static final String GUID = "7a16371c-cd4a-494d-b622-963cd89f5212"; @Nullable private final Artifact inputManifest; private final Runfiles runfiles; private final Artifact outputManifest; - private final boolean filesetTree; + @Nullable private final String filesetRoot; private final boolean enableRunfiles; private final boolean inprocessSymlinkCreation; private final boolean skipRunfilesManifests; @@ -59,7 +60,7 @@ public final class SymlinkTreeAction extends AbstractAction { * @param runfiles the input runfiles * @param outputManifest the generated symlink tree manifest (must have "MANIFEST" base name). * Symlink tree root will be set to the artifact's parent directory. - * @param filesetTree true if this is fileset symlink tree + * @param filesetRoot non-null if this is a fileset symlink tree */ public SymlinkTreeAction( ActionOwner owner, @@ -67,13 +68,13 @@ public SymlinkTreeAction( Artifact inputManifest, @Nullable Runfiles runfiles, Artifact outputManifest, - boolean filesetTree) { + String filesetRoot) { this( owner, inputManifest, runfiles, outputManifest, - filesetTree, + filesetRoot, config.getActionEnvironment(), config.runfilesEnabled(), config.inprocessSymlinkCreation(), @@ -90,7 +91,7 @@ public SymlinkTreeAction( * @param runfiles the input runfiles * @param outputManifest the generated symlink tree manifest (must have "MANIFEST" base name). * Symlink tree root will be set to the artifact's parent directory. - * @param filesetTree true if this is fileset symlink tree, + * @param filesetRoot non-null if this is a fileset symlink tree, */ @AutoCodec.Instantiator public SymlinkTreeAction( @@ -98,29 +99,29 @@ public SymlinkTreeAction( Artifact inputManifest, @Nullable Runfiles runfiles, Artifact outputManifest, - boolean filesetTree, + @Nullable String filesetRoot, ActionEnvironment env, boolean enableRunfiles, boolean inprocessSymlinkCreation, boolean skipRunfilesManifests) { super( owner, - skipRunfilesManifests && enableRunfiles && !filesetTree + skipRunfilesManifests && enableRunfiles && (filesetRoot == null) ? NestedSetBuilder.emptySet(Order.STABLE_ORDER) : NestedSetBuilder.create(Order.STABLE_ORDER, inputManifest), ImmutableSet.of(outputManifest), env); Preconditions.checkArgument(outputManifest.getPath().getBaseName().equals("MANIFEST")); Preconditions.checkArgument( - (runfiles == null) == filesetTree, "Runfiles must be null iff this is a fileset action"); - this.inputManifest = - skipRunfilesManifests && enableRunfiles && !filesetTree ? null : inputManifest; + (runfiles == null) == (filesetRoot != null), + "Runfiles must be null iff this is a fileset action"); this.runfiles = runfiles; this.outputManifest = outputManifest; - this.filesetTree = filesetTree; + this.filesetRoot = filesetRoot; this.enableRunfiles = enableRunfiles; this.inprocessSymlinkCreation = inprocessSymlinkCreation; - this.skipRunfilesManifests = skipRunfilesManifests && enableRunfiles && !filesetTree; + this.skipRunfilesManifests = skipRunfilesManifests && enableRunfiles && (filesetRoot == null); + this.inputManifest = this.skipRunfilesManifests ? null : inputManifest; } public Artifact getInputManifest() { @@ -137,7 +138,11 @@ public Artifact getOutputManifest() { } public boolean isFilesetTree() { - return filesetTree; + return filesetRoot != null; + } + + public PathFragment getFilesetRoot() { + return PathFragment.create(filesetRoot); } public boolean isRunfilesEnabled() { @@ -148,10 +153,6 @@ public boolean inprocessSymlinkCreation() { return inprocessSymlinkCreation; } - public boolean skipRunfilesManifests() { - return skipRunfilesManifests; - } - @Override public String getMnemonic() { return "SymlinkTree"; @@ -159,14 +160,14 @@ public String getMnemonic() { @Override protected String getRawProgressMessage() { - return (filesetTree ? "Creating Fileset tree " : "Creating runfiles tree ") + return (isFilesetTree() ? "Creating Fileset tree " : "Creating runfiles tree ") + outputManifest.getExecPath().getParentDirectory().getPathString(); } @Override protected void computeKey(ActionKeyContext actionKeyContext, Fingerprint fp) { fp.addString(GUID); - fp.addBoolean(filesetTree); + fp.addNullableString(filesetRoot); fp.addBoolean(enableRunfiles); fp.addBoolean(inprocessSymlinkCreation); fp.addBoolean(skipRunfilesManifests); diff --git a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java index 877fc47ff5df97..067adb53f870de 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java @@ -13,14 +13,16 @@ // limitations under the License. package com.google.devtools.build.lib.exec; -import static java.nio.charset.StandardCharsets.ISO_8859_1; 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.common.collect.Lists; import com.google.devtools.build.lib.actions.Artifact; 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.shell.Command; import com.google.devtools.build.lib.shell.CommandException; import com.google.devtools.build.lib.util.CommandBuilder; @@ -33,9 +35,7 @@ import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Symlinks; -import java.io.BufferedReader; import java.io.IOException; -import java.io.InputStreamReader; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -184,32 +184,13 @@ Command createCommand(Path execRoot, BinTools binTools, Map shel .build(); } - static Map readSymlinksFromFilesetManifest(Path manifest) - throws IOException { - Map result = new HashMap<>(); - try (BufferedReader reader = - new BufferedReader( - new InputStreamReader( - // ISO_8859 is used to write the manifest in {Runfiles,Fileset}ManifestAction. - manifest.getInputStream(), ISO_8859_1))) { - String line; - int lineNumber = 0; - while ((line = reader.readLine()) != null) { - // If the input has metadata (for fileset), they appear in every other line. - if (++lineNumber % 2 == 0) { - continue; - } - int spaceIndex = line.indexOf(' '); - result.put( - PathFragment.create(line.substring(0, spaceIndex)), - PathFragment.create(line.substring(spaceIndex + 1))); - } - if (lineNumber % 2 != 0) { - throw new IOException( - "Possibly corrupted manifest file '" + manifest.getPathString() + "'"); - } + static ImmutableMap processFilesetLinks( + ImmutableList links, PathFragment root, PathFragment execRoot) { + Map symlinks = new HashMap<>(); + for (FilesetOutputSymlink symlink : links) { + symlinks.put(root.getRelative(symlink.getName()), symlink.reconstituteTargetPath(execRoot)); } - return result; + return ImmutableMap.copyOf(symlinks); } private static final class Directory { 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 442d2d4a0197ff..38b872978fd6d2 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 @@ -80,12 +80,14 @@ public void createSymlinks( } else { Preconditions.checkState(action.isFilesetTree()); Preconditions.checkNotNull(inputManifest); - try { - symlinks = SymlinkTreeHelper.readSymlinksFromFilesetManifest(inputManifest); - } catch (IOException e) { - throw new EnvironmentalExecException( - "Failed to read from manifest '" + inputManifest.getPathString() + "'", e); - } + + symlinks = + SymlinkTreeHelper.processFilesetLinks( + actionExecutionContext + .getArtifactExpander() + .getFileset(action.getInputManifest()), + action.getFilesetRoot(), + actionExecutionContext.getExecRoot().asFragment()); } outputService.createSymlinkTree( diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java b/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java index a065b06311be5d..4f7c31e6880dfe 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java @@ -193,7 +193,7 @@ ManifestAndRunfiles createApkBuilderSymlinks(RuleContext ruleContext) { inputManifest, runfiles, outputManifest, - /*filesetTree=*/ false)); + /* filesetRoot = */ null)); return new ManifestAndRunfiles(outputManifest, runfiles); } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeActionTest.java index 1b01f433099e64..24a70d774faae5 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeActionTest.java @@ -69,7 +69,7 @@ public void testComputeKey() throws Exception { ? new Runfiles.Builder("TESTING", false).addArtifact(runfile).build() : new Runfiles.Builder("TESTING", false).addArtifact(runfile2).build(), outputManifest, - /*filesetTree=*/ false, + /*filesetRoot=*/ null, createActionEnvironment( attributesToFlip.contains(RunfilesActionAttributes.FIXED_ENVIRONMENT), attributesToFlip.contains(RunfilesActionAttributes.VARIABLE_ENVIRONMENT)), @@ -84,7 +84,7 @@ public void testComputeKey() throws Exception { inputManifest, /*runfiles=*/ null, outputManifest, - /*filesetTree=*/ true, + /*filesetRoot=*/ "root", createActionEnvironment( attributesToFlip.contains(FilesetActionAttributes.FIXED_ENVIRONMENT), attributesToFlip.contains(FilesetActionAttributes.VARIABLE_ENVIRONMENT)), @@ -102,7 +102,7 @@ public void testComputeKey() throws Exception { ? new Runfiles.Builder("TESTING", false).addArtifact(runfile).build() : new Runfiles.Builder("TESTING", false).addArtifact(runfile2).build(), outputManifest, - /*filesetTree=*/ false, + /*filesetRoot=*/ null, createActionEnvironment( attributesToFlip.contains(SkipManifestAttributes.FIXED_ENVIRONMENT), attributesToFlip.contains(SkipManifestAttributes.VARIABLE_ENVIRONMENT)), @@ -130,7 +130,7 @@ public void testNullRunfilesThrows() { inputManifest, /*runfiles=*/ null, outputManifest, - /*filesetTree=*/ false, + /*filesetRoot=*/ null, createActionEnvironment(false, false), /*enableRunfiles=*/ true, /*inprocessSymlinkCreation=*/ false, diff --git a/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeHelperTest.java b/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeHelperTest.java index 8e8144598708e8..e510e4e423a506 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeHelperTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeHelperTest.java @@ -15,18 +15,16 @@ package com.google.devtools.build.lib.exec; import static com.google.common.truth.Truth.assertThat; -import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.actions.FilesetOutputSymlink; +import com.google.devtools.build.lib.actions.HasDigest; import com.google.devtools.build.lib.shell.Command; import com.google.devtools.build.lib.vfs.FileSystem; -import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; -import java.io.FileNotFoundException; -import java.io.IOException; import java.util.Map; import org.junit.Test; import org.junit.runner.RunWith; @@ -56,50 +54,53 @@ public void checkCreatedSpawn() { } @Test - public void readManifest() throws Exception { - Path execRoot = fs.getPath("/my/workspace"); - execRoot.createDirectoryAndParents(); - Path inputManifestPath = execRoot.getRelative("input_manifest"); - FileSystemUtils.writeContentAsLatin1(inputManifestPath, "from to\nmetadata"); + public void readManifest() { + PathFragment execRoot = PathFragment.create("/my/workspace"); + + FilesetOutputSymlink link = + FilesetOutputSymlink.createForTesting( + PathFragment.create("from"), PathFragment.create("to"), execRoot); + Map symlinks = - SymlinkTreeHelper.readSymlinksFromFilesetManifest(inputManifestPath); - assertThat(symlinks).containsExactly(PathFragment.create("from"), PathFragment.create("to")); + SymlinkTreeHelper.processFilesetLinks( + ImmutableList.of(link), PathFragment.create("root"), execRoot); + assertThat(symlinks) + .containsExactly(PathFragment.create("root/from"), PathFragment.create("to")); } @Test - public void readMultilineManifest() throws Exception { - Path execRoot = fs.getPath("/my/workspace"); - execRoot.createDirectoryAndParents(); - Path inputManifestPath = execRoot.getRelative("input_manifest"); - FileSystemUtils.writeContentAsLatin1( - inputManifestPath, "from to\nmetadata\n/foo /bar\nmetadata"); + public void readMultilineManifest() { + PathFragment execRoot = PathFragment.create("/my/workspace"); + + FilesetOutputSymlink link1 = + FilesetOutputSymlink.createForTesting( + PathFragment.create("from"), PathFragment.create("to"), execRoot); + FilesetOutputSymlink link2 = + FilesetOutputSymlink.createForTesting( + PathFragment.create("foo"), PathFragment.create("/bar"), execRoot); + FilesetOutputSymlink link3 = + FilesetOutputSymlink.createAlreadyRelativized( + PathFragment.create("rel"), PathFragment.create("path"), HasDigest.EMPTY, true, true); + FilesetOutputSymlink link4 = + FilesetOutputSymlink.createAlreadyRelativized( + PathFragment.create("rel2"), + PathFragment.create("/path"), + HasDigest.EMPTY, + false, + false); + Map symlinks = - SymlinkTreeHelper.readSymlinksFromFilesetManifest(inputManifestPath); + SymlinkTreeHelper.processFilesetLinks( + ImmutableList.of(link1, link2, link3, link4), PathFragment.create("root2"), execRoot); assertThat(symlinks) .containsExactly( - PathFragment.create("from"), + PathFragment.create("root2/from"), PathFragment.create("to"), - PathFragment.create("/foo"), - PathFragment.create("/bar")); - } - - @Test - public void readCorruptManifest() throws Exception { - Path execRoot = fs.getPath("/my/workspace"); - execRoot.createDirectoryAndParents(); - Path inputManifestPath = execRoot.getRelative("input_manifest"); - FileSystemUtils.writeContentAsLatin1(inputManifestPath, "from to"); - assertThrows( - IOException.class, - () -> SymlinkTreeHelper.readSymlinksFromFilesetManifest(inputManifestPath)); - } - - @Test - public void readNonExistentManifestFails() { - Path execRoot = fs.getPath("/my/workspace"); - Path inputManifestPath = execRoot.getRelative("input_manifest"); - assertThrows( - FileNotFoundException.class, - () -> SymlinkTreeHelper.readSymlinksFromFilesetManifest(inputManifestPath)); + PathFragment.create("root2/foo"), + PathFragment.create("/bar"), + PathFragment.create("root2/rel"), + execRoot.getRelative("path"), + PathFragment.create("root2/rel2"), + PathFragment.create("/path")); } } diff --git a/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategyTest.java b/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategyTest.java index c50c7f62f4b13c..839a4bb64dd615 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategyTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategyTest.java @@ -92,7 +92,7 @@ public void outputServiceInteraction() throws Exception { inputManifest, runfiles, outputManifest, - /*filesetTree=*/ false, + /*filesetRoot=*/ null, ActionEnvironment.EMPTY, /*enableRunfiles=*/ true, /*inprocessSymlinkCreation=*/ false, @@ -138,7 +138,7 @@ public void inprocessSymlinkCreation() throws Exception { inputManifest, runfiles, outputManifest, - /*filesetTree=*/ false, + /*filesetRoot=*/ null, ActionEnvironment.EMPTY, /*enableRunfiles=*/ true, /*inprocessSymlinkCreation=*/ true,