Fix transition whitelist attribute's default value check to work across multiple workspaces.
Test was originally cloning the //tools directory into the main repository which was why the test was passing. Also added additional tests ensuring bazel users can create their own whitelist if they want to and failing on a bad whitelist value.
#7056
RELNOTES: None
PiperOrigin-RevId: 249828279
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
index d465c70..1060e11 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
@@ -703,11 +703,20 @@
throw new EvalException(
location, "_whitelist_function_transition attribute must have a default value");
}
- if (!attr.getDefaultValueUnchecked()
- .equals(FunctionSplitTransitionWhitelist.WHITELIST_LABEL)) {
+ Label defaultLabel = (Label) attr.getDefaultValueUnchecked();
+ // Check the label value for package and target name, to make sure this works properly
+ // in Bazel where it is expected to be found under @bazel_tools.
+ if (!defaultLabel
+ .getPackageName()
+ .equals(FunctionSplitTransitionWhitelist.WHITELIST_LABEL.getPackageName())
+ || !defaultLabel
+ .getName()
+ .equals(FunctionSplitTransitionWhitelist.WHITELIST_LABEL.getName())) {
throw new EvalException(
location,
- "_whitelist_function_transition attribute does not have the expected value "
+ "_whitelist_function_transition attribute ("
+ + defaultLabel
+ + ") does not have the expected value "
+ FunctionSplitTransitionWhitelist.WHITELIST_LABEL);
}
hasFunctionTransitionWhitelist = true;
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
index 89b7ea1..7c921b0 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
@@ -2591,7 +2591,8 @@
reporter.removeHandler(failFastHandler);
getConfiguredTarget("//test:my_rule");
assertContainsEvent(
- "_whitelist_function_transition attribute does not have the expected value");
+ " _whitelist_function_transition attribute (//test:my_other_rule) does not have the"
+ + " expected value");
}
@Test
diff --git a/src/test/shell/bazel/whitelist_test.sh b/src/test/shell/bazel/whitelist_test.sh
index e3c2b12..b8d476d 100755
--- a/src/test/shell/bazel/whitelist_test.sh
+++ b/src/test/shell/bazel/whitelist_test.sh
@@ -14,7 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
-# test behavior with
+# test behavior with starlark transitions and
# //src/main/java/com/google/devtools/build/lib/analysis/Whitelist.java
#
@@ -29,17 +29,17 @@
#
# repo structure:
# ${WORKSPACE_DIR}/
-# hotsauce/
-# BUILD
-# rules.bzl
-# rule_with_transition
-# repo2/
# vinegar/
-# WORKSPACE
-# local_repository
-# rules.bzl
-# rule_with_external_dep
-# BUILD
+# WORKSPACE
+# local_repository
+# rules.bzl
+# rule_with_external_dep
+# BUILD
+# repo2/
+# hotsauce/
+# BUILD
+# rules.bzl
+# rule_with_transition
#
# The whitelist for starlark transitions is set to a package group of "//..."
function test_whitelist_includes_external_deps() {
@@ -61,7 +61,7 @@
implementation = _rule_with_transition_impl,
attrs = {
"dep": attr.label(cfg = my_transition),
- "_whitelist_function_transition": attr.label(default = "@//tools/whitelists/function_transition_whitelist"),
+ "_whitelist_function_transition": attr.label(default = "@bazel_tools//tools/whitelists/function_transition_whitelist"),
}
)
EOF
@@ -103,4 +103,109 @@
}
+# Test using a bad whitelist value
+#
+# repo structure:
+# ${WORKSPACE_DIR}/
+# vinegar/
+# rules.bzl
+# rule_with_transition
+# BUILD
+#
+# The whitelist for starlark transitions is set to a package group of "//..."
+function test_whitelist_bad_value() {
+ mkdir -p vinegar
+ cat > vinegar/rules.bzl <<EOF
+def _my_transition_impl(settings, attr):
+ _ignore = (settings, attr)
+ return {'//command_line_option:test_arg': ['tapatio']}
+my_transition = transition(
+ implementation = _my_transition_impl,
+ inputs = [],
+ outputs = ["//command_line_option:test_arg"]
+)
+def _rule_with_transition_impl(ctx):
+ return []
+rule_with_transition = rule(
+ implementation = _rule_with_transition_impl,
+ cfg = my_transition,
+ attrs = {
+ "_whitelist_function_transition": attr.label(default = "@bazel_tools//tools/whitelists/bad"),
+ }
+)
+EOF
+ cat > vinegar/BUILD<<EOF
+load("//vinegar:rules.bzl", "rule_with_transition")
+rule_with_transition(
+ name = "vinegar",
+)
+EOF
+
+ bazel build //vinegar --experimental_starlark_config_transitions \
+ >& $TEST_log && fail "Expected failure"
+ expect_log "_whitelist_function_transition attribute (@bazel_tools//tools/whitelists/bad:bad)"
+ expect_log "does not have the expected value //tools/whitelists/function_transition_whitelist:function_transition_whitelist"
+}
+
+
+# Test using their own repo's whitelist value
+#
+# repo structure:
+# ${WORKSPACE_DIR}/
+# vinegar/
+# rules.bzl
+# rule_with_transition
+# BUILD
+#
+# The whitelist for starlark transitions is set to a package group of "//..."
+function test_whitelist_own_rep() {
+ mkdir -p tools/whitelists/function_transition_whitelist
+ cat > tools/whitelists/function_transition_whitelist/BUILD <<EOF
+package_group(
+ name = "function_transition_whitelist",
+ packages = ["//vinegar/..."],
+)
+
+filegroup(
+ name = "srcs",
+ srcs = glob(["**"]),
+ visibility = ["//tools/whitelists:__pkg__"],
+)
+EOF
+
+ mkdir -p vinegar
+ cat > vinegar/rules.bzl <<EOF
+def _my_transition_impl(settings, attr):
+ _ignore = (settings, attr)
+ return {'//command_line_option:test_arg': ['tapatio']}
+my_transition = transition(
+ implementation = _my_transition_impl,
+ inputs = [],
+ outputs = ["//command_line_option:test_arg"]
+)
+def _rule_with_transition_impl(ctx):
+ return []
+rule_with_transition = rule(
+ implementation = _rule_with_transition_impl,
+ attrs = {
+ "dep" : attr.label(cfg = my_transition),
+ "_whitelist_function_transition": attr.label(default = "@//tools/whitelists/function_transition_whitelist"),
+ }
+)
+EOF
+ cat > vinegar/BUILD<<EOF
+load("//vinegar:rules.bzl", "rule_with_transition")
+rule_with_transition(
+ name = "vinegar",
+ dep = ":acetic-acid"
+)
+rule_with_transition(name = "acetic-acid")
+EOF
+
+ bazel cquery "deps(//vinegar)" --test_arg=hotlanta --transitions=full \
+ --noimplicit_deps --experimental_starlark_config_transitions \
+ >& $TEST_log || fail "failed to query //vinegar"
+ expect_log "test_arg:\[hotlanta\] -> \[\[\"tapatio\"\]\]"
+}
+
run_suite "whitelist tests"
