Avoid passing Tristate and Distrib type attributes to attr struct
These types are not Starlark-compatible, and, due to an existing bug, these are propagated anyway for native rule targets which include them as implicit attributes. Without this change, one may assign to these types in Starlark but later get a catastrophic error when an object of these types is passed to another function, such as dir().
Progress toward #7281.
RELNOTES: None.
PiperOrigin-RevId: 243336671
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttributesCollection.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttributesCollection.java
index dabb841..a8b0be5 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttributesCollection.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttributesCollection.java
@@ -155,6 +155,12 @@
return;
}
+ // Some legacy native attribute types do not have a valid Starlark type. Avoid exposing
+ // these to Starlark.
+ if (type == BuildType.DISTRIBUTIONS || type == BuildType.TRISTATE) {
+ return;
+ }
+
// TODO(mstaib): Remove the LABEL_DICT_UNARY special case of this conditional
// LABEL_DICT_UNARY was previously not treated as a dependency-bearing type, and was put into
// Skylark as a Map<String, Label>; this special case preserves that behavior temporarily.
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java
index 05f9cd8..0390def 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java
@@ -2747,6 +2747,29 @@
assertThat(result).isEqualTo("//test:zzz");
}
+ @Test
+ public void testAllCcLibraryAttrsAreValidTypes() throws Exception {
+ scratch.file(
+ "test/aspect.bzl",
+ "def _impl(target, ctx):",
+ " for entry in dir(ctx.rule.attr):",
+ " val = getattr(ctx.rule.attr, entry, None)",
+ " # Only legitimate Starlark values can be passed to dir(), so this effectively",
+ " # verifies val is an appropriate Starlark type.",
+ " _test_dir = dir(val)",
+ " return []",
+ "",
+ "MyAspect = aspect(",
+ " implementation=_impl,",
+ ")");
+ scratch.file("test/BUILD", "cc_library(", " name = 'xxx',", ")");
+ AnalysisResult analysisResult =
+ update(ImmutableList.of("test/aspect.bzl%MyAspect"), "//test:xxx");
+ AspectValue aspectValue = analysisResult.getAspects().iterator().next();
+ ConfiguredAspect configuredAspect = aspectValue.getConfiguredAspect();
+ assertThat(configuredAspect).isNotNull();
+ }
+
/** SkylarkAspectTest with "keep going" flag */
@RunWith(JUnit4.class)
public static final class WithKeepGoing extends SkylarkDefinedAspectsTest {
