From d69b200b6486975a93324de6f0adc1a8ce742faa Mon Sep 17 00:00:00 2001 From: Samuel Littley Date: Tue, 20 Aug 2024 16:49:15 +0100 Subject: [PATCH 1/5] Allow the `name_scheme` argument to `python_wheel` to be a list instead of a single string. --- ChangeLog | 4 ++++ VERSION | 2 +- build_defs/python.build_defs | 14 +++++++------- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/ChangeLog b/ChangeLog index 9aa716bd..715da77c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +Version 1.7.2 +------------- + * Allow the `name_scheme` argument to `python_wheel` to be a list instead of a single string. + Version 1.7.1 ------------- * Bug: change so that every URL is passed with its individual --urls flag in python_wheel (#154) diff --git a/VERSION b/VERSION index 943f9cbc..f8a696c8 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.7.1 +1.7.2 diff --git a/build_defs/python.build_defs b/build_defs/python.build_defs index 87aa5ae6..a7ed3819 100644 --- a/build_defs/python.build_defs +++ b/build_defs/python.build_defs @@ -507,7 +507,7 @@ def pip_library(name:str, version:str, labels:list=[], hashes:list=None, package def python_wheel(name:str, version:str, labels:list=[], hashes:list=None, package_name:str=None, outs:list=None, post_install_commands:list=None, patch:str|list=None, licences:list=None, test_only:bool&testonly=False, repo:str=None, zip_safe:bool=True, visibility:list=None, - deps:list=[], name_scheme:str=None, strip:list=['*.pyc', 'tests'], binary = False, + deps:list=[], name_scheme:str|list=None, strip:list=['*.pyc', 'tests'], binary = False, entry_points={}, tool:str=CONFIG.PYTHON.WHEEL_TOOL, prereleases:bool=CONFIG.PYTHON.PRERELEASES, tool_verbosity:str=CONFIG.PYTHON.VERBOSITY): """Downloads a Python wheel and extracts it. @@ -538,8 +538,8 @@ def python_wheel(name:str, version:str, labels:list=[], hashes:list=None, packag zip_safe (bool): Flag to indicate whether a pex including this rule will be zip-safe. visibility (list): Visibility declaration. deps (list): Dependencies of this rule. - name_scheme (str): The templatized wheel naming scheme (available template variables - are `url_base`, `package_name`, `initial`, and `version`). + name_scheme (str | list): The templatized wheel naming scheme(s) (available template variables + are `url_base`, `package_name`, `initial`, and `version`). strip (list): Files to strip after install. Note that these are done at any level. binary (bool): Whether this wheel should be executable. This assumes that the wheel will contain a __main__ module with a main() function. If this is not the case, then entry_points should be used. @@ -559,10 +559,10 @@ def python_wheel(name:str, version:str, labels:list=[], hashes:list=None, packag urls = [] if url_base: if name_scheme: - urls += [name_scheme.format(url_base=url_base, - package_name=package_name, - initial=initial, - version=version)] + if isinstance(name_scheme, str): + name_scheme = [name_scheme] + urls += [scheme.format(url_base=url_base, package_name=package_name, initial=initial, version=version) + for scheme in name_scheme] elif CONFIG.PYTHON.WHEEL_NAME_SCHEME: urls += [scheme.format(url_base=url_base, package_name=package_name, initial=initial, version=version) for scheme in CONFIG.PYTHON.WHEEL_NAME_SCHEME] From cf5aa5a7146159a0f24314ecdf403a1cc9c4e1c0 Mon Sep 17 00:00:00 2001 From: Samuel Littley Date: Tue, 20 Aug 2024 18:13:13 +0100 Subject: [PATCH 2/5] Update the comment and tidy a bit --- build_defs/python.build_defs | 48 ++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/build_defs/python.build_defs b/build_defs/python.build_defs index a7ed3819..87b8939c 100644 --- a/build_defs/python.build_defs +++ b/build_defs/python.build_defs @@ -504,6 +504,15 @@ def pip_library(name:str, version:str, labels:list=[], hashes:list=None, package visibility = visibility, ) +# A reasonable set of possible wheel naming schemes. +# Look for an arch-specific wheel first; in some cases there can be both (e.g. protobuf +# has optional arch-specific bits) and we prefer the one with the cool stuff. +_DEFAULT_WHEEL_NAME_SCHEMES = [ + '{url_base}/{package_name}-{version}-${{OS}}-${{ARCH}}.whl', + '{url_base}/{package_name}-{version}-${{OS}}_${{ARCH}}.whl', + '{url_base}/{package_name}-{version}.whl', +] + def python_wheel(name:str, version:str, labels:list=[], hashes:list=None, package_name:str=None, outs:list=None, post_install_commands:list=None, patch:str|list=None, licences:list=None, test_only:bool&testonly=False, repo:str=None, zip_safe:bool=True, visibility:list=None, @@ -515,11 +524,14 @@ def python_wheel(name:str, version:str, labels:list=[], hashes:list=None, packag This is a lightweight pip-free alternative to pip_library which supports cross-compiling. Rather than leaning on pip which is difficult to achieve reproducible builds with and support on different platforms, this rule is a simple wrapper around curl and unzip. - Unless otherwise specified, the wheels are expected to adhere to common naming schemes, - such as: - -[--].whl - -[-_].whl - -.whl + + Wheel URLs are resolved in one or both of two ways: + * If `repo` is passed or `wheel_repo` is set in the .plzconfig, URLs are constructed using + `name_scheme` if passed, else `wheel_name_scheme` if set in the .plzconfig, else using some + reasonable defaults defined in _DEFAULT_WHEEL_NAME_SCHEMES. + * If `tool` is passed or `wheel_tool` is set in the .plzconfig, the tool is used to look up + URLs. If `repo` is also passed or `wheel_repo` is also set in the .plzconfig, the URLs + generated from the naming schemes are passed to the resolver tool. Args: name (str): Name of the rule. Also doubles as the name of the package if package_name @@ -561,27 +573,15 @@ def python_wheel(name:str, version:str, labels:list=[], hashes:list=None, packag if name_scheme: if isinstance(name_scheme, str): name_scheme = [name_scheme] - urls += [scheme.format(url_base=url_base, package_name=package_name, initial=initial, version=version) - for scheme in name_scheme] elif CONFIG.PYTHON.WHEEL_NAME_SCHEME: - urls += [scheme.format(url_base=url_base, package_name=package_name, initial=initial, version=version) - for scheme in CONFIG.PYTHON.WHEEL_NAME_SCHEME] + name_scheme = CONFIG.PYTHON.WHEEL_NAME_SCHEME else: - # Populate urls using a reasonable set of possible wheel naming schemes. - # Look for an arch-specific wheel first; in some cases there can be both (e.g. protobuf - # has optional arch-specific bits) and we prefer the one with the cool stuff. - urls += ['{url_base}/{package_name}-{version}-${{OS}}-${{ARCH}}.whl'.format(url_base=url_base, - package_name=package_name, - initial=initial, - version=version), - '{url_base}/{package_name}-{version}-${{OS}}_${{ARCH}}.whl'.format(url_base=url_base, - package_name=package_name, - initial=initial, - version=version), - '{url_base}/{package_name}-{version}.whl'.format(url_base=url_base, - package_name=package_name, - initial=initial, - version=version)] + name_scheme = _DEFAULT_WHEEL_NAME_SCHEMES + + urls += [ + scheme.format(url_base=url_base, package_name=package_name, initial=initial, version=version) + for scheme in name_scheme + ] file_rule = None if tool: From ff2123205a701992eb93ed481b67514b2d5e1e74 Mon Sep 17 00:00:00 2001 From: Samuel Littley Date: Tue, 20 Aug 2024 18:45:42 +0100 Subject: [PATCH 3/5] Simplify the build def further, update the comment, and add a test --- .plzconfig | 2 +- ChangeLog | 1 + build_defs/python.build_defs | 14 ++++++-------- test/BUILD | 14 ++++++++++++++ test/name_scheme_test.py | 15 +++++++++++++++ 5 files changed, 37 insertions(+), 9 deletions(-) create mode 100644 test/name_scheme_test.py diff --git a/.plzconfig b/.plzconfig index d7c78602..94d43532 100644 --- a/.plzconfig +++ b/.plzconfig @@ -1,5 +1,5 @@ [Please] -Version = >=17.10.1 +Version = >=17.10.3 [Build] hashcheckers = sha256 diff --git a/ChangeLog b/ChangeLog index 715da77c..d42d5917 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,6 +1,7 @@ Version 1.7.2 ------------- * Allow the `name_scheme` argument to `python_wheel` to be a list instead of a single string. + * Update required Please version to 17.10.3 to avoid a memory consumption bug. Version 1.7.1 ------------- diff --git a/build_defs/python.build_defs b/build_defs/python.build_defs index 87b8939c..94202cf4 100644 --- a/build_defs/python.build_defs +++ b/build_defs/python.build_defs @@ -516,8 +516,9 @@ _DEFAULT_WHEEL_NAME_SCHEMES = [ def python_wheel(name:str, version:str, labels:list=[], hashes:list=None, package_name:str=None, outs:list=None, post_install_commands:list=None, patch:str|list=None, licences:list=None, test_only:bool&testonly=False, repo:str=None, zip_safe:bool=True, visibility:list=None, - deps:list=[], name_scheme:str|list=None, strip:list=['*.pyc', 'tests'], binary = False, - entry_points={}, tool:str=CONFIG.PYTHON.WHEEL_TOOL, prereleases:bool=CONFIG.PYTHON.PRERELEASES, + deps:list=[], name_scheme:str|list=CONFIG.PYTHON.WHEEL_NAME_SCHEME, + strip:list=['*.pyc', 'tests'], binary = False, entry_points={}, + tool:str=CONFIG.PYTHON.WHEEL_TOOL, prereleases:bool=CONFIG.PYTHON.PRERELEASES, tool_verbosity:str=CONFIG.PYTHON.VERBOSITY): """Downloads a Python wheel and extracts it. @@ -570,13 +571,10 @@ def python_wheel(name:str, version:str, labels:list=[], hashes:list=None, packag fail('python.wheel_repo is not set in the config, must pass repo explicitly to python_wheel') urls = [] if url_base: - if name_scheme: - if isinstance(name_scheme, str): - name_scheme = [name_scheme] - elif CONFIG.PYTHON.WHEEL_NAME_SCHEME: - name_scheme = CONFIG.PYTHON.WHEEL_NAME_SCHEME - else: + if not name_scheme: name_scheme = _DEFAULT_WHEEL_NAME_SCHEMES + elif isinstance(name_scheme, str): + name_scheme = [name_scheme] urls += [ scheme.format(url_base=url_base, package_name=package_name, initial=initial, version=version) diff --git a/test/BUILD b/test/BUILD index 4931c08d..8701dfe8 100644 --- a/test/BUILD +++ b/test/BUILD @@ -182,3 +182,17 @@ plz_e2e_test( labels = ["py3"], deps = ["//third_party/python:grpcio"], ) + +# Test that python_wheel targets can have name_scheme as a list or a string + +python_test( + name = "name_scheme_test", + srcs = ["name_scheme_test.py"], + labels = [ + "py3", + ], + deps = [ + "//third_party/python:click", + "//third_party/python:click-log", + ], +) \ No newline at end of file diff --git a/test/name_scheme_test.py b/test/name_scheme_test.py new file mode 100644 index 00000000..37c19eb4 --- /dev/null +++ b/test/name_scheme_test.py @@ -0,0 +1,15 @@ +import unittest + + +class NameSchemeTest(unittest.TestCase): + + # The click python_wheel has no name_scheme argument, and is thus using the default value which + # is a list. + def test_click_is_importable(self): + import click + self.assertIsNotNone(click.command) + + # The click-log python_wheel has an explicit name_scheme argument which is a string. + def test_click_log_is_importable(self): + import click_log + self.assertIsNotNone(click_log.basic_config) From b4ba94cfd498fcdb3a0e4b10132b397b2792c7e7 Mon Sep 17 00:00:00 2001 From: Samuel Littley Date: Tue, 20 Aug 2024 18:47:33 +0100 Subject: [PATCH 4/5] Add missing newline at eof --- test/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/BUILD b/test/BUILD index 8701dfe8..d9a80d4c 100644 --- a/test/BUILD +++ b/test/BUILD @@ -195,4 +195,4 @@ python_test( "//third_party/python:click", "//third_party/python:click-log", ], -) \ No newline at end of file +) From 0692f8f22d8973d1de7867bab28ab28f65a0f361 Mon Sep 17 00:00:00 2001 From: Samuel Littley Date: Wed, 21 Aug 2024 09:37:24 +0100 Subject: [PATCH 5/5] Add a comment on the packages we are using for testing --- third_party/python/BUILD | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/third_party/python/BUILD b/third_party/python/BUILD index 46d58643..66b49fd3 100644 --- a/third_party/python/BUILD +++ b/third_party/python/BUILD @@ -574,6 +574,8 @@ filegroup( ], ) +# Note: this package is used for //test:name_scheme_test as an example of something using a list of +# name schemes python_wheel( name = "click", hashes = [], @@ -582,6 +584,8 @@ python_wheel( deps = [], ) +# Note: this package is used for //test:name_scheme_test as an example of something using a single +# string name scheme. python_wheel( name = "click-log", package_name = "click_log",