From 306beafde413ce137f2bc50c3f3bcfb75a2f3792 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 23 Mar 2024 23:23:45 +0900 Subject: [PATCH] fix(whl_library): only add group machinery when it is needed 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 #1760 --- CHANGELOG.md | 3 + .../generate_whl_library_build_bazel.bzl | 49 +++++----- python/private/bzlmod/pip.bzl | 35 ++++--- .../generate_build_bazel_tests.bzl | 92 ++++++------------- 4 files changed, 73 insertions(+), 106 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff61b2be2c..436e607bbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/python/pip_install/private/generate_whl_library_build_bazel.bzl b/python/pip_install/private/generate_whl_library_build_bazel.bzl index 19650d16d7..b1219096ba 100644 --- a/python/pip_install/private/generate_whl_library_build_bazel.bzl +++ b/python/pip_install/private/generate_whl_library_build_bazel.bzl @@ -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}, @@ -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): @@ -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, ) diff --git a/python/private/bzlmod/pip.bzl b/python/private/bzlmod/pip.bzl index a017089803..13d1fa3842 100644 --- a/python/private/bzlmod/pip.bzl +++ b/python/private/bzlmod/pip.bzl @@ -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: @@ -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, []) diff --git a/tests/pip_install/whl_library/generate_build_bazel_tests.bzl b/tests/pip_install/whl_library/generate_build_bazel_tests.bzl index 72423aaec4..11611b9a4a 100644 --- a/tests/pip_install/whl_library/generate_build_bazel_tests.bzl +++ b/tests/pip_install/whl_library/generate_build_bazel_tests.bzl @@ -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=[], @@ -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( @@ -113,7 +103,7 @@ filegroup( ) filegroup( - name = "_whl", + name = "whl", srcs = ["foo.whl"], data = [ "@pypi_bar_baz//:whl", @@ -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=[], @@ -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( @@ -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"], @@ -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( @@ -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=[], @@ -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( @@ -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 = [ @@ -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_",