Allow controlling the prefix added to repos/packages (#459)

This consists of a breaking change for users who were directly depending on the prior pip_parse labels.

Author: person142

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.   |  <code>"pip_parsed_deps"</code> |
+| name |  The name of the generated repository. The generated repositories     containing each requirement will be of the form &lt;name&gt;_&lt;requirement-name&gt;.   |  <code>"pip_parsed_deps"</code> |
 | kwargs |  Additional keyword arguments for the underlying     <code>pip_repository</code> rule.   |  none |
 
 

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",
     ],
 )
 

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()

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",
             ],
         )
 

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 <name>_<requirement-name>.
         **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
     )

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")

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
-

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:

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,

python/pip_install/extract_wheels/lib/whl_filegroup_test.py

@@ -24,16 +24,16 @@ 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,
             extras={},
             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__":