From bdfd1d0360f35b2f030369eba15f7f57c0c42588 Mon Sep 17 00:00:00 2001 From: Philipp Schrader Date: Wed, 13 Sep 2023 06:37:24 -0700 Subject: [PATCH] Rename `pycross_wheel_library` to `py_wheel_library` and make it work This patch changes the name of the rule to reflect the fact that it's not exactly the same as the one in rules_pycross. I also took this opportunity to delete the superfluous `namespace_pkgs.py` library (plus test) since we have a nearly identical version already in the main repo. I added a test to validate that the rule functions at a basic level. References: #1360 --- WORKSPACE | 13 +- .../tools/wheel_installer/BUILD.bazel | 1 + .../rules_pycross/pycross/private/BUILD.bazel | 35 ++++ .../pycross/private/providers.bzl | 6 +- .../pycross/private/py_wheel_library_test.py | 48 +++++ .../pycross/private/tools/BUILD.bazel | 33 +--- .../pycross/private/tools/namespace_pkgs.py | 109 ----------- .../private/tools/namespace_pkgs_test.py | 179 ------------------ .../pycross/private/tools/wheel_installer.py | 28 ++- .../pycross/private/wheel_library.bzl | 18 +- 10 files changed, 133 insertions(+), 337 deletions(-) create mode 100644 third_party/rules_pycross/pycross/private/BUILD.bazel create mode 100644 third_party/rules_pycross/pycross/private/py_wheel_library_test.py delete mode 100644 third_party/rules_pycross/pycross/private/tools/namespace_pkgs.py delete mode 100644 third_party/rules_pycross/pycross/private/tools/namespace_pkgs_test.py diff --git a/WORKSPACE b/WORKSPACE index 7438bb8257..420ad35277 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -34,7 +34,7 @@ python_register_multi_toolchains( python_versions = MINOR_MAPPING.values(), ) -load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") +load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive", "http_file") # Used for Bazel CI http_archive( @@ -86,3 +86,14 @@ pip_parse( load("@publish_deps//:requirements.bzl", "install_deps") install_deps() + +# This wheel is purely here to validate the wheel extraction code. It's not +# intended for anything else. +http_file( + name = "wheel_for_testing", + downloaded_file_path = "numpy-1.25.2-cp311-cp311-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", + sha256 = "0d60fbae8e0019865fc4784745814cff1c421df5afee233db6d88ab4f14655a2", + urls = [ + "https://files.pythonhosted.org/packages/50/67/3e966d99a07d60a21a21d7ec016e9e4c2642a86fea251ec68677daf71d4d/numpy-1.25.2-cp311-cp311-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", + ], +) diff --git a/python/pip_install/tools/wheel_installer/BUILD.bazel b/python/pip_install/tools/wheel_installer/BUILD.bazel index 6360ca5c70..0eadcc25f6 100644 --- a/python/pip_install/tools/wheel_installer/BUILD.bazel +++ b/python/pip_install/tools/wheel_installer/BUILD.bazel @@ -9,6 +9,7 @@ py_library( "wheel.py", "wheel_installer.py", ], + visibility = ["//third_party/rules_pycross/pycross/private:__subpackages__"], deps = [ requirement("installer"), requirement("pip"), diff --git a/third_party/rules_pycross/pycross/private/BUILD.bazel b/third_party/rules_pycross/pycross/private/BUILD.bazel new file mode 100644 index 0000000000..903abe8523 --- /dev/null +++ b/third_party/rules_pycross/pycross/private/BUILD.bazel @@ -0,0 +1,35 @@ +# Copyright 2023 Jeremy Volkman. All rights reserved. +# Copyright 2023 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +load("//python:defs.bzl", "py_test") +load(":wheel_library.bzl", "py_wheel_library") + +py_wheel_library( + name = "extracted_wheel_for_testing", + wheel = "@wheel_for_testing//file", +) + +py_test( + name = "py_wheel_library_test", + srcs = [ + "py_wheel_library_test.py", + ], + data = [ + ":extracted_wheel_for_testing", + ], + deps = [ + "//python/runfiles", + ], +) diff --git a/third_party/rules_pycross/pycross/private/providers.bzl b/third_party/rules_pycross/pycross/private/providers.bzl index f55e98693a..47fc9f7271 100644 --- a/third_party/rules_pycross/pycross/private/providers.bzl +++ b/third_party/rules_pycross/pycross/private/providers.bzl @@ -13,9 +13,9 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Pycross providers.""" +"""Python providers.""" -PycrossWheelInfo = provider( +PyWheelInfo = provider( doc = "Information about a Python wheel.", fields = { "name_file": "File: A file containing the canonical name of the wheel.", @@ -23,7 +23,7 @@ PycrossWheelInfo = provider( }, ) -PycrossTargetEnvironmentInfo = provider( +PyTargetEnvironmentInfo = provider( doc = "A target environment description.", fields = { "file": "The JSON file containing target environment information.", diff --git a/third_party/rules_pycross/pycross/private/py_wheel_library_test.py b/third_party/rules_pycross/pycross/private/py_wheel_library_test.py new file mode 100644 index 0000000000..d42fadaad5 --- /dev/null +++ b/third_party/rules_pycross/pycross/private/py_wheel_library_test.py @@ -0,0 +1,48 @@ +# Copyright 2023 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import unittest +from pathlib import Path + +from rules_python.python.runfiles import runfiles + +RUNFILES = runfiles.Create() + + +class TestPyWheelLibrary(unittest.TestCase): + def setUp(self): + self.extraction_dir = Path( + RUNFILES.Rlocation( + "rules_python/third_party/rules_pycross/pycross/private/extracted_wheel_for_testing" + ) + ) + self.assertTrue(self.extraction_dir.exists(), self.extraction_dir) + self.assertTrue(self.extraction_dir.is_dir(), self.extraction_dir) + + def test_file_presence(self): + """Validate that the basic file layout looks good.""" + for path in ( + "bin/f2py", + "site-packages/numpy.libs/libgfortran-daac5196.so.5.0.0", + "site-packages/numpy/dtypes.py", + "site-packages/numpy/core/_umath_tests.cpython-311-aarch64-linux-gnu.so", + ): + print(self.extraction_dir / path) + self.assertTrue( + (self.extraction_dir / path).exists(), f"{path} does not exist" + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/third_party/rules_pycross/pycross/private/tools/BUILD.bazel b/third_party/rules_pycross/pycross/private/tools/BUILD.bazel index 867b771aae..a87e6aa67e 100644 --- a/third_party/rules_pycross/pycross/private/tools/BUILD.bazel +++ b/third_party/rules_pycross/pycross/private/tools/BUILD.bazel @@ -13,41 +13,14 @@ # See the License for the specific language governing permissions and # limitations under the License. -load("@rules_python//python:defs.bzl", "py_binary", "py_library", "py_test") - -package(default_visibility = ["//visibility:private"]) - -py_library( - name = "namespace_pkgs", - srcs = [ - "namespace_pkgs.py", - ], -) - -py_test( - name = "namespace_pkgs_test", - size = "small", - srcs = [ - "namespace_pkgs_test.py", - ], - tags = [ - "unit", - # TODO(philsc): Make this work. - "manual", - ], - deps = [ - ":namespace_pkgs", - ], -) +load("//python:defs.bzl", "py_binary") py_binary( name = "wheel_installer", srcs = ["wheel_installer.py"], visibility = ["//visibility:public"], deps = [ - ":namespace_pkgs", - # TODO(philsc): Make this work with what's available in rules_python. - #"@rules_pycross_pypi_deps_absl_py//:pkg", - #"@rules_pycross_pypi_deps_installer//:pkg", + "//python/pip_install/tools/wheel_installer:lib", + "@pypi__installer//:lib", ], ) diff --git a/third_party/rules_pycross/pycross/private/tools/namespace_pkgs.py b/third_party/rules_pycross/pycross/private/tools/namespace_pkgs.py deleted file mode 100644 index 59300ffcd1..0000000000 --- a/third_party/rules_pycross/pycross/private/tools/namespace_pkgs.py +++ /dev/null @@ -1,109 +0,0 @@ -"""Utility functions to discover python package types""" -import os -import textwrap -from pathlib import Path # supported in >= 3.4 -from typing import List -from typing import Optional -from typing import Set - - -def implicit_namespace_packages( - directory: str, ignored_dirnames: Optional[List[str]] = None -) -> Set[Path]: - """Discovers namespace packages implemented using the 'native namespace packages' method. - - AKA 'implicit namespace packages', which has been supported since Python 3.3. - See: https://packaging.python.org/guides/packaging-namespace-packages/#native-namespace-packages - - Args: - directory: The root directory to recursively find packages in. - ignored_dirnames: A list of directories to exclude from the search - - Returns: - The set of directories found under root to be packages using the native namespace method. - """ - namespace_pkg_dirs: Set[Path] = set() - standard_pkg_dirs: Set[Path] = set() - directory_path = Path(directory) - ignored_dirname_paths: List[Path] = [Path(p) for p in ignored_dirnames or ()] - # Traverse bottom-up because a directory can be a namespace pkg because its child contains module files. - for dirpath, dirnames, filenames in map( - lambda t: (Path(t[0]), *t[1:]), os.walk(directory_path, topdown=False) - ): - if "__init__.py" in filenames: - standard_pkg_dirs.add(dirpath) - continue - elif ignored_dirname_paths: - is_ignored_dir = dirpath in ignored_dirname_paths - child_of_ignored_dir = any( - d in dirpath.parents for d in ignored_dirname_paths - ) - if is_ignored_dir or child_of_ignored_dir: - continue - - dir_includes_py_modules = _includes_python_modules(filenames) - parent_of_namespace_pkg = any( - Path(dirpath, d) in namespace_pkg_dirs for d in dirnames - ) - parent_of_standard_pkg = any( - Path(dirpath, d) in standard_pkg_dirs for d in dirnames - ) - parent_of_pkg = parent_of_namespace_pkg or parent_of_standard_pkg - if ( - (dir_includes_py_modules or parent_of_pkg) - and - # The root of the directory should never be an implicit namespace - dirpath != directory_path - ): - namespace_pkg_dirs.add(dirpath) - return namespace_pkg_dirs - - -def add_pkgutil_style_namespace_pkg_init(dir_path: Path) -> None: - """Adds 'pkgutil-style namespace packages' init file to the given directory - - See: https://packaging.python.org/guides/packaging-namespace-packages/#pkgutil-style-namespace-packages - - Args: - dir_path: The directory to create an __init__.py for. - - Raises: - ValueError: If the directory already contains an __init__.py file - """ - ns_pkg_init_filepath = os.path.join(dir_path, "__init__.py") - - if os.path.isfile(ns_pkg_init_filepath): - raise ValueError("%s already contains an __init__.py file." % dir_path) - - with open(ns_pkg_init_filepath, "w") as ns_pkg_init_f: - # See https://packaging.python.org/guides/packaging-namespace-packages/#pkgutil-style-namespace-packages - ns_pkg_init_f.write( - textwrap.dedent( - """\ - # __path__ manipulation added by bazelbuild/rules_python to support namespace pkgs. - __path__ = __import__('pkgutil').extend_path(__path__, __name__) - """ - ) - ) - - -def _includes_python_modules(files: List[str]) -> bool: - """ - In order to only transform directories that Python actually considers namespace pkgs - we need to detect if a directory includes Python modules. - - Which files are loadable as modules is extension based, and the particular set of extensions - varies by platform. - - See: - 1. https://github.com/python/cpython/blob/7d9d25dbedfffce61fc76bc7ccbfa9ae901bf56f/Lib/importlib/machinery.py#L19 - 2. PEP 420 -- Implicit Namespace Packages, Specification - https://www.python.org/dev/peps/pep-0420/#specification - 3. dynload_shlib.c and dynload_win.c in python/cpython. - """ - module_suffixes = { - ".py", # Source modules - ".pyc", # Compiled bytecode modules - ".so", # Unix extension modules - ".pyd", # https://docs.python.org/3/faq/windows.html#is-a-pyd-file-the-same-as-a-dll - } - return any(Path(f).suffix in module_suffixes for f in files) diff --git a/third_party/rules_pycross/pycross/private/tools/namespace_pkgs_test.py b/third_party/rules_pycross/pycross/private/tools/namespace_pkgs_test.py deleted file mode 100644 index 49945f9b8c..0000000000 --- a/third_party/rules_pycross/pycross/private/tools/namespace_pkgs_test.py +++ /dev/null @@ -1,179 +0,0 @@ -import os -import pathlib -import shutil -import tempfile -import unittest -from typing import Optional -from typing import Set - -from pycross.private.tools import namespace_pkgs - - -class TempDir: - def __init__(self) -> None: - self.dir = tempfile.mkdtemp() - - def root(self) -> str: - return self.dir - - def add_dir(self, rel_path: str) -> None: - d = pathlib.Path(self.dir, rel_path) - d.mkdir(parents=True) - - def add_file(self, rel_path: str, contents: Optional[str] = None) -> None: - f = pathlib.Path(self.dir, rel_path) - f.parent.mkdir(parents=True, exist_ok=True) - if contents: - with open(str(f), "w") as writeable_f: - writeable_f.write(contents) - else: - f.touch() - - def remove(self) -> None: - shutil.rmtree(self.dir) - - -class TestImplicitNamespacePackages(unittest.TestCase): - def assertPathsEqual(self, actual: Set[pathlib.Path], expected: Set[str]) -> None: - self.assertEqual(actual, {pathlib.Path(p) for p in expected}) - - def test_in_current_directory(self) -> None: - directory = TempDir() - directory.add_file("foo/bar/biz.py") - directory.add_file("foo/bee/boo.py") - directory.add_file("foo/buu/__init__.py") - directory.add_file("foo/buu/bii.py") - cwd = os.getcwd() - os.chdir(directory.root()) - expected = { - "foo", - "foo/bar", - "foo/bee", - } - try: - actual = namespace_pkgs.implicit_namespace_packages(".") - self.assertPathsEqual(actual, expected) - finally: - os.chdir(cwd) - directory.remove() - - def test_finds_correct_namespace_packages(self) -> None: - directory = TempDir() - directory.add_file("foo/bar/biz.py") - directory.add_file("foo/bee/boo.py") - directory.add_file("foo/buu/__init__.py") - directory.add_file("foo/buu/bii.py") - - expected = { - directory.root() + "/foo", - directory.root() + "/foo/bar", - directory.root() + "/foo/bee", - } - actual = namespace_pkgs.implicit_namespace_packages(directory.root()) - self.assertPathsEqual(actual, expected) - - def test_ignores_empty_directories(self) -> None: - directory = TempDir() - directory.add_file("foo/bar/biz.py") - directory.add_dir("foo/cat") - - expected = { - directory.root() + "/foo", - directory.root() + "/foo/bar", - } - actual = namespace_pkgs.implicit_namespace_packages(directory.root()) - self.assertPathsEqual(actual, expected) - - def test_empty_case(self) -> None: - directory = TempDir() - directory.add_file("foo/__init__.py") - directory.add_file("foo/bar/__init__.py") - directory.add_file("foo/bar/biz.py") - - actual = namespace_pkgs.implicit_namespace_packages(directory.root()) - self.assertEqual(actual, set()) - - def test_ignores_non_module_files_in_directories(self) -> None: - directory = TempDir() - directory.add_file("foo/__init__.pyi") - directory.add_file("foo/py.typed") - - actual = namespace_pkgs.implicit_namespace_packages(directory.root()) - self.assertEqual(actual, set()) - - def test_parent_child_relationship_of_namespace_pkgs(self): - directory = TempDir() - directory.add_file("foo/bar/biff/my_module.py") - directory.add_file("foo/bar/biff/another_module.py") - - expected = { - directory.root() + "/foo", - directory.root() + "/foo/bar", - directory.root() + "/foo/bar/biff", - } - actual = namespace_pkgs.implicit_namespace_packages(directory.root()) - self.assertPathsEqual(actual, expected) - - def test_parent_child_relationship_of_namespace_and_standard_pkgs(self): - directory = TempDir() - directory.add_file("foo/bar/biff/__init__.py") - directory.add_file("foo/bar/biff/another_module.py") - - expected = { - directory.root() + "/foo", - directory.root() + "/foo/bar", - } - actual = namespace_pkgs.implicit_namespace_packages(directory.root()) - self.assertPathsEqual(actual, expected) - - def test_parent_child_relationship_of_namespace_and_nested_standard_pkgs(self): - directory = TempDir() - directory.add_file("foo/bar/__init__.py") - directory.add_file("foo/bar/biff/another_module.py") - directory.add_file("foo/bar/biff/__init__.py") - directory.add_file("foo/bar/boof/big_module.py") - directory.add_file("foo/bar/boof/__init__.py") - directory.add_file("fim/in_a_ns_pkg.py") - - expected = { - directory.root() + "/foo", - directory.root() + "/fim", - } - actual = namespace_pkgs.implicit_namespace_packages(directory.root()) - self.assertPathsEqual(actual, expected) - - def test_recognized_all_nonstandard_module_types(self): - directory = TempDir() - directory.add_file("ayy/my_module.pyc") - directory.add_file("bee/ccc/dee/eee.so") - directory.add_file("eff/jee/aych.pyd") - - expected = { - directory.root() + "/ayy", - directory.root() + "/bee", - directory.root() + "/bee/ccc", - directory.root() + "/bee/ccc/dee", - directory.root() + "/eff", - directory.root() + "/eff/jee", - } - actual = namespace_pkgs.implicit_namespace_packages(directory.root()) - self.assertPathsEqual(actual, expected) - - def test_skips_ignored_directories(self): - directory = TempDir() - directory.add_file("foo/boo/my_module.py") - directory.add_file("foo/bar/another_module.py") - - expected = { - directory.root() + "/foo", - directory.root() + "/foo/bar", - } - actual = namespace_pkgs.implicit_namespace_packages( - directory.root(), - ignored_dirnames=[directory.root() + "/foo/boo"], - ) - self.assertPathsEqual(actual, expected) - - -if __name__ == "__main__": - unittest.main() diff --git a/third_party/rules_pycross/pycross/private/tools/wheel_installer.py b/third_party/rules_pycross/pycross/private/tools/wheel_installer.py index 6d3673669b..c15e25a9aa 100644 --- a/third_party/rules_pycross/pycross/private/tools/wheel_installer.py +++ b/third_party/rules_pycross/pycross/private/tools/wheel_installer.py @@ -1,19 +1,35 @@ +# Copyright 2023 Jeremy Volkman. All rights reserved. +# Copyright 2023 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + """ A tool that invokes pypa/build to build the given sdist tarball. """ +import argparse import os import shutil +import sys import tempfile from pathlib import Path from typing import Any -from absl import app -from absl.flags import argparse_flags from installer import install from installer.destinations import SchemeDictionaryDestination from installer.sources import WheelFile -from pycross.private.tools import namespace_pkgs + +from rules_python.python.pip_install.tools.wheel_installer import namespace_pkgs def setup_namespace_pkg_compatibility(wheel_dir: Path) -> None: @@ -73,7 +89,7 @@ def main(args: Any) -> None: destination=destination, # Additional metadata that is generated by the installation tool. additional_metadata={ - "INSTALLER": b"https://github.com/jvolkman/rules_pycross", + "INSTALLER": b"https://github.com/bazelbuild/rules_python/tree/main/third_party/rules_pycross", }, ) finally: @@ -83,7 +99,7 @@ def main(args: Any) -> None: def parse_flags(argv) -> Any: - parser = argparse_flags.ArgumentParser(description="Extract a Python wheel.") + parser = argparse.ArgumentParser(description="Extract a Python wheel.") parser.add_argument( "--wheel", @@ -119,4 +135,4 @@ def parse_flags(argv) -> Any: if "BUILD_WORKING_DIRECTORY" in os.environ: os.chdir(os.environ["BUILD_WORKING_DIRECTORY"]) - app.run(main, flags_parser=parse_flags) + main(parse_flags(sys.argv)) diff --git a/third_party/rules_pycross/pycross/private/wheel_library.bzl b/third_party/rules_pycross/pycross/private/wheel_library.bzl index 25a2497abe..c7af91e588 100644 --- a/third_party/rules_pycross/pycross/private/wheel_library.bzl +++ b/third_party/rules_pycross/pycross/private/wheel_library.bzl @@ -13,19 +13,19 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Implementation of the pycross_wheel_library rule.""" +"""Implementation of the py_wheel_library rule.""" load("@bazel_skylib//lib:paths.bzl", "paths") load("@rules_python//python:defs.bzl", "PyInfo") -load(":providers.bzl", "PycrossWheelInfo") +load(":providers.bzl", "PyWheelInfo") -def _pycross_wheel_library_impl(ctx): +def _py_wheel_library_impl(ctx): out = ctx.actions.declare_directory(ctx.attr.name) wheel_target = ctx.attr.wheel - if PycrossWheelInfo in wheel_target: - wheel_file = wheel_target[PycrossWheelInfo].wheel_file - name_file = wheel_target[PycrossWheelInfo].name_file + if PyWheelInfo in wheel_target: + wheel_file = wheel_target[PyWheelInfo].wheel_file + name_file = wheel_target[PyWheelInfo].name_file else: wheel_file = ctx.file.wheel name_file = None @@ -103,8 +103,8 @@ def _pycross_wheel_library_impl(ctx): ), ] -pycross_wheel_library = rule( - implementation = _pycross_wheel_library_impl, +py_wheel_library = rule( + implementation = _py_wheel_library_impl, attrs = { "deps": attr.label_list( doc = "A list of this wheel's Python library dependencies.", @@ -129,7 +129,7 @@ This option is required to support some packages which cannot handle the convers mandatory = True, ), "_tool": attr.label( - default = Label("//pycross/private/tools:wheel_installer"), + default = Label("//third_party/rules_pycross/pycross/private/tools:wheel_installer"), cfg = "exec", executable = True, ),