Remove the 3-arg minimum on shell actions

RELNOTES: None.
PiperOrigin-RevId: 208855272

5 files changed, 103 insertions, 14 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
index ce9eeeeb03..7a6d05fa00 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.analysis.actions;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.CharMatcher;
+import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
@@ -306,7 +307,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
           failMessage =
               "error executing shell command: "
                   + "'"
-                  + truncate(Iterables.get(getArguments(), 2), 200)
+                  + truncate(Joiner.on(" ").join(getArguments()), 200)
                   + "'";
         } catch (CommandLineExpansionException commandLineExpansionException) {
           failMessage =
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java
index 90343eba5d..72e2a5f802 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java
@@ -290,9 +290,7 @@ public class SkylarkActionFactory implements SkylarkActionFactoryApi {
       }
     } else if (commandUnchecked instanceof SkylarkList) {
       SkylarkList commandList = (SkylarkList) commandUnchecked;
-      if (commandList.size() < 3) {
-        throw new EvalException(null, "'command' list has to be of size at least 3");
-      }
+
       @SuppressWarnings("unchecked")
       List<String> command = commandList.getContents(String.class, "command");
       builder.setShellCommand(command);
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 d3568b2017..92e384a4b7 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
@@ -400,16 +400,6 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
         "  command = 'dummy_command')");
   }
 
-  @Test
-  public void testCreateSpawnActionCommandsListTooShort() throws Exception {
-    checkErrorContains(
-        createRuleContext("//foo:foo"),
-        "'command' list has to be of size at least 3",
-        "ruleContext.actions.run_shell(",
-        "  outputs = ruleContext.files.srcs,",
-        "  command = ['dummy_command', '--arg'])");
-  }
-
   private void setupToolInInputsTest(String... ruleImpl) throws Exception {
     ImmutableList.Builder<String> lines = ImmutableList.builder();
     lines.add("def _main_rule_impl(ctx):");
diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD
index eafe9aadea..3349e90d2b 100644
--- a/src/test/shell/bazel/BUILD
+++ b/src/test/shell/bazel/BUILD
@@ -353,6 +353,14 @@ sh_test(
 )
 
 sh_test(
+    name = "skylark_rule_test",
+    size = "large",
+    srcs = ["skylark_rule_test.sh"],
+    data = [":test-deps"],
+    tags = ["no_windows"],
+)
+
+sh_test(
     name = "disk_cache_test",
     size = "small",
     srcs = ["disk_cache_test.sh"],
diff --git a/src/test/shell/bazel/skylark_rule_test.sh b/src/test/shell/bazel/skylark_rule_test.sh
new file mode 100755
index 0000000000..edb21b969c
--- /dev/null
+++ b/src/test/shell/bazel/skylark_rule_test.sh
@@ -0,0 +1,92 @@
+#!/bin/bash
+#
+# Copyright 2018 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# Tests building with rules defined in Skylark.
+
+# Load the test setup defined in the parent directory
+CURRENT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+source "${CURRENT_DIR}/../integration_test_setup.sh" \
+  || { echo "integration_test_setup.sh not found!" >&2; exit 1; }
+
+# Test a basic skylark rule which touches an output file
+function test_basic_output() {
+  mkdir -p test
+  cat << EOF >> test/BUILD
+load(":skylark.bzl", "test_rule")
+
+test_rule(
+    name = "test",
+    out = "output.txt"
+)
+EOF
+
+  cat << 'EOF' >> test/skylark.bzl
+def _test_impl(ctx):
+  ctx.actions.run_shell(outputs = [ctx.outputs.out],
+                        command = ["touch", ctx.outputs.out.path])
+  files_to_build = depset([ctx.outputs.out])
+  return DefaultInfo(
+      files = files_to_build,
+  )
+
+test_rule = rule(
+    implementation=_test_impl,
+    attrs = {
+        "out": attr.output(mandatory = True),
+    },
+)
+EOF
+
+  bazel build //test:test &> $TEST_log \
+      || fail "should have generated output successfully"
+}
+
+# Test a basic skylark rule which is valid except the action fails on execution.
+function test_execution_failure() {
+  mkdir -p test
+  cat << EOF >> test/BUILD
+load(":skylark.bzl", "test_rule")
+
+test_rule(
+    name = "test",
+    out = "output.txt"
+)
+EOF
+
+  cat << 'EOF' >> test/skylark.bzl
+def _test_impl(ctx):
+  ctx.actions.run_shell(outputs = [ctx.outputs.out],
+                        command = ["not_a_command", ctx.outputs.out.path])
+  files_to_build = depset([ctx.outputs.out])
+  return DefaultInfo(
+      files = files_to_build,
+  )
+
+test_rule = rule(
+    implementation=_test_impl,
+    attrs = {
+        "out": attr.output(mandatory = True),
+    },
+)
+EOF
+
+  ! bazel build //test:test &> $TEST_log \
+      || fail "Should have resulted in an execution error"
+
+  expect_log "error executing shell command"
+}
+
+run_suite "skylark rule definition tests"