From e485f7549b56f6a653386eed3ac04badaf3bcb55 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius Date: Sun, 30 Apr 2023 18:22:56 +0900 Subject: [PATCH 1/2] fix(bzlmod): correctly template repository macros for requirements, etc It seems that the macros for specifying the requirements break when the user starts using `incompatible_generate_aliases=True`. This PR fixes this. Testing done: 1. Modify the example: ``` $ git diff diff --git a/examples/bzlmod/MODULE.bazel b/examples/bzlmod/MODULE.bazel index ce91228..1750210 100644 --- a/examples/bzlmod/MODULE.bazel +++ b/examples/bzlmod/MODULE.bazel @@ -26,6 +26,7 @@ register_toolchains( pip = use_extension("@rules_python//python:extensions.bzl", "pip") pip.parse( name = "pip", + incompatible_generate_aliases=True, requirements_lock = "//:requirements_lock.txt", requirements_windows = "//:requirements_windows.txt", ) ``` 2. Run `bazel build ...` and check that it is still working. --- python/pip_install/pip_repository.bzl | 6 +++++- .../pip_repository_requirements_bzlmod.bzl.tmpl | 8 ++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/python/pip_install/pip_repository.bzl b/python/pip_install/pip_repository.bzl index fce0dcdd47..d08ad1a582 100644 --- a/python/pip_install/pip_repository.bzl +++ b/python/pip_install/pip_repository.bzl @@ -367,6 +367,10 @@ def _pip_repository_bzlmod_impl(rctx): else: build_contents += _bzlmod_pkg_aliases(repo_name, bzl_packages) + macro_tmpl = "@@{name}//:{{}}_{{}}".format(name = rctx.attr.name) + if rctx.attr.incompatible_generate_aliases: + macro_tmpl = "@@{name}//{{}}:{{}}".format(name = rctx.attr.name) + rctx.file("BUILD.bazel", build_contents) rctx.template("requirements.bzl", rctx.attr._template, substitutions = { "%%ALL_REQUIREMENTS%%": _format_repr_list([ @@ -377,8 +381,8 @@ def _pip_repository_bzlmod_impl(rctx): "@{}//{}:whl".format(repo_name, p) if rctx.attr.incompatible_generate_aliases else "@{}_{}//:whl".format(rctx.attr.name, p) for p in bzl_packages ]), - "%%NAME%%": rctx.attr.name, "%%REQUIREMENTS_LOCK%%": str(requirements_txt), + "%%TMPL%%": macro_tmpl, }) pip_repository_bzlmod_attrs = { diff --git a/python/pip_install/pip_repository_requirements_bzlmod.bzl.tmpl b/python/pip_install/pip_repository_requirements_bzlmod.bzl.tmpl index 462829d074..b46e599921 100644 --- a/python/pip_install/pip_repository_requirements_bzlmod.bzl.tmpl +++ b/python/pip_install/pip_repository_requirements_bzlmod.bzl.tmpl @@ -12,13 +12,13 @@ def _clean_name(name): return name.replace("-", "_").replace(".", "_").lower() def requirement(name): - return "@@%%NAME%%//:" + _clean_name(name) + "_pkg" + return "%%TMPL%%".format(_clean_name(name), "pkg") def whl_requirement(name): - return "@@%%NAME%%//:" + _clean_name(name) + "_whl" + return "%%TMPL%%".format(_clean_name(name), "whl") def data_requirement(name): - return "@@%%NAME%%//:" + _clean_name(name) + "_data" + return "%%TMPL%%".format(_clean_name(name), "data") def dist_info_requirement(name): - return "@@%%NAME%%//:" + _clean_name(name) + "_dist_info" + return "%%TMPL%%".format(_clean_name(name), "dist_info") From 915b1ecc2a519658a11553ffd5d12a7b5cab7926 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius Date: Mon, 1 May 2023 12:14:42 +0900 Subject: [PATCH 2/2] comment: improve code layout --- python/pip_install/pip_repository.bzl | 9 +++++++-- .../pip_repository_requirements_bzlmod.bzl.tmpl | 8 ++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/python/pip_install/pip_repository.bzl b/python/pip_install/pip_repository.bzl index d08ad1a582..1ea7bca88a 100644 --- a/python/pip_install/pip_repository.bzl +++ b/python/pip_install/pip_repository.bzl @@ -367,9 +367,14 @@ def _pip_repository_bzlmod_impl(rctx): else: build_contents += _bzlmod_pkg_aliases(repo_name, bzl_packages) - macro_tmpl = "@@{name}//:{{}}_{{}}".format(name = rctx.attr.name) + # NOTE: we are using the canonical name with the double '@' in order to + # always uniquely identify a repository, as the labels are being passed as + # a string and the resolution of the label happens at the call-site of the + # `requirement`, et al. macros. if rctx.attr.incompatible_generate_aliases: macro_tmpl = "@@{name}//{{}}:{{}}".format(name = rctx.attr.name) + else: + macro_tmpl = "@@{name}//:{{}}_{{}}".format(name = rctx.attr.name) rctx.file("BUILD.bazel", build_contents) rctx.template("requirements.bzl", rctx.attr._template, substitutions = { @@ -381,8 +386,8 @@ def _pip_repository_bzlmod_impl(rctx): "@{}//{}:whl".format(repo_name, p) if rctx.attr.incompatible_generate_aliases else "@{}_{}//:whl".format(rctx.attr.name, p) for p in bzl_packages ]), + "%%MACRO_TMPL%%": macro_tmpl, "%%REQUIREMENTS_LOCK%%": str(requirements_txt), - "%%TMPL%%": macro_tmpl, }) pip_repository_bzlmod_attrs = { diff --git a/python/pip_install/pip_repository_requirements_bzlmod.bzl.tmpl b/python/pip_install/pip_repository_requirements_bzlmod.bzl.tmpl index b46e599921..1b2e2178bb 100644 --- a/python/pip_install/pip_repository_requirements_bzlmod.bzl.tmpl +++ b/python/pip_install/pip_repository_requirements_bzlmod.bzl.tmpl @@ -12,13 +12,13 @@ def _clean_name(name): return name.replace("-", "_").replace(".", "_").lower() def requirement(name): - return "%%TMPL%%".format(_clean_name(name), "pkg") + return "%%MACRO_TMPL%%".format(_clean_name(name), "pkg") def whl_requirement(name): - return "%%TMPL%%".format(_clean_name(name), "whl") + return "%%MACRO_TMPL%%".format(_clean_name(name), "whl") def data_requirement(name): - return "%%TMPL%%".format(_clean_name(name), "data") + return "%%MACRO_TMPL%%".format(_clean_name(name), "data") def dist_info_requirement(name): - return "%%TMPL%%".format(_clean_name(name), "dist_info") + return "%%MACRO_TMPL%%".format(_clean_name(name), "dist_info")