From cdeb5d828e7cda8aa2c5ca83cbd8a405de264806 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Tue, 7 Nov 2023 12:04:14 +0900 Subject: [PATCH 1/7] feat: expose 'pip_utils.normalize_name' function --- CHANGELOG.md | 3 +++ examples/pip_parse_vendored/BUILD.bazel | 2 +- examples/pip_parse_vendored/requirements.bzl | 14 ++++++-------- python/pip.bzl | 19 +++++++++++-------- python/pip_install/pip_repository.bzl | 4 +++- .../pip_repository_requirements.bzl.tmpl | 13 +++++-------- python/private/bzlmod/requirements.bzl.tmpl | 17 ++++++++--------- 7 files changed, 37 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab4c4d2d5..33bc3ace2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,6 +86,9 @@ Breaking changes: * (pip) Support for using [PEP621](https://peps.python.org/pep-0621/) compliant `pyproject.toml` for creating a resolved `requirements.txt` file. +* (utils) Added a `pip_utils` struct with a `normalize_name` function to allow users + to find out how `rules_python` would normalize a PyPI distribution name. + ## [0.26.0] - 2023-10-06 ### Changed diff --git a/examples/pip_parse_vendored/BUILD.bazel b/examples/pip_parse_vendored/BUILD.bazel index 8741c5aaa..ddf328192 100644 --- a/examples/pip_parse_vendored/BUILD.bazel +++ b/examples/pip_parse_vendored/BUILD.bazel @@ -19,7 +19,7 @@ genrule( cmd = " | ".join([ "cat $<", # Insert our load statement after the existing one so we don't produce a file with buildifier warnings - """sed -e '/^load.*.whl_library/i\\'$$'\\n''load("@python39//:defs.bzl", "interpreter")'""", + """sed -e '/^load.*.pip.bzl/i\\'$$'\\n''load("@python39//:defs.bzl", "interpreter")'""", # Replace the bazel 6.0.0 specific comment with something that bazel 5.4.0 would produce. # This enables this example to be run as a test under bazel 5.4.0. """sed -e 's#@//#//#'""", diff --git a/examples/pip_parse_vendored/requirements.bzl b/examples/pip_parse_vendored/requirements.bzl index 48371ed0e..438561e74 100644 --- a/examples/pip_parse_vendored/requirements.bzl +++ b/examples/pip_parse_vendored/requirements.bzl @@ -5,6 +5,7 @@ from //:requirements.txt """ load("@python39//:defs.bzl", "interpreter") +load("@rules_python//python:pip.bzl", "pip_utils") load("@rules_python//python/pip_install:pip_repository.bzl", "whl_library") all_requirements = ["@pip//certifi:pkg", "@pip//charset_normalizer:pkg", "@pip//idna:pkg", "@pip//requests:pkg", "@pip//urllib3:pkg"] @@ -19,25 +20,22 @@ _packages = [("pip_certifi", "certifi==2023.7.22 --hash=sha256:539cc1d13202e _config = {"download_only": False, "enable_implicit_namespace_pkgs": False, "environment": {}, "extra_pip_args": [], "isolated": True, "pip_data_exclude": [], "python_interpreter": "python3", "python_interpreter_target": interpreter, "quiet": True, "repo": "pip", "repo_prefix": "pip_", "timeout": 600} _annotations = {} -def _clean_name(name): - return name.replace("-", "_").replace(".", "_").lower() - def requirement(name): - return "@pip//{}:{}".format(_clean_name(name), "pkg") + return "@pip//{}:{}".format(pip_utils.normalize_name(name), "pkg") def whl_requirement(name): - return "@pip//{}:{}".format(_clean_name(name), "whl") + return "@pip//{}:{}".format(pip_utils.normalize_name(name), "whl") def data_requirement(name): - return "@pip//{}:{}".format(_clean_name(name), "data") + return "@pip//{}:{}".format(pip_utils.normalize_name(name), "data") def dist_info_requirement(name): - return "@pip//{}:{}".format(_clean_name(name), "dist_info") + return "@pip//{}:{}".format(pip_utils.normalize_name(name), "dist_info") def entry_point(pkg, script = None): if not script: script = pkg - return "@pip_" + _clean_name(pkg) + "//:rules_python_wheel_entry_point_" + script + return "@pip_" + pip_utils.normalize_name(pkg) + "//:rules_python_wheel_entry_point_" + script def _get_annotation(requirement): # This expects to parse `setuptools==58.2.0 --hash=sha256:2551203ae6955b9876741a26ab3e767bb3242dafe86a32a749ea0d78b6792f11` diff --git a/python/pip.bzl b/python/pip.bzl index fd02a5685..26e99fea6 100644 --- a/python/pip.bzl +++ b/python/pip.bzl @@ -23,6 +23,7 @@ load("//python/pip_install:pip_repository.bzl", "pip_repository", _package_annot load("//python/pip_install:requirements.bzl", _compile_pip_requirements = "compile_pip_requirements") load("//python/private:bzlmod_enabled.bzl", "BZLMOD_ENABLED") load("//python/private:full_version.bzl", "full_version") +load("//python/private:normalize_name.bzl", "normalize_name") load("//python/private:render_pkg_aliases.bzl", "NO_MATCH_ERROR_MESSAGE_TEMPLATE") compile_pip_requirements = _compile_pip_requirements @@ -86,7 +87,7 @@ _process_requirements( requirements_bzl = """\ # Generated by python/pip.bzl -load("@{rules_python}//python:pip.bzl", "whl_library_alias") +load("@{rules_python}//python:pip.bzl", "whl_library_alias", "pip_utils") {load_statements} _wheel_names = [] @@ -106,20 +107,17 @@ def _process_requirements(pkg_labels, python_version, repo_prefix): {process_requirements_calls} -def _clean_name(name): - return name.replace("-", "_").replace(".", "_").lower() - def requirement(name): - return "{macro_tmpl}".format(_clean_name(name), "pkg") + return "{macro_tmpl}".format(pip_utils.normalize_name(name), "pkg") def whl_requirement(name): - return "{macro_tmpl}".format(_clean_name(name), "whl") + return "{macro_tmpl}".format(pip_utils.normalize_name(name), "whl") def data_requirement(name): - return "{macro_tmpl}".format(_clean_name(name), "data") + return "{macro_tmpl}".format(pip_utils.normalize_name(name), "data") def dist_info_requirement(name): - return "{macro_tmpl}".format(_clean_name(name), "dist_info") + return "{macro_tmpl}".format(pip_utils.normalize_name(name), "dist_info") def entry_point(pkg, script = None): fail("Not implemented yet") @@ -278,3 +276,8 @@ def multi_pip_parse(name, default_version, python_versions, python_interpreter_t default_version = default_version, pip_parses = pip_parses, ) + +# Extra utilities visible to rules_python users. +pip_utils = struct( + normalize_name = normalize_name, +) diff --git a/python/pip_install/pip_repository.bzl b/python/pip_install/pip_repository.bzl index b841772f1..79e3972eb 100644 --- a/python/pip_install/pip_repository.bzl +++ b/python/pip_install/pip_repository.bzl @@ -279,6 +279,8 @@ def _pip_repository_impl(rctx): bzl_packages = dict(sorted([[name, normalize_name(name)] for name, _ in parsed_requirements_txt.requirements])) imports = [ + # NOTE: Maintain the order consistent with `buildifier` + 'load("@rules_python//python:pip.bzl", "pip_utils")', 'load("@rules_python//python/pip_install:pip_repository.bzl", "whl_library")', ] @@ -337,7 +339,7 @@ def _pip_repository_impl(rctx): "%%ANNOTATIONS%%": _format_dict(_repr_dict(annotations)), "%%CONFIG%%": _format_dict(_repr_dict(config)), "%%EXTRA_PIP_ARGS%%": json.encode(options), - "%%IMPORTS%%": "\n".join(sorted(imports)), + "%%IMPORTS%%": "\n".join(imports), "%%MACRO_TMPL%%": macro_tmpl, "%%NAME%%": rctx.attr.name, "%%PACKAGES%%": _format_repr_list( diff --git a/python/pip_install/pip_repository_requirements.bzl.tmpl b/python/pip_install/pip_repository_requirements.bzl.tmpl index 23c83117b..7a9e54c50 100644 --- a/python/pip_install/pip_repository_requirements.bzl.tmpl +++ b/python/pip_install/pip_repository_requirements.bzl.tmpl @@ -18,25 +18,22 @@ _packages = %%PACKAGES%% _config = %%CONFIG%% _annotations = %%ANNOTATIONS%% -def _clean_name(name): - return name.replace("-", "_").replace(".", "_").lower() - def requirement(name): - return "%%MACRO_TMPL%%".format(_clean_name(name), "pkg") + return "%%MACRO_TMPL%%".format(pip_utils.normalize_name(name), "pkg") def whl_requirement(name): - return "%%MACRO_TMPL%%".format(_clean_name(name), "whl") + return "%%MACRO_TMPL%%".format(pip_utils.normalize_name(name), "whl") def data_requirement(name): - return "%%MACRO_TMPL%%".format(_clean_name(name), "data") + return "%%MACRO_TMPL%%".format(pip_utils.normalize_name(name), "data") def dist_info_requirement(name): - return "%%MACRO_TMPL%%".format(_clean_name(name), "dist_info") + return "%%MACRO_TMPL%%".format(pip_utils.normalize_name(name), "dist_info") def entry_point(pkg, script = None): if not script: script = pkg - return "@%%NAME%%_" + _clean_name(pkg) + "//:rules_python_wheel_entry_point_" + script + return "@%%NAME%%_" + pip_utils.normalize_name(pkg) + "//:rules_python_wheel_entry_point_" + script def _get_annotation(requirement): # This expects to parse `setuptools==58.2.0 --hash=sha256:2551203ae6955b9876741a26ab3e767bb3242dafe86a32a749ea0d78b6792f11` diff --git a/python/private/bzlmod/requirements.bzl.tmpl b/python/private/bzlmod/requirements.bzl.tmpl index 5ed1e49cc..3a68853e0 100644 --- a/python/private/bzlmod/requirements.bzl.tmpl +++ b/python/private/bzlmod/requirements.bzl.tmpl @@ -3,6 +3,8 @@ @generated by rules_python pip.parse bzlmod extension. """ +load("@rules_python//python:pip.bzl", "pip_utils") + all_requirements = %%ALL_REQUIREMENTS%% all_whl_requirements_by_package = %%ALL_WHL_REQUIREMENTS_BY_PACKAGE%% @@ -11,20 +13,17 @@ all_whl_requirements = all_whl_requirements_by_package.values() all_data_requirements = %%ALL_DATA_REQUIREMENTS%% -def _clean_name(name): - return name.replace("-", "_").replace(".", "_").lower() - def requirement(name): - return "%%MACRO_TMPL%%".format(_clean_name(name), "pkg") + return "%%MACRO_TMPL%%".format(pip_utils.normalize_name(name), "pkg") def whl_requirement(name): - return "%%MACRO_TMPL%%".format(_clean_name(name), "whl") + return "%%MACRO_TMPL%%".format(pip_utils.normalize_name(name), "whl") def data_requirement(name): - return "%%MACRO_TMPL%%".format(_clean_name(name), "data") + return "%%MACRO_TMPL%%".format(pip_utils.normalize_name(name), "data") def dist_info_requirement(name): - return "%%MACRO_TMPL%%".format(_clean_name(name), "dist_info") + return "%%MACRO_TMPL%%".format(pip_utils.normalize_name(name), "dist_info") def entry_point(pkg, script = None): """entry_point returns the target of the canonical label of the package entrypoints. @@ -43,7 +42,7 @@ py_console_script_binary( ) ``` """.format( - pkg = _clean_name(pkg), - pkg_label = "%%MACRO_TMPL%%".format(_clean_name(pkg), "pkg"), + pkg = pip_utils.normalize_name(pkg), + pkg_label = "%%MACRO_TMPL%%".format(pip_utils.normalize_name(pkg), "pkg"), script = script, )) From 2cf3636b34a92021c70b91ef63ee04a4b14f406a Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Wed, 8 Nov 2023 22:08:18 +0900 Subject: [PATCH 2/7] fix entry_point error message --- python/private/bzlmod/pip_repository.bzl | 2 +- python/private/bzlmod/requirements.bzl.tmpl | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/python/private/bzlmod/pip_repository.bzl b/python/private/bzlmod/pip_repository.bzl index e4e59b59d..9e6b0f466 100644 --- a/python/private/bzlmod/pip_repository.bzl +++ b/python/private/bzlmod/pip_repository.bzl @@ -56,7 +56,7 @@ def _pip_repository_impl(rctx): for p in bzl_packages }), "%%MACRO_TMPL%%": macro_tmpl, - "%%NAME%%": rctx.attr.name, + "%%NAME%%": rctx.attr.repo_name, }) pip_repository_attrs = { diff --git a/python/private/bzlmod/requirements.bzl.tmpl b/python/private/bzlmod/requirements.bzl.tmpl index 3a68853e0..29e0ed46a 100644 --- a/python/private/bzlmod/requirements.bzl.tmpl +++ b/python/private/bzlmod/requirements.bzl.tmpl @@ -28,8 +28,8 @@ def dist_info_requirement(name): def entry_point(pkg, script = None): """entry_point returns the target of the canonical label of the package entrypoints. """ - if not script: - script = pkg + actual_script = script or pkg + fail("""Please replace this instance of entry_point with the following: ``` @@ -37,12 +37,10 @@ load("@rules_python//python/entry_points:py_console_script_binary.bzl", "py_cons py_console_script_binary( name = "{pkg}", - pkg = "@%%{pkg_label}", - script = "{script}", + pkg = "@%%NAME%%//{pkg}",{script} ) ``` """.format( pkg = pip_utils.normalize_name(pkg), - pkg_label = "%%MACRO_TMPL%%".format(pip_utils.normalize_name(pkg), "pkg"), - script = script, + script = "" if not script else "\n script = %s" % actual_script, )) From c9cbb9d7961d2507947a37ecee301944da4a1d6a Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Wed, 8 Nov 2023 22:08:27 +0900 Subject: [PATCH 3/7] normalize keys --- python/pip_install/pip_repository.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/pip_install/pip_repository.bzl b/python/pip_install/pip_repository.bzl index 79e3972eb..4487b921d 100644 --- a/python/pip_install/pip_repository.bzl +++ b/python/pip_install/pip_repository.bzl @@ -333,7 +333,7 @@ def _pip_repository_impl(rctx): for p in bzl_packages.values() ]), "%%ALL_WHL_REQUIREMENTS_BY_PACKAGE%%": _format_dict(_repr_dict({ - name: macro_tmpl.format(p, "whl") + normalize_name(name): macro_tmpl.format(p, "whl") for name, p in bzl_packages.items() })), "%%ANNOTATIONS%%": _format_dict(_repr_dict(annotations)), From a056784b884e7dc136b4aadef027512bd6c9ff3b Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Wed, 8 Nov 2023 22:07:55 +0900 Subject: [PATCH 4/7] improve changelog to mention how to get the values --- CHANGELOG.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 33bc3ace2..d6d56c316 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,8 +33,11 @@ A brief description of the categories of changes: dependencies is now done as part of `py_repositories` call. * (pip_parse) The generated `requirements.bzl` file now has an additional symbol - `all_whl_requirements_by_package` which provides a map from the original package name - (as it appears in requirements.txt) to the target that provides the built wheel file. + `all_whl_requirements_by_package` which provides a map from the normalized + PyPI package name to the target that provides the built wheel file. Use + `pip_utils.normalize_name` function from `@rules_python//python:pip.bzl` to + convert a PyPI package name to a key in the `all_whl_requirements_by_package` + map. * (pip_parse) The flag `incompatible_generate_aliases` has been flipped to `True` by default on `non-bzlmod` setups allowing users to use the same label From 03fdf7dee6537a4ee061fff0d851328ec68a440a Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Wed, 8 Nov 2023 22:21:05 +0900 Subject: [PATCH 5/7] fix fail message --- python/private/bzlmod/requirements.bzl.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/private/bzlmod/requirements.bzl.tmpl b/python/private/bzlmod/requirements.bzl.tmpl index 29e0ed46a..b99322dd9 100644 --- a/python/private/bzlmod/requirements.bzl.tmpl +++ b/python/private/bzlmod/requirements.bzl.tmpl @@ -42,5 +42,5 @@ py_console_script_binary( ``` """.format( pkg = pip_utils.normalize_name(pkg), - script = "" if not script else "\n script = %s" % actual_script, + script = "" if not script else "\n script = \"%s\"," % actual_script, )) From ca93fec625aeff271a8ea220c1e9edff13abbbdb Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Wed, 8 Nov 2023 22:21:27 +0900 Subject: [PATCH 6/7] example: vendor requirements --- examples/pip_parse_vendored/requirements.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/pip_parse_vendored/requirements.bzl b/examples/pip_parse_vendored/requirements.bzl index 438561e74..21a255631 100644 --- a/examples/pip_parse_vendored/requirements.bzl +++ b/examples/pip_parse_vendored/requirements.bzl @@ -10,7 +10,7 @@ load("@rules_python//python/pip_install:pip_repository.bzl", "whl_library") all_requirements = ["@pip//certifi:pkg", "@pip//charset_normalizer:pkg", "@pip//idna:pkg", "@pip//requests:pkg", "@pip//urllib3:pkg"] -all_whl_requirements_by_package = {"certifi": "@pip//certifi:whl", "charset-normalizer": "@pip//charset_normalizer:whl", "idna": "@pip//idna:whl", "requests": "@pip//requests:whl", "urllib3": "@pip//urllib3:whl"} +all_whl_requirements_by_package = {"certifi": "@pip//certifi:whl", "charset_normalizer": "@pip//charset_normalizer:whl", "idna": "@pip//idna:whl", "requests": "@pip//requests:whl", "urllib3": "@pip//urllib3:whl"} all_whl_requirements = all_whl_requirements_by_package.values() From ef167e61def90cd3fcf1fddfeb00cb4ffe2eb8ac Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Thu, 9 Nov 2023 13:35:28 +0900 Subject: [PATCH 7/7] comment: simplify bzl_packages in the legacy codebase --- python/pip_install/pip_repository.bzl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/python/pip_install/pip_repository.bzl b/python/pip_install/pip_repository.bzl index 4487b921d..07ca3c22f 100644 --- a/python/pip_install/pip_repository.bzl +++ b/python/pip_install/pip_repository.bzl @@ -276,7 +276,7 @@ def _pip_repository_impl(rctx): packages = [(normalize_name(name), requirement) for name, requirement in parsed_requirements_txt.requirements] - bzl_packages = dict(sorted([[name, normalize_name(name)] for name, _ in parsed_requirements_txt.requirements])) + bzl_packages = sorted([normalize_name(name) for name, _ in parsed_requirements_txt.requirements]) imports = [ # NOTE: Maintain the order consistent with `buildifier` @@ -316,7 +316,7 @@ def _pip_repository_impl(rctx): if rctx.attr.incompatible_generate_aliases: macro_tmpl = "@%s//{}:{}" % rctx.attr.name - aliases = render_pkg_aliases(repo_name = rctx.attr.name, bzl_packages = bzl_packages.values()) + aliases = render_pkg_aliases(repo_name = rctx.attr.name, bzl_packages = bzl_packages) for path, contents in aliases.items(): rctx.file(path, contents) else: @@ -326,15 +326,15 @@ def _pip_repository_impl(rctx): rctx.template("requirements.bzl", rctx.attr._template, substitutions = { "%%ALL_DATA_REQUIREMENTS%%": _format_repr_list([ macro_tmpl.format(p, "data") - for p in bzl_packages.values() + for p in bzl_packages ]), "%%ALL_REQUIREMENTS%%": _format_repr_list([ macro_tmpl.format(p, "pkg") - for p in bzl_packages.values() + for p in bzl_packages ]), "%%ALL_WHL_REQUIREMENTS_BY_PACKAGE%%": _format_dict(_repr_dict({ - normalize_name(name): macro_tmpl.format(p, "whl") - for name, p in bzl_packages.items() + p: macro_tmpl.format(p, "whl") + for p in bzl_packages })), "%%ANNOTATIONS%%": _format_dict(_repr_dict(annotations)), "%%CONFIG%%": _format_dict(_repr_dict(config)),