From b855757bb9a5a98780a5ca585a4a36f619ee0b08 Mon Sep 17 00:00:00 2001 From: Josh Wilson Date: Sun, 18 Apr 2021 22:17:49 -0500 Subject: [PATCH] Allow controlling the prefix added to repos/packages --- docs/pip.md | 2 +- examples/pip_parse/BUILD | 5 +-- examples/pip_parse/WORKSPACE | 5 +-- examples/pip_parse/pip_parse_test.py | 22 +++++----- python/pip.bzl | 5 ++- python/pip_install/extract_wheels/__init__.py | 13 +++--- .../extract_wheels/lib/arguments.py | 6 ++- .../extract_wheels/lib/arguments_test.py | 11 ++++- .../pip_install/extract_wheels/lib/bazel.py | 42 ++++++------------- .../extract_wheels/lib/whl_filegroup_test.py | 8 ++-- .../parse_requirements_to_bzl/__init__.py | 19 +++++---- .../extract_single_wheel/__init__.py | 4 +- .../parse_requirements_to_bzl_test.py | 3 +- python/pip_install/pip_repository.bzl | 16 ++++++- 14 files changed, 89 insertions(+), 72 deletions(-) diff --git a/docs/pip.md b/docs/pip.md index acbe752400..de8d1db686 100644 --- a/docs/pip.md +++ b/docs/pip.md @@ -216,7 +216,7 @@ alias( | Name | Description | Default Value | | :-------------: | :-------------: | :-------------: | | requirements_lock | A fully resolved 'requirements.txt' pip requirement file containing the transitive set of your dependencies. If this file is passed instead of 'requirements' no resolve will take place and pip_repository will create individual repositories for each of your dependencies so that wheels are fetched/built only for the targets specified by 'build/run/test'. | none | -| name | The name of the generated repository. | "pip_parsed_deps" | +| name | The name of the generated repository. The generated repositories containing each requirement will be of the form <name>_<requirement-name>. | "pip_parsed_deps" | | kwargs | Additional keyword arguments for the underlying pip_repository rule. | none | diff --git a/examples/pip_parse/BUILD b/examples/pip_parse/BUILD index 53ee925f76..d0f0529bed 100644 --- a/examples/pip_parse/BUILD +++ b/examples/pip_parse/BUILD @@ -1,9 +1,8 @@ load( - "@pip_parsed_deps//:requirements.bzl", + "@pypi//:requirements.bzl", "data_requirement", "dist_info_requirement", "entry_point", - "requirement", ) load("@rules_python//python:defs.bzl", "py_binary", "py_test") load("@rules_python//python:pip.bzl", "compile_pip_requirements") @@ -38,7 +37,7 @@ py_binary( name = "main", srcs = ["main.py"], deps = [ - requirement("requests"), + "@pypi_requests//:pkg", ], ) diff --git a/examples/pip_parse/WORKSPACE b/examples/pip_parse/WORKSPACE index 7e47ed22c3..6c18864661 100644 --- a/examples/pip_parse/WORKSPACE +++ b/examples/pip_parse/WORKSPACE @@ -34,12 +34,11 @@ pip_parse( # style env vars are read, but env vars that control requests and urllib3 # can be passed # environment = {"HTTPS_PROXY": "http://my.proxy.fun/"}, - - # Uses the default repository name "pip_parsed_deps" + name = "pypi", requirements_lock = "//:requirements_lock.txt", ) -load("@pip_parsed_deps//:requirements.bzl", "install_deps") +load("@pypi//:requirements.bzl", "install_deps") # Initialize repositories for all packages in requirements_lock.txt. install_deps() diff --git a/examples/pip_parse/pip_parse_test.py b/examples/pip_parse/pip_parse_test.py index 7cc8a7a03d..361d59f71d 100644 --- a/examples/pip_parse/pip_parse_test.py +++ b/examples/pip_parse/pip_parse_test.py @@ -46,12 +46,12 @@ def test_data(self): self.assertListEqual( env.split(" "), [ - "external/pip_parsed_deps_pypi__s3cmd/s3cmd-2.1.0.data/data/share/doc/packages/s3cmd/INSTALL.md", - "external/pip_parsed_deps_pypi__s3cmd/s3cmd-2.1.0.data/data/share/doc/packages/s3cmd/LICENSE", - "external/pip_parsed_deps_pypi__s3cmd/s3cmd-2.1.0.data/data/share/doc/packages/s3cmd/NEWS", - "external/pip_parsed_deps_pypi__s3cmd/s3cmd-2.1.0.data/data/share/doc/packages/s3cmd/README.md", - "external/pip_parsed_deps_pypi__s3cmd/s3cmd-2.1.0.data/data/share/man/man1/s3cmd.1", - "external/pip_parsed_deps_pypi__s3cmd/s3cmd-2.1.0.data/scripts/s3cmd", + "external/pypi_s3cmd/s3cmd-2.1.0.data/data/share/doc/packages/s3cmd/INSTALL.md", + "external/pypi_s3cmd/s3cmd-2.1.0.data/data/share/doc/packages/s3cmd/LICENSE", + "external/pypi_s3cmd/s3cmd-2.1.0.data/data/share/doc/packages/s3cmd/NEWS", + "external/pypi_s3cmd/s3cmd-2.1.0.data/data/share/doc/packages/s3cmd/README.md", + "external/pypi_s3cmd/s3cmd-2.1.0.data/data/share/man/man1/s3cmd.1", + "external/pypi_s3cmd/s3cmd-2.1.0.data/scripts/s3cmd", ], ) @@ -61,11 +61,11 @@ def test_dist_info(self): self.assertListEqual( env.split(" "), [ - "external/pip_parsed_deps_pypi__requests/requests-2.25.1.dist-info/LICENSE", - "external/pip_parsed_deps_pypi__requests/requests-2.25.1.dist-info/METADATA", - "external/pip_parsed_deps_pypi__requests/requests-2.25.1.dist-info/RECORD", - "external/pip_parsed_deps_pypi__requests/requests-2.25.1.dist-info/WHEEL", - "external/pip_parsed_deps_pypi__requests/requests-2.25.1.dist-info/top_level.txt", + "external/pypi_requests/requests-2.25.1.dist-info/LICENSE", + "external/pypi_requests/requests-2.25.1.dist-info/METADATA", + "external/pypi_requests/requests-2.25.1.dist-info/RECORD", + "external/pypi_requests/requests-2.25.1.dist-info/WHEEL", + "external/pypi_requests/requests-2.25.1.dist-info/top_level.txt", ], ) diff --git a/python/pip.bzl b/python/pip.bzl index ca74aba2fb..cac11c2197 100644 --- a/python/pip.bzl +++ b/python/pip.bzl @@ -96,6 +96,7 @@ def pip_install(requirements, name = "pip", **kwargs): pip_repository( name = name, requirements = requirements, + repo_prefix = "pypi__", **kwargs ) @@ -171,7 +172,8 @@ def pip_parse(requirements_lock, name = "pip_parsed_deps", **kwargs): of 'requirements' no resolve will take place and pip_repository will create individual repositories for each of your dependencies so that wheels are fetched/built only for the targets specified by 'build/run/test'. - name (str, optional): The name of the generated repository. + name (str, optional): The name of the generated repository. The generated repositories + containing each requirement will be of the form _. **kwargs (dict): Additional keyword arguments for the underlying `pip_repository` rule. """ @@ -182,6 +184,7 @@ def pip_parse(requirements_lock, name = "pip_parsed_deps", **kwargs): pip_repository( name = name, requirements_lock = requirements_lock, + repo_prefix = "{}_".format(name), incremental = True, **kwargs ) diff --git a/python/pip_install/extract_wheels/__init__.py b/python/pip_install/extract_wheels/__init__.py index 5fdf9eae75..bf5a18ce04 100644 --- a/python/pip_install/extract_wheels/__init__.py +++ b/python/pip_install/extract_wheels/__init__.py @@ -68,8 +68,8 @@ def main() -> None: # relative requirements to be correctly resolved. The --wheel-dir is therefore required to be repointed back to the # current calling working directory (the repo root in .../external/name), where the wheel files should be written to pip_args = ( - [sys.executable, "-m", "pip"] + - (["--isolated"] if args.isolated else []) + + [sys.executable, "-m", "pip"] + + (["--isolated"] if args.isolated else []) + ["wheel", "-r", args.requirements] + ["--wheel-dir", os.getcwd()] + deserialized_args["extra_pip_args"] @@ -86,11 +86,14 @@ def main() -> None: repo_label = "@%s" % args.repo targets = [ - '"%s%s"' - % ( + '"{}{}"'.format( repo_label, bazel.extract_wheel( - whl, extras, deserialized_args["pip_data_exclude"], args.enable_implicit_namespace_pkgs + whl, + extras, + deserialized_args["pip_data_exclude"], + args.enable_implicit_namespace_pkgs, + args.repo_prefix, ), ) for whl in glob.glob("*.whl") diff --git a/python/pip_install/extract_wheels/lib/arguments.py b/python/pip_install/extract_wheels/lib/arguments.py index 00901372f3..9c8f49a46d 100644 --- a/python/pip_install/extract_wheels/lib/arguments.py +++ b/python/pip_install/extract_wheels/lib/arguments.py @@ -30,6 +30,11 @@ def parse_common_args(parser: ArgumentParser) -> ArgumentParser: action="store", help="Extra environment variables to set on the pip environment.", ) + parser.add_argument( + "--repo-prefix", + required=True, + help="Prefix to prepend to packages", + ) return parser @@ -45,4 +50,3 @@ def deserialize_structured_args(args): else: args[arg_name] = [] return args - diff --git a/python/pip_install/extract_wheels/lib/arguments_test.py b/python/pip_install/extract_wheels/lib/arguments_test.py index 53e19a30b1..89ab291538 100644 --- a/python/pip_install/extract_wheels/lib/arguments_test.py +++ b/python/pip_install/extract_wheels/lib/arguments_test.py @@ -10,16 +10,25 @@ def test_arguments(self) -> None: parser = argparse.ArgumentParser() parser = arguments.parse_common_args(parser) repo_name = "foo" + repo_prefix = "pypi_" index_url = "--index_url=pypi.org/simple" extra_pip_args = [index_url] args_dict = vars(parser.parse_args( - args=["--repo", repo_name, f"--extra_pip_args={json.dumps({'arg': extra_pip_args})}"])) + args=[ + "--repo", + repo_name, + f"--extra_pip_args={json.dumps({'arg': extra_pip_args})}", + "--repo-prefix", + repo_prefix, + ] + )) args_dict = arguments.deserialize_structured_args(args_dict) self.assertIn("repo", args_dict) self.assertIn("extra_pip_args", args_dict) self.assertEqual(args_dict["pip_data_exclude"], []) self.assertEqual(args_dict["enable_implicit_namespace_pkgs"], False) self.assertEqual(args_dict["repo"], repo_name) + self.assertEqual(args_dict["repo_prefix"], repo_prefix) self.assertEqual(args_dict["extra_pip_args"], extra_pip_args) def test_deserialize_structured_args(self) -> None: diff --git a/python/pip_install/extract_wheels/lib/bazel.py b/python/pip_install/extract_wheels/lib/bazel.py index e07a193053..3411756660 100644 --- a/python/pip_install/extract_wheels/lib/bazel.py +++ b/python/pip_install/extract_wheels/lib/bazel.py @@ -215,17 +215,7 @@ def install_deps(): ) -DEFAULT_PACKAGE_PREFIX = "pypi__" - - -def whl_library_repo_prefix(parent_repo: str) -> str: - return "{parent}_{default_package_prefix}".format( - parent=parent_repo, - default_package_prefix=DEFAULT_PACKAGE_PREFIX - ) - - -def sanitise_name(name: str, prefix: str = DEFAULT_PACKAGE_PREFIX) -> str: +def sanitise_name(name: str, prefix: str) -> str: """Sanitises the name to be compatible with Bazel labels. There are certain requirements around Bazel labels that we need to consider. From the Bazel docs: @@ -268,12 +258,12 @@ def setup_namespace_pkg_compatibility(wheel_dir: str) -> None: namespace_pkgs.add_pkgutil_style_namespace_pkg_init(ns_pkg_dir) -def sanitised_library_label(whl_name: str) -> str: - return '"//%s"' % sanitise_name(whl_name) +def sanitised_library_label(whl_name: str, prefix: str) -> str: + return '"//%s"' % sanitise_name(whl_name, prefix) -def sanitised_file_label(whl_name: str) -> str: - return '"//%s:%s"' % (sanitise_name(whl_name), WHEEL_FILE_LABEL) +def sanitised_file_label(whl_name: str, prefix: str) -> str: + return '"//%s:%s"' % (sanitise_name(whl_name, prefix), WHEEL_FILE_LABEL) def _whl_name_to_repo_root(whl_name: str, repo_prefix: str) -> str: @@ -293,8 +283,8 @@ def extract_wheel( extras: Dict[str, Set[str]], pip_data_exclude: List[str], enable_implicit_namespace_pkgs: bool, + repo_prefix: str, incremental: bool = False, - incremental_repo_prefix: Optional[str] = None, ) -> Optional[str]: """Extracts wheel into given directory and creates py_library and filegroup targets. @@ -305,8 +295,6 @@ def extract_wheel( enable_implicit_namespace_pkgs: if true, disables conversion of implicit namespace packages and will unzip as-is incremental: If true the extract the wheel in a format suitable for an external repository. This effects the names of libraries and their dependencies, which point to other external repositories. - incremental_repo_prefix: If incremental is true, use this prefix when creating labels from wheel - names instead of the default. Returns: The Bazel label for the extracted wheel, in the form '//path/to/wheel'. @@ -316,7 +304,7 @@ def extract_wheel( if incremental: directory = "." else: - directory = sanitise_name(whl.name) + directory = sanitise_name(whl.name, prefix=repo_prefix) os.mkdir(directory) # copy the original wheel @@ -333,25 +321,21 @@ def extract_wheel( whl_deps = sorted(whl.dependencies(extras_requested)) if incremental: - # check for mypy Optional validity - if incremental_repo_prefix is None: - raise TypeError( - "incremental_repo_prefix arguement cannot be None if incremental == True") sanitised_dependencies = [ - sanitised_repo_library_label(d, repo_prefix=incremental_repo_prefix) for d in whl_deps + sanitised_repo_library_label(d, repo_prefix=repo_prefix) for d in whl_deps ] sanitised_wheel_file_dependencies = [ - sanitised_repo_file_label(d, repo_prefix=incremental_repo_prefix) for d in whl_deps + sanitised_repo_file_label(d, repo_prefix=repo_prefix) for d in whl_deps ] else: sanitised_dependencies = [ - sanitised_library_label(d) for d in whl_deps + sanitised_library_label(d, prefix=repo_prefix) for d in whl_deps ] sanitised_wheel_file_dependencies = [ - sanitised_file_label(d) for d in whl_deps + sanitised_file_label(d, prefix=repo_prefix) for d in whl_deps ] - library_name = PY_LIBRARY_LABEL if incremental else sanitise_name(whl.name) + library_name = PY_LIBRARY_LABEL if incremental else sanitise_name(whl.name, repo_prefix) directory_path = Path(directory) entry_points = [] @@ -365,7 +349,7 @@ def extract_wheel( with open(os.path.join(directory, "BUILD.bazel"), "w") as build_file: contents = generate_build_file_contents( - library_name, + PY_LIBRARY_LABEL if incremental else sanitise_name(whl.name, repo_prefix), sanitised_dependencies, sanitised_wheel_file_dependencies, pip_data_exclude, diff --git a/python/pip_install/extract_wheels/lib/whl_filegroup_test.py b/python/pip_install/extract_wheels/lib/whl_filegroup_test.py index 4fd0159d71..39a0d4c65f 100644 --- a/python/pip_install/extract_wheels/lib/whl_filegroup_test.py +++ b/python/pip_install/extract_wheels/lib/whl_filegroup_test.py @@ -24,8 +24,8 @@ def tearDown(self): def _run( self, + repo_prefix: str, incremental: bool = False, - incremental_repo_prefix: Optional[str] = None, ) -> None: generated_bazel_dir = bazel.extract_wheel( self.wheel_path, @@ -33,7 +33,7 @@ def _run( pip_data_exclude=[], enable_implicit_namespace_pkgs=False, incremental=incremental, - incremental_repo_prefix=incremental_repo_prefix + repo_prefix=repo_prefix ) # Take off the leading // from the returned label. # Assert that the raw wheel ends up in the package. @@ -44,10 +44,10 @@ def _run( self.assertIn('filegroup', build_file_content) def test_nonincremental(self) -> None: - self._run() + self._run(repo_prefix="prefix_") def test_incremental(self) -> None: - self._run(incremental=True, incremental_repo_prefix="test") + self._run(incremental=True, repo_prefix="prefix_") if __name__ == "__main__": diff --git a/python/pip_install/parse_requirements_to_bzl/__init__.py b/python/pip_install/parse_requirements_to_bzl/__init__.py index 5163b3e4d8..e22d4a348f 100644 --- a/python/pip_install/parse_requirements_to_bzl/__init__.py +++ b/python/pip_install/parse_requirements_to_bzl/__init__.py @@ -60,16 +60,19 @@ def generate_parsed_requirements_contents(all_args: argparse.Namespace) -> str: args.setdefault("python_interpreter", sys.executable) # Pop this off because it wont be used as a config argument to the whl_library rule. requirements_lock = args.pop("requirements_lock") - repo_prefix = bazel.whl_library_repo_prefix(args["repo"]) install_req_and_lines = parse_install_requirements(requirements_lock, args["extra_pip_args"]) - repo_names_and_reqs = repo_names_and_requirements(install_req_and_lines, repo_prefix) - all_requirements = ", ".join( - [bazel.sanitised_repo_library_label(ir.name, repo_prefix=repo_prefix) for ir, _ in install_req_and_lines] - ) - all_whl_requirements = ", ".join( - [bazel.sanitised_repo_file_label(ir.name, repo_prefix=repo_prefix) for ir, _ in install_req_and_lines] + repo_names_and_reqs = repo_names_and_requirements( + install_req_and_lines, args["repo_prefix"] ) + all_requirements = ", ".join([ + bazel.sanitised_repo_library_label(ir.name, repo_prefix=args["repo_prefix"]) + for ir, _ in install_req_and_lines + ]) + all_whl_requirements = ", ".join([ + bazel.sanitised_repo_file_label(ir.name, repo_prefix=args["repo_prefix"]) + for ir, _ in install_req_and_lines + ]) return textwrap.dedent("""\ load("@rules_python//python/pip_install:pip_repository.bzl", "whl_library") @@ -112,7 +115,7 @@ def install_deps(): all_whl_requirements=all_whl_requirements, repo_names_and_reqs=repo_names_and_reqs, args=args, - repo_prefix=repo_prefix, + repo_prefix=args["repo_prefix"], py_library_label=bazel.PY_LIBRARY_LABEL, wheel_file_label=bazel.WHEEL_FILE_LABEL, data_label=bazel.DATA_LABEL, diff --git a/python/pip_install/parse_requirements_to_bzl/extract_single_wheel/__init__.py b/python/pip_install/parse_requirements_to_bzl/extract_single_wheel/__init__.py index 2c1a12494c..39371161cf 100644 --- a/python/pip_install/parse_requirements_to_bzl/extract_single_wheel/__init__.py +++ b/python/pip_install/parse_requirements_to_bzl/extract_single_wheel/__init__.py @@ -30,7 +30,7 @@ def main() -> None: pip_args = ( [sys.executable, "-m", "pip"] + - (["--isolated"] if args.isolated else []) + + (["--isolated"] if args.isolated else []) + ["wheel", "--no-deps"] + deserialized_args["extra_pip_args"] ) @@ -67,5 +67,5 @@ def main() -> None: deserialized_args["pip_data_exclude"], args.enable_implicit_namespace_pkgs, incremental=True, - incremental_repo_prefix=bazel.whl_library_repo_prefix(args.repo) + repo_prefix=args.repo_prefix, ) diff --git a/python/pip_install/parse_requirements_to_bzl/parse_requirements_to_bzl_test.py b/python/pip_install/parse_requirements_to_bzl/parse_requirements_to_bzl_test.py index 65a19df146..ad603f4e08 100644 --- a/python/pip_install/parse_requirements_to_bzl/parse_requirements_to_bzl_test.py +++ b/python/pip_install/parse_requirements_to_bzl/parse_requirements_to_bzl_test.py @@ -6,7 +6,6 @@ from python.pip_install.parse_requirements_to_bzl import generate_parsed_requirements_contents from python.pip_install.extract_wheels.lib.bazel import ( sanitised_repo_library_label, - whl_library_repo_prefix, sanitised_repo_file_label ) @@ -21,7 +20,7 @@ def test_generated_requirements_bzl(self) -> None: requirements_lock.flush() args = argparse.Namespace() args.requirements_lock = requirements_lock.name - args.repo = "pip_parsed_deps" + args.repo_prefix = "pip_parsed_deps_pypi__" extra_pip_args = ["--index-url=pypi.org/simple"] pip_data_exclude = ["**.foo"] args.extra_pip_args = json.dumps({"arg": extra_pip_args}) diff --git a/python/pip_install/pip_repository.bzl b/python/pip_install/pip_repository.bzl index c3007e1519..8e69da7612 100644 --- a/python/pip_install/pip_repository.bzl +++ b/python/pip_install/pip_repository.bzl @@ -138,7 +138,7 @@ def _pip_repository_impl(rctx): rctx.path(rctx.attr.requirements), ] - args += ["--repo", rctx.attr.name] + args += ["--repo", rctx.attr.name, "--repo-prefix", rctx.attr.repo_prefix] args = _parse_optional_attrs(rctx, args) result = rctx.execute( @@ -207,6 +207,18 @@ python_interpreter. default = True, doc = "If True, suppress printing stdout and stderr output to the terminal.", ), + "repo_prefix": attr.string( + doc = """ +Prefix for the generated packages. For non-incremental mode the +packages will be of the form + +@///... + +For incremental mode the packages will be of the form + +@//... +""", + ), # 600 is documented as default here: https://docs.bazel.build/versions/master/skylark/lib/repository_ctx.html#execute "timeout": attr.int( default = 600, @@ -293,6 +305,8 @@ def _impl_whl_library(rctx): rctx.attr.requirement, "--repo", rctx.attr.repo, + "--repo-prefix", + rctx.attr.repo_prefix, ] args = _parse_optional_attrs(rctx, args)