fix: Allow passing a tuple to the `tags` attribute. (#1191)
Starlark rules allow giving the tags as a tuple. The helper function
that added the special migration tag assumed tags was always a list,
resulting in an error when it tried to concatenate a list and tuple.
To fix, check if tags is a tuple and concatenate a tuple if so. The
input type of the tags attribute is preserved so that a test verifying
tags can be passed to the underlying rule can be implemented (this test
is to verify there isn't a regression during the rewrite to Starlark).
diff --git a/docs/BUILD.bazel b/docs/BUILD.bazel
index 27af3e7..938ba85 100644
--- a/docs/BUILD.bazel
+++ b/docs/BUILD.bazel
@@ -80,6 +80,9 @@
"//python/private:stamp.bzl",
"//python/private:util.bzl",
],
+ deps = [
+ "//python/private:util_bzl",
+ ],
)
# TODO: Stardoc does not guarantee consistent outputs accross platforms (Unix/Windows).
diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel
index 4068ea4..f454f42 100644
--- a/python/private/BUILD.bazel
+++ b/python/private/BUILD.bazel
@@ -44,7 +44,11 @@
bzl_library(
name = "util_bzl",
srcs = ["util.bzl"],
- visibility = ["//python:__subpackages__"],
+ visibility = [
+ "//docs:__subpackages__",
+ "//python:__subpackages__",
+ ],
+ deps = ["@bazel_skylib//lib:types"],
)
# @bazel_tools can't define bzl_library itself, so we just put a wrapper around it.
diff --git a/python/private/util.bzl b/python/private/util.bzl
index 25a50aa..f0d4373 100644
--- a/python/private/util.bzl
+++ b/python/private/util.bzl
@@ -1,5 +1,7 @@
"""Functionality shared by multiple pieces of code."""
+load("@bazel_skylib//lib:types.bzl", "types")
+
def copy_propagating_kwargs(from_kwargs, into_kwargs = None):
"""Copies args that must be compatible between two targets with a dependency relationship.
@@ -36,8 +38,23 @@
_MIGRATION_TAG = "__PYTHON_RULES_MIGRATION_DO_NOT_USE_WILL_BREAK__"
def add_migration_tag(attrs):
+ """Add a special tag to `attrs` to aid migration off native rles.
+
+ Args:
+ attrs: dict of keyword args. The `tags` key will be modified in-place.
+
+ Returns:
+ The same `attrs` object, but modified.
+ """
if "tags" in attrs and attrs["tags"] != None:
- attrs["tags"] = attrs["tags"] + [_MIGRATION_TAG]
+ tags = attrs["tags"]
+
+ # Preserve the input type: this allows a test verifying the underlying
+ # rule can accept the tuple for the tags argument.
+ if types.is_tuple(tags):
+ attrs["tags"] = tags + (_MIGRATION_TAG,)
+ else:
+ attrs["tags"] = tags + [_MIGRATION_TAG]
else:
attrs["tags"] = [_MIGRATION_TAG]
return attrs
diff --git a/tools/build_defs/python/tests/base_tests.bzl b/tools/build_defs/python/tests/base_tests.bzl
index 715aea7..467611f 100644
--- a/tools/build_defs/python/tests/base_tests.bzl
+++ b/tools/build_defs/python/tests/base_tests.bzl
@@ -15,7 +15,7 @@
load("@rules_testing//lib:analysis_test.bzl", "analysis_test")
load("@rules_testing//lib:truth.bzl", "matching")
-load("@rules_testing//lib:util.bzl", rt_util = "util")
+load("@rules_testing//lib:util.bzl", "PREVENT_IMPLICIT_BUILDING_TAGS", rt_util = "util")
load("//python:defs.bzl", "PyInfo")
load("//tools/build_defs/python/tests:py_info_subject.bzl", "py_info_subject")
load("//tools/build_defs/python/tests:util.bzl", pt_util = "util")
@@ -99,5 +99,26 @@
_tests.append(_test_data_sets_uses_shared_library)
+def _test_tags_can_be_tuple(name, config):
+ # We don't use a helper because we want to ensure that value passed is
+ # a tuple.
+ config.base_test_rule(
+ name = name + "_subject",
+ tags = ("one", "two") + tuple(PREVENT_IMPLICIT_BUILDING_TAGS),
+ )
+ analysis_test(
+ name = name,
+ target = name + "_subject",
+ impl = _test_tags_can_be_tuple_impl,
+ )
+
+def _test_tags_can_be_tuple_impl(env, target):
+ env.expect.that_target(target).tags().contains_at_least([
+ "one",
+ "two",
+ ])
+
+_tests.append(_test_tags_can_be_tuple)
+
def create_base_tests(config):
return pt_util.create_tests(_tests, config = config)
