Allows symlink tree actions to construct fileset links without consuming the input manifest. Instead, construct the links directly from the Fileset links propagated through the action execution values.
PiperOrigin-RevId: 292183775
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 c7cf526..ed2bef9 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 @@
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 475ce4d..ce05c09 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 @@
* @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 @@
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 @@
* @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 @@
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 boolean isFilesetTree() {
- return filesetTree;
+ return filesetRoot != null;
+ }
+
+ public PathFragment getFilesetRoot() {
+ return PathFragment.create(filesetRoot);
}
public boolean isRunfilesEnabled() {
@@ -148,10 +153,6 @@
return inprocessSymlinkCreation;
}
- public boolean skipRunfilesManifests() {
- return skipRunfilesManifests;
- }
-
@Override
public String getMnemonic() {
return "SymlinkTree";
@@ -159,14 +160,14 @@
@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 877fc47..067adb5 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 @@
.build();
}
- static Map<PathFragment, PathFragment> readSymlinksFromFilesetManifest(Path manifest)
- throws IOException {
- Map<PathFragment, PathFragment> 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<PathFragment, PathFragment> processFilesetLinks(
+ ImmutableList<FilesetOutputSymlink> links, PathFragment root, PathFragment execRoot) {
+ Map<PathFragment, PathFragment> 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 442d2d4..38b8729 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 @@
} 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 a065b06..4f7c31e 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 @@
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 1b01f43..24a70d7 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 @@
? 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 @@
inputManifest,
/*runfiles=*/ null,
outputManifest,
- /*filesetTree=*/ true,
+ /*filesetRoot=*/ "root",
createActionEnvironment(
attributesToFlip.contains(FilesetActionAttributes.FIXED_ENVIRONMENT),
attributesToFlip.contains(FilesetActionAttributes.VARIABLE_ENVIRONMENT)),
@@ -102,7 +102,7 @@
? 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 @@
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 8e81445..e510e4e 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 @@
}
@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<PathFragment, PathFragment> 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<PathFragment, PathFragment> 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 c50c7f6..839a4bb 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 @@
inputManifest,
runfiles,
outputManifest,
- /*filesetTree=*/ false,
+ /*filesetRoot=*/ null,
ActionEnvironment.EMPTY,
/*enableRunfiles=*/ true,
/*inprocessSymlinkCreation=*/ false,
@@ -138,7 +138,7 @@
inputManifest,
runfiles,
outputManifest,
- /*filesetTree=*/ false,
+ /*filesetRoot=*/ null,
ActionEnvironment.EMPTY,
/*enableRunfiles=*/ true,
/*inprocessSymlinkCreation=*/ true,
