Skip to content

feat: support per-wheel enable_implicit_namespace_pkgs in whl_mods (WIP) #3279

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+95 −1
Open

feat: support per-wheel enable_implicit_namespace_pkgs in whl_mods (WIP) #3279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a284239
one-shot attempt
chrisirhc Sep 18, 2025
508b6a1
working tests
chrisirhc Sep 18, 2025
59d75f4
simplify tes
chrisirhc Sep 18, 2025
2de4153
make tests pass
chrisirhc Sep 18, 2025
1e187c4
attempt to use whl_mod approach
chrisirhc Sep 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions python/private/pypi/extension.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ def _whl_mods_impl(whl_mods_dict):
data = mods.data,
data_exclude_glob = mods.data_exclude_glob,
srcs_exclude_glob = mods.srcs_exclude_glob,
enable_implicit_namespace_pkgs = mods.enable_implicit_namespace_pkgs,
))

_whl_mods_repo(
Expand Down Expand Up @@ -178,6 +179,7 @@ You cannot use both the additive_build_content and additive_build_content_file a
data = whl_mod.data,
data_exclude_glob = whl_mod.data_exclude_glob,
srcs_exclude_glob = whl_mod.srcs_exclude_glob,
enable_implicit_namespace_pkgs = whl_mod.enable_implicit_namespace_pkgs,
)

config = build_config(module_ctx = module_ctx, enable_pipstar = enable_pipstar)
Expand Down Expand Up @@ -741,6 +743,12 @@ cannot have a child module that uses the same `hub_name`.
doc = "The whl name that the modifications are used for.",
mandatory = True,
),
"enable_implicit_namespace_pkgs": attr.bool(
doc = """\
(bool, optional): Override the global setting for generating __init__.py
files for namespace packages. If None, uses the repository-level setting.
""",
),
Comment on lines +746 to +751
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using attr.bool here will not work as intended. The goal is to have a tri-state value (True, False, or not set to use the global default), but attr.bool only supports True and False. If a user does not specify this attribute, it will default to False, incorrectly overriding the global setting.

To correctly implement the tri-state logic, I suggest using attr.string with a set of allowed values, for example "True", "False", and "auto" (for the default when not set).

You will also need to update the code that consumes this attribute to handle string values, which I've commented on in python/private/pypi/generate_whl_library_build_bazel.bzl.

        "enable_implicit_namespace_pkgs": attr.string(
            doc = """\
(string, optional): Override the global setting for generating __init__.py
files for namespace packages. Can be "True", "False", or "auto".
If "auto", uses the repository-level setting.
""",
            default = "auto",
            values = ["True", "False", "auto"],
        ),

}
return attrs

Expand Down
2 changes: 2 additions & 0 deletions python/private/pypi/generate_whl_library_build_bazel.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ def generate_whl_library_build_bazel(
kwargs["copy_executables"] = annotation.copy_executables
kwargs["data_exclude"] = kwargs.get("data_exclude", []) + annotation.data_exclude_glob
kwargs["srcs_exclude"] = annotation.srcs_exclude_glob
if hasattr(annotation, "enable_implicit_namespace_pkgs") and annotation.enable_implicit_namespace_pkgs != None:
kwargs["enable_implicit_namespace_pkgs"] = annotation.enable_implicit_namespace_pkgs
Comment on lines +112 to +113
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

To accommodate the change from attr.bool to attr.string for enable_implicit_namespace_pkgs in whl_mods, and to correctly handle values from package_annotation (which can be boolean or None), this logic needs to be updated.

The new logic should check for None (from package_annotation), "auto" (the new default for whl_mods), and then convert string "True"/"False" or boolean True/False to the final boolean value that whl_library_targets expects.

Suggested change
if hasattr(annotation, "enable_implicit_namespace_pkgs") and annotation.enable_implicit_namespace_pkgs != None:
kwargs["enable_implicit_namespace_pkgs"] = annotation.enable_implicit_namespace_pkgs
val = getattr(annotation, "enable_implicit_namespace_pkgs", None)
if val == True or val == "True":
kwargs["enable_implicit_namespace_pkgs"] = True
elif val == False or val == "False":
kwargs["enable_implicit_namespace_pkgs"] = False

if annotation.additive_build_content:
additional_content.append(annotation.additive_build_content)
if default_python_version:
Expand Down
6 changes: 5 additions & 1 deletion python/private/pypi/package_annotation.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ def package_annotation(
copy_executables = {},
data = [],
data_exclude_glob = [],
srcs_exclude_glob = []):
srcs_exclude_glob = [],
enable_implicit_namespace_pkgs = None):
"""Annotations to apply to the BUILD file content from package generated from a `pip_repository` rule.

[cf]: https://github.com/bazelbuild/bazel-skylib/blob/main/docs/copy_file_doc.md
Expand All @@ -35,6 +36,8 @@ def package_annotation(
data_exclude_glob (list, optional): A list of exclude glob patterns to add as `data` to the generated
`py_library` target.
srcs_exclude_glob (list, optional): A list of labels to add as `srcs` to the generated `py_library` target.
enable_implicit_namespace_pkgs (bool, optional): Override the global setting for generating __init__.py
files for namespace packages. If None, uses the repository-level setting.
Comment on lines +39 to +40
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The type hint (bool, optional) is slightly misleading as the function also accepts None to use the repository-level setting. It would be clearer to specify that None is a valid type.

Suggested change
enable_implicit_namespace_pkgs (bool, optional): Override the global setting for generating __init__.py
files for namespace packages. If None, uses the repository-level setting.
enable_implicit_namespace_pkgs (bool | None, optional): Override the global setting for generating __init__.py
files for namespace packages. If None, uses the repository-level setting.


Returns:
str: A json encoded string of the provided content.
Expand All @@ -46,4 +49,5 @@ def package_annotation(
data = data,
data_exclude_glob = data_exclude_glob,
srcs_exclude_glob = srcs_exclude_glob,
enable_implicit_namespace_pkgs = enable_implicit_namespace_pkgs,
))
77 changes: 77 additions & 0 deletions tests/pypi/extension/extension_tests.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,20 @@ simple==0.0.1 \
],
)

def _whl_mod(*, hub_name, whl_name, additive_build_content = None, additive_build_content_file = None, copy_executables = {}, copy_files = {}, data = [], data_exclude_glob = [], srcs_exclude_glob = [], enable_implicit_namespace_pkgs = None):
return struct(
hub_name = hub_name,
whl_name = whl_name,
additive_build_content = additive_build_content,
additive_build_content_file = additive_build_content_file,
copy_executables = copy_executables,
copy_files = copy_files,
data = data,
data_exclude_glob = data_exclude_glob,
srcs_exclude_glob = srcs_exclude_glob,
enable_implicit_namespace_pkgs = enable_implicit_namespace_pkgs,
)
Comment on lines +52 to +64
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To align with the suggested change of using attr.string for enable_implicit_namespace_pkgs, this test helper should be updated to use string values. The default should be "auto".

Suggested change
def _whl_mod(*, hub_name, whl_name, additive_build_content = None, additive_build_content_file = None, copy_executables = {}, copy_files = {}, data = [], data_exclude_glob = [], srcs_exclude_glob = [], enable_implicit_namespace_pkgs = None):
return struct(
hub_name = hub_name,
whl_name = whl_name,
additive_build_content = additive_build_content,
additive_build_content_file = additive_build_content_file,
copy_executables = copy_executables,
copy_files = copy_files,
data = data,
data_exclude_glob = data_exclude_glob,
srcs_exclude_glob = srcs_exclude_glob,
enable_implicit_namespace_pkgs = enable_implicit_namespace_pkgs,
)
def _whl_mod(*, hub_name, whl_name, additive_build_content = None, additive_build_content_file = None, copy_executables = {}, copy_files = {}, data = [], data_exclude_glob = [], srcs_exclude_glob = [], enable_implicit_namespace_pkgs = "auto"):
return struct(
hub_name = hub_name,
whl_name = whl_name,
additive_build_content = additive_build_content,
additive_build_content_file = additive_build_content_file,
copy_executables = copy_executables,
copy_files = copy_files,
data = data,
data_exclude_glob = data_exclude_glob,
srcs_exclude_glob = srcs_exclude_glob,
enable_implicit_namespace_pkgs = enable_implicit_namespace_pkgs,
)


def _mod(*, name, default = [], parse = [], override = [], whl_mods = [], is_root = True):
return struct(
name = name,
Expand Down Expand Up @@ -234,6 +248,69 @@ def _test_build_pipstar_platform(env):

_tests.append(_test_build_pipstar_platform)

def _test_whl_mods_with_namespace_pkgs(env):
pypi = _parse_modules(
env,
module_ctx = _mock_mctx(
_mod(
name = "rules_python",
parse = [
_parse(
hub_name = "pypi",
python_version = "3.15",
requirements_lock = "requirements.txt",
),
],
whl_mods = [
_whl_mod(
hub_name = "pypi",
whl_name = "simple",
additive_build_content = "# Custom build content",
enable_implicit_namespace_pkgs = True,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To align with the suggested change of using attr.string for enable_implicit_namespace_pkgs, this should be a string.

Suggested change
enable_implicit_namespace_pkgs = True,
enable_implicit_namespace_pkgs = "True",

),
],
),
),
available_interpreters = {
"python_3_15_host": "unit_test_interpreter_target",
},
minor_mapping = {"3.15": "3.15.19"},
)

pypi.exposed_packages().contains_exactly({"pypi": ["simple"]})
pypi.hub_group_map().contains_exactly({"pypi": {}})
pypi.hub_whl_map().contains_exactly({"pypi": {
"simple": {
"pypi_315_simple": [
whl_config_setting(
version = "3.15",
),
],
},
}})
pypi.whl_libraries().contains_exactly({
"pypi_315_simple": {
"dep_template": "@pypi//{name}:{target}",
"python_interpreter_target": "unit_test_interpreter_target",
"requirement": "simple==0.0.1 --hash=sha256:deadbeef --hash=sha256:deadbaaf",
},
})
pypi.whl_mods().contains_exactly({
"pypi": {
"simple": struct(
build_content = "# Custom build content",
copy_files = {},
copy_executables = {},
data = [],
data_exclude_glob = [],
srcs_exclude_glob = [],
enable_implicit_namespace_pkgs = True,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The parse_modules function passes the attribute value through without conversion. With the suggested change to attr.string, this assertion should check for a string value.

Suggested change
enable_implicit_namespace_pkgs = True,
enable_implicit_namespace_pkgs = "True",

),
},
})

_tests.append(_test_whl_mods_with_namespace_pkgs)

def extension_test_suite(name):
"""Create the test suite.

Expand Down
3 changes: 3 additions & 0 deletions tests/pypi/generate_whl_library_build_bazel/generate_whl_library_build_bazel_tests.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ whl_library_targets_from_requires(
"data_exclude_all",
],
dep_template = "@pypi//{name}:{target}",
enable_implicit_namespace_pkgs = True,
entry_points = {
"foo": "bar.py",
},
Expand Down Expand Up @@ -139,6 +140,7 @@ whl_library_targets_from_requires(
data_exclude_glob = ["data_exclude_all"],
srcs_exclude_glob = ["srcs_exclude_all"],
additive_build_content = """# SOMETHING SPECIAL AT THE END""",
enable_implicit_namespace_pkgs = True,
),
group_name = "qux",
group_deps = ["foo", "fox", "qux"],
Expand Down Expand Up @@ -211,6 +213,7 @@ whl_library_targets_from_requires(

_tests.append(_test_all_with_loads)


def generate_whl_library_build_bazel_test_suite(name):
"""Create the test suite.

Expand Down