Support directory expansion in Skylark Args.
This is controlled by a flag "expand_directories", whose default value is controlled by --incompatible_expand_directories.
Example:
directory = ctx.actions.declare_directory('dir')
...
args = ctx.actions.args()
args.add_all([directory])
The resulting command line will now contain the directory contents instead of the path to the directory.
RELNOTES[INC]: Adds --incompatible_expand_directories to automatically expand directories in skylark command lines. Design doc: https://docs.google.com/document/d/11agWFiOUiz2htBLj6swPTob5z78TrCxm8DQE4uJLOwM
PiperOrigin-RevId: 210155043
diff --git a/site/docs/skylark/backward-compatibility.md b/site/docs/skylark/backward-compatibility.md
index 1a20b05..3e2d509 100644
--- a/site/docs/skylark/backward-compatibility.md
+++ b/site/docs/skylark/backward-compatibility.md
@@ -45,6 +45,7 @@
* [Remove native http archive](#remove-native-http-archive)
* [New-style JavaInfo constructor](#new-style-java_info)
* [Disallow tools in action inputs](#disallow-tools-in-action-inputs)
+* [Expand directories in Args](#expand-directories-in-args)
### Dictionary concatenation
@@ -387,4 +388,32 @@
* Flag: `--incompatible_no_support_tools_in_action_inputs`
* Default: `false`
-<!-- Add new options here -->
+
+### Expand directories in Args
+
+Previously, directories created by
+[`ctx.actions.declare_directory`](lib/actions.html#declare_directory) expanded
+to the path of the directory when added to an [`Args`](lib/Args.html) object.
+
+With this flag enabled, directories are instead replaced by the full file
+contents of that directory when passed to `args.add_all()` or
+`args.add_joined()`. (Directories may not be passed to `args.add()`.)
+
+If you want the old behavior on a case-by-case basis (perhaps your tool can
+handle directories on the command line), you can pass `expand_directories=False`
+to the `args.add_all()` or `args.add_joined()` call.
+
+```
+d = ctx.action.declare_directory(“dir”)
+# ... Some action runs and produces [“dir/file1”, “dir/file2”] ...
+f = ctx.action.declare_file(“file”)
+args = ctx.action.args()
+args.add_all([d, f])
+ -> Used to expand to ["dir", "file"]
+ Now expands to [“dir/file1”, “dir/file2”, “file”]
+```
+
+* Flag: `--incompatible_expand_directories`
+* Default: `false`
+
+!-- Add new options here -->
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 755a909..19ca017 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
@@ -19,6 +19,7 @@
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.Artifact;
@@ -68,8 +69,10 @@
import com.google.protobuf.GeneratedMessage;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.UUID;
import javax.annotation.Nullable;
@@ -167,6 +170,9 @@
action =
new ParameterFileWriteAction(
ruleContext.getActionOwner(),
+ skylarkSemantics.incompatibleExpandDirectories()
+ ? args.getTreeArtifacts()
+ : ImmutableList.of(),
(Artifact) output,
args.build(),
args.parameterFileType,
@@ -343,6 +349,11 @@
.setFlagFormatString(args.flagFormatString)
.setUseAlways(args.useAlways)
.setCharset(StandardCharsets.UTF_8)
+ .setInputs(
+ skylarkSemantics.incompatibleExpandDirectories()
+ && !context.getConfiguration().deferParamFiles()
+ ? args.getTreeArtifacts()
+ : ImmutableList.of())
.build();
}
builder.addCommandLine(args.commandLine.build(), paramFileInfo);
@@ -539,6 +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 ParameterFileType parameterFileType = ParameterFileType.SHELL_QUOTED;
private String flagFormatString;
private boolean useAlways;
@@ -586,6 +599,7 @@
/* formatJoined= */ null,
/* omitIfEmpty= */ false,
/* uniquify= */ false,
+ skylarkSemantics.incompatibleExpandDirectories(),
/* terminateWith= */ null,
loc);
@@ -621,6 +635,7 @@
Object beforeEach,
Boolean omitIfEmpty,
Boolean uniquify,
+ Object expandDirectories,
Object terminateWith,
Location loc)
throws EvalException {
@@ -647,6 +662,9 @@
/* formatJoined= */ null,
omitIfEmpty,
uniquify,
+ expandDirectories == Runtime.UNBOUND
+ ? skylarkSemantics.incompatibleExpandDirectories()
+ : (Boolean) expandDirectories,
terminateWith != Runtime.NONE ? (String) terminateWith : null,
loc);
return Runtime.NONE;
@@ -662,6 +680,7 @@
Object formatJoined,
Boolean omitIfEmpty,
Boolean uniquify,
+ Object expandDirectories,
Location loc)
throws EvalException {
if (isImmutable()) {
@@ -687,6 +706,9 @@
formatJoined != Runtime.NONE ? (String) formatJoined : null,
omitIfEmpty,
uniquify,
+ expandDirectories == Runtime.UNBOUND
+ ? skylarkSemantics.incompatibleExpandDirectories()
+ : (Boolean) expandDirectories,
/* terminateWith= */ null,
loc);
return Runtime.NONE;
@@ -703,23 +725,33 @@
String formatJoined,
boolean omitIfEmpty,
boolean uniquify,
+ boolean expandDirectories,
String terminateWith,
Location loc)
throws EvalException {
SkylarkCustomCommandLine.VectorArg.Builder vectorArg;
if (value instanceof SkylarkNestedSet) {
- NestedSet<?> nestedSet = ((SkylarkNestedSet) value).getSet(Object.class);
+ SkylarkNestedSet skylarkNestedSet = ((SkylarkNestedSet) value);
+ NestedSet<Object> nestedSet = skylarkNestedSet.getSet(Object.class);
+ if (expandDirectories) {
+ potentialTreeArtifacts.add(nestedSet);
+ }
vectorArg = new SkylarkCustomCommandLine.VectorArg.Builder(nestedSet);
} else {
- SkylarkList skylarkList = (SkylarkList) value;
+ @SuppressWarnings("unchecked")
+ SkylarkList<Object> skylarkList = (SkylarkList<Object>) value;
+ if (expandDirectories) {
+ scanForTreeArtifacts(skylarkList);
+ }
vectorArg = new SkylarkCustomCommandLine.VectorArg.Builder(skylarkList);
}
validateMapEach(mapEach, loc);
- validateFormatString("format_each", formatEach);
- validateFormatString("format_joined", formatJoined);
+ validateFormatString("format_each", formatEach, loc);
+ validateFormatString("format_joined", formatJoined, loc);
vectorArg
.setLocation(loc)
.setArgName(argName)
+ .setExpandDirectories(expandDirectories)
.setMapAll(mapAll)
.setFormatEach(formatEach)
.setBeforeEach(beforeEach)
@@ -769,13 +801,13 @@
}
}
- private void validateFormatString(String argumentName, @Nullable String formatStr)
+ private void validateFormatString(String argumentName, @Nullable String formatStr, Location loc)
throws EvalException {
if (formatStr != null
&& skylarkSemantics.incompatibleDisallowOldStyleArgsAdd()
&& !SingleStringArgFormatter.isValid(formatStr)) {
throw new EvalException(
- null,
+ loc,
String.format(
"Invalid value for parameter \"%s\": Expected string with a single \"%%s\"",
argumentName));
@@ -784,7 +816,8 @@
private void addScalarArg(Object value, String format, BaseFunction mapFn, Location loc)
throws EvalException {
- validateFormatString("format", format);
+ validateNoTreeArtifact(value, loc);
+ validateFormatString("format", format, loc);
if (format == null && mapFn == null) {
commandLine.add(value);
} else {
@@ -794,6 +827,18 @@
}
}
+ private void validateNoTreeArtifact(Object value, Location loc) throws EvalException {
+ if (skylarkSemantics.incompatibleExpandDirectories()
+ && (value instanceof Artifact)
+ && ((Artifact) value).isTreeArtifact()) {
+ throw new EvalException(
+ loc,
+ "Cannot add directories to Args#add since they may expand to multiple values. "
+ + "Either use Args#add_all (if you want expansion) "
+ + "or args.add(directory.path) (if you do not).");
+ }
+ }
+
@Override
public void useParamsFile(String paramFileArg, Boolean useAlways) throws EvalException {
if (isImmutable()) {
@@ -848,6 +893,22 @@
public void repr(SkylarkPrinter printer) {
printer.append("context.args() object");
}
+
+ ImmutableSet<Artifact> getTreeArtifacts() {
+ for (Iterable<Object> collection : potentialTreeArtifacts) {
+ scanForTreeArtifacts(collection);
+ }
+ potentialTreeArtifacts.clear();
+ return ImmutableSet.copyOf(treeArtifacts);
+ }
+
+ private void scanForTreeArtifacts(Iterable<?> objects) {
+ for (Object object : objects) {
+ if ((object instanceof Artifact) && ((Artifact) object).isTreeArtifact()) {
+ treeArtifacts.add((Artifact) object);
+ }
+ }
+ }
}
@Override
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 3e44975..eb404bb 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
@@ -20,6 +20,8 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Interner;
import com.google.devtools.build.lib.actions.ActionKeyContext;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
import com.google.devtools.build.lib.actions.CommandLine;
import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.actions.CommandLineItem;
@@ -64,15 +66,18 @@
private static final int HAS_MAP_ALL = 1 << 1;
private static final int HAS_MAP_EACH = 1 << 2;
private static final int IS_NESTED_SET = 1 << 3;
- private static final int UNIQUIFY = 1 << 4;
- private static final int OMIT_IF_EMPTY = 1 << 5;
- private static final int HAS_ARG_NAME = 1 << 6;
- private static final int HAS_FORMAT_EACH = 1 << 7;
- private static final int HAS_BEFORE_EACH = 1 << 8;
- private static final int HAS_JOIN_WITH = 1 << 9;
- private static final int HAS_FORMAT_JOINED = 1 << 10;
- private static final int HAS_TERMINATE_WITH = 1 << 11;
+ private static final int EXPAND_DIRECTORIES = 1 << 4;
+ private static final int UNIQUIFY = 1 << 5;
+ private static final int OMIT_IF_EMPTY = 1 << 6;
+ private static final int HAS_ARG_NAME = 1 << 7;
+ private static final int HAS_FORMAT_EACH = 1 << 8;
+ private static final int HAS_BEFORE_EACH = 1 << 9;
+ private static final int HAS_JOIN_WITH = 1 << 10;
+ private static final int HAS_FORMAT_JOINED = 1 << 11;
+ private static final int HAS_TERMINATE_WITH = 1 << 12;
+ private static final UUID EXPAND_DIRECTORIES_UUID =
+ UUID.fromString("9d7520d2-a187-11e8-98d0-529269fb1459");
private static final UUID UNIQUIFY_UUID =
UUID.fromString("7f494c3e-faea-4498-a521-5d3bc6ee19eb");
private static final UUID OMIT_IF_EMPTY_UUID =
@@ -107,6 +112,7 @@
features |= arg.mapAll != null ? HAS_MAP_ALL : 0;
features |= arg.mapEach != null ? HAS_MAP_EACH : 0;
features |= arg.nestedSet != null ? IS_NESTED_SET : 0;
+ features |= arg.expandDirectories ? EXPAND_DIRECTORIES : 0;
features |= arg.uniquify ? UNIQUIFY : 0;
features |= arg.omitIfEmpty ? OMIT_IF_EMPTY : 0;
features |= arg.argName != null ? HAS_ARG_NAME : 0;
@@ -168,6 +174,7 @@
List<Object> arguments,
int argi,
ImmutableList.Builder<String> builder,
+ @Nullable ArtifactExpander artifactExpander,
SkylarkSemantics skylarkSemantics)
throws CommandLineExpansionException {
final Location location =
@@ -185,12 +192,18 @@
originalValues = arguments.subList(argi, argi + count);
argi += count;
}
+ List<Object> expandedValues = originalValues;
+ if (artifactExpander != null && (features & EXPAND_DIRECTORIES) != 0) {
+ if (hasTreeArtifact(originalValues)) {
+ expandedValues = expandTreeArtifacts(artifactExpander, originalValues);
+ }
+ }
List<String> stringValues;
if (mapEach != null) {
- stringValues = new ArrayList<>(originalValues.size());
- applyMapEach(mapEach, originalValues, stringValues::add, location, skylarkSemantics);
+ stringValues = new ArrayList<>(expandedValues.size());
+ applyMapEach(mapEach, expandedValues, stringValues::add, location, skylarkSemantics);
} else if (mapAll != null) {
- Object result = applyMapFn(mapAll, originalValues, location, skylarkSemantics);
+ Object result = applyMapFn(mapAll, expandedValues, location, skylarkSemantics);
if (!(result instanceof List)) {
throw new CommandLineExpansionException(
errorMessage(
@@ -199,13 +212,13 @@
null));
}
List resultAsList = (List) result;
- if (resultAsList.size() != originalValues.size()) {
+ if (resultAsList.size() != expandedValues.size()) {
throw new CommandLineExpansionException(
errorMessage(
String.format(
"map_fn must return a list of the same length as the input. "
+ "Found list of length %d, expected %d.",
- resultAsList.size(), originalValues.size()),
+ resultAsList.size(), expandedValues.size()),
location,
null));
}
@@ -217,10 +230,10 @@
stringValues.add(CommandLineItem.expandToCommandLine(resultAsList.get(i)));
}
} else {
- int count = originalValues.size();
- stringValues = new ArrayList<>(originalValues.size());
+ int count = expandedValues.size();
+ stringValues = new ArrayList<>(expandedValues.size());
for (int i = 0; i < count; ++i) {
- stringValues.add(CommandLineItem.expandToCommandLine(originalValues.get(i)));
+ stringValues.add(CommandLineItem.expandToCommandLine(expandedValues.get(i)));
}
}
// It's safe to uniquify at this stage, any transformations after this
@@ -291,6 +304,33 @@
return argi;
}
+ private static boolean hasTreeArtifact(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()) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private static List<Object> expandTreeArtifacts(
+ Artifact.ArtifactExpander artifactExpander, List<Object> originalValues) {
+ 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);
+ } else {
+ expandedValues.add(object);
+ }
+ }
+ return expandedValues;
+ }
+
private int addToFingerprint(
List<Object> arguments,
int argi,
@@ -335,6 +375,9 @@
}
}
}
+ if ((features & EXPAND_DIRECTORIES) != 0) {
+ fingerprint.addUUID(EXPAND_DIRECTORIES_UUID);
+ }
if ((features & UNIQUIFY) != 0) {
fingerprint.addUUID(UNIQUIFY_UUID);
}
@@ -380,7 +423,7 @@
SkylarkSemantics skylarkSemantics)
throws CommandLineExpansionException {
ImmutableList.Builder<String> builder = ImmutableList.builder();
- argi = eval(arguments, argi, builder, skylarkSemantics);
+ argi = eval(arguments, argi, builder, null, skylarkSemantics);
for (String s : builder.build()) {
fingerprint.addString(s);
}
@@ -392,6 +435,7 @@
@Nullable private final NestedSet<?> nestedSet;
private Location location;
public String argName;
+ private boolean expandDirectories;
private BaseFunction mapAll;
private BaseFunction mapEach;
private String formatEach;
@@ -422,6 +466,11 @@
return this;
}
+ Builder setExpandDirectories(boolean expandDirectories) {
+ this.expandDirectories = expandDirectories;
+ return this;
+ }
+
Builder setMapAll(BaseFunction mapAll) {
this.mapAll = mapAll;
return this;
@@ -666,11 +715,17 @@
@Override
public Iterable<String> arguments() throws CommandLineExpansionException {
+ return arguments(null);
+ }
+
+ @Override
+ public Iterable<String> arguments(@Nullable ArtifactExpander artifactExpander)
+ throws CommandLineExpansionException {
ImmutableList.Builder<String> result = ImmutableList.builder();
for (int argi = 0; argi < arguments.size(); ) {
Object arg = arguments.get(argi++);
if (arg instanceof VectorArg) {
- argi = ((VectorArg) arg).eval(arguments, argi, result, skylarkSemantics);
+ argi = ((VectorArg) arg).eval(arguments, argi, result, artifactExpander, skylarkSemantics);
} else if (arg instanceof ScalarArg) {
argi = ((ScalarArg) arg).eval(arguments, argi, result, skylarkSemantics);
} else {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
index 8a2b397..076f156 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
@@ -269,6 +269,21 @@
public boolean incompatibleDisallowOldStyleArgsAdd;
@Option(
+ name = "incompatible_expand_directories",
+ defaultValue = "false",
+ category = "incompatible changes",
+ documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+ effectTags = {OptionEffectTag.UNKNOWN},
+ metadataTags = {
+ OptionMetadataTag.INCOMPATIBLE_CHANGE,
+ OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+ },
+ help =
+ "Controls whether directories are expanded to the list of files under that directory "
+ + "when added to Args, instead of replaced by the path of the directory.")
+ public boolean incompatibleExpandDirectories;
+
+ @Option(
name = "incompatible_new_actions_api",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.SKYLARK_SEMANTICS,
@@ -400,6 +415,7 @@
.incompatibleDisallowLegacyJavaInfo(incompatibleDisallowLegacyJavaInfo)
.incompatibleDisallowOldStyleArgsAdd(incompatibleDisallowOldStyleArgsAdd)
.incompatibleDisallowSlashOperator(incompatibleDisallowSlashOperator)
+ .incompatibleExpandDirectories(incompatibleExpandDirectories)
.incompatibleGenerateJavaCommonSourceJar(incompatibleGenerateJavaCommonSourceJar)
.incompatibleNewActionsApi(incompatibleNewActionsApi)
.incompatibleNoSupportToolsInActionInputs(incompatibleNoSupportToolsInActionInputs)
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/CommandLineArgsApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/CommandLineArgsApi.java
index 2062772..0a34af6 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/CommandLineArgsApi.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/CommandLineArgsApi.java
@@ -140,8 +140,11 @@
"The object to append. It will be converted to a string using the standard "
+ "conversion mentioned above. Since there is no <code>map_each</code> "
+ "parameter for this function, <code>value</code> should be either a "
- + "string or a <code>File</code>."
- + ""
+ + "string or a <code>File</code>. A directory <code>File</code> must be "
+ + "passed to <a href='#add_all'><code>add_all()</code> or "
+ + "<a href='#add_joined'><code>add_joined()</code></a> instead of this method."
+ + "<p><i>Deprecated behavior:</i> <code>value</code> may be a directory "
+ + "unless --noincompatible_expand_directories is passed.</p>"
+ "<p><i>Deprecated behavior:</i> <code>value</code> may also be a "
+ "list, tuple, or depset of multiple items to append."),
@Param(
@@ -213,7 +216,11 @@
+ "<p>Most of the processing occurs over a list of arguments to be appended, as per "
+ "the following steps:"
+ "<ol>"
- + "<li>If <code>map_each</code> is given, it is applied to each input item, and the "
+ + "<li>Each directory <code>File</code> item is replaced by all <code>File</code>s "
+ + "recursively contained in that directory, unless "
+ + "<code>expand_directories=False</code> is specified."
+ + "</li>"
+ + "<li>If <code>map_each</code> is given, it is applied to each item, and the "
+ " resulting lists of strings are concatenated to form the initial argument "
+ " list. Otherwise, the initial argument list is the result of applying the "
+ " standard conversion to each item."
@@ -325,6 +332,17 @@
+ "but it can be useful if <code>map_each</code> emits the same string for "
+ "multiple items."),
@Param(
+ name = "expand_directories",
+ type = Boolean.class,
+ named = true,
+ positional = false,
+ defaultValue = "unbound",
+ doc =
+ "If true, any directories in <code>values</code> will be expanded to a flat list "
+ + "of files. This happens before <code>map_each</code> is applied. "
+ + "The default value of this flag is controlled by "
+ + "--incompatible_expand_directories."),
+ @Param(
name = "terminate_with",
type = String.class,
named = true,
@@ -346,6 +364,7 @@
Object beforeEach,
Boolean omitIfEmpty,
Boolean uniquify,
+ Object expandDirectories,
Object terminateWith,
Location loc)
throws EvalException;
@@ -443,7 +462,14 @@
named = true,
positional = false,
defaultValue = "False",
- doc = "Same as for <a href='#add_all.uniquify'><code>add_all</code></a>.")
+ doc = "Same as for <a href='#add_all.uniquify'><code>add_all</code></a>."),
+ @Param(
+ name = "expand_directories",
+ type = Boolean.class,
+ named = true,
+ positional = false,
+ defaultValue = "unbound",
+ doc = "Same as for <a href='#add_all.expand_directories'><code>add_all</code></a>.")
},
useLocation = true)
public Runtime.NoneType addJoined(
@@ -455,6 +481,7 @@
Object formatJoined,
Boolean omitIfEmpty,
Boolean uniquify,
+ Object expandDirectories,
Location loc)
throws EvalException;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
index ac13aad..187bf0f 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
@@ -71,6 +71,8 @@
public abstract boolean incompatibleDisallowSlashOperator();
+ public abstract boolean incompatibleExpandDirectories();
+
public abstract boolean incompatibleGenerateJavaCommonSourceJar();
public abstract boolean incompatibleNewActionsApi();
@@ -119,6 +121,7 @@
.incompatibleDisallowLegacyJavaInfo(false)
.incompatibleDisallowOldStyleArgsAdd(false)
.incompatibleDisallowSlashOperator(false)
+ .incompatibleExpandDirectories(false)
.incompatibleGenerateJavaCommonSourceJar(false)
.incompatibleNewActionsApi(false)
.incompatibleNoSupportToolsInActionInputs(false)
@@ -165,6 +168,8 @@
public abstract Builder incompatibleDisallowSlashOperator(boolean value);
+ public abstract Builder incompatibleExpandDirectories(boolean value);
+
public abstract Builder incompatibleGenerateJavaCommonSourceJar(boolean value);
public abstract Builder incompatibleNewActionsApi(boolean value);
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
index 3dd878c..eff7ba3 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
@@ -136,6 +136,7 @@
"--incompatible_disallow_legacy_javainfo=" + rand.nextBoolean(),
"--incompatible_disallow_old_style_args_add=" + rand.nextBoolean(),
"--incompatible_disallow_slash_operator=" + rand.nextBoolean(),
+ "--incompatible_expand_directories=" + rand.nextBoolean(),
"--incompatible_generate_javacommon_source_jar=" + rand.nextBoolean(),
"--incompatible_new_actions_api=" + rand.nextBoolean(),
"--incompatible_no_support_tools_in_action_inputs=" + rand.nextBoolean(),
@@ -170,6 +171,7 @@
.incompatibleDisallowLegacyJavaInfo(rand.nextBoolean())
.incompatibleDisallowOldStyleArgsAdd(rand.nextBoolean())
.incompatibleDisallowSlashOperator(rand.nextBoolean())
+ .incompatibleExpandDirectories(rand.nextBoolean())
.incompatibleGenerateJavaCommonSourceJar(rand.nextBoolean())
.incompatibleNewActionsApi(rand.nextBoolean())
.incompatibleNoSupportToolsInActionInputs(rand.nextBoolean())
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
index b8dc016..742da5a 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
@@ -26,6 +26,8 @@
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.Artifact.ArtifactExpanderImpl;
+import com.google.devtools.build.lib.actions.CommandLine;
import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.actions.CompositeRunfilesSupplier;
import com.google.devtools.build.lib.actions.RunfilesSupplier;
@@ -41,6 +43,7 @@
import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.analysis.skylark.SkylarkActionFactory;
+import com.google.devtools.build.lib.analysis.skylark.SkylarkActionFactory.Args;
import com.google.devtools.build.lib.analysis.skylark.SkylarkCustomCommandLine;
import com.google.devtools.build.lib.analysis.skylark.SkylarkRuleContext;
import com.google.devtools.build.lib.cmdline.Label;
@@ -56,6 +59,7 @@
import com.google.devtools.build.lib.syntax.EvalUtils;
import com.google.devtools.build.lib.syntax.Printer;
import com.google.devtools.build.lib.syntax.Runtime;
+import com.google.devtools.build.lib.syntax.SkylarkList;
import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
import com.google.devtools.build.lib.testutil.MoreAsserts;
@@ -2873,6 +2877,128 @@
return ((SkylarkActionFactory.Args) evalRuleContextCode(lines)).build();
}
+ @Test
+ public void testDirectoryInArgs() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_expand_directories");
+ SkylarkList<?> result =
+ (SkylarkList<?>)
+ evalRuleContextCode(
+ "args = ruleContext.actions.args()",
+ "directory = ruleContext.actions.declare_directory('dir')",
+ "def _short_path(f): return f.short_path", // For easier assertions
+ "args.add_all([directory], map_each=_short_path)",
+ "args, directory");
+ Args args = (Args) result.get(0);
+ Artifact directory = (Artifact) result.get(1);
+ CommandLine commandLine = args.build();
+
+ // When asking for arguments without an artifact expander we just return the directory
+ assertThat(commandLine.arguments()).containsExactly("foo/dir");
+
+ // Now ask for one with an expanded directory
+ Artifact file1 = getBinArtifactWithNoOwner("foo/dir/file1");
+ Artifact file2 = getBinArtifactWithNoOwner("foo/dir/file2");
+ ArtifactExpanderImpl artifactExpander =
+ new ArtifactExpanderImpl(
+ ImmutableMap.of(directory, ImmutableList.of(file1, file2)), ImmutableMap.of());
+ assertThat(commandLine.arguments(artifactExpander))
+ .containsExactly("foo/dir/file1", "foo/dir/file2");
+ }
+
+ @Test
+ public void testDirectoryInArgsIncompatibleFlagOff() throws Exception {
+ setSkylarkSemanticsOptions("--noincompatible_expand_directories");
+ SkylarkList<?> result =
+ (SkylarkList<?>)
+ evalRuleContextCode(
+ "args = ruleContext.actions.args()",
+ "directory = ruleContext.actions.declare_directory('dir')",
+ "def _short_path(f): return f.short_path", // For easier assertions
+ "args.add_all([directory], map_each=_short_path)",
+ "args, directory");
+ Args args = (Args) result.get(0);
+ Artifact directory = (Artifact) result.get(1);
+ CommandLine commandLine = args.build();
+
+ Artifact file1 = getBinArtifactWithNoOwner("foo/dir/file1");
+ Artifact file2 = getBinArtifactWithNoOwner("foo/dir/file2");
+ ArtifactExpanderImpl artifactExpander =
+ new ArtifactExpanderImpl(
+ ImmutableMap.of(directory, ImmutableList.of(file1, file2)), ImmutableMap.of());
+ // Do not expand the directory, incompatible flag is off
+ assertThat(commandLine.arguments(artifactExpander)).containsExactly("foo/dir");
+ }
+
+ @Test
+ public void testDirectoryInArgsExpandDirectories() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_expand_directories");
+ SkylarkList<?> result =
+ (SkylarkList<?>)
+ evalRuleContextCode(
+ "args = ruleContext.actions.args()",
+ "directory = ruleContext.actions.declare_directory('dir')",
+ "def _short_path(f): return f.short_path", // For easier assertions
+ "args.add_all([directory], map_each=_short_path, expand_directories=True)",
+ "args.add_all([directory], map_each=_short_path, expand_directories=False)",
+ "args, directory");
+ Args args = (Args) result.get(0);
+ Artifact directory = (Artifact) result.get(1);
+ CommandLine commandLine = args.build();
+
+ Artifact file1 = getBinArtifactWithNoOwner("foo/dir/file1");
+ Artifact file2 = getBinArtifactWithNoOwner("foo/dir/file2");
+ ArtifactExpanderImpl artifactExpander =
+ new ArtifactExpanderImpl(
+ ImmutableMap.of(directory, ImmutableList.of(file1, file2)), ImmutableMap.of());
+ // First expanded, then not expanded (two separate calls)
+ assertThat(commandLine.arguments(artifactExpander))
+ .containsExactly("foo/dir/file1", "foo/dir/file2", "foo/dir");
+ }
+
+ @Test
+ public void testDirectoryInScalarArgsFails() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_expand_directories");
+ checkErrorContains(
+ "Cannot add directories to Args#add",
+ "args = ruleContext.actions.args()",
+ "directory = ruleContext.actions.declare_directory('dir')",
+ "args.add(directory)");
+ }
+
+ @Test
+ public void testDirectoryInScalarArgsIsOkWithoutIncompatibleFlag() throws Exception {
+ setSkylarkSemanticsOptions("--noincompatible_expand_directories");
+ Args args =
+ (Args)
+ evalRuleContextCode(
+ "args = ruleContext.actions.args()",
+ "directory = ruleContext.actions.declare_directory('dir')",
+ "args.add(directory)",
+ "args");
+ assertThat(Iterables.getOnlyElement(args.build().arguments())).endsWith("foo/dir");
+ }
+
+ @Test
+ public void testParamFileHasDirectoryAsInput() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_expand_directories");
+ SkylarkRuleContext ctx = dummyRuleContext();
+ SkylarkList<?> result =
+ (SkylarkList<?>)
+ evalRuleContextCode(
+ ctx,
+ "args = ruleContext.actions.args()",
+ "directory = ruleContext.actions.declare_directory('dir')",
+ "args.add_all([directory])",
+ "params = ruleContext.actions.declare_file('params')",
+ "ruleContext.actions.write(params, args)",
+ "params, directory");
+ Artifact params = (Artifact) result.get(0);
+ Artifact directory = (Artifact) result.get(1);
+ ActionAnalysisMetadata action =
+ ctx.getRuleContext().getAnalysisEnvironment().getLocalGeneratingAction(params);
+ assertThat(action.getInputs()).contains(directory);
+ }
+
private void setupThrowFunction(BuiltinFunction func) throws Exception {
throwFunction = func;
throwFunction.configure(
