diff options
3 files changed, 11 insertions, 166 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 61ecebee60..6631eca95e 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 @@ -362,38 +362,6 @@ public final class CustomCommandLine extends CommandLine { } } - private static final class JoinExpandedTreeArtifactExecPathsArg - extends TreeArtifactExpansionArgvFragment { - - private final String delimiter; - private final Artifact treeArtifact; - - private JoinExpandedTreeArtifactExecPathsArg(String delimiter, Artifact treeArtifact) { - Preconditions.checkArgument( - treeArtifact.isTreeArtifact(), "%s is not a TreeArtifact", treeArtifact); - this.delimiter = delimiter; - this.treeArtifact = treeArtifact; - } - - @Override - void eval(ImmutableList.Builder<String> builder, ArtifactExpander artifactExpander) { - Set<Artifact> expandedArtifacts = new TreeSet<>(); - artifactExpander.expand(treeArtifact, expandedArtifacts); - - if (!expandedArtifacts.isEmpty()) { - builder.add(Artifact.joinExecPaths(delimiter, expandedArtifacts)); - } - } - - @Override - public String describe() { - return String.format( - "JoinExpandedTreeArtifactExecPathsArg{ delimiter: %s, treeArtifact: %s}", - delimiter, - treeArtifact.getExecPathString()); - } - } - private static final class ExpandedTreeArtifactExecPathsArg extends TreeArtifactExpansionArgvFragment { private final Artifact treeArtifact; @@ -423,32 +391,6 @@ public final class CustomCommandLine extends CommandLine { } /** - * 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 - Object substituteTreeArtifact(Map<Artifact, TreeFileArtifact> substitutionMap) { - Artifact treeFileArtifact = substitutionMap.get(placeHolderTreeArtifact); - Preconditions.checkNotNull(treeFileArtifact, "Artifact to substitute: %s", - placeHolderTreeArtifact); - return String.format(template, treeFileArtifact.getExecPath()); - } - } - - /** * An argument object that evaluates to the exec path of a {@link TreeFileArtifact}, enclosing * the associated {@link TreeFileArtifact}. */ @@ -615,24 +557,6 @@ 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, @Nullable Artifact treeArtifact) { - Preconditions.checkNotNull(arg); - if (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. @@ -648,40 +572,24 @@ public final class CustomCommandLine extends CommandLine { } /** - * 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. + * 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 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 + * @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 addPlaceholderTreeArtifactFormattedExecPath( - String template, @Nullable Artifact treeArtifact) { - Preconditions.checkNotNull(template); + public Builder addPlaceholderTreeArtifactExecPath(String arg, @Nullable Artifact treeArtifact) { + Preconditions.checkNotNull(arg); if (treeArtifact != null) { - arguments.add(new TreeFileArtifactExecPathWithTemplateArg(template, treeArtifact)); + arguments.add(arg); + arguments.add(new TreeFileArtifactExecPathArg(treeArtifact)); } return this; } /** - * Adds a string joined together by the exec paths of all {@link TreeFileArtifact}s under - * {@code treeArtifact}. - * - * @param delimiter the delimiter used to join the artifact exec paths. - * @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; - } - - /** * Adds the exec paths (one argument per exec path) of all {@link TreeFileArtifact}s under * {@code treeArtifact}. * 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 ba0439e1d6..ad1f4137ff 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 @@ -19,11 +19,9 @@ 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; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; -import com.google.devtools.build.lib.analysis.actions.CommandLine; import com.google.devtools.build.lib.analysis.actions.CustomCommandLine; import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.CustomArgv; import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.CustomMultiArgv; @@ -35,7 +33,6 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; 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; @@ -230,7 +227,6 @@ public class CustomCommandLineTest { .add("foo", VectorArg.of((NestedSet<String>) null)) .add("foo", VectorArg.of(NestedSetBuilder.<String>emptySet(Order.STABLE_ORDER))) .addPlaceholderTreeArtifactExecPath("foo", null) - .addPlaceholderTreeArtifactFormattedExecPath("foo", null) .add((CustomArgv) null) .add((CustomMultiArgv) null) .build(); @@ -247,12 +243,6 @@ public class CustomCommandLineTest { npt.testMethod(obj, clazz.getMethod("add", String.class, Object.class)); npt.testMethod( obj, clazz.getMethod("addPlaceholderTreeArtifactExecPath", String.class, Artifact.class)); - npt.testMethod( - obj, - clazz.getMethod( - "addPlaceholderTreeArtifactFormattedExecPath", String.class, Artifact.class)); - npt.testMethod( - obj, clazz.getMethod("addJoinExpandedTreeArtifactExecPath", String.class, Artifact.class)); npt.testMethod(obj, clazz.getMethod("addExpandedTreeArtifactExecPaths", Artifact.class)); npt.setDefault(Iterable.class, ImmutableList.of("foo")); @@ -261,8 +251,6 @@ public class CustomCommandLineTest { npt.setDefault(Iterable.class, ImmutableList.of(PathFragment.create("foo"))); npt.setDefault(Artifact.class, treeArtifact); - npt.testMethod( - obj, clazz.getMethod("addJoinExpandedTreeArtifactExecPath", String.class, Artifact.class)); npt.testMethod(obj, clazz.getMethod("addExpandedTreeArtifactExecPaths", Artifact.class)); } @@ -294,24 +282,6 @@ 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"); @@ -330,39 +300,6 @@ public class CustomCommandLineTest { } - @Test - public void testJoinExpandedTreeArtifactExecPath() { - Artifact treeArtifact = createTreeArtifact("myTreeArtifact"); - - CommandLine commandLine = CustomCommandLine.builder() - .add("hello") - .addJoinExpandedTreeArtifactExecPath(":", treeArtifact) - .build(); - - assertThat(commandLine.arguments()).containsExactly( - "hello", - "JoinExpandedTreeArtifactExecPathsArg{ delimiter: :, treeArtifact: myTreeArtifact}"); - - final Iterable<TreeFileArtifact> treeFileArtifacts = ImmutableList.of( - createTreeFileArtifact(treeArtifact, "children/child1"), - createTreeFileArtifact(treeArtifact, "children/child2")); - - ArtifactExpander artifactExpander = new ArtifactExpander() { - @Override - public void expand(Artifact artifact, Collection<? super Artifact> output) { - for (TreeFileArtifact treeFileArtifact : treeFileArtifacts) { - if (treeFileArtifact.getParent().equals(artifact)) { - output.add(treeFileArtifact); - } - } - } - }; - - assertThat(commandLine.arguments(artifactExpander)).containsExactly( - "hello", - "myTreeArtifact/children/child1:myTreeArtifact/children/child2"); - } - private Artifact createTreeArtifact(String rootRelativePath) { PathFragment relpath = PathFragment.create(rootRelativePath); return new SpecialArtifact( 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 0520b7ae3c..2f789a5d0d 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 @@ -97,7 +97,7 @@ public class ParamFileWriteActionTest extends BuildViewTestCase { assertThat(content.trim()) .isEqualTo( "--flag1\n" - + "artifact/myTreeFileArtifact/artifacts/treeFileArtifact1:" + + "artifact/myTreeFileArtifact/artifacts/treeFileArtifact1\n" + "artifact/myTreeFileArtifact/artifacts/treeFileArtifact2"); } @@ -140,7 +140,7 @@ public class ParamFileWriteActionTest extends BuildViewTestCase { private CommandLine createTreeArtifactExpansionCommandLine() { return CustomCommandLine.builder() .add("--flag1") - .addJoinExpandedTreeArtifactExecPath(":", treeArtifact) + .addExpandedTreeArtifactExecPaths(treeArtifact) .build(); } |