aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar laszlocsomor <laszlocsomor@google.com>2017-07-14 17:06:05 +0200
committerGravatar Jakob Buchgraber <buchgr@google.com>2017-07-17 10:10:39 +0200
commita76c94be7c56b93fc5a2f9ececfba7ac1f61f69c (patch)
tree09c1ab12473aa8ac6b453b24db098e8b347fe7a9
parent681b8174d5ae989cf9489716e4c15a54c2d36bc4 (diff)
CustomCommandLine.Builder: clean up its interface
In this commit: - remove unused methods and classes - turn CustomCommandLine.ArgvFragment into an interface - remove the CustomCommandLine.TreeFileArtifactArgvFragment abstract class; it only had one remaining subclass - add @Nullable annotations where nulls are fine - add Precondition checks for non-nullable args - simplify the interface by removing add* methods that can be composed of other add* methods; this makes it easier to see what the callers do with the Builder - remove add* methods that add a single argument followed by a list of other elements (or a joined string of them); these had a bug in that they didn't check if the collection was empty (only that it was not null), and if it was empty then the single argument was still added though it was not followed by any value - fix call sites of add* methods where we previously could have added a flag with an empty collection - audit every affected call site RELNOTES: none PiperOrigin-RevId: 161957521
-rw-r--r--src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java312
-rw-r--r--src/main/java/com/google/devtools/build/lib/analysis/actions/ParamFileHelper.java3
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java10
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java6
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/LibraryRGeneratorActionBuilder.java37
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java5
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainerConverter.java10
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/ResourceShrinkerActionBuilder.java18
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/android/RobolectricResourceSymbolsActionBuilder.java11
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/java/DeployArchiveBuilder.java17
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java54
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java30
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/java/ResourceJarActionBuilder.java4
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/java/SingleJarActionBuilder.java5
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java36
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java16
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/objc/LipoSupport.java29
-rw-r--r--src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java174
-rw-r--r--src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java4
19 files changed, 380 insertions, 401 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java
index 601229bd14..b05b0394c6 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java
@@ -20,6 +20,7 @@ import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableList.Builder;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
@@ -43,40 +44,20 @@ import javax.annotation.Nullable;
@Immutable
public final class CustomCommandLine extends CommandLine {
- private abstract static class ArgvFragment {
- abstract void eval(ImmutableList.Builder<String> builder);
+ private interface ArgvFragment {
+ void eval(ImmutableList.Builder<String> builder);
}
/**
- * A command line argument for {@link TreeFileArtifact}.
+ * A command line argument that can expand enclosed TreeArtifacts into a list of child {@link
+ * TreeFileArtifact}s at execution time before argument evaluation.
*
- * <p>Since {@link TreeFileArtifact} is not known or available at analysis time, subclasses should
- * enclose its parent TreeFileArtifact instead at analysis time. This interface provides method
- * {@link #substituteTreeArtifact} to generate another argument object that replaces the enclosed
- * TreeArtifact with one of its {@link TreeFileArtifact} at execution time.
- */
- private abstract static class TreeFileArtifactArgvFragment {
- /**
- * Substitutes this ArgvFragment with another arg object, with the original TreeArtifacts
- * contained in this ArgvFragment replaced by their associated TreeFileArtifacts.
- *
- * @param substitutionMap A map between TreeArtifacts and their associated TreeFileArtifacts
- * used to replace them.
- */
- abstract Object substituteTreeArtifact(Map<Artifact, TreeFileArtifact> substitutionMap);
- }
-
- /**
- * A command line argument that can expand enclosed TreeArtifacts into a list of child
- * {@link TreeFileArtifact}s at execution time before argument evaluation.
- *
- * <p>The main difference between this class and {@link TreeFileArtifactArgvFragment} is that
- * {@link TreeFileArtifactArgvFragment} is used in {@link SpawnActionTemplate} to substitutes a
+ * <p>The main difference between this class and {@link TreeFileArtifactExecPathArg} is that
+ * {@link TreeFileArtifactExecPathArg} is used in {@link SpawnActionTemplate} to substitutes a
* TreeArtifact with *one* of its child TreeFileArtifacts, while this class expands a TreeArtifact
* into *all* of its child TreeFileArtifacts.
- *
*/
- private abstract static class TreeArtifactExpansionArgvFragment extends ArgvFragment {
+ private abstract static class TreeArtifactExpansionArgvFragment implements ArgvFragment {
/**
* Evaluates this argument fragment into an argument string and adds it into {@code builder}.
* The enclosed TreeArtifact will be expanded using {@code artifactExpander}.
@@ -98,13 +79,13 @@ public final class CustomCommandLine extends CommandLine {
* <p>Internally this method just calls {@link #describe}.
*/
@Override
- void eval(ImmutableList.Builder<String> builder) {
+ public void eval(ImmutableList.Builder<String> builder) {
builder.add(describe());
}
}
// It's better to avoid anonymous classes if we want to serialize command lines
- private static final class JoinExecPathsArg extends ArgvFragment {
+ private static final class JoinExecPathsArg implements ArgvFragment {
private final String delimiter;
private final Iterable<Artifact> artifacts;
@@ -115,7 +96,7 @@ public final class CustomCommandLine extends CommandLine {
}
@Override
- void eval(ImmutableList.Builder<String> builder) {
+ public void eval(ImmutableList.Builder<String> builder) {
builder.add(Artifact.joinExecPaths(delimiter, artifacts));
}
}
@@ -180,7 +161,7 @@ public final class CustomCommandLine extends CommandLine {
}
}
- private static final class PathWithTemplateArg extends ArgvFragment {
+ private static final class PathWithTemplateArg implements ArgvFragment {
private final String template;
private final PathFragment[] paths;
@@ -191,13 +172,13 @@ public final class CustomCommandLine extends CommandLine {
}
@Override
- void eval(ImmutableList.Builder<String> builder) {
+ public void eval(ImmutableList.Builder<String> builder) {
// PathFragment.toString() uses getPathString()
builder.add(String.format(template, (Object[]) paths));
}
}
- private static final class ParamFileArgument extends ArgvFragment {
+ private static final class ParamFileArgument implements ArgvFragment {
private final String paramFilePrefix;
private final PathFragment path;
@@ -207,84 +188,37 @@ public final class CustomCommandLine extends CommandLine {
}
@Override
- void eval(ImmutableList.Builder<String> builder) {
+ public void eval(ImmutableList.Builder<String> builder) {
builder.add(paramFilePrefix + path);
}
}
- /**
- * An argument object that evaluates to a formatted string for {@link TreeFileArtifact} exec
- * paths, enclosing the associated string format template and {@link TreeFileArtifact}s.
- */
- private static final class TreeFileArtifactExecPathWithTemplateArg
- extends TreeFileArtifactArgvFragment {
-
- private final String template;
- private final Artifact placeHolderTreeArtifact;
-
- private TreeFileArtifactExecPathWithTemplateArg(String template, Artifact artifact) {
- Preconditions.checkArgument(artifact.isTreeArtifact(), "%s must be a TreeArtifact",
- artifact);
- this.template = template;
- this.placeHolderTreeArtifact = artifact;
- }
-
- @Override
- ArgvFragment substituteTreeArtifact(Map<Artifact, TreeFileArtifact> substitutionMap) {
- Artifact treeFileArtifact = substitutionMap.get(placeHolderTreeArtifact);
- Preconditions.checkNotNull(treeFileArtifact, "Artifact to substitute: %s",
- placeHolderTreeArtifact);
-
- return new PathWithTemplateArg(template, treeFileArtifact.getExecPath());
- }
- }
-
// TODO(bazel-team): CustomArgv and CustomMultiArgv is going to be difficult to expose
// in Skylark. Maybe we can get rid of them by refactoring JavaCompileAction. It also
// raises immutability / serialization issues.
- /**
- * Custom Java code producing a String argument. Usage of this class is discouraged.
- */
- public abstract static class CustomArgv extends ArgvFragment {
+ /** Custom Java code producing a String argument. Usage of this class is discouraged. */
+ public abstract static class CustomArgv implements ArgvFragment {
@Override
- void eval(ImmutableList.Builder<String> builder) {
+ public void eval(ImmutableList.Builder<String> builder) {
builder.add(argv());
}
public abstract String argv();
}
- /**
- * Custom Java code producing a List of String arguments. Usage of this class is discouraged.
- */
- public abstract static class CustomMultiArgv extends ArgvFragment {
+ /** Custom Java code producing a List of String arguments. Usage of this class is discouraged. */
+ public abstract static class CustomMultiArgv implements ArgvFragment {
@Override
- void eval(ImmutableList.Builder<String> builder) {
+ public void eval(ImmutableList.Builder<String> builder) {
builder.addAll(argv());
}
public abstract Iterable<String> argv();
}
- private static final class JoinPathsArg extends ArgvFragment {
-
- private final String delimiter;
- private final Iterable<PathFragment> paths;
-
- private JoinPathsArg(String delimiter, Iterable<PathFragment> paths) {
- this.delimiter = delimiter;
- this.paths = CollectionUtils.makeImmutable(paths);
- }
-
- @Override
- void eval(ImmutableList.Builder<String> builder) {
- builder.add(Joiner.on(delimiter).join(paths));
- }
- }
-
- private static final class JoinStringsArg extends ArgvFragment {
+ private static final class JoinStringsArg implements ArgvFragment {
private final String delimiter;
private final Iterable<String> strings;
@@ -295,12 +229,12 @@ public final class CustomCommandLine extends CommandLine {
}
@Override
- void eval(ImmutableList.Builder<String> builder) {
+ public void eval(ImmutableList.Builder<String> builder) {
builder.add(Joiner.on(delimiter).join(strings));
}
}
- private static final class JoinValuesTransformed<T> extends ArgvFragment {
+ private static final class JoinValuesTransformed<T> implements ArgvFragment {
private final String delimiter;
private final Iterable<T> values;
@@ -314,7 +248,7 @@ public final class CustomCommandLine extends CommandLine {
}
@Override
- void eval(ImmutableList.Builder<String> builder) {
+ public void eval(ImmutableList.Builder<String> builder) {
StringBuilder arg = new StringBuilder();
Iterator<T> parts = values.iterator();
if (parts.hasNext()) {
@@ -331,18 +265,19 @@ public final class CustomCommandLine extends CommandLine {
/**
* Arguments that intersperse strings between the items in a sequence. There are two forms of
* interspersing, and either may be used by this implementation:
+ *
* <ul>
- * <li>before each - a string is added before each item in a sequence. e.g.
- * {@code -f foo -f bar -f baz}
- * <li>format each - a format string is used to format each item in a sequence. e.g.
- * {@code -I/foo -I/bar -I/baz} for the format {@code "-I%s"}
+ * <li>before each - a string is added before each item in a sequence. e.g. {@code -f foo -f bar
+ * -f baz}
+ * <li>format each - a format string is used to format each item in a sequence. e.g. {@code
+ * -I/foo -I/bar -I/baz} for the format {@code "-I%s"}
* </ul>
*
- * <p>This class could be used both with both the "before" and "format" features at the same
- * time, but this is probably more confusion than it is worth. If you need this functionality,
- * consider using "before" only but storing the strings pre-formated in a {@link NestedSet}.
+ * <p>This class could be used both with both the "before" and "format" features at the same time,
+ * but this is probably more confusion than it is worth. If you need this functionality, consider
+ * using "before" only but storing the strings pre-formatted in a {@link NestedSet}.
*/
- private static final class InterspersingArgs extends ArgvFragment {
+ private static final class InterspersingArgs implements ArgvFragment {
private final Iterable<?> sequence;
private final String beforeEach;
private final String formatEach;
@@ -370,7 +305,7 @@ public final class CustomCommandLine extends CommandLine {
}
@Override
- void eval(ImmutableList.Builder<String> builder) {
+ public void eval(ImmutableList.Builder<String> builder) {
for (Object item : sequence) {
if (item == null) {
continue;
@@ -388,12 +323,11 @@ public final class CustomCommandLine extends CommandLine {
}
}
-
/**
- * An argument object that evaluates to the exec path of a {@link TreeFileArtifact}, enclosing
- * the associated {@link TreeFileArtifact}.
+ * An argument object that evaluates to the exec path of a {@link TreeFileArtifact}, enclosing the
+ * associated {@link TreeFileArtifact}.
*/
- private static final class TreeFileArtifactExecPathArg extends TreeFileArtifactArgvFragment {
+ private static final class TreeFileArtifactExecPathArg {
private final Artifact placeHolderTreeArtifact;
private TreeFileArtifactExecPathArg(Artifact artifact) {
@@ -401,7 +335,13 @@ public final class CustomCommandLine extends CommandLine {
placeHolderTreeArtifact = artifact;
}
- @Override
+ /**
+ * Substitutes this ArgvFragment with another arg object, with the original TreeArtifacts
+ * contained in this ArgvFragment replaced by their associated TreeFileArtifacts.
+ *
+ * @param substitutionMap A map between TreeArtifacts and their associated TreeFileArtifacts
+ * used to replace them.
+ */
Object substituteTreeArtifact(Map<Artifact, TreeFileArtifact> substitutionMap) {
Artifact artifact = substitutionMap.get(placeHolderTreeArtifact);
Preconditions.checkNotNull(artifact, "Artifact to substitute: %s", placeHolderTreeArtifact);
@@ -431,56 +371,39 @@ public final class CustomCommandLine extends CommandLine {
// toString() results.
private final List<Object> arguments = new ArrayList<>();
- public Builder add(CharSequence arg) {
+ public Builder add(@Nullable CharSequence arg) {
if (arg != null) {
arguments.add(arg);
}
return this;
}
- public Builder add(Label arg) {
+ public Builder add(@Nullable Label arg) {
if (arg != null) {
arguments.add(arg);
}
return this;
}
- public Builder add(String arg, Iterable<String> args) {
- if (arg != null && args != null) {
- arguments.add(arg);
- arguments.add(
- InterspersingArgs.fromStrings(args, /*beforeEach=*/ null, /*formatEach=*/ null));
- }
- return this;
- }
-
- public Builder add(Iterable<String> args) {
- if (args != null) {
+ public Builder add(@Nullable Iterable<String> args) {
+ if (args != null && !Iterables.isEmpty(args)) {
arguments.add(
InterspersingArgs.fromStrings(args, /*beforeEach=*/ null, /*formatEach=*/ null));
}
return this;
}
- public Builder addExecPath(String arg, Artifact artifact) {
- if (arg != null && artifact != null) {
+ public Builder addExecPath(String arg, @Nullable Artifact artifact) {
+ Preconditions.checkNotNull(arg);
+ if (artifact != null) {
arguments.add(arg);
arguments.add(artifact.getExecPath());
}
return this;
}
- public Builder addExecPaths(String arg, Iterable<Artifact> artifacts) {
- if (arg != null && artifacts != null) {
- arguments.add(arg);
- arguments.add(
- InterspersingArgs.fromExecPaths(artifacts, /*beforeEach=*/ null, /*formatEach=*/ null));
- }
- return this;
- }
-
- public Builder addExecPaths(Iterable<Artifact> artifacts) {
- if (artifacts != null) {
+ public Builder addExecPaths(@Nullable Iterable<Artifact> artifacts) {
+ if (artifacts != null && !Iterables.isEmpty(artifacts)) {
arguments.add(
InterspersingArgs.fromExecPaths(artifacts, /*beforeEach=*/ null, /*formatEach=*/ null));
}
@@ -488,40 +411,23 @@ public final class CustomCommandLine extends CommandLine {
}
/**
- * Adds a flag with the exec path of a placeholder TreeArtifact. When the command line is used
- * in an action template, the placeholder will be replaced by the exec path of a
- * {@link TreeFileArtifact} inside the TreeArtifact at execution time for each expanded action.
- *
- * @param arg the name of the argument
- * @param treeArtifact the TreeArtifact that will be evaluated to one of its child
- * {@link TreeFileArtifact} at execution time
- */
- public Builder addPlaceholderTreeArtifactExecPath(String arg, Artifact treeArtifact) {
- if (arg != null && treeArtifact != null) {
- arguments.add(arg);
- arguments.add(new TreeFileArtifactExecPathArg(treeArtifact));
- }
- return this;
- }
-
- /**
* Adds a placeholder TreeArtifact exec path. When the command line is used in an action
* template, the placeholder will be replaced by the exec path of a {@link TreeFileArtifact}
* inside the TreeArtifact at execution time for each expanded action.
*
- * @param treeArtifact the TreeArtifact that will be evaluated to one of its child
- * {@link TreeFileArtifact} at execution time
+ * @param treeArtifact the TreeArtifact that will be evaluated to one of its child {@link
+ * TreeFileArtifact} at execution time
*/
- public Builder addPlaceholderTreeArtifactExecPath(Artifact treeArtifact) {
+ public Builder addPlaceholderTreeArtifactExecPath(@Nullable Artifact treeArtifact) {
if (treeArtifact != null) {
arguments.add(new TreeFileArtifactExecPathArg(treeArtifact));
}
return this;
}
- public Builder addJoinStrings(String arg, String delimiter, Iterable<String> strings) {
- if (arg != null && strings != null) {
- arguments.add(arg);
+ public Builder addJoinStrings(String delimiter, @Nullable Iterable<String> strings) {
+ Preconditions.checkNotNull(delimiter);
+ if (strings != null && !Iterables.isEmpty(strings)) {
arguments.add(new JoinStringsArg(delimiter, strings));
}
return this;
@@ -534,37 +440,38 @@ public final class CustomCommandLine extends CommandLine {
* this class, expansion of the nested set is deferred until action execution instead of
* retained on the heap.
*
- * @param arg The argument
* @param delimiter A delimiter string placed in between each transformed value
* @param values The values to expand into a list
* @param toString A function that transforms a value into a string
*/
public <T> Builder addJoinValues(
- String arg, String delimiter, Iterable<T> values, Function<T, String> toString) {
- if (arg != null && arguments != null) {
- arguments.add(arg);
+ String delimiter, @Nullable Iterable<T> values, Function<T, String> toString) {
+ Preconditions.checkNotNull(delimiter);
+ Preconditions.checkNotNull(toString);
+ if (values != null && !Iterables.isEmpty(values)) {
arguments.add(new JoinValuesTransformed<T>(delimiter, values, toString));
}
return this;
}
-
- public Builder addJoinExecPaths(String arg, String delimiter, Iterable<Artifact> artifacts) {
- if (arg != null && artifacts != null) {
- arguments.add(arg);
+
+ public Builder addJoinExecPaths(String delimiter, @Nullable Iterable<Artifact> artifacts) {
+ Preconditions.checkNotNull(delimiter);
+ if (artifacts != null && !Iterables.isEmpty(artifacts)) {
arguments.add(new JoinExecPathsArg(delimiter, artifacts));
}
return this;
}
- public Builder addPath(PathFragment path) {
+ public Builder addPath(@Nullable PathFragment path) {
if (path != null) {
arguments.add(path);
}
return this;
}
- public Builder addPaths(String template, PathFragment... path) {
- if (template != null && path != null) {
+ public Builder addPaths(String template, @Nullable PathFragment... path) {
+ Preconditions.checkNotNull(template);
+ if (path != null && path.length > 0) {
arguments.add(new PathWithTemplateArg(template, path));
}
return this;
@@ -573,41 +480,22 @@ public final class CustomCommandLine extends CommandLine {
/**
* Adds a param file as an argument.
*
+ * <p>Memory consumption consideration: though `addPaths` could also do the job of this method,
+ * this one is more memory-efficient because it delays string constructions as much as possible.
+ * Using `addPaths` would look like: <code>.addPaths(paramFilePrefix + "%s", paramFile)</code>,
+ * meaning we'd eagerly create an extra String for every param file.
+ *
* @param paramFilePrefix The character that denotes a param file, commonly '@'
* @param paramFile The param file artifact
*/
public Builder addParamFile(String paramFilePrefix, Artifact paramFile) {
+ Preconditions.checkNotNull(paramFilePrefix);
+ Preconditions.checkNotNull(paramFile);
arguments.add(new ParamFileArgument(paramFilePrefix, paramFile.getExecPath()));
return this;
}
/**
- * Adds a formatted string containing the exec path of a placeholder TreeArtifact. When the
- * command line is used in an action template, the placeholder will be replaced by the exec path
- * of a {@link TreeFileArtifact} inside the TreeArtifact at execution time for each expanded
- * action.
- *
- * @param template the string format template containing a single string format specifier (%s)
- * to be replaced by the artifact exec path string.
- * @param treeArtifact the TreeArtifact that will be evaluated to one of their child
- * {@link TreeFileArtifact} at execution time
- */
- public Builder addPlaceholderTreeArtifactFormattedExecPath(
- String template, Artifact treeArtifact) {
- if (template != null && treeArtifact != null) {
- arguments.add(new TreeFileArtifactExecPathWithTemplateArg(template, treeArtifact));
- }
- return this;
- }
-
- public Builder addJoinPaths(String delimiter, Iterable<PathFragment> paths) {
- if (delimiter != null && paths != null) {
- arguments.add(new JoinPathsArg(delimiter, paths));
- }
- return this;
- }
-
- /**
* Adds a string joined together by the exec paths of all {@link TreeFileArtifact}s under
* {@code treeArtifact}.
*
@@ -615,6 +503,8 @@ public final class CustomCommandLine extends CommandLine {
* @param treeArtifact the TreeArtifact containing the {@link TreeFileArtifact}s to join.
*/
public Builder addJoinExpandedTreeArtifactExecPath(String delimiter, Artifact treeArtifact) {
+ Preconditions.checkNotNull(delimiter);
+ Preconditions.checkNotNull(treeArtifact);
arguments.add(new JoinExpandedTreeArtifactExecPathsArg(delimiter, treeArtifact));
return this;
}
@@ -626,46 +516,51 @@ public final class CustomCommandLine extends CommandLine {
* @param treeArtifact the TreeArtifact containing the {@link TreeFileArtifact}s to add.
*/
public Builder addExpandedTreeArtifactExecPaths(Artifact treeArtifact) {
+ Preconditions.checkNotNull(treeArtifact);
arguments.add(new ExpandedTreeArtifactExecPathsArg(treeArtifact));
return this;
}
- public Builder addBeforeEachPath(String repeated, Iterable<PathFragment> paths) {
- if (repeated != null && paths != null) {
+ public Builder addBeforeEachPath(String repeated, @Nullable Iterable<PathFragment> paths) {
+ Preconditions.checkNotNull(repeated);
+ if (paths != null && !Iterables.isEmpty(paths)) {
arguments.add(InterspersingArgs.fromStrings(paths, repeated, /*formatEach=*/ null));
}
return this;
}
- public Builder addBeforeEach(String repeated, Iterable<String> strings) {
- if (repeated != null && strings != null) {
+ public Builder addBeforeEach(String repeated, @Nullable Iterable<String> strings) {
+ Preconditions.checkNotNull(repeated);
+ if (strings != null && !Iterables.isEmpty(strings)) {
arguments.add(InterspersingArgs.fromStrings(strings, repeated, /*formatEach=*/ null));
}
return this;
}
- public Builder addBeforeEachExecPath(String repeated, Iterable<Artifact> artifacts) {
- if (repeated != null && artifacts != null) {
+ public Builder addBeforeEachExecPath(String repeated, @Nullable Iterable<Artifact> artifacts) {
+ Preconditions.checkNotNull(repeated);
+ if (artifacts != null && !Iterables.isEmpty(artifacts)) {
arguments.add(InterspersingArgs.fromExecPaths(artifacts, repeated, /*formatEach=*/ null));
}
return this;
}
- public Builder addFormatEach(String format, Iterable<String> strings) {
- if (format != null && strings != null) {
+ public Builder addFormatEach(String format, @Nullable Iterable<String> strings) {
+ Preconditions.checkNotNull(format);
+ if (strings != null && !Iterables.isEmpty(strings)) {
arguments.add(InterspersingArgs.fromStrings(strings, /*beforeEach=*/null, format));
}
return this;
}
- public Builder add(CustomArgv arg) {
+ public Builder add(@Nullable CustomArgv arg) {
if (arg != null) {
arguments.add(arg);
}
return this;
}
- public Builder add(CustomMultiArgv arg) {
+ public Builder add(@Nullable CustomMultiArgv arg) {
if (arg != null) {
arguments.add(arg);
}
@@ -683,13 +578,12 @@ public final class CustomCommandLine extends CommandLine {
private final ImmutableList<Object> arguments;
-
/**
* A map between enclosed TreeArtifacts and their associated {@link TreeFileArtifacts} for
* substitution.
*
- * <p> This map is used to support TreeArtifact substitutions in
- * {@link TreeFileArtifactArgvFragment}s.
+ * <p>This map is used to support TreeArtifact substitutions in {@link
+ * TreeFileArtifactExecPathArg}s.
*/
private final Map<Artifact, TreeFileArtifact> substitutionMap;
@@ -706,8 +600,8 @@ public final class CustomCommandLine extends CommandLine {
/**
* Given the list of {@link TreeFileArtifact}s, returns another CustomCommandLine that replaces
- * their parent TreeArtifacts with the TreeFileArtifacts in all
- * {@link TreeFileArtifactArgvFragment} argument objects.
+ * their parent TreeArtifacts with the TreeFileArtifacts in all {@link
+ * TreeFileArtifactExecPathArg} argument objects.
*/
@VisibleForTesting
public CustomCommandLine evaluateTreeFileArtifacts(Iterable<TreeFileArtifact> treeFileArtifacts) {
@@ -750,14 +644,14 @@ public final class CustomCommandLine extends CommandLine {
}
/**
- * If the given arg is a {@link TreeFileArtifactArgvFragment} and we have its associated
+ * If the given arg is a {@link TreeFileArtifactExecPathArg} and we have its associated
* TreeArtifact substitution map, returns another argument object that has its enclosing
* TreeArtifact substituted by one of its {@link TreeFileArtifact}. Otherwise, returns the given
* arg unmodified.
*/
private Object substituteTreeFileArtifactArgvFragment(Object arg) {
- if (arg instanceof TreeFileArtifactArgvFragment) {
- TreeFileArtifactArgvFragment argvFragment = (TreeFileArtifactArgvFragment) arg;
+ if (arg instanceof TreeFileArtifactExecPathArg) {
+ TreeFileArtifactExecPathArg argvFragment = (TreeFileArtifactExecPathArg) arg;
return argvFragment.substituteTreeArtifact(
Preconditions.checkNotNull(substitutionMap, argvFragment));
} else {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/ParamFileHelper.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/ParamFileHelper.java
index 7a3e16b207..d75d31300e 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/ParamFileHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/ParamFileHelper.java
@@ -91,6 +91,9 @@ public final class ParamFileHelper {
List<String> executableArgs, ParamFileInfo paramFileInfo, Artifact parameterFile) {
return CustomCommandLine.builder()
.add(executableArgs)
+ // Bazel creates a lot of parameter files, so use a param-file-specific adder method instead
+ // of `addPaths`, which would work too but create extra String garbage:
+ // .addPaths(paramFileInfo.getFlag() + "%s", parameterFile)
.addParamFile(paramFileInfo.getFlag(), parameterFile)
.build();
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java
index a77e25094c..3f8e47fc48 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java
@@ -240,12 +240,14 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor
CustomCommandLine.Builder cmdLineArgs = CustomCommandLine.builder();
if (!transitiveAars.isEmpty()) {
- cmdLineArgs.addJoinValues(
- "--android_libraries", ",", transitiveAars, AndroidLocalTestBase::aarCmdLineArg);
+ cmdLineArgs
+ .add("--android_libraries")
+ .addJoinValues(",", transitiveAars, AndroidLocalTestBase::aarCmdLineArg);
}
if (!strictAars.isEmpty()) {
- cmdLineArgs.addJoinValues(
- "--strict_libraries", ",", strictAars, AndroidLocalTestBase::aarCmdLineArg);
+ cmdLineArgs
+ .add("--strict_libraries")
+ .addJoinValues(",", strictAars, AndroidLocalTestBase::aarCmdLineArg);
}
RunfilesSupport runfilesSupport =
RunfilesSupport.withExecutable(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java
index fd827b6962..4589c93dd1 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java
@@ -427,16 +427,16 @@ public class AndroidResourcesProcessorBuilder {
}
ImmutableList<String> filteredResources = resourceFilter.getFilteredResources();
if (!filteredResources.isEmpty()) {
- builder.addJoinStrings("--prefilteredResources", ",", filteredResources);
+ builder.add("--prefilteredResources").addJoinStrings(",", filteredResources);
}
if (!uncompressedExtensions.isEmpty()) {
- builder.addJoinStrings("--uncompressedExtensions", ",", uncompressedExtensions);
+ builder.add("--uncompressedExtensions").addJoinStrings(",", uncompressedExtensions);
}
if (!crunchPng) {
builder.add("--useAaptCruncher=no");
}
if (!assetsToIgnore.isEmpty()) {
- builder.addJoinStrings("--assetsToIgnore", ",", assetsToIgnore);
+ builder.add("--assetsToIgnore").addJoinStrings(",", assetsToIgnore);
}
if (debug) {
builder.add("--debug");
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/LibraryRGeneratorActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/LibraryRGeneratorActionBuilder.java
index e4baef192a..52cab02505 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/LibraryRGeneratorActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/LibraryRGeneratorActionBuilder.java
@@ -13,7 +13,6 @@
// limitations under the License.
package com.google.devtools.build.lib.rules.android;
-import com.google.common.base.Function;
import com.google.common.base.Strings;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
@@ -29,10 +28,6 @@ import com.google.devtools.build.lib.collect.nestedset.Order;
/** Builder for the action that generates the R class for libraries. */
public class LibraryRGeneratorActionBuilder {
- static final Function<ResourceContainer, Artifact> TO_SYMBOL_ARTIFACT =
- ResourceContainer::getSymbols;
- static final Function<ResourceContainer, String> TO_SYMBOL_PATH =
- (ResourceContainer container) -> container.getSymbols().getExecPathString();
private String javaPackage;
private Iterable<ResourceContainer> deps = ImmutableList.<ResourceContainer>of();
@@ -74,20 +69,26 @@ public class LibraryRGeneratorActionBuilder {
builder.add("--packageForR").add(javaPackage);
}
- FluentIterable<ResourceContainer> symbolProviders =
- FluentIterable.from(deps).append(resourceContainer);
-
- builder.addJoinStrings(
- "--symbols",
- ruleContext.getConfiguration().getHostPathSeparator(),
- symbolProviders.transform(TO_SYMBOL_PATH));
- inputs.addTransitive(
- NestedSetBuilder.wrap(
- Order.NAIVE_LINK_ORDER, symbolProviders.transform(TO_SYMBOL_ARTIFACT)));
-
- builder.addExecPath("--classJarOutput", rJavaClassJar);
+ // Memory consumption consideration: normally we'd not convert this FluentIterable to a list, to
+ // keep a potentially quadratic memory usage linear. However, since `symbolProviders` is wrapped
+ // in a NestedSet a few lines below, and NestedSetBuilder.wrap calls ImmutableList.copyOf on its
+ // argument, we're not using extra memory.
+ ImmutableList<Artifact> symbolProviders =
+ FluentIterable.from(deps)
+ .append(resourceContainer)
+ .transform(ResourceContainer::getSymbols)
+ .toList();
+
+ if (!symbolProviders.isEmpty()) {
+ builder
+ .add("--symbols")
+ .addJoinExecPaths(ruleContext.getConfiguration().getHostPathSeparator(), symbolProviders);
+ }
+ inputs.addTransitive(NestedSetBuilder.wrap(Order.NAIVE_LINK_ORDER, symbolProviders));
- builder.addExecPath("--androidJar", sdk.getAndroidJar());
+ builder
+ .addExecPath("--classJarOutput", rJavaClassJar)
+ .addExecPath("--androidJar", sdk.getAndroidJar());
inputs.add(sdk.getAndroidJar());
// Create the spawn action.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java
index 12215da639..3cbb11b841 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java
@@ -101,8 +101,9 @@ public class RClassGeneratorActionBuilder {
// TODO(corysmith): Remove NestedSet as we are already flattening it.
Iterable<ResourceContainer> depResources = dependencies.getResources();
if (depResources.iterator().hasNext()) {
- builder.addJoinStrings(
- "--libraries", ",", Iterables.transform(depResources, chooseDepsToArg(version)));
+ builder
+ .add("--libraries")
+ .addJoinStrings(",", Iterables.transform(depResources, chooseDepsToArg(version)));
inputs.addTransitive(
NestedSetBuilder.wrap(
Order.NAIVE_LINK_ORDER,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainerConverter.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainerConverter.java
index 98393b2045..e980b9995f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainerConverter.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainerConverter.java
@@ -240,12 +240,14 @@ public class ResourceContainerConverter {
if (dependencies != null) {
if (!dependencies.getTransitiveResources().isEmpty()) {
- cmdBuilder.addJoinValues(
- "--data", toArg.listSeparator(), dependencies.getTransitiveResources(), toArg);
+ cmdBuilder
+ .add("--data")
+ .addJoinValues(toArg.listSeparator(), dependencies.getTransitiveResources(), toArg);
}
if (!dependencies.getDirectResources().isEmpty()) {
- cmdBuilder.addJoinValues(
- "--directData", toArg.listSeparator(), dependencies.getDirectResources(), toArg);
+ cmdBuilder
+ .add("--directData")
+ .addJoinValues(toArg.listSeparator(), dependencies.getDirectResources(), toArg);
}
// This flattens the nested set. Since each ResourceContainer needs to be transformed into
// Artifacts, and the NestedSetBuilder.wrap doesn't support lazy Iterator evaluation
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceShrinkerActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceShrinkerActionBuilder.java
index 60d414ddc4..ef077eb5dc 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceShrinkerActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceShrinkerActionBuilder.java
@@ -165,10 +165,10 @@ public class ResourceShrinkerActionBuilder {
inputs.add(sdk.getAndroidJar());
if (!uncompressedExtensions.isEmpty()) {
- commandLine.addJoinStrings("--uncompressedExtensions", ",", uncompressedExtensions);
+ commandLine.add("--uncompressedExtensions").addJoinStrings(",", uncompressedExtensions);
}
if (!assetsToIgnore.isEmpty()) {
- commandLine.addJoinStrings("--assetsToIgnore", ",", assetsToIgnore);
+ commandLine.add("--assetsToIgnore").addJoinStrings(",", assetsToIgnore);
}
if (ruleContext.getConfiguration().getCompilationMode() != CompilationMode.OPT) {
commandLine.add("--debug");
@@ -201,14 +201,18 @@ public class ResourceShrinkerActionBuilder {
inputs.add(primaryResources.getManifest());
List<Artifact> dependencyManifests = getManifests(dependencyResources);
- commandLine.addJoinExecPaths(
- "--dependencyManifests",
- ruleContext.getConfiguration().getHostPathSeparator(),
- dependencyManifests);
+ if (!dependencyManifests.isEmpty()) {
+ commandLine
+ .add("--dependencyManifests")
+ .addJoinExecPaths(
+ ruleContext.getConfiguration().getHostPathSeparator(), dependencyManifests);
+ }
inputs.addAll(dependencyManifests);
List<String> resourcePackages = getResourcePackages(primaryResources, dependencyResources);
- commandLine.addJoinStrings("--resourcePackages", ",", resourcePackages);
+ if (!resourcePackages.isEmpty()) {
+ commandLine.add("--resourcePackages").addJoinStrings(",", resourcePackages);
+ }
commandLine.addExecPath("--shrunkResourceApk", resourceApkOut);
outputs.add(resourceApkOut);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/RobolectricResourceSymbolsActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/RobolectricResourceSymbolsActionBuilder.java
index 20fe4104fd..23b52fd94a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/RobolectricResourceSymbolsActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/RobolectricResourceSymbolsActionBuilder.java
@@ -84,11 +84,12 @@ public class RobolectricResourceSymbolsActionBuilder {
inputs.add(sdk.getAndroidJar());
if (!Iterables.isEmpty(dependencies.getResources())) {
- builder.addJoinValues(
- "--data",
- RESOURCE_CONTAINER_TO_ARG.listSeparator(),
- dependencies.getResources(),
- RESOURCE_CONTAINER_TO_ARG);
+ builder
+ .add("--data")
+ .addJoinValues(
+ RESOURCE_CONTAINER_TO_ARG.listSeparator(),
+ dependencies.getResources(),
+ RESOURCE_CONTAINER_TO_ARG);
}
// This flattens the nested set.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/DeployArchiveBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/java/DeployArchiveBuilder.java
index 5aa4340af5..b87666a527 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/DeployArchiveBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/DeployArchiveBuilder.java
@@ -176,13 +176,11 @@ public class DeployArchiveBuilder {
}
args.add("--normalize");
if (javaMainClass != null) {
- args.add("--main_class");
- args.add(javaMainClass);
+ args.add("--main_class").add(javaMainClass);
}
if (!deployManifestLines.isEmpty()) {
- args.add("--deploy_manifest_lines");
- args.add(deployManifestLines);
+ args.add("--deploy_manifest_lines").add(deployManifestLines);
}
if (buildInfoFiles != null) {
@@ -194,12 +192,15 @@ public class DeployArchiveBuilder {
args.add("--exclude_build_data");
}
if (launcher != null) {
- args.add("--java_launcher");
- args.add(launcher.getExecPathString());
+ args.add("--java_launcher").add(launcher.getExecPathString());
}
- args.addExecPaths("--classpath_resources", classpathResources);
- args.addExecPaths("--sources", runtimeClasspath);
+ if (!classpathResources.isEmpty()) {
+ args.add("--classpath_resources").addExecPaths(classpathResources);
+ }
+ if (!Iterables.isEmpty(runtimeClasspath)) {
+ args.add("--sources").addExecPaths(runtimeClasspath);
+ }
return args;
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
index b45cb28a0e..0fbe097fcf 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
@@ -429,8 +429,8 @@ public final class JavaCompileAction extends SpawnAction {
CustomCommandLine.builder().addPath(javaExecutable).add(javaBuilderJvmFlags);
if (!instrumentationJars.isEmpty()) {
builder
+ .add("-cp")
.addJoinExecPaths(
- "-cp",
pathDelimiter,
Iterables.concat(instrumentationJars, ImmutableList.of(javaBuilderJar)))
.add(javaBuilderMainClass);
@@ -679,35 +679,34 @@ public final class JavaCompileAction extends SpawnAction {
result.addExecPath("--output_deps_proto", outputDepsProto);
}
if (!extdirInputs.isEmpty()) {
- result.addExecPaths("--extclasspath", extdirInputs);
+ result.add("--extclasspath").addExecPaths(extdirInputs);
}
if (!bootclasspathEntries.isEmpty()) {
- result.addExecPaths("--bootclasspath", bootclasspathEntries);
+ result.add("--bootclasspath").addExecPaths(bootclasspathEntries);
}
if (!sourcePathEntries.isEmpty()) {
- result.addExecPaths("--sourcepath", sourcePathEntries);
+ result.add("--sourcepath").addExecPaths(sourcePathEntries);
}
if (!processorPath.isEmpty()) {
- result.addExecPaths("--processorpath", processorPath);
+ result.add("--processorpath").addExecPaths(processorPath);
}
if (!processorNames.isEmpty()) {
- result.add("--processors", processorNames);
+ result.add("--processors").add(processorNames);
}
if (!processorFlags.isEmpty()) {
- result.add("--javacopts", processorFlags);
+ result.add("--javacopts").add(processorFlags);
}
if (!sourceJars.isEmpty()) {
- result.addExecPaths("--source_jars", sourceJars);
+ result.add("--source_jars").addExecPaths(sourceJars);
}
if (!sourceFiles.isEmpty()) {
- result.addExecPaths("--sources", sourceFiles);
+ result.add("--sources").addExecPaths(sourceFiles);
}
if (!javacOpts.isEmpty()) {
- result.add("--javacopts", javacOpts);
+ result.add("--javacopts").add(javacOpts);
}
if (ruleKind != null) {
- result.add("--rule_kind");
- result.add(ruleKind);
+ result.add("--rule_kind").add(ruleKind);
}
if (targetLabel != null) {
result.add("--target_label");
@@ -725,34 +724,37 @@ public final class JavaCompileAction extends SpawnAction {
}
if (!classpathEntries.isEmpty()) {
- result.addExecPaths("--classpath", classpathEntries);
+ result.add("--classpath").addExecPaths(classpathEntries);
}
// strict_java_deps controls whether the mapping from jars to targets is
// written out and whether we try to minimize the compile-time classpath.
if (strictJavaDeps != BuildConfiguration.StrictDepsMode.OFF) {
- result.add("--strict_java_deps");
- result.add(strictJavaDeps.toString());
- result.add(new JarsToTargetsArgv(classpathEntries, directJars));
+ result
+ .add("--strict_java_deps")
+ .add(strictJavaDeps.toString())
+ .add(new JarsToTargetsArgv(classpathEntries, directJars));
if (configuration.getFragment(JavaConfiguration.class).getReduceJavaClasspath()
== JavaClasspathMode.JAVABUILDER) {
result.add("--reduce_classpath");
if (!compileTimeDependencyArtifacts.isEmpty()) {
- result.addExecPaths("--deps_artifacts", compileTimeDependencyArtifacts);
+ result.add("--deps_artifacts").addExecPaths(compileTimeDependencyArtifacts);
}
}
}
if (metadata != null) {
- result.add("--post_processor");
- result.addExecPath(JACOCO_INSTRUMENTATION_PROCESSOR, metadata);
- result.addPath(
- configuration
- .getCoverageMetadataDirectory(targetLabel.getPackageIdentifier().getRepository())
- .getExecPath());
- result.add("-*Test");
- result.add("-*TestCase");
+ result
+ .add("--post_processor")
+ .addExecPath(JACOCO_INSTRUMENTATION_PROCESSOR, metadata)
+ .addPath(
+ configuration
+ .getCoverageMetadataDirectory(
+ targetLabel.getPackageIdentifier().getRepository())
+ .getExecPath())
+ .add("-*Test")
+ .add("-*TestCase");
}
return result.build();
}
@@ -960,7 +962,7 @@ public final class JavaCompileAction extends SpawnAction {
this.targetLabel = targetLabel;
return this;
}
-
+
public Builder setTestOnly(boolean testOnly) {
this.testOnly = testOnly;
return this;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java
index 1bd31013bc..046149453f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java
@@ -23,6 +23,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.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionInput;
@@ -502,19 +503,24 @@ public class JavaHeaderCompileAction extends SpawnAction {
result.add("--temp_dir").addPath(tempDirectory);
- result.addExecPaths("--bootclasspath", bootclasspathEntries);
+ if (!Iterables.isEmpty(bootclasspathEntries)) {
+ result.add("--bootclasspath").addExecPaths(bootclasspathEntries);
+ }
- result.addExecPaths("--sources", sourceFiles);
+ if (!sourceFiles.isEmpty()) {
+ result.add("--sources").addExecPaths(sourceFiles);
+ }
if (!sourceJars.isEmpty()) {
- result.addExecPaths("--source_jars", sourceJars);
+ result.add("--source_jars").addExecPaths(sourceJars);
}
- result.add("--javacopts", javacOpts);
+ if (!javacOpts.isEmpty()) {
+ result.add("--javacopts").add(javacOpts);
+ }
if (ruleKind != null) {
- result.add("--rule_kind");
- result.add(ruleKind);
+ result.add("--rule_kind").add(ruleKind);
}
if (targetLabel != null) {
result.add("--target_label");
@@ -527,7 +533,9 @@ public class JavaHeaderCompileAction extends SpawnAction {
result.add("@" + targetLabel);
}
}
- result.addExecPaths("--classpath", classpathEntries);
+ if (!classpathEntries.isEmpty()) {
+ result.add("--classpath").addExecPaths(classpathEntries);
+ }
return result;
}
@@ -536,18 +544,18 @@ public class JavaHeaderCompileAction extends SpawnAction {
CustomCommandLine.Builder result = CustomCommandLine.builder();
baseCommandLine(result, classpathEntries);
if (!processorNames.isEmpty()) {
- result.add("--processors", processorNames);
+ result.add("--processors").add(processorNames);
}
if (!processorFlags.isEmpty()) {
- result.add("--javacopts", processorFlags);
+ result.add("--javacopts").add(processorFlags);
}
if (!processorPath.isEmpty()) {
- result.addExecPaths("--processorpath", processorPath);
+ result.add("--processorpath").addExecPaths(processorPath);
}
if (strictJavaDeps != BuildConfiguration.StrictDepsMode.OFF) {
result.add(new JavaCompileAction.JarsToTargetsArgv(classpathEntries, directJars));
if (!compileTimeDependencyArtifacts.isEmpty()) {
- result.addExecPaths("--deps_artifacts", compileTimeDependencyArtifacts);
+ result.add("--deps_artifacts").addExecPaths(compileTimeDependencyArtifacts);
}
}
return result.build();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/ResourceJarActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/java/ResourceJarActionBuilder.java
index eaa06555fb..dbac08d772 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/ResourceJarActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/ResourceJarActionBuilder.java
@@ -106,7 +106,7 @@ public class ResourceJarActionBuilder {
.add("--exclude_build_data")
.addExecPath("--output", outputJar);
if (!resourceJars.isEmpty()) {
- command.addExecPaths("--sources", resourceJars);
+ command.add("--sources").addExecPaths(resourceJars);
}
if (!resources.isEmpty() || !messages.isEmpty()) {
command.add("--resources");
@@ -119,7 +119,7 @@ public class ResourceJarActionBuilder {
}
}
if (!classpathResources.isEmpty()) {
- command.addExecPaths("--classpath_resources", classpathResources);
+ command.add("--classpath_resources").addExecPaths(classpathResources);
}
// TODO(b/37444705): remove this logic and always call useParameterFile once the bug is fixed
// Most resource jar actions are very small and expanding the argument list for
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/SingleJarActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/java/SingleJarActionBuilder.java
index 578f94b6e4..8d5b5caee7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/SingleJarActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/SingleJarActionBuilder.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.rules.java;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ExecutionRequirements;
import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType;
@@ -94,7 +95,9 @@ public final class SingleJarActionBuilder {
CustomCommandLine.Builder args = CustomCommandLine.builder();
args.addExecPath("--output", outputJar);
args.add(SOURCE_JAR_COMMAND_LINE_ARGS);
- args.addExecPaths("--sources", resourceJars);
+ if (!Iterables.isEmpty(resourceJars)) {
+ args.add("--sources").addExecPaths(resourceJars);
+ }
if (!resources.isEmpty()) {
args.add("--resources");
for (Map.Entry<PathFragment, Artifact> resource : resources.entrySet()) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
index 39ed3e564c..81126eab69 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
@@ -34,7 +34,6 @@ import static java.nio.charset.StandardCharsets.ISO_8859_1;
import static java.util.stream.Collectors.toCollection;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Joiner;
import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
@@ -1099,25 +1098,34 @@ public abstract class CompilationSupport {
.getPrerequisite("$dummy_lib", Mode.TARGET, ObjcProvider.class)
.get(LIBRARY));
- CustomCommandLine commandLine =
+ CustomCommandLine.Builder commandLineBuilder =
CustomCommandLine.builder()
.addExecPath("--input_archive", j2objcArchive)
.addExecPath("--output_archive", prunedJ2ObjcArchive)
.addExecPath("--dummy_archive", dummyArchive)
- .addExecPath("--xcrunwrapper", xcrunwrapper(ruleContext).getExecutable())
- .addJoinExecPaths("--dependency_mapping_files", ",", j2ObjcDependencyMappingFiles)
- .addJoinExecPaths("--header_mapping_files", ",", j2ObjcHeaderMappingFiles)
- .addJoinExecPaths(
- "--archive_source_mapping_files", ",", j2ObjcArchiveSourceMappingFiles)
- .add("--entry_classes")
- .add(Joiner.on(",").join(entryClasses))
- .build();
+ .addExecPath("--xcrunwrapper", xcrunwrapper(ruleContext).getExecutable());
+ if (!j2ObjcDependencyMappingFiles.isEmpty()) {
+ commandLineBuilder
+ .add("--dependency_mapping_files")
+ .addJoinExecPaths(",", j2ObjcDependencyMappingFiles);
+ }
+ if (!j2ObjcHeaderMappingFiles.isEmpty()) {
+ commandLineBuilder
+ .add("--header_mapping_files")
+ .addJoinExecPaths(",", j2ObjcHeaderMappingFiles);
+ }
+ if (!j2ObjcArchiveSourceMappingFiles.isEmpty()) {
+ commandLineBuilder
+ .add("--archive_source_mapping_files")
+ .addJoinExecPaths(",", j2ObjcArchiveSourceMappingFiles);
+ }
+ commandLineBuilder.add("--entry_classes").addJoinStrings(",", entryClasses);
ruleContext.registerAction(
new ParameterFileWriteAction(
ruleContext.getActionOwner(),
paramFile,
- commandLine,
+ commandLineBuilder.build(),
ParameterFile.ParameterFileType.UNQUOTED,
ISO_8859_1));
ruleContext.registerAction(
@@ -1139,7 +1147,7 @@ public abstract class CompilationSupport {
.build(ruleContext));
}
}
-
+
/** Returns archives arising from j2objc transpilation after dead code removal. */
protected Iterable<Artifact> computeAndStripPrunedJ2ObjcArchives(
J2ObjcEntryClassProvider j2ObjcEntryClassProvider,
@@ -1421,9 +1429,7 @@ public abstract class CompilationSupport {
.add(appleConfiguration.getXcodeVersion().toStringWithMinimumComponents(2))
.add("--");
for (ObjcHeaderThinningInfo info : infos) {
- cmdLine.addJoinPaths(
- ":",
- Lists.newArrayList(info.sourceFile.getExecPath(), info.headersListFile.getExecPath()));
+ cmdLine.addJoinExecPaths(":", ImmutableList.of(info.sourceFile, info.headersListFile));
builder.addInput(info.sourceFile).addOutput(info.headersListFile);
}
ruleContext.registerAction(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java
index 60a24514e0..f2cf27fe0b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java
@@ -492,8 +492,10 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF
ImmutableList.Builder<Artifact> sourceJarOutputFiles = ImmutableList.builder();
if (!Iterables.isEmpty(sourceJars)) {
sourceJarOutputFiles.addAll(sourceJarOutputs(ruleContext));
- argBuilder.addJoinExecPaths("--src_jars", ",", sourceJars);
- argBuilder.add(sourceJarFlags(ruleContext));
+ argBuilder
+ .add("--src_jars")
+ .addJoinExecPaths(",", sourceJars)
+ .add(sourceJarFlags(ruleContext));
}
Iterable<String> translationFlags = ruleContext
@@ -504,7 +506,7 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF
NestedSet<Artifact> depsHeaderMappingFiles =
depJ2ObjcMappingFileProvider.getHeaderMappingFiles();
if (!depsHeaderMappingFiles.isEmpty()) {
- argBuilder.addJoinExecPaths("--header-mapping", ",", depsHeaderMappingFiles);
+ argBuilder.add("--header-mapping").addJoinExecPaths(",", depsHeaderMappingFiles);
}
boolean experimentalJ2ObjcHeaderMap =
@@ -516,7 +518,7 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF
NestedSet<Artifact> depsClassMappingFiles = depJ2ObjcMappingFileProvider.getClassMappingFiles();
if (!depsClassMappingFiles.isEmpty()) {
- argBuilder.addJoinExecPaths("--mapping", ",", depsClassMappingFiles);
+ argBuilder.add("--mapping").addJoinExecPaths(",", depsClassMappingFiles);
}
Artifact archiveSourceMappingFile = j2ObjcOutputArchiveSourceMappingFile(ruleContext);
@@ -538,7 +540,7 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF
NestedSet<Artifact> compileTimeJars =
compArgsProvider.getRecursiveJavaCompilationArgs().getCompileTimeJars();
if (!compileTimeJars.isEmpty()) {
- argBuilder.addJoinExecPaths("-classpath", ":", compileTimeJars);
+ argBuilder.add("-classpath").addJoinExecPaths(":", compileTimeJars);
}
argBuilder.addExecPaths(sources);
@@ -584,10 +586,10 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF
if (experimentalJ2ObjcHeaderMap) {
CustomCommandLine.Builder headerMapCommandLine = CustomCommandLine.builder();
if (!Iterables.isEmpty(sources)) {
- headerMapCommandLine.addJoinExecPaths("--source_files", ",", sources);
+ headerMapCommandLine.add("--source_files").addJoinExecPaths(",", sources);
}
if (!Iterables.isEmpty(sourceJars)) {
- headerMapCommandLine.addJoinExecPaths("--source_jars", ",", sourceJars);
+ headerMapCommandLine.add("--source_jars").addJoinExecPaths(",", sourceJars);
}
headerMapCommandLine.addExecPath("--output_mapping_file", outputHeaderMappingFile);
ruleContext.registerAction(new SpawnAction.Builder()
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/LipoSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/LipoSupport.java
index 41487ecc43..bad646175a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/LipoSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/LipoSupport.java
@@ -29,7 +29,7 @@ import com.google.devtools.build.lib.rules.apple.ApplePlatform;
*/
public class LipoSupport {
private final RuleContext ruleContext;
-
+
public LipoSupport(RuleContext ruleContext) {
this.ruleContext = ruleContext;
}
@@ -46,18 +46,21 @@ public class LipoSupport {
public LipoSupport registerCombineArchitecturesAction(
NestedSet<Artifact> inputBinaries, Artifact outputBinary, ApplePlatform platform) {
if (inputBinaries.toList().size() > 1) {
- ruleContext.registerAction(ObjcRuleClasses.spawnAppleEnvActionBuilder(
- ruleContext.getFragment(AppleConfiguration.class), platform)
- .setMnemonic("ObjcCombiningArchitectures")
- .addTransitiveInputs(inputBinaries)
- .addOutput(outputBinary)
- .setExecutable(CompilationSupport.xcrunwrapper(ruleContext))
- .setCommandLine(CustomCommandLine.builder()
- .add(ObjcRuleClasses.LIPO)
- .addExecPaths("-create", inputBinaries)
- .addExecPath("-o", outputBinary)
- .build())
- .build(ruleContext));
+ ruleContext.registerAction(
+ ObjcRuleClasses.spawnAppleEnvActionBuilder(
+ ruleContext.getFragment(AppleConfiguration.class), platform)
+ .setMnemonic("ObjcCombiningArchitectures")
+ .addTransitiveInputs(inputBinaries)
+ .addOutput(outputBinary)
+ .setExecutable(CompilationSupport.xcrunwrapper(ruleContext))
+ .setCommandLine(
+ CustomCommandLine.builder()
+ .add(ObjcRuleClasses.LIPO)
+ .add("-create")
+ .addExecPaths(inputBinaries)
+ .addExecPath("-o", outputBinary)
+ .build())
+ .build(ruleContext));
} else {
ruleContext.registerAction(new SymlinkAction(
ruleContext.getActionOwner(),
diff --git a/src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java b/src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java
index ea061beb2d..f7995b42d3 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java
@@ -18,6 +18,7 @@ import static org.junit.Assert.fail;
import com.google.common.base.Function;
import com.google.common.collect.ImmutableList;
+import com.google.common.testing.NullPointerTester;
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
@@ -32,7 +33,6 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.testutil.Scratch;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.Collection;
-import javax.annotation.Nullable;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -71,15 +71,18 @@ public class CustomCommandLineTest {
@Test
public void testStringsArgs() {
- CustomCommandLine cl = CustomCommandLine.builder().add("--arg",
- ImmutableList.of("a", "b")).build();
+ CustomCommandLine cl =
+ CustomCommandLine.builder().add("--arg").add(ImmutableList.of("a", "b")).build();
assertThat(cl.arguments()).isEqualTo(ImmutableList.of("--arg", "a", "b"));
}
@Test
public void testArtifactJoinStringArgs() {
- CustomCommandLine cl = CustomCommandLine.builder().addJoinStrings("--path", ":",
- ImmutableList.of("foo", "bar")).build();
+ CustomCommandLine cl =
+ CustomCommandLine.builder()
+ .add("--path")
+ .addJoinStrings(":", ImmutableList.of("foo", "bar"))
+ .build();
assertThat(cl.arguments()).isEqualTo(ImmutableList.of("--path", "foo:bar"));
}
@@ -87,17 +90,8 @@ public class CustomCommandLineTest {
public void testJoinValues() {
CustomCommandLine cl =
CustomCommandLine.builder()
- .addJoinValues(
- "--path",
- ":",
- ImmutableList.of("foo", "bar", "baz"),
- new Function<String, String>() {
- @Nullable
- @Override
- public String apply(@Nullable String s) {
- return s.toUpperCase();
- }
- })
+ .add("--path")
+ .addJoinValues(":", ImmutableList.of("foo", "bar", "baz"), String::toUpperCase)
.build();
assertThat(cl.arguments()).isEqualTo(ImmutableList.of("--path", "FOO:BAR:BAZ"));
}
@@ -110,8 +104,11 @@ public class CustomCommandLineTest {
@Test
public void testArtifactExecPathsArgs() {
- CustomCommandLine cl = CustomCommandLine.builder().addExecPaths("--path",
- ImmutableList.of(artifact1, artifact2)).build();
+ CustomCommandLine cl =
+ CustomCommandLine.builder()
+ .add("--path")
+ .addExecPaths(ImmutableList.of(artifact1, artifact2))
+ .build();
assertThat(cl.arguments())
.isEqualTo(ImmutableList.of("--path", "dir/file1.txt", "dir/file2.txt"));
}
@@ -125,8 +122,11 @@ public class CustomCommandLineTest {
@Test
public void testArtifactJoinExecPathArgs() {
- CustomCommandLine cl = CustomCommandLine.builder().addJoinExecPaths("--path", ":",
- ImmutableList.of(artifact1, artifact2)).build();
+ CustomCommandLine cl =
+ CustomCommandLine.builder()
+ .add("--path")
+ .addJoinExecPaths(":", ImmutableList.of(artifact1, artifact2))
+ .build();
assertThat(cl.arguments()).isEqualTo(ImmutableList.of("--path", "dir/file1.txt:dir/file2.txt"));
}
@@ -137,13 +137,6 @@ public class CustomCommandLineTest {
}
@Test
- public void testJoinPathArgs() {
- CustomCommandLine cl = CustomCommandLine.builder().addJoinPaths(":",
- ImmutableList.of(artifact1.getExecPath(), artifact2.getExecPath())).build();
- assertThat(cl.arguments()).isEqualTo(ImmutableList.of("dir/file1.txt:dir/file2.txt"));
- }
-
- @Test
public void testPathsArgs() {
CustomCommandLine cl = CustomCommandLine.builder().addPaths("%s:%s",
artifact1.getExecPath(), artifact1.getRootRelativePath()).build();
@@ -174,12 +167,15 @@ public class CustomCommandLineTest {
@Test
public void testCombinedArgs() {
- CustomCommandLine cl = CustomCommandLine.builder()
- .add("--arg")
- .add("--args", ImmutableList.of("abc"))
- .addExecPaths("--path1", ImmutableList.of(artifact1))
- .addExecPath("--path2", artifact2)
- .build();
+ CustomCommandLine cl =
+ CustomCommandLine.builder()
+ .add("--arg")
+ .add("--args")
+ .add(ImmutableList.of("abc"))
+ .add("--path1")
+ .addExecPaths(ImmutableList.of(artifact1))
+ .addExecPath("--path2", artifact2)
+ .build();
assertThat(cl.arguments())
.isEqualTo(
ImmutableList.of(
@@ -187,13 +183,73 @@ public class CustomCommandLineTest {
}
@Test
- public void testAddNulls() {
- CustomCommandLine cl = CustomCommandLine.builder()
- .add("--args", null)
- .addExecPaths(null, ImmutableList.of(artifact1))
- .addExecPath(null, null)
- .build();
+ public void testAddNulls() throws Exception {
+ Artifact treeArtifact = createTreeArtifact("myTreeArtifact");
+ assertThat(treeArtifact).isNotNull();
+
+ CustomCommandLine cl =
+ CustomCommandLine.builder()
+ .add((CharSequence) null)
+ .add((Label) null)
+ .add((Iterable<String>) null)
+ .add(ImmutableList.<String>of())
+ .addExecPaths(null)
+ .addExecPaths(ImmutableList.of())
+ .addExecPath("foo", null)
+ .addPlaceholderTreeArtifactExecPath(null)
+ .addJoinStrings("foo", null)
+ .addJoinStrings("foo", ImmutableList.of())
+ .addJoinValues("foo", null, String::toString)
+ .addJoinValues("foo", ImmutableList.of(), String::toString)
+ .addJoinExecPaths("foo", null)
+ .addJoinExecPaths("foo", ImmutableList.of())
+ .addPath(null)
+ .addPaths("foo")
+ .addPaths("foo", (PathFragment[]) null)
+ .addPaths("foo", new PathFragment[0])
+ .addBeforeEachPath("foo", null)
+ .addBeforeEachPath("foo", ImmutableList.of())
+ .addBeforeEach("foo", null)
+ .addBeforeEach("foo", ImmutableList.of())
+ .addBeforeEachExecPath("foo", null)
+ .addBeforeEachExecPath("foo", ImmutableList.of())
+ .addFormatEach("%s", null)
+ .addFormatEach("%s", ImmutableList.of())
+ .add((CustomArgv) null)
+ .add((CustomMultiArgv) null)
+ .build();
assertThat(cl.arguments()).isEqualTo(ImmutableList.of());
+
+ CustomCommandLine.Builder obj = CustomCommandLine.builder();
+ Class<CustomCommandLine.Builder> clazz = CustomCommandLine.Builder.class;
+ NullPointerTester npt =
+ new NullPointerTester()
+ .setDefault(Artifact.class, artifact1)
+ .setDefault(String.class, "foo")
+ .setDefault(PathFragment[].class, new PathFragment[] {PathFragment.create("foo")});
+
+ npt.testMethod(obj, clazz.getMethod("addExecPath", String.class, Artifact.class));
+ npt.testMethod(obj, clazz.getMethod("addParamFile", String.class, Artifact.class));
+ npt.testMethod(obj, clazz.getMethod("addPaths", String.class, PathFragment[].class));
+
+ npt.setDefault(Iterable.class, ImmutableList.of("foo"));
+ npt.testMethod(obj, clazz.getMethod("addJoinStrings", String.class, Iterable.class));
+ npt.testMethod(
+ obj, clazz.getMethod("addJoinValues", String.class, Iterable.class, Function.class));
+ npt.testMethod(obj, clazz.getMethod("addBeforeEach", String.class, Iterable.class));
+ npt.testMethod(obj, clazz.getMethod("addFormatEach", String.class, Iterable.class));
+
+ npt.setDefault(Iterable.class, ImmutableList.of(artifact1));
+ npt.testMethod(obj, clazz.getMethod("addJoinExecPaths", String.class, Iterable.class));
+ npt.testMethod(obj, clazz.getMethod("addBeforeEachExecPath", String.class, Iterable.class));
+
+ npt.setDefault(Iterable.class, ImmutableList.of(PathFragment.create("foo")));
+ npt.testMethod(obj, clazz.getMethod("addBeforeEachPath", String.class, Iterable.class));
+
+ npt.setDefault(Artifact.class, treeArtifact);
+ npt.testMethod(
+ obj, clazz.getMethod("addJoinExpandedTreeArtifactExecPath", String.class, Artifact.class));
+ npt.testMethod(obj, clazz.getMethod("addExpandedTreeArtifactExecPaths", Artifact.class));
}
@Test
@@ -201,10 +257,13 @@ public class CustomCommandLineTest {
Artifact treeArtifactOne = createTreeArtifact("myArtifact/treeArtifact1");
Artifact treeArtifactTwo = createTreeArtifact("myArtifact/treeArtifact2");
- CustomCommandLine commandLineTemplate = CustomCommandLine.builder()
- .addPlaceholderTreeArtifactExecPath("--argOne", treeArtifactOne)
- .addPlaceholderTreeArtifactExecPath("--argTwo", treeArtifactTwo)
- .build();
+ CustomCommandLine commandLineTemplate =
+ CustomCommandLine.builder()
+ .add("--argOne")
+ .addPlaceholderTreeArtifactExecPath(treeArtifactOne)
+ .add("--argTwo")
+ .addPlaceholderTreeArtifactExecPath(treeArtifactTwo)
+ .build();
TreeFileArtifact treeFileArtifactOne = createTreeFileArtifact(
treeArtifactOne, "children/child1");
@@ -224,32 +283,17 @@ public class CustomCommandLineTest {
}
@Test
- public void testTreeFileArtifactExecPathWithTemplateArgs() {
- Artifact treeArtifact = createTreeArtifact("myArtifact/treeArtifact1");
-
- CustomCommandLine commandLineTemplate = CustomCommandLine.builder()
- .addPlaceholderTreeArtifactFormattedExecPath("path:%s", treeArtifact)
- .build();
-
- TreeFileArtifact treeFileArtifact = createTreeFileArtifact(
- treeArtifact, "children/child1");
-
- CustomCommandLine commandLine = commandLineTemplate.evaluateTreeFileArtifacts(
- ImmutableList.of(treeFileArtifact));
-
- assertThat(commandLine.arguments()).containsExactly(
- "path:myArtifact/treeArtifact1/children/child1");
- }
-
- @Test
public void testTreeFileArtifactArgThrowWithoutSubstitution() {
Artifact treeArtifactOne = createTreeArtifact("myArtifact/treeArtifact1");
Artifact treeArtifactTwo = createTreeArtifact("myArtifact/treeArtifact2");
- CustomCommandLine commandLineTemplate = CustomCommandLine.builder()
- .addPlaceholderTreeArtifactExecPath("--argOne", treeArtifactOne)
- .addPlaceholderTreeArtifactExecPath("--argTwo", treeArtifactTwo)
- .build();
+ CustomCommandLine commandLineTemplate =
+ CustomCommandLine.builder()
+ .add("--argOne")
+ .addPlaceholderTreeArtifactExecPath(treeArtifactOne)
+ .add("--argTwo")
+ .addPlaceholderTreeArtifactExecPath(treeArtifactTwo)
+ .build();
try {
commandLineTemplate.arguments();
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java
index 8711b47a4b..6e5de2fce5 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java
@@ -132,7 +132,9 @@ public class ParamFileWriteActionTest extends BuildViewTestCase {
return CustomCommandLine.builder()
.add("--flag1")
.add("--flag2")
- .add("--flag3", ImmutableList.of("value1", "value2"))
+ .add("--flag3")
+ .add("value1")
+ .add("value2")
.build();
}