Support filesets in ctx.actions.args().
For the bazel people: Fileset is an internal packaging rule that isn't open sourced.
This change makes filesets a directory and adds directory expansion support in ctx.actions.args(). This should make these artifacts similar to tree artifacts.
RELNOTES: Support fileset expansion in ctx.actions.args(). Controlled by --incompatible_expand_directories.
PiperOrigin-RevId: 211861523
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
index db97a74..290ea13 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
@@ -474,8 +474,6 @@
/**
* Returns true iff this is a TreeArtifact representing a directory tree containing Artifacts.
*/
- // TODO(rduan): Document this Skylark method once TreeArtifact is no longer experimental.
- @Override
public boolean isTreeArtifact() {
return false;
}
@@ -487,6 +485,11 @@
return false;
}
+ @Override
+ public boolean isDirectory() {
+ return isTreeArtifact() || isFileset();
+ }
+
/**
* Returns true iff metadata cache must return constant metadata for the
* given artifact.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java
index be4d487..b4b0eda 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java
@@ -172,7 +172,7 @@
new ParameterFileWriteAction(
ruleContext.getActionOwner(),
skylarkSemantics.incompatibleExpandDirectories()
- ? args.getTreeArtifacts()
+ ? args.getDirectoryArtifacts()
: ImmutableList.of(),
(Artifact) output,
args.build(),
@@ -352,7 +352,7 @@
.setInputs(
skylarkSemantics.incompatibleExpandDirectories()
&& !context.getConfiguration().deferParamFiles()
- ? args.getTreeArtifacts()
+ ? args.getDirectoryArtifacts()
: ImmutableList.of())
.build();
}
@@ -550,8 +550,8 @@
private final Mutability mutability;
private final SkylarkSemantics skylarkSemantics;
private final SkylarkCustomCommandLine.Builder commandLine;
- private List<NestedSet<Object>> potentialTreeArtifacts = new ArrayList<>();
- private final Set<Artifact> treeArtifacts = new HashSet<>();
+ private List<NestedSet<Object>> potentialDirectoryArtifacts = new ArrayList<>();
+ private final Set<Artifact> directoryArtifacts = new HashSet<>();
private ParameterFileType parameterFileType = ParameterFileType.SHELL_QUOTED;
private String flagFormatString;
private boolean useAlways;
@@ -734,14 +734,14 @@
SkylarkNestedSet skylarkNestedSet = ((SkylarkNestedSet) value);
NestedSet<Object> nestedSet = skylarkNestedSet.getSet(Object.class);
if (expandDirectories) {
- potentialTreeArtifacts.add(nestedSet);
+ potentialDirectoryArtifacts.add(nestedSet);
}
vectorArg = new SkylarkCustomCommandLine.VectorArg.Builder(nestedSet);
} else {
@SuppressWarnings("unchecked")
SkylarkList<Object> skylarkList = (SkylarkList<Object>) value;
if (expandDirectories) {
- scanForTreeArtifacts(skylarkList);
+ scanForDirectories(skylarkList);
}
vectorArg = new SkylarkCustomCommandLine.VectorArg.Builder(skylarkList);
}
@@ -816,7 +816,7 @@
private void addScalarArg(Object value, String format, BaseFunction mapFn, Location loc)
throws EvalException {
- validateNoTreeArtifact(value, loc);
+ validateNoDirectory(value, loc);
validateFormatString("format", format, loc);
if (format == null && mapFn == null) {
commandLine.add(value);
@@ -827,10 +827,8 @@
}
}
- private void validateNoTreeArtifact(Object value, Location loc) throws EvalException {
- if (skylarkSemantics.incompatibleExpandDirectories()
- && (value instanceof Artifact)
- && ((Artifact) value).isTreeArtifact()) {
+ private void validateNoDirectory(Object value, Location loc) throws EvalException {
+ if (skylarkSemantics.incompatibleExpandDirectories() && isDirectory(value)) {
throw new EvalException(
loc,
"Cannot add directories to Args#add since they may expand to multiple values. "
@@ -839,6 +837,10 @@
}
}
+ private static boolean isDirectory(Object object) {
+ return ((object instanceof Artifact) && ((Artifact) object).isDirectory());
+ }
+
@Override
public void useParamsFile(String paramFileArg, Boolean useAlways) throws EvalException {
if (isImmutable()) {
@@ -903,18 +905,18 @@
}
}
- ImmutableSet<Artifact> getTreeArtifacts() {
- for (Iterable<Object> collection : potentialTreeArtifacts) {
- scanForTreeArtifacts(collection);
+ ImmutableSet<Artifact> getDirectoryArtifacts() {
+ for (Iterable<Object> collection : potentialDirectoryArtifacts) {
+ scanForDirectories(collection);
}
- potentialTreeArtifacts.clear();
- return ImmutableSet.copyOf(treeArtifacts);
+ potentialDirectoryArtifacts.clear();
+ return ImmutableSet.copyOf(directoryArtifacts);
}
- private void scanForTreeArtifacts(Iterable<?> objects) {
+ private void scanForDirectories(Iterable<?> objects) {
for (Object object : objects) {
- if ((object instanceof Artifact) && ((Artifact) object).isTreeArtifact()) {
- treeArtifacts.add((Artifact) object);
+ if (isDirectory(object)) {
+ directoryArtifacts.add((Artifact) object);
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkCustomCommandLine.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkCustomCommandLine.java
index eb404bb..68ffce3 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkCustomCommandLine.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkCustomCommandLine.java
@@ -26,11 +26,16 @@
import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.actions.CommandLineItem;
import com.google.devtools.build.lib.actions.SingleStringArgFormatter;
+import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.concurrent.BlazeInterners;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.events.NullEventHandler;
+import com.google.devtools.build.lib.exec.FilesetManifest;
+import com.google.devtools.build.lib.exec.FilesetManifest.RelativeSymlinkBehavior;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
+import com.google.devtools.build.lib.skylarkbuildapi.FileApi;
+import com.google.devtools.build.lib.skylarkbuildapi.FileRootApi;
import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
import com.google.devtools.build.lib.syntax.BaseFunction;
import com.google.devtools.build.lib.syntax.Environment;
@@ -41,6 +46,8 @@
import com.google.devtools.build.lib.syntax.SkylarkList;
import com.google.devtools.build.lib.syntax.SkylarkSemantics;
import com.google.devtools.build.lib.util.Fingerprint;
+import com.google.devtools.build.lib.vfs.PathFragment;
+import java.io.IOException;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.IllegalFormatException;
@@ -194,8 +201,8 @@
}
List<Object> expandedValues = originalValues;
if (artifactExpander != null && (features & EXPAND_DIRECTORIES) != 0) {
- if (hasTreeArtifact(originalValues)) {
- expandedValues = expandTreeArtifacts(artifactExpander, originalValues);
+ if (hasDirectory(originalValues)) {
+ expandedValues = expandDirectories(artifactExpander, originalValues);
}
}
List<String> stringValues;
@@ -304,26 +311,38 @@
return argi;
}
- private static boolean hasTreeArtifact(List<Object> originalValues) {
+ private static boolean hasDirectory(List<Object> originalValues) {
int n = originalValues.size();
for (int i = 0; i < n; ++i) {
Object object = originalValues.get(i);
- if ((object instanceof Artifact) && ((Artifact) object).isTreeArtifact()) {
+ if (isDirectory(object)) {
return true;
}
}
return false;
}
- private static List<Object> expandTreeArtifacts(
- Artifact.ArtifactExpander artifactExpander, List<Object> originalValues) {
+ private static boolean isDirectory(Object object) {
+ return ((object instanceof Artifact) && ((Artifact) object).isDirectory());
+ }
+
+ private static List<Object> expandDirectories(
+ Artifact.ArtifactExpander artifactExpander, List<Object> originalValues)
+ throws CommandLineExpansionException {
List<Object> expandedValues;
int n = originalValues.size();
expandedValues = new ArrayList<>(n);
for (int i = 0; i < n; ++i) {
Object object = originalValues.get(i);
- if (object instanceof Artifact && ((Artifact) object).isTreeArtifact()) {
- artifactExpander.expand((Artifact) object, expandedValues);
+ if (isDirectory(object)) {
+ Artifact artifact = (Artifact) object;
+ if (artifact.isTreeArtifact()) {
+ artifactExpander.expand((Artifact) object, expandedValues);
+ } else if (artifact.isFileset()) {
+ expandFileset(artifactExpander, artifact, expandedValues);
+ } else {
+ throw new AssertionError("Unknown artifact type.");
+ }
} else {
expandedValues.add(object);
}
@@ -331,6 +350,23 @@
return expandedValues;
}
+ private static void expandFileset(
+ Artifact.ArtifactExpander artifactExpander, Artifact fileset, List<Object> expandedValues)
+ throws CommandLineExpansionException {
+ try {
+ FilesetManifest filesetManifest =
+ FilesetManifest.constructFilesetManifest(
+ artifactExpander.getFileset(fileset),
+ fileset.getExecPath(),
+ RelativeSymlinkBehavior.IGNORE);
+ for (PathFragment relativePath : filesetManifest.getEntries().keySet()) {
+ expandedValues.add(new FilesetSymlinkFile(fileset, relativePath));
+ }
+ } catch (IOException e) {
+ throw new CommandLineExpansionException("Could not expand fileset: " + e.getMessage());
+ }
+ }
+
private int addToFingerprint(
List<Object> arguments,
int argi,
@@ -931,4 +967,85 @@
this.cause = cause;
}
}
+
+ /**
+ * When we expand filesets the user might still expect a File object (since the results may be fed
+ * into map_each. Therefore we synthesize a File object from the fileset symlink.
+ */
+ static class FilesetSymlinkFile implements FileApi, CommandLineItem {
+ private final Artifact fileset;
+ private final PathFragment execPath;
+
+ public FilesetSymlinkFile(Artifact fileset, PathFragment execPath) {
+ this.fileset = fileset;
+ this.execPath = execPath;
+ }
+
+ private PathFragment getExecPath() {
+ return execPath;
+ }
+
+ @Override
+ public String getDirname() {
+ PathFragment parent = getExecPath().getParentDirectory();
+ return (parent == null) ? "/" : parent.getSafePathString();
+ }
+
+ @Override
+ public String getFilename() {
+ return getExecPath().getBaseName();
+ }
+
+ @Override
+ public String getExtension() {
+ return getExecPath().getFileExtension();
+ }
+
+ @Override
+ public Label getOwnerLabel() {
+ return fileset.getOwnerLabel();
+ }
+
+ @Override
+ public FileRootApi getRoot() {
+ return fileset.getRoot();
+ }
+
+ @Override
+ public boolean isSourceArtifact() {
+ // This information is lost to us.
+ // Since the symlinks are always in the output tree, settle for saying "no"
+ return false;
+ }
+
+ @Override
+ public boolean isDirectory() {
+ return false;
+ }
+
+ @Override
+ public String getRunfilesPathString() {
+ PathFragment relativePath = execPath.relativeTo(fileset.getExecPath());
+ return fileset.getRunfilesPath().getRelative(relativePath).getPathString();
+ }
+
+ @Override
+ public String getExecPathString() {
+ return getExecPath().getPathString();
+ }
+
+ @Override
+ public String expandToCommandLine() {
+ return getExecPathString();
+ }
+
+ @Override
+ public void repr(SkylarkPrinter printer) {
+ if (isSourceArtifact()) {
+ printer.append("<source file " + getRunfilesPathString() + ">");
+ } else {
+ printer.append("<generated file " + getRunfilesPathString() + ">");
+ }
+ }
+ }
}
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 eb30532..4920e8a 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
@@ -483,9 +483,10 @@
// Aggregate top-level Filesets with Filesets nested in Runfiles. Both should be used to update
// the FileSystem context.
state.expandedFilesets.forEach(filesetMappings::put);
+ ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> expandedFilesets =
+ ImmutableMap.copyOf(filesetMappings);
try {
- state.updateFileSystemContext(skyframeActionExecutor, env, metadataHandler,
- ImmutableMap.copyOf(filesetMappings));
+ state.updateFileSystemContext(skyframeActionExecutor, env, metadataHandler, expandedFilesets);
} catch (IOException e) {
throw new ActionExecutionException(
"Failed to update filesystem context: ", e, action, /*catastrophe=*/ false);
@@ -495,7 +496,7 @@
perActionFileCache,
metadataHandler,
Collections.unmodifiableMap(state.expandedArtifacts),
- Collections.unmodifiableMap(state.expandedFilesets),
+ expandedFilesets,
topLevelFilesets,
state.actionFileSystem,
skyframeDepsResult)) {
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/FileApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/FileApi.java
index 1c69792..06bd6c6 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/FileApi.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/FileApi.java
@@ -40,40 +40,45 @@
)
public interface FileApi extends SkylarkValue {
- @SkylarkCallable(name = "dirname", structField = true,
- doc = "The name of the directory containing this file. It's taken from "
- + "<a href=\"#path\">path</a> and is always relative to the execution directory.")
- public String getDirname();
+ @SkylarkCallable(
+ name = "dirname",
+ structField = true,
+ doc =
+ "The name of the directory containing this file. It's taken from "
+ + "<a href=\"#path\">path</a> and is always relative to the execution directory.")
+ String getDirname();
- @SkylarkCallable(name = "basename", structField = true,
+ @SkylarkCallable(
+ name = "basename",
+ structField = true,
doc = "The base name of this file. This is the name of the file inside the directory.")
- public String getFilename();
+ String getFilename();
@SkylarkCallable(name = "extension", structField = true, doc = "The file extension of this file.")
- public String getExtension();
-
- @SkylarkCallable(name = "owner", structField = true, allowReturnNones = true,
- doc = "A label of a target that produces this File."
- )
- public Label getOwnerLabel();
+ String getExtension();
@SkylarkCallable(
- name = "root",
- structField = true,
- doc = "The root beneath which this file resides."
- )
- public FileRootApi getRoot();
+ name = "owner",
+ structField = true,
+ allowReturnNones = true,
+ doc = "A label of a target that produces this File.")
+ Label getOwnerLabel();
@SkylarkCallable(
- name = "is_source",
- structField = true,
- doc = "Returns true if this is a source file, i.e. it is not generated."
- )
- public boolean isSourceArtifact();
+ name = "root",
+ structField = true,
+ doc = "The root beneath which this file resides.")
+ FileRootApi getRoot();
+
+ @SkylarkCallable(
+ name = "is_source",
+ structField = true,
+ doc = "Returns true if this is a source file, i.e. it is not generated.")
+ boolean isSourceArtifact();
// TODO(rduan): Document this Skylark method once TreeArtifact is no longer experimental.
@SkylarkCallable(name = "is_directory", structField = true, documented = false)
- public boolean isTreeArtifact();
+ boolean isDirectory();
@SkylarkCallable(
name = "short_path",
@@ -81,23 +86,21 @@
doc =
"The path of this file relative to its root. This excludes the aforementioned "
+ "<i>root</i>, i.e. configuration-specific fragments of the path. This is also the "
- + "path under which the file is mapped if it's in the runfiles of a binary."
- )
- public String getRunfilesPathString();
+ + "path under which the file is mapped if it's in the runfiles of a binary.")
+ String getRunfilesPathString();
@SkylarkCallable(
- name = "path",
- structField = true,
- doc =
- "The execution path of this file, relative to the workspace's execution directory. It "
- + "consists of two parts, an optional first part called the <i>root</i> (see also the "
- + "<a href=\"root.html\">root</a> module), and the second part which is the "
- + "<code>short_path</code>. The root may be empty, which it usually is for "
- + "non-generated files. For generated files it usually contains a "
- + "configuration-specific path fragment that encodes things like the target CPU "
- + "architecture that was used while building said file. Use the "
- + "<code>short_path</code> for the path under which the file is mapped if it's in the "
- + "runfiles of a binary."
- )
- public String getExecPathString();
+ name = "path",
+ structField = true,
+ doc =
+ "The execution path of this file, relative to the workspace's execution directory. It "
+ + "consists of two parts, an optional first part called the <i>root</i> (see also "
+ + "the <a href=\"root.html\">root</a> module), and the second part which is the "
+ + "<code>short_path</code>. The root may be empty, which it usually is for "
+ + "non-generated files. For generated files it usually contains a "
+ + "configuration-specific path fragment that encodes things like the target CPU "
+ + "architecture that was used while building said file. Use the "
+ + "<code>short_path</code> for the path under which the file is mapped if it's in "
+ + "the runfiles of a binary.")
+ String getExecPathString();
}
