From 3c82bfe03d578d521c957613239f49604d431946 Mon Sep 17 00:00:00 2001 From: cparsons Date: Wed, 15 Aug 2018 11:37:05 -0700 Subject: Remove the 3-arg minimum on shell actions RELNOTES: None. PiperOrigin-RevId: 208855272 --- .../build/lib/analysis/actions/SpawnAction.java | 3 +- .../lib/analysis/skylark/SkylarkActionFactory.java | 4 +- .../SkylarkRuleImplementationFunctionsTest.java | 10 --- src/test/shell/bazel/BUILD | 8 ++ src/test/shell/bazel/skylark_rule_test.sh | 92 ++++++++++++++++++++++ 5 files changed, 103 insertions(+), 14 deletions(-) create mode 100755 src/test/shell/bazel/skylark_rule_test.sh 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 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 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 @@ -352,6 +352,14 @@ sh_test( tags = ["no_windows"], ) +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", 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" -- cgit v1.2.3