feat(pip_parse): support referencing dependencies to packages via hub

aignas · aignas · commit d3aa5355a403 · 2024-04-17T17:59:10.000+09:00
With this change we can in theory have multi-platform libraries in the dependency cycle and use the pip hub repo for the dependencies. With this we can also make the contents of `whl_library` not depend on what platform the actual dependencies are. This allows us to support the following topologies: * A platform-specific wheel depends on cross-platform wheel. * A cross-platform wheel depends on cross-platform wheel. * A whl_library can have `select` dependencies based on the interpreter version, e.g. pull in a `tomli` dependency only when the Python interpreter is less than 3.11. Relates to bazel-contrib#1663. Work towards bazel-contrib#735.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -67,6 +67,13 @@ A brief description of the categories of changes:
   downloader. If you see any issues, report in
   [#1357](https://github.com/bazelbuild/rules_python/issues/1357). The URLs for
   the whl and sdist files will be written to the lock file.
+* (pip_parse): A new flag `use_hub_alias_dependencies` has been added that is going
+  to become default in the next release. This makes use of `dep_template` flag
+  in the `whl_library` rule. This also affects the
+  `experimental_requirement_cycles` feature where the dependencies that are in
+  a group would be only accessible via the hub repo aliases. If you still
+  depend on legacy labels instead of the hub repo aliases and you use the
+  `experimental_requirement_cycles`, now is a good time to migrate.
 
 [0.XX.0]: https://github.com/bazelbuild/rules_python/releases/tag/0.XX.0
 [python_default_visibility]: gazelle/README.md#directive-python_default_visibility
diff --git a/python/pip_install/pip_repository.bzl b/python/pip_install/pip_repository.bzl
@@ -365,9 +365,12 @@ def _pip_repository_impl(rctx):
         "python_interpreter": _get_python_interpreter_attr(rctx),
         "quiet": rctx.attr.quiet,
         "repo": rctx.attr.name,
-        "repo_prefix": "{}_".format(rctx.attr.name),
         "timeout": rctx.attr.timeout,
     }
+    if rctx.attr.use_hub_alias_dependencies:
+        config["dep_template"] = "@{}//{{name}}:{{target}}".format(rctx.attr.name)
+    else:
+        config["repo_prefix"] = "{}_".format(rctx.attr.name)
 
     if rctx.attr.python_interpreter_target:
         config["python_interpreter_target"] = str(rctx.attr.python_interpreter_target)
@@ -387,6 +390,13 @@ def _pip_repository_impl(rctx):
 
     rctx.file("BUILD.bazel", _BUILD_FILE_CONTENTS)
     rctx.template("requirements.bzl", rctx.attr._template, substitutions = {
+        "    # %%GROUP_LIBRARY%%": """\
+    group_repo = "{name}__groups"
+    group_library(
+        name = group_repo,
+        repo_prefix = "{name}_",
+        groups = all_requirement_groups,
+    )""".format(name = rctx.attr.name) if not rctx.attr.use_hub_alias_dependencies else "",
         "%%ALL_DATA_REQUIREMENTS%%": _format_repr_list([
             macro_tmpl.format(p, "data")
             for p in bzl_packages
@@ -595,6 +605,8 @@ python_interpreter. An example value: "@python3_x86_64-unknown-linux-gnu//:pytho
     "repo_prefix": attr.string(
         doc = """
 Prefix for the generated packages will be of the form `@<prefix><sanitized-package-name>//...`
+
+DEPRECATED. Only left for people who vendor requirements.bzl.
 """,
     ),
     # 600 is documented as default here: https://docs.bazel.build/versions/master/skylark/lib/repository_ctx.html#execute
@@ -637,6 +649,15 @@ attributes.
         allow_single_file = True,
         doc = "Override the requirements_lock attribute when the host platform is Windows",
     ),
+    "use_hub_alias_dependencies": attr.bool(
+        default = False,
+        doc = """\
+Controls if the hub alias dependencies are used. If set to true, then the
+group_library will be included in the hub repo.
+
+True will become default in a subsequent release.
+""",
+    ),
     "_template": attr.label(
         default = ":pip_repository_requirements.bzl.tmpl",
     ),
@@ -886,7 +907,7 @@ def _whl_library_impl(rctx):
         entry_points[entry_point_without_py] = entry_point_script_name
 
     build_file_contents = generate_whl_library_build_bazel(
-        repo_prefix = rctx.attr.repo_prefix,
+        dep_template = rctx.attr.dep_template or "@{}_{{name}}//:{{target}}".format(rctx.attr.repo_prefix),
         whl_name = whl_path.basename,
         dependencies = metadata["deps"],
         dependencies_by_platform = metadata["deps_by_platform"],
@@ -941,6 +962,13 @@ whl_library_attrs = dict({
         ),
         allow_files = True,
     ),
+    "dep_template": attr.string(
+        doc = """
+The dep template to use for referencing the dependencies. It should have `{name}`
+and `{target}` tokens that will be replaced with the normalized distribution name
+and the target that we need respectively.
+""",
+    ),
     "filename": attr.string(
         doc = "Download the whl file to this filename. Only used when the `urls` is passed. If not specified, will be auto-detected from the `urls`.",
     ),
diff --git a/python/pip_install/pip_repository_requirements.bzl.tmpl b/python/pip_install/pip_repository_requirements.bzl.tmpl
@@ -58,12 +58,7 @@ def install_deps(**whl_library_kwargs):
         for requirement in group_requirements
     }
 
-    group_repo = "%%NAME%%__groups"
-    group_library(
-        name = group_repo,
-        repo_prefix = "%%NAME%%_",
-        groups = all_requirement_groups,
-    )
+    # %%GROUP_LIBRARY%%
 
     # Install wheels which may be participants in a group
     whl_config = dict(_config)
diff --git a/python/pip_install/private/generate_group_library_build_bazel.bzl b/python/pip_install/private/generate_group_library_build_bazel.bzl
@@ -22,9 +22,10 @@ load(
     "WHEEL_FILE_PUBLIC_LABEL",
 )
 load("//python/private:normalize_name.bzl", "normalize_name")
+load("//python/private:text_util.bzl", "render")
 
 _PRELUDE = """\
-load("@rules_python//python:defs.bzl", "py_library", "py_binary")
+load("@rules_python//python:defs.bzl", "py_library")
 """
 
 _GROUP_TEMPLATE = """\
@@ -62,26 +63,39 @@ def _generate_group_libraries(repo_prefix, group_name, group_members):
           which make up the group.
     """
 
-    lib_dependencies = [
-        "@%s%s//:%s" % (repo_prefix, normalize_name(d), PY_LIBRARY_IMPL_LABEL)
-        for d in group_members
-    ]
-    whl_file_deps = [
-        "@%s%s//:%s" % (repo_prefix, normalize_name(d), WHEEL_FILE_IMPL_LABEL)
-        for d in group_members
-    ]
-    visibility = [
-        "@%s%s//:__pkg__" % (repo_prefix, normalize_name(d))
-        for d in group_members
-    ]
+    group_members = sorted(group_members)
+
+    if repo_prefix:
+        lib_dependencies = [
+            "@%s%s//:%s" % (repo_prefix, normalize_name(d), PY_LIBRARY_IMPL_LABEL)
+            for d in group_members
+        ]
+        whl_file_deps = [
+            "@%s%s//:%s" % (repo_prefix, normalize_name(d), WHEEL_FILE_IMPL_LABEL)
+            for d in group_members
+        ]
+        visibility = [
+            "@%s%s//:__pkg__" % (repo_prefix, normalize_name(d))
+            for d in group_members
+        ]
+    else:
+        lib_dependencies = [
+            "//%s:%s" % (normalize_name(d), PY_LIBRARY_IMPL_LABEL)
+            for d in group_members
+        ]
+        whl_file_deps = [
+            "//%s:%s" % (normalize_name(d), WHEEL_FILE_IMPL_LABEL)
+            for d in group_members
+        ]
+        visibility = ["//:__subpackages__"]
 
     return _GROUP_TEMPLATE.format(
         name = normalize_name(group_name),
         whl_public_label = WHEEL_FILE_PUBLIC_LABEL,
-        whl_deps = repr(whl_file_deps),
+        whl_deps = render.indent(render.list(whl_file_deps)).lstrip(),
         lib_public_label = PY_LIBRARY_PUBLIC_LABEL,
-        lib_deps = repr(lib_dependencies),
-        visibility = repr(visibility),
+        lib_deps = render.indent(render.list(lib_dependencies)).lstrip(),
+        visibility = render.indent(render.list(visibility)).lstrip(),
     )
 
 def generate_group_library_build_bazel(
diff --git a/python/pip_install/private/generate_whl_library_build_bazel.bzl b/python/pip_install/private/generate_whl_library_build_bazel.bzl
@@ -213,7 +213,7 @@ selects.config_setting_group(
 
 def generate_whl_library_build_bazel(
         *,
-        repo_prefix,
+        dep_template,
         whl_name,
         dependencies,
         dependencies_by_platform,
@@ -226,7 +226,7 @@ def generate_whl_library_build_bazel(
     """Generate a BUILD file for an unzipped Wheel
 
     Args:
-        repo_prefix: the repo prefix that should be used for dependency lists.
+        dep_template: the dependency template that should be used for dependency lists.
         whl_name: the whl_name that this is generated for.
         dependencies: a list of PyPI packages that are dependencies to the py_library.
         dependencies_by_platform: a dict[str, list] of PyPI packages that may vary by platform.
@@ -328,38 +328,49 @@ def generate_whl_library_build_bazel(
     lib_dependencies = _render_list_and_select(
         deps = dependencies,
         deps_by_platform = dependencies_by_platform,
-        tmpl = "@{}{{}}//:{}".format(repo_prefix, PY_LIBRARY_PUBLIC_LABEL),
+        tmpl = dep_template.format(name = "{}", target = PY_LIBRARY_PUBLIC_LABEL),
     )
 
     whl_file_deps = _render_list_and_select(
         deps = dependencies,
         deps_by_platform = dependencies_by_platform,
-        tmpl = "@{}{{}}//:{}".format(repo_prefix, WHEEL_FILE_PUBLIC_LABEL),
+        tmpl = dep_template.format(name = "{}", target = WHEEL_FILE_PUBLIC_LABEL),
     )
 
     # If this library is a member of a group, its public label aliases need to
     # point to the group implementation rule not the implementation rules. We
     # also need to mark the implementation rules as visible to the group
     # implementation.
-    if group_name:
-        group_repo = repo_prefix + "_groups"
-        label_tmpl = "\"@{}//:{}_{{}}\"".format(group_repo, normalize_name(group_name))
-        impl_vis = ["@{}//:__pkg__".format(group_repo)]
+    if group_name and "//:" in dep_template:
+        # This is the legacy behaviour where the group library is outside the hub repo
+        label_tmpl = dep_template.format(
+            name = "_groups",
+            target = normalize_name(group_name) + "_{}",
+        )
+        impl_vis = [dep_template.format(
+            name = "_groups",
+            target = "__pkg__",
+        )]
         additional_content.extend([
             "",
             render.alias(
                 name = PY_LIBRARY_PUBLIC_LABEL,
-                actual = label_tmpl.format(PY_LIBRARY_PUBLIC_LABEL),
+                actual = repr(label_tmpl.format(PY_LIBRARY_PUBLIC_LABEL)),
             ),
             "",
             render.alias(
                 name = WHEEL_FILE_PUBLIC_LABEL,
-                actual = label_tmpl.format(WHEEL_FILE_PUBLIC_LABEL),
+                actual = repr(label_tmpl.format(WHEEL_FILE_PUBLIC_LABEL)),
             ),
         ])
         py_library_label = PY_LIBRARY_IMPL_LABEL
         whl_file_label = WHEEL_FILE_IMPL_LABEL
 
+    elif group_name:
+        py_library_label = PY_LIBRARY_PUBLIC_LABEL
+        whl_file_label = WHEEL_FILE_PUBLIC_LABEL
+        impl_vis = [dep_template.format(name = "", target = "__subpackages__")]
+
     else:
         py_library_label = PY_LIBRARY_PUBLIC_LABEL
         whl_file_label = WHEEL_FILE_PUBLIC_LABEL
diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel
@@ -226,6 +226,7 @@ bzl_library(
         ":normalize_name_bzl",
         ":text_util_bzl",
         ":version_label_bzl",
+        "//python/pip_install/private:generate_group_library_build_bazel_bzl",
     ],
 )
 
diff --git a/python/private/bzlmod/pip.bzl b/python/private/bzlmod/pip.bzl
@@ -18,7 +18,6 @@ load("@bazel_features//:features.bzl", "bazel_features")
 load("@pythons_hub//:interpreters.bzl", "DEFAULT_PYTHON_VERSION", "INTERPRETER_LABELS")
 load(
     "//python/pip_install:pip_repository.bzl",
-    "group_library",
     "locked_requirements_label",
     "pip_repository_attrs",
     "use_isolated",
@@ -101,7 +100,7 @@ You cannot use both the additive_build_content and additive_build_content_file a
             whl_mods = whl_mods,
         )
 
-def _create_whl_repos(module_ctx, pip_attr, whl_map, whl_overrides, simpleapi_cache):
+def _create_whl_repos(module_ctx, pip_attr, whl_map, whl_overrides, group_map, simpleapi_cache):
     python_interpreter_target = pip_attr.python_interpreter_target
 
     # if we do not have the python_interpreter set in the attributes
@@ -129,6 +128,7 @@ def _create_whl_repos(module_ctx, pip_attr, whl_map, whl_overrides, simpleapi_ca
         hub_name,
         version_label(pip_attr.python_version),
     )
+    major_minor = _major_minor_version(pip_attr.python_version)
 
     requirements_lock = locked_requirements_label(module_ctx, pip_attr)
 
@@ -171,12 +171,11 @@ def _create_whl_repos(module_ctx, pip_attr, whl_map, whl_overrides, simpleapi_ca
             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,
-        )
+        # TODO @aignas 2024-04-05: how do we support different requirement
+        # cycles for different abis/oses? For now we will need the users to
+        # assume the same groups across all versions/platforms until we start
+        # using an alternative cycle resolution strategy.
+        group_map[hub_name] = pip_attr.experimental_requirement_cycles
     else:
         whl_group_mapping = {}
         requirement_cycles = {}
@@ -201,8 +200,6 @@ def _create_whl_repos(module_ctx, pip_attr, whl_map, whl_overrides, simpleapi_ca
             cache = simpleapi_cache,
         )
 
-    major_minor = _major_minor_version(pip_attr.python_version)
-
     # Create a new wheel library for each of the different whls
     for whl_name, requirement_line in requirements:
         # We are not using the "sanitized name" because the user
@@ -219,7 +216,7 @@ def _create_whl_repos(module_ctx, pip_attr, whl_map, whl_overrides, simpleapi_ca
         repo_name = "{}_{}".format(pip_name, whl_name)
         whl_library_args = dict(
             repo = pip_name,
-            repo_prefix = pip_name + "_",
+            dep_template = "@{}//{{name}}:{{target}}".format(hub_name),
             requirement = requirement_line,
         )
         maybe_args = dict(
@@ -421,6 +418,7 @@ def _pip_impl(module_ctx):
     # dict[hub, dict[whl, dict[version, str pip]]]
     # Where hub, whl, and pip are the repo names
     hub_whl_map = {}
+    hub_group_map = {}
 
     simpleapi_cache = {}
 
@@ -459,7 +457,7 @@ def _pip_impl(module_ctx):
             else:
                 pip_hub_map[pip_attr.hub_name].python_versions.append(pip_attr.python_version)
 
-            _create_whl_repos(module_ctx, pip_attr, hub_whl_map, whl_overrides, simpleapi_cache)
+            _create_whl_repos(module_ctx, pip_attr, hub_whl_map, whl_overrides, hub_group_map, simpleapi_cache)
 
     for hub_name, whl_map in hub_whl_map.items():
         pip_repository(
@@ -470,6 +468,7 @@ def _pip_impl(module_ctx):
                 for key, value in whl_map.items()
             },
             default_version = _major_minor_version(DEFAULT_PYTHON_VERSION),
+            groups = hub_group_map.get(hub_name),
         )
 
 def _pip_parse_ext_attrs():
diff --git a/python/private/bzlmod/pip_repository.bzl b/python/private/bzlmod/pip_repository.bzl
@@ -32,6 +32,7 @@ def _pip_repository_impl(rctx):
             for key, values in rctx.attr.whl_map.items()
         },
         default_version = rctx.attr.default_version,
+        requirement_cycles = rctx.attr.groups,
     )
     for path, contents in aliases.items():
         rctx.file(path, contents)
@@ -68,6 +69,9 @@ This is the default python version in the format of X.Y. This should match
 what is setup by the 'python' extension using the 'is_default = True'
 setting.""",
     ),
+    "groups": attr.string_list_dict(
+        mandatory = False,
+    ),
     "repo_name": attr.string(
         mandatory = True,
         doc = "The apparent name of the repo. This is needed because in bzlmod, the name attribute becomes the canonical name.",
diff --git a/python/private/render_pkg_aliases.bzl b/python/private/render_pkg_aliases.bzl
diff --git a/tests/pip_hub_repository/render_pkg_aliases/render_pkg_aliases_test.bzl b/tests/pip_hub_repository/render_pkg_aliases/render_pkg_aliases_test.bzl
diff --git a/tests/pip_install/group_library/generate_build_bazel_tests.bzl b/tests/pip_install/group_library/generate_build_bazel_tests.bzl
diff --git a/tests/pip_install/whl_library/generate_build_bazel_tests.bzl b/tests/pip_install/whl_library/generate_build_bazel_tests.bzl

Original file line number	Diff line number	Diff line change
`@@ -226,6 +226,7 @@ bzl_library(`
`226`	`226`	`":normalize_name_bzl",`
`227`	`227`	`":text_util_bzl",`
`228`	`228`	`":version_label_bzl",`
	`229`	`+ "//python/pip_install/private:generate_group_library_build_bazel_bzl",`
`229`	`230`	`],`
`230`	`231`	`)`
`231`	`232`