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
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 86d6542..7ef502e 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.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 @@
}
private static Attribute.Builder<?> createAttribute(
- Type<?> type, SkylarkDict<String, Object> arguments, FuncallExpression ast, Environment env,
- Location loc) throws EvalException, ConversionException {
+ Type<?> type, SkylarkDict<String, Object> 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 @@
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 @@
SkylarkDict<String, Object> 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 @@
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 @@
CONFIGURATION_ARG,
cfg),
ast,
- env,
- ast.getLocation());
+ env);
ImmutableList<SkylarkAspect> skylarkAspects =
ImmutableList.copyOf(aspects.getContents(SkylarkAspect.class, "aspects"));
return new Descriptor(attribute, skylarkAspects);
@@ -902,7 +900,7 @@
cfg);
try {
Attribute.Builder<?> attribute =
- createAttribute(BuildType.LABEL_LIST, kwargs, ast, env, ast.getLocation());
+ createAttribute(BuildType.LABEL_LIST, kwargs, ast, env);
ImmutableList<SkylarkAspect> 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 0872174..47b4a98 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 @@
}
@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 @@
" 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 @@
assertThat(analysisResultOfAspect.hasError()).isFalse();
}
-
@Test
public void aspectFragmentAccessSuccess() throws Exception {
getConfiguredTargetForAspectFragment(
@@ -1251,7 +1250,6 @@
};
}
-
@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 8c6df5b..da072b9 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 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 @@
*/
@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 @@
}
@Test
+
public void structsAsDeclaredProvidersTest() throws Exception {
evalAndExport(
"data = struct(x = 1)"
@@ -1115,4 +1119,30 @@
" 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.");
+ }
+ }
}
+
