fix: Using canonical name in requirements.bzl (bazel-contrib#1176)

linzhp · web-flow · commit 1383bd4fa06c · 2023-05-17T17:08:33.000Z
When `incompatible_generate_aliases = False`, `pip.parse` doesn't work
in bzlmod, because requirements.bzl has:

```
all_requirements = ["@rules_python~0.21.0~pip~pip_yapf//:pkg"]

all_whl_requirements = ["@rules_python~0.21.0~pip~pip_yapf//:whl"]
```

Starting Bazel 6, canonical names should be referred with double "@".

The reason why `incompatible_generate_aliases = True` worked is because
it uses apparent name by parsing the canonical label with `repo_name =
rctx.attr.name.split("~")[-1]`. This is dangerous because Bazel may
change its canonical name pattern in future versions.

This PR adds a new attribute "repo_name" to `pip_repository_bzlmod`, so
we have access to the human-readable name in the implementation.
diff --git a/docs/pip_repository.md b/docs/pip_repository.md
diff --git a/examples/bzlmod/BUILD.bazel b/examples/bzlmod/BUILD.bazel
@@ -1,4 +1,5 @@
-load("@pip//:requirements.bzl", "requirement")
+load("@bazel_skylib//rules:build_test.bzl", "build_test")
+load("@pip//:requirements.bzl", "all_requirements", "all_whl_requirements", "requirement")
 load("@python3_9//:defs.bzl", py_test_with_transition = "py_test")
 load("@rules_python//python:defs.bzl", "py_binary", "py_library", "py_test")
 load("@rules_python//python:pip.bzl", "compile_pip_requirements")
@@ -43,3 +44,13 @@ py_test_with_transition(
     main = "test.py",
     deps = [":lib"],
 )
+
+build_test(
+    name = "all_wheels",
+    targets = all_whl_requirements,
+)
+
+build_test(
+    name = "all_requirements",
+    targets = all_requirements,
+)
diff --git a/examples/bzlmod/MODULE.bazel b/examples/bzlmod/MODULE.bazel
@@ -4,6 +4,7 @@ module(
     compatibility_level = 1,
 )
 
+bazel_dep(name = "bazel_skylib", version = "1.4.1")
 bazel_dep(name = "rules_python", version = "0.0.0")
 local_path_override(
     module_name = "rules_python",
@@ -26,6 +27,8 @@ register_toolchains(
 pip = use_extension("@rules_python//python/extensions:pip.bzl", "pip")
 pip.parse(
     name = "pip",
+    # Intentionally set it false because the "true" case is already covered by examples/bzlmod_build_file_generation
+    incompatible_generate_aliases = False,
     requirements_lock = "//:requirements_lock.txt",
     requirements_windows = "//:requirements_windows.txt",
 )
diff --git a/python/extensions/pip.bzl b/python/extensions/pip.bzl
@@ -34,6 +34,7 @@ def _pip_impl(module_ctx):
             # this does not create the install_deps() macro.
             pip_repository_bzlmod(
                 name = attr.name,
+                repo_name = attr.name,
                 requirements_lock = attr.requirements_lock,
                 incompatible_generate_aliases = attr.incompatible_generate_aliases,
             )
diff --git a/python/pip_install/pip_repository.bzl b/python/pip_install/pip_repository.bzl
@@ -358,7 +358,7 @@ def _pip_repository_bzlmod_impl(rctx):
 
     bzl_packages = sorted([name for name, _ in packages])
 
-    repo_name = rctx.attr.name.split("~")[-1]
+    repo_name = rctx.attr.repo_name
 
     build_contents = _BUILD_FILE_CONTENTS
 
@@ -379,11 +379,11 @@ def _pip_repository_bzlmod_impl(rctx):
     rctx.file("BUILD.bazel", build_contents)
     rctx.template("requirements.bzl", rctx.attr._template, substitutions = {
         "%%ALL_REQUIREMENTS%%": _format_repr_list([
-            "@{}//{}".format(repo_name, p) if rctx.attr.incompatible_generate_aliases else "@{}_{}//:pkg".format(rctx.attr.name, p)
+            macro_tmpl.format(p, p) if rctx.attr.incompatible_generate_aliases else macro_tmpl.format(p, "pkg")
             for p in bzl_packages
         ]),
         "%%ALL_WHL_REQUIREMENTS%%": _format_repr_list([
-            "@{}//{}:whl".format(repo_name, p) if rctx.attr.incompatible_generate_aliases else "@{}_{}//:whl".format(rctx.attr.name, p)
+            macro_tmpl.format(p, "whl")
             for p in bzl_packages
         ]),
         "%%MACRO_TMPL%%": macro_tmpl,
@@ -395,6 +395,10 @@ pip_repository_bzlmod_attrs = {
         default = False,
         doc = "Allow generating aliases in '@pip//:<pkg>' -> '@pip_<pkg>//:pkg'. This replaces the aliases generated by the `bzlmod` tooling.",
     ),
+    "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",
+    ),
     "requirements_darwin": attr.label(
         allow_single_file = True,
         doc = "Override the requirements_lock attribute when the host platform is Mac OS",

Original file line number	Diff line number	Diff line change
`@@ -34,6 +34,7 @@ def _pip_impl(module_ctx):`
`34`	`34`	`# this does not create the install_deps() macro.`
`35`	`35`	`pip_repository_bzlmod(`
`36`	`36`	`name = attr.name,`
	`37`	`+ repo_name = attr.name,`
`37`	`38`	`requirements_lock = attr.requirements_lock,`
`38`	`39`	`incompatible_generate_aliases = attr.incompatible_generate_aliases,`
`39`	`40`	`)`