Skip to content

Commit

Permalink
fix(whl_library): only add group machinery when it is needed (bazelbu…
Browse files Browse the repository at this point in the history 
…ild#1822)

This removes the `alias` additions when the package groups are not used
fixing alias that attempt to traverse the entire py_library dependency
tree.

I noticed that for some code that I want to write for `whl_library`
having it explicit whether we are using groups or not in the `pip.parse`
and or `whl_library` makes the code easier to understand and write.

Fixes bazelbuild#1760
  • Loading branch information
@aignas
aignas committed Mar 27, 2024
1 parent e8039db commit 5631f05
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 106 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ A brief description of the categories of changes:
* (gazelle) In `project` or `package` generation modes, do not generate `py_test`
rules when there are no test files and do not set `main = "__test__.py"` when
that file doesn't exist.
* (whl_library) The group redirection is only added when the package is part of
the group potentially fixing aspects that want to traverse a `py_library` graph.
Fixes [#1760](https://github.com/bazelbuild/rules_python/issues/1760).

### Added

Expand Down
49 changes: 24 additions & 25 deletions python/pip_install/private/generate_whl_library_build_bazel.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,14 @@ filegroup(
)
filegroup(
name = "{whl_file_impl_label}",
name = "{whl_file_label}",
srcs = ["{whl_name}"],
data = {whl_file_deps},
visibility = {impl_vis},
)
py_library(
name = "{py_library_impl_label}",
name = "{py_library_label}",
srcs = glob(
["site-packages/**/*.py"],
exclude={srcs_exclude},
Expand All @@ -89,16 +89,6 @@ py_library(
tags = {tags},
visibility = {impl_vis},
)
alias(
name = "{py_library_public_label}",
actual = "{py_library_actual_label}",
)
alias(
name = "{whl_file_public_label}",
actual = "{whl_file_actual_label}",
)
"""

def _plat_label(plat):
Expand Down Expand Up @@ -353,36 +343,45 @@ def generate_whl_library_build_bazel(
# implementation.
if group_name:
group_repo = repo_prefix + "_groups"
library_impl_label = "@%s//:%s_%s" % (group_repo, normalize_name(group_name), PY_LIBRARY_PUBLIC_LABEL)
whl_impl_label = "@%s//:%s_%s" % (group_repo, normalize_name(group_name), WHEEL_FILE_PUBLIC_LABEL)
impl_vis = "@%s//:__pkg__" % (group_repo,)
label_tmpl = "\"@{}//:{}_{{}}\"".format(group_repo, normalize_name(group_name))
impl_vis = ["@{}//:__pkg__".format(group_repo)]
additional_content.extend([
"",
render.alias(
name = PY_LIBRARY_PUBLIC_LABEL,
actual = label_tmpl.format(PY_LIBRARY_PUBLIC_LABEL),
),
"",
render.alias(
name = WHEEL_FILE_PUBLIC_LABEL,
actual = label_tmpl.format(WHEEL_FILE_PUBLIC_LABEL),
),
])
py_library_label = PY_LIBRARY_IMPL_LABEL
whl_file_label = WHEEL_FILE_IMPL_LABEL

else:
library_impl_label = PY_LIBRARY_IMPL_LABEL
whl_impl_label = WHEEL_FILE_IMPL_LABEL
impl_vis = "//visibility:private"
py_library_label = PY_LIBRARY_PUBLIC_LABEL
whl_file_label = WHEEL_FILE_PUBLIC_LABEL
impl_vis = ["//visibility:public"]

contents = "\n".join(
[
_BUILD_TEMPLATE.format(
loads = "\n".join(loads),
py_library_public_label = PY_LIBRARY_PUBLIC_LABEL,
py_library_impl_label = PY_LIBRARY_IMPL_LABEL,
py_library_actual_label = library_impl_label,
py_library_label = py_library_label,
dependencies = render.indent(lib_dependencies, " " * 4).lstrip(),
whl_file_deps = render.indent(whl_file_deps, " " * 4).lstrip(),
data_exclude = repr(_data_exclude),
whl_name = whl_name,
whl_file_public_label = WHEEL_FILE_PUBLIC_LABEL,
whl_file_impl_label = WHEEL_FILE_IMPL_LABEL,
whl_file_actual_label = whl_impl_label,
whl_file_label = whl_file_label,
tags = repr(tags),
data_label = DATA_LABEL,
dist_info_label = DIST_INFO_LABEL,
entry_point_prefix = WHEEL_ENTRY_POINT_PREFIX,
srcs_exclude = repr(srcs_exclude),
data = repr(data),
impl_vis = repr([impl_vis]),
impl_vis = repr(impl_vis),
),
] + additional_content,
)
Expand Down
35 changes: 20 additions & 15 deletions python/private/bzlmod/pip.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,23 +152,27 @@ def _create_whl_repos(module_ctx, pip_attr, whl_map, whl_overrides):
for mod, whl_name in pip_attr.whl_modifications.items():
whl_modifications[whl_name] = mod

requirement_cycles = {
name: [normalize_name(whl_name) for whl_name in whls]
for name, whls in pip_attr.experimental_requirement_cycles.items()
}
if pip_attr.experimental_requirement_cycles:
requirement_cycles = {
name: [normalize_name(whl_name) for whl_name in whls]
for name, whls in pip_attr.experimental_requirement_cycles.items()
}

whl_group_mapping = {
whl_name: group_name
for group_name, group_whls in requirement_cycles.items()
for whl_name in group_whls
}
whl_group_mapping = {
whl_name: group_name
for group_name, group_whls in requirement_cycles.items()
for whl_name in group_whls
}

group_repo = "%s__groups" % (pip_name,)
group_library(
name = group_repo,
repo_prefix = pip_name + "_",
groups = pip_attr.experimental_requirement_cycles,
)
group_repo = "%s__groups" % (pip_name,)
group_library(
name = group_repo,
repo_prefix = pip_name + "_",
groups = pip_attr.experimental_requirement_cycles,
)
else:
whl_group_mapping = {}
requirement_cycles = {}

# Create a new wheel library for each of the different whls
for whl_name, requirement_line in requirements:
Expand All @@ -177,6 +181,7 @@ def _create_whl_repos(module_ctx, pip_attr, whl_map, whl_overrides):
# to.
annotation = whl_modifications.get(whl_name)
whl_name = normalize_name(whl_name)

group_name = whl_group_mapping.get(whl_name)
group_deps = requirement_cycles.get(group_name, [])

Expand Down
92 changes: 26 additions & 66 deletions tests/pip_install/whl_library/generate_build_bazel_tests.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,17 @@ filegroup(
)
filegroup(
name = "_whl",
name = "whl",
srcs = ["foo.whl"],
data = [
"@pypi_bar_baz//:whl",
"@pypi_foo//:whl",
],
visibility = ["//visibility:private"],
visibility = ["//visibility:public"],
)
py_library(
name = "_pkg",
name = "pkg",
srcs = glob(
["site-packages/**/*.py"],
exclude=[],
Expand All @@ -67,17 +67,7 @@ py_library(
"@pypi_foo//:pkg",
],
tags = ["tag1", "tag2"],
visibility = ["//visibility:private"],
)
alias(
name = "pkg",
actual = "_pkg",
)
alias(
name = "whl",
actual = "_whl",
visibility = ["//visibility:public"],
)
"""
actual = generate_whl_library_build_bazel(
Expand Down Expand Up @@ -113,7 +103,7 @@ filegroup(
)
filegroup(
name = "_whl",
name = "whl",
srcs = ["foo.whl"],
data = [
"@pypi_bar_baz//:whl",
Expand All @@ -130,11 +120,11 @@ filegroup(
"//conditions:default": [],
},
),
visibility = ["//visibility:private"],
visibility = ["//visibility:public"],
)
py_library(
name = "_pkg",
name = "pkg",
srcs = glob(
["site-packages/**/*.py"],
exclude=[],
Expand Down Expand Up @@ -165,17 +155,7 @@ py_library(
},
),
tags = ["tag1", "tag2"],
visibility = ["//visibility:private"],
)
alias(
name = "pkg",
actual = "_pkg",
)
alias(
name = "whl",
actual = "_whl",
visibility = ["//visibility:public"],
)
config_setting(
Expand Down Expand Up @@ -275,17 +255,17 @@ filegroup(
)
filegroup(
name = "_whl",
name = "whl",
srcs = ["foo.whl"],
data = [
"@pypi_bar_baz//:whl",
"@pypi_foo//:whl",
],
visibility = ["//visibility:private"],
visibility = ["//visibility:public"],
)
py_library(
name = "_pkg",
name = "pkg",
srcs = glob(
["site-packages/**/*.py"],
exclude=["srcs_exclude_all"],
Expand All @@ -305,17 +285,7 @@ py_library(
"@pypi_foo//:pkg",
],
tags = ["tag1", "tag2"],
visibility = ["//visibility:private"],
)
alias(
name = "pkg",
actual = "_pkg",
)
alias(
name = "whl",
actual = "_whl",
visibility = ["//visibility:public"],
)
copy_file(
Expand Down Expand Up @@ -373,17 +343,17 @@ filegroup(
)
filegroup(
name = "_whl",
name = "whl",
srcs = ["foo.whl"],
data = [
"@pypi_bar_baz//:whl",
"@pypi_foo//:whl",
],
visibility = ["//visibility:private"],
visibility = ["//visibility:public"],
)
py_library(
name = "_pkg",
name = "pkg",
srcs = glob(
["site-packages/**/*.py"],
exclude=[],
Expand All @@ -403,17 +373,7 @@ py_library(
"@pypi_foo//:pkg",
],
tags = ["tag1", "tag2"],
visibility = ["//visibility:private"],
)
alias(
name = "pkg",
actual = "_pkg",
)
alias(
name = "whl",
actual = "_whl",
visibility = ["//visibility:public"],
)
py_binary(
Expand Down Expand Up @@ -502,16 +462,6 @@ py_library(
visibility = ["@pypi__groups//:__pkg__"],
)
alias(
name = "pkg",
actual = "@pypi__groups//:qux_pkg",
)
alias(
name = "whl",
actual = "@pypi__groups//:qux_whl",
)
config_setting(
name = "is_linux_x86_64",
constraint_values = [
Expand All @@ -520,6 +470,16 @@ config_setting(
],
visibility = ["//visibility:private"],
)
alias(
name = "pkg",
actual = "@pypi__groups//:qux_pkg",
)
alias(
name = "whl",
actual = "@pypi__groups//:qux_whl",
)
"""
actual = generate_whl_library_build_bazel(
repo_prefix = "pypi_",
Expand Down

0 comments on commit 5631f05

Please sign in to comment.