Implement CustomCommandLine support for Skylark.

RELNOTES: Support ctx.actions.args() for more efficient Skylark command line construction.
PiperOrigin-RevId: 166694148

1 files changed, 176 insertions, 0 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
index 975a98b251..1a443a17f5 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
@@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.CommandLineExpansionException;
 import com.google.devtools.build.lib.actions.CompositeRunfilesSupplier;
 import com.google.devtools.build.lib.actions.RunfilesSupplier;
 import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
@@ -1735,6 +1736,181 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
     getConfiguredTarget("//test:silly");
   }
 
+  @Test
+  public void testLazyArgs() throws Exception {
+    SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
+    evalRuleContextCode(
+        ruleContext,
+        "def map_scalar(val): return 'mapped' + val",
+        "def map_vector(vals): return [x + 1 for x in vals]",
+        "args = ruleContext.actions.args()",
+        "args.add('--foo')",
+        "args.add('foo', format='format%s')",
+        "args.add('foo', map_fn=map_scalar)",
+        "args.add([1, 2])",
+        "args.add([1, 2], join_with=':')",
+        "args.add([1, 2], before_each='-before')",
+        "args.add([1, 2], format='format/%s')",
+        "args.add([1, 2], map_fn=map_vector)",
+        "args.add([1, 2], format='format/%s', join_with=':')",
+        "args.add(ruleContext.files.srcs)",
+        "args.add(ruleContext.files.srcs, format='format/%s')",
+        "ruleContext.actions.run(",
+        "  inputs = depset(ruleContext.files.srcs),",
+        "  outputs = ruleContext.files.srcs,",
+        "  arguments = args,",
+        "  executable = ruleContext.files.tools[0],",
+        ")");
+    SpawnAction action =
+        (SpawnAction)
+            Iterables.getOnlyElement(
+                ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions());
+    assertThat(action.getArguments())
+        .containsExactly(
+            "foo/t.exe",
+            "--foo",
+            "formatfoo",
+            "mappedfoo",
+            "1",
+            "2",
+            "1:2",
+            "-before",
+            "1",
+            "-before",
+            "2",
+            "format/1",
+            "format/2",
+            "2",
+            "3",
+            "format/1:format/2",
+            "foo/a.txt",
+            "foo/b.img",
+            "format/foo/a.txt",
+            "format/foo/b.img")
+        .inOrder();
+  }
+
+  @Test
+  public void testScalarJoinWithErrorMessage() throws Exception {
+    SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
+    checkError(
+        ruleContext,
+        "'join_with' is not supported for scalar arguments",
+        "args = ruleContext.actions.args()\n" + "args.add(1, join_with=':')");
+  }
+
+  @Test
+  public void testScalarBeforeEachErrorMessage() throws Exception {
+    SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
+    checkError(
+        ruleContext,
+        "'before_each' is not supported for scalar arguments",
+        "args = ruleContext.actions.args()\n" + "args.add(1, before_each='illegal')");
+  }
+
+  @Test
+  public void testLazyArgIllegalFormatString() throws Exception {
+    SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
+    evalRuleContextCode(
+        ruleContext,
+        "args = ruleContext.actions.args()",
+        "args.add([1, 2], format='format/%s%s')", // Expects two args, will only be given one
+        "ruleContext.actions.run(",
+        "  inputs = depset(ruleContext.files.srcs),",
+        "  outputs = ruleContext.files.srcs,",
+        "  arguments = args,",
+        "  executable = ruleContext.files.tools[0],",
+        ")");
+    SpawnAction action =
+        (SpawnAction)
+            Iterables.getOnlyElement(
+                ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions());
+    try {
+      action.getArguments();
+      fail();
+    } catch (CommandLineExpansionException e) {
+      assertThat(e.getMessage()).contains("not enough arguments");
+    }
+  }
+
+  @Test
+  public void testLazyArgBadMapFn() throws Exception {
+    SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
+    evalRuleContextCode(
+        ruleContext,
+        "args = ruleContext.actions.args()",
+        "def bad_fn(args): 'hello'.nosuchmethod()",
+        "args.add([1, 2], map_fn=bad_fn)",
+        "ruleContext.actions.run(",
+        "  inputs = depset(ruleContext.files.srcs),",
+        "  outputs = ruleContext.files.srcs,",
+        "  arguments = args,",
+        "  executable = ruleContext.files.tools[0],",
+        ")");
+    SpawnAction action =
+        (SpawnAction)
+            Iterables.getOnlyElement(
+                ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions());
+    try {
+      action.getArguments();
+      fail();
+    } catch (CommandLineExpansionException e) {
+      assertThat(e.getMessage()).contains("type 'string' has no method nosuchmethod()");
+    }
+  }
+
+  @Test
+  public void testLazyArgMapFnReturnsWrongType() throws Exception {
+    SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
+    evalRuleContextCode(
+        ruleContext,
+        "args = ruleContext.actions.args()",
+        "def bad_fn(args): return None",
+        "args.add([1, 2], map_fn=bad_fn)",
+        "ruleContext.actions.run(",
+        "  inputs = depset(ruleContext.files.srcs),",
+        "  outputs = ruleContext.files.srcs,",
+        "  arguments = args,",
+        "  executable = ruleContext.files.tools[0],",
+        ")");
+    SpawnAction action =
+        (SpawnAction)
+            Iterables.getOnlyElement(
+                ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions());
+    try {
+      action.getArguments();
+      fail();
+    } catch (CommandLineExpansionException e) {
+      assertThat(e.getMessage()).contains("map_fn must return a list, got NoneType");
+    }
+  }
+
+  @Test
+  public void createShellWithLazyArgs() throws Exception {
+    SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
+    evalRuleContextCode(
+        ruleContext,
+        "args = ruleContext.actions.args()",
+        "args.add('--foo')",
+        "ruleContext.actions.run_shell(",
+        "  inputs = ruleContext.files.srcs,",
+        "  outputs = ruleContext.files.srcs,",
+        "  arguments = args,",
+        "  mnemonic = 'DummyMnemonic',",
+        "  command = 'dummy_command',",
+        "  progress_message = 'dummy_message',",
+        "  use_default_shell_env = True)");
+    SpawnAction action =
+        (SpawnAction)
+            Iterables.getOnlyElement(
+                ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions());
+    List<String> args = action.getArguments();
+    // We don't need to assert the entire arg list, just check that
+    // the dummy empty string is inserted followed by '--foo'
+    assertThat(args.get(args.size() - 2)).isEmpty();
+    assertThat(Iterables.getLast(args)).isEqualTo("--foo");
+  }
+
   private void setupThrowFunction(BuiltinFunction func) throws Exception {
     throwFunction = func;
     throwFunction.configure(