From f7c552ca3aa86dc1cf11e701b4bcf2cf9ea7efe7 Mon Sep 17 00:00:00 2001 From: Vladimir Moskva Date: Thu, 12 Jan 2017 17:17:15 +0000 Subject: Mandatory cfg parameter for labels if executable=1 is provided RELNOTES[INC]: All executable labels must also have a cfg parameter specified. -- PiperOrigin-RevId: 144332992 MOS_MIGRATED_REVID=144332992 --- .../devtools/build/lib/rules/SkylarkAttr.java | 22 ++++++++-------- .../build/lib/skylark/SkylarkAspectsTest.java | 8 +++--- .../lib/skylark/SkylarkRuleClassFunctionsTest.java | 30 ++++++++++++++++++++++ 3 files changed, 43 insertions(+), 17 deletions(-) (limited to 'src') diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java index 86d6542037..7ef502efb9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java +++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java @@ -19,7 +19,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.Attribute.AllowedValueSet; @@ -159,8 +158,8 @@ public final class SkylarkAttr { } private static Attribute.Builder createAttribute( - Type type, SkylarkDict arguments, FuncallExpression ast, Environment env, - Location loc) throws EvalException, ConversionException { + Type type, SkylarkDict arguments, FuncallExpression ast, Environment env) + throws EvalException, ConversionException { // We use an empty name now so that we can set it later. // This trick makes sense only in the context of Skylark (builtin rules should not use it). Attribute.Builder builder = Attribute.attr("", type); @@ -205,11 +204,11 @@ public final class SkylarkAttr { if (containsNonNoneKey(arguments, EXECUTABLE_ARG) && (Boolean) arguments.get(EXECUTABLE_ARG)) { builder.setPropertyFlag("EXECUTABLE"); if (!containsNonNoneKey(arguments, CONFIGURATION_ARG)) { - String message = "Argument `cfg = \"host\"`, `cfg = \"data\"`, or `cfg = \"target\"` " - + "is required if `executable = True` is provided for a label. Please see " + throw new EvalException( + ast.getLocation(), + "cfg parameter is mandatory when executable=True is provided. Please see " + "https://www.bazel.build/versions/master/docs/skylark/rules.html#configurations " - + "for more details."; - env.handleEvent(Event.warn(loc, message)); + + "for more details."); } } @@ -325,7 +324,7 @@ public final class SkylarkAttr { SkylarkDict kwargs, Type type, FuncallExpression ast, Environment env) throws EvalException { try { - return new Descriptor(createAttribute(type, kwargs, ast, env, ast.getLocation())); + return new Descriptor(createAttribute(type, kwargs, ast, env)); } catch (ConversionException e) { throw new EvalException(ast.getLocation(), e.getMessage()); } @@ -356,7 +355,7 @@ public final class SkylarkAttr { Preconditions.checkNotNull(maybeGetNonConfigurableReason(type), type); try { return new Descriptor( - createAttribute(type, kwargs, ast, env, ast.getLocation()) + createAttribute(type, kwargs, ast, env) .nonconfigurable(whyNotConfigurableReason)); } catch (ConversionException e) { throw new EvalException(ast.getLocation(), e.getMessage()); @@ -622,8 +621,7 @@ public final class SkylarkAttr { CONFIGURATION_ARG, cfg), ast, - env, - ast.getLocation()); + env); ImmutableList skylarkAspects = ImmutableList.copyOf(aspects.getContents(SkylarkAspect.class, "aspects")); return new Descriptor(attribute, skylarkAspects); @@ -902,7 +900,7 @@ public final class SkylarkAttr { cfg); try { Attribute.Builder attribute = - createAttribute(BuildType.LABEL_LIST, kwargs, ast, env, ast.getLocation()); + createAttribute(BuildType.LABEL_LIST, kwargs, ast, env); ImmutableList skylarkAspects = ImmutableList.copyOf(aspects.getContents(SkylarkAspect.class, "aspects")); return new Descriptor(attribute, skylarkAspects); diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java index 0872174ea0..47b4a98f41 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java @@ -1052,7 +1052,7 @@ public class SkylarkAspectsTest extends AnalysisTestCase { } @Test - public void multipleExecutablesInTarget() throws Exception { + public void testMultipleExecutablesInTarget() throws Exception { scratch.file("foo/extension.bzl", "def _aspect_impl(target, ctx):", " return struct()", @@ -1061,8 +1061,8 @@ public class SkylarkAspectsTest extends AnalysisTestCase { " pass", "my_rule = rule(_main_rule_impl,", " attrs = { ", - " 'exe1' : attr.label(executable = True, allow_files = True),", - " 'exe2' : attr.label(executable = True, allow_files = True),", + " 'exe1' : attr.label(executable = True, allow_files = True, cfg = 'host'),", + " 'exe2' : attr.label(executable = True, allow_files = True, cfg = 'host'),", " },", ")" ); @@ -1081,7 +1081,6 @@ public class SkylarkAspectsTest extends AnalysisTestCase { assertThat(analysisResultOfAspect.hasError()).isFalse(); } - @Test public void aspectFragmentAccessSuccess() throws Exception { getConfiguredTargetForAspectFragment( @@ -1251,7 +1250,6 @@ public class SkylarkAspectsTest extends AnalysisTestCase { }; } - @Test public void aspectOutputsToBinDirectory() throws Exception { scratch.file( diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java index 8c6df5b6d6..da072b93ec 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java @@ -56,7 +56,9 @@ import com.google.devtools.build.lib.util.FileTypeSet; import java.util.Collection; import org.junit.Assert; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -65,6 +67,7 @@ import org.junit.runners.JUnit4; */ @RunWith(JUnit4.class) public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase { + @Rule public ExpectedException thrown = ExpectedException.none(); @Before public final void createBuildFile() throws Exception { @@ -1083,6 +1086,7 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase { } @Test + public void structsAsDeclaredProvidersTest() throws Exception { evalAndExport( "data = struct(x = 1)" @@ -1115,4 +1119,30 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase { " pass", "aspect(_impl, attr_aspects=['*', 'foo'])"); } + + @Test + public void testMandatoryConfigParameterForExecutableLabels() throws Exception { + scratch.file("third_party/foo/extension.bzl", + "def _main_rule_impl(ctx):", + " pass", + "my_rule = rule(_main_rule_impl,", + " attrs = { ", + " 'exe' : attr.label(executable = True, allow_files = True),", + " },", + ")" + ); + scratch.file("third_party/foo/BUILD", + "load('extension', 'my_rule')", + "my_rule(name = 'main', exe = ':tool.sh')" + ); + + try { + createRuleContext("//third_party/foo:main"); + Assert.fail(); + } catch (AssertionError e) { + assertThat(e.getMessage()).contains("cfg parameter is mandatory when executable=True is " + + "provided."); + } + } } + -- cgit v1.2.3