Implement support for Skylark param files.

PiperOrigin-RevId: 169234249

1 files changed, 105 insertions, 13 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 2db09cc216..0add5edade 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
@@ -31,6 +31,7 @@ import com.google.devtools.build.lib.analysis.DefaultInfo;
 import com.google.devtools.build.lib.analysis.FilesToRunProvider;
 import com.google.devtools.build.lib.analysis.Runfiles;
 import com.google.devtools.build.lib.analysis.actions.FileWriteAction;
+import com.google.devtools.build.lib.analysis.actions.ParameterFileWriteAction;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction;
 import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction.Substitution;
@@ -58,7 +59,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.regex.Pattern;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -1737,7 +1737,6 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
     getConfiguredTarget("//test:silly");
   }
 
-  @Ignore
   @Test
   public void testLazyArgs() throws Exception {
     SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
@@ -1760,7 +1759,7 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
         "ruleContext.actions.run(",
         "  inputs = depset(ruleContext.files.srcs),",
         "  outputs = ruleContext.files.srcs,",
-        "  arguments = args,",
+        "  arguments = [args],",
         "  executable = ruleContext.files.tools[0],",
         ")");
     SpawnAction action =
@@ -1792,7 +1791,73 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
         .inOrder();
   }
 
-  @Ignore
+  @Test
+  public void testMultipleLazyArgsMixedWithStrings() throws Exception {
+    SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
+    evalRuleContextCode(
+        ruleContext,
+        "foo_args = ruleContext.actions.args()",
+        "foo_args.add('--foo')",
+        "bar_args = ruleContext.actions.args()",
+        "bar_args.add('--bar')",
+        "ruleContext.actions.run(",
+        "  inputs = depset(ruleContext.files.srcs),",
+        "  outputs = ruleContext.files.srcs,",
+        "  arguments = ['hello', foo_args, 'world', bar_args, 'works'],",
+        "  executable = ruleContext.files.tools[0],",
+        ")");
+    SpawnAction action =
+        (SpawnAction)
+            Iterables.getOnlyElement(
+                ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions());
+    assertThat(action.getArguments())
+        .containsExactly("foo/t.exe", "hello", "--foo", "world", "--bar", "works")
+        .inOrder();
+  }
+
+  @Test
+  public void testLazyArgsWithParamFile() throws Exception {
+    SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
+    evalRuleContextCode(
+        ruleContext,
+        "foo_args = ruleContext.actions.args()",
+        "foo_args.add('--foo')",
+        "foo_args.use_param_file('--file=%s', use_always=True)",
+        "output=ruleContext.actions.declare_file('out')",
+        "ruleContext.actions.run(",
+        "  inputs = depset(ruleContext.files.srcs),",
+        "  outputs = [output],",
+        "  arguments = [foo_args],",
+        "  executable = ruleContext.files.tools[0],",
+        ")");
+    List<ActionAnalysisMetadata> actions =
+        ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions();
+    assertThat(actions.stream().anyMatch(a -> a instanceof ParameterFileWriteAction)).isTrue();
+    SpawnAction action =
+        (SpawnAction) actions.stream().filter(a -> a instanceof SpawnAction).findAny().get();
+    // Assert that there is a file argument. Don't bother matching the exact string
+    assertThat(action.getArguments().stream().anyMatch(arg -> arg.matches("--file=.*"))).isTrue();
+  }
+
+  @Test
+  public void testLazyArgsWithParamFileInvalidFormatString() throws Exception {
+    SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
+    checkError(
+        ruleContext,
+        "Invalid value for parameter \"param_file_arg\": Expected string with a single \"%s\"",
+        "args = ruleContext.actions.args()\n" + "args.use_param_file('--file=')");
+  }
+
+  @Test
+  public void testLazyArgsWithParamFileInvalidFormat() throws Exception {
+    SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
+    checkError(
+        ruleContext,
+        "Invalid value for parameter \"format\": Expected one of \"shell\", \"multiline\"",
+        "args = ruleContext.actions.args()\n"
+            + "args.use_param_file('--file=%s', format='illegal')");
+  }
+
   @Test
   public void testScalarJoinWithErrorMessage() throws Exception {
     SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
@@ -1802,7 +1867,6 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
         "args = ruleContext.actions.args()\n" + "args.add(1, join_with=':')");
   }
 
-  @Ignore
   @Test
   public void testScalarBeforeEachErrorMessage() throws Exception {
     SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
@@ -1812,7 +1876,6 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
         "args = ruleContext.actions.args()\n" + "args.add(1, before_each='illegal')");
   }
 
-  @Ignore
   @Test
   public void testLazyArgIllegalFormatString() throws Exception {
     SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
@@ -1823,7 +1886,7 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
         "ruleContext.actions.run(",
         "  inputs = depset(ruleContext.files.srcs),",
         "  outputs = ruleContext.files.srcs,",
-        "  arguments = args,",
+        "  arguments = [args],",
         "  executable = ruleContext.files.tools[0],",
         ")");
     SpawnAction action =
@@ -1838,7 +1901,6 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
     }
   }
 
-  @Ignore
   @Test
   public void testLazyArgBadMapFn() throws Exception {
     SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
@@ -1850,7 +1912,7 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
         "ruleContext.actions.run(",
         "  inputs = depset(ruleContext.files.srcs),",
         "  outputs = ruleContext.files.srcs,",
-        "  arguments = args,",
+        "  arguments = [args],",
         "  executable = ruleContext.files.tools[0],",
         ")");
     SpawnAction action =
@@ -1865,7 +1927,6 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
     }
   }
 
-  @Ignore
   @Test
   public void testLazyArgMapFnReturnsWrongType() throws Exception {
     SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
@@ -1877,7 +1938,7 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
         "ruleContext.actions.run(",
         "  inputs = depset(ruleContext.files.srcs),",
         "  outputs = ruleContext.files.srcs,",
-        "  arguments = args,",
+        "  arguments = [args],",
         "  executable = ruleContext.files.tools[0],",
         ")");
     SpawnAction action =
@@ -1892,7 +1953,6 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
     }
   }
 
-  @Ignore
   @Test
   public void createShellWithLazyArgs() throws Exception {
     SkylarkRuleContext ruleContext = createRuleContext("//foo:foo");
@@ -1903,7 +1963,7 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
         "ruleContext.actions.run_shell(",
         "  inputs = ruleContext.files.srcs,",
         "  outputs = ruleContext.files.srcs,",
-        "  arguments = args,",
+        "  arguments = [args],",
         "  mnemonic = 'DummyMnemonic',",
         "  command = 'dummy_command',",
         "  progress_message = 'dummy_message',",
@@ -1919,6 +1979,38 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
     assertThat(Iterables.getLast(args)).isEqualTo("--foo");
   }
 
+  @Test
+  public void testLazyArgsObjectImmutability() throws Exception {
+    scratch.file(
+        "test/BUILD",
+        "load('/test/rules', 'main_rule', 'dep_rule')",
+        "dep_rule(name = 'dep')",
+        "main_rule(name = 'main', deps = [':dep'])");
+    scratch.file(
+        "test/rules.bzl",
+        "def _main_impl(ctx):",
+        "  dep = ctx.attr.deps[0]",
+        "  args = dep.dep_arg",
+        "  args.add('hello')",
+        "main_rule = rule(",
+        "  implementation = _main_impl,",
+        "  attrs = {",
+        "    'deps': attr.label_list()",
+        "  },",
+        "  outputs = {'file': 'output.txt'},",
+        ")",
+        "def _dep_impl(ctx):",
+        "  args = ctx.actions.args()",
+        "  return struct(dep_arg = args)",
+        "dep_rule = rule(implementation = _dep_impl)");
+    try {
+      getConfiguredTarget("//test:main");
+      fail("Should have been unable to mutate frozen args object");
+    } catch (AssertionError e) {
+      assertThat(e).hasMessageThat().contains("cannot modify frozen value");
+    }
+  }
+
   private void setupThrowFunction(BuiltinFunction func) throws Exception {
     throwFunction = func;
     throwFunction.configure(