diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib')
17 files changed, 268 insertions, 335 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(), |