From aa8528932b25f7bafc85fcaf238a7bf77c2babad Mon Sep 17 00:00:00 2001 From: Henry Fuller Date: Fri, 5 Mar 2021 12:31:00 -0800 Subject: [PATCH 01/13] Create support for incremental repo's. Refactor pip_repository rule to invoke different scripts based on the value of the incremental attribute to the rule. Create a new macro in repositories.bzl which in the context of an incremental repo will instantiate all the child repos representing individual python packages. Refactor code which is repeated between the create_incremental_repo scripts and the extract wheels script. --- python/pip.bzl | 11 + .../pip_install/create_incremental_repo/BUILD | 0 .../create_incremental_repo/__init__.py | 110 ++++++++++ .../create_incremental_repo/__main__.py | 5 + .../extract_single_wheel/__init__.py | 52 +++++ .../extract_single_wheel/__main__.py | 4 + python/pip_install/extract_wheels/__init__.py | 28 +-- .../pip_install/extract_wheels/lib/bazel.py | 85 ++++++-- .../extract_wheels/lib/utilities.py | 21 ++ python/pip_install/pip_repository.bzl | 190 +++++++++++++----- 10 files changed, 420 insertions(+), 86 deletions(-) create mode 100644 python/pip_install/create_incremental_repo/BUILD create mode 100644 python/pip_install/create_incremental_repo/__init__.py create mode 100644 python/pip_install/create_incremental_repo/__main__.py create mode 100644 python/pip_install/create_incremental_repo/extract_single_wheel/__init__.py create mode 100644 python/pip_install/create_incremental_repo/extract_single_wheel/__main__.py create mode 100644 python/pip_install/extract_wheels/lib/utilities.py diff --git a/python/pip.bzl b/python/pip.bzl index 44e4167f1d..7feee25bfd 100644 --- a/python/pip.bzl +++ b/python/pip.bzl @@ -56,6 +56,17 @@ def pip_install(requirements, name = "pip", **kwargs): **kwargs ) +def pip_install_incremental(requirements_lock, name = "pip_incremental", **kwargs): + # Just in case our dependencies weren't already fetched + pip_install_dependencies() + + pip_repository( + name = name, + requirements_lock = requirements_lock, + incremental = True, + **kwargs + ) + def pip_repositories(): # buildifier: disable=print print("DEPRECATED: the pip_repositories rule has been replaced with pip_install, please see rules_python 0.1 release notes") diff --git a/python/pip_install/create_incremental_repo/BUILD b/python/pip_install/create_incremental_repo/BUILD new file mode 100644 index 0000000000..e69de29bb2 diff --git a/python/pip_install/create_incremental_repo/__init__.py b/python/pip_install/create_incremental_repo/__init__.py new file mode 100644 index 0000000000..17d725b052 --- /dev/null +++ b/python/pip_install/create_incremental_repo/__init__.py @@ -0,0 +1,110 @@ +import argparse +import textwrap +import sys + +from python.pip_install.extract_wheels.lib import bazel, utilities +from pip._internal.req import parse_requirements, constructors +from pip._internal.network.session import PipSession + + +def parse_install_requirements(requirements_lock): + return [ + constructors.install_req_from_parsed_requirement(pr) + for pr in parse_requirements(requirements_lock, session=PipSession()) + ] + + +def repo_names_and_requirements(install_reqs, repo_prefix): + return [ + ( + bazel.sanitise_name(ir.name, prefix=repo_prefix), + str(ir.req) + ) + for ir in install_reqs + ] + + +def generate_incremental_requirements_contents(all_args) -> str: + """ + Parse each requirement from the requirements_lock file, and prepare arguments for each + repository rule, which will represent the individual requirements. + + Generates a requirements.bzl file containing a macro (install_deps()) which instantiates + a repository rule for each requirment in the lock file. + """ + + args = dict(all_args.__dict__) + args.setdefault("python_interpreter", sys.executable) + # Pop this off because it wont be used as a config argurment to thw whl_library rule. + requirements_lock = args.pop("requirements_lock") + repo_prefix = bazel.create_incremental_repo_prefix(args["repo"]) + + install_reqs = parse_install_requirements(requirements_lock) + repo_names_and_reqs = repo_names_and_requirements(install_reqs, repo_prefix) + all_requirements = ", ".join( + [bazel.sanitised_repo_library_label(ir.name, repo_prefix=repo_prefix) for ir in install_reqs] + ) + all_whl_requirements = ", ".join( + [bazel.sanitised_repo_file_label(ir.name, repo_prefix=repo_prefix) for ir in install_reqs] + ) + return textwrap.dedent(f"""\ + load("@rules_python//python/pip_install:pip_repository.bzl", "whl_library") + + all_requirements = [{all_requirements}] + + all_whl_requirements = [{all_whl_requirements}] + + _packages = {repo_names_and_reqs} + _config = {args} + + def _clean_name(name): + return name.replace("-", "_").replace(".", "_").lower() + + def requirement(name): + return "@{repo_prefix}" + _clean_name(name) + "//:pkg" + + def whl_requirement(name): + return "@{repo_prefix}" + _clean_name(name) + "//:whl" + + def install_deps(): + for name, requirement in _packages: + whl_library( + name = name, + requirement = requirement, + **_config, + ) +""") + + +def main() -> None: + parser = argparse.ArgumentParser( + description="Create rules to incrementally fetch needed \ +dependencies from a fully resolved requirements lock file." + ) + parser.add_argument( + "--requirements_lock", + action="store", + required=True, + help="Path to fully resolved requirements.txt to use as the source of repos.", + ) + parser.add_argument( + "--quiet", + type=bool, + action="store", + required=True, + help="Whether to print stdout / stderr from child repos.", + ) + parser.add_argument( + "--timeout", + type=int, + action="store", + required=True, + help="timeout to use for pip operation.", + ) + utilities.parse_common_args(parser) + args = parser.parse_args() + + with open("requirements.bzl", "w") as requirement_file: + requirement_file.write( + generate_incremental_requirements_contents(args) + ) diff --git a/python/pip_install/create_incremental_repo/__main__.py b/python/pip_install/create_incremental_repo/__main__.py new file mode 100644 index 0000000000..23271a5cf9 --- /dev/null +++ b/python/pip_install/create_incremental_repo/__main__.py @@ -0,0 +1,5 @@ +"""Main entry point.""" +from python.pip_install.create_incremental_repo import main + +if __name__ == "__main__": + main() diff --git a/python/pip_install/create_incremental_repo/extract_single_wheel/__init__.py b/python/pip_install/create_incremental_repo/extract_single_wheel/__init__.py new file mode 100644 index 0000000000..8de8a7c0f3 --- /dev/null +++ b/python/pip_install/create_incremental_repo/extract_single_wheel/__init__.py @@ -0,0 +1,52 @@ +import argparse +import sys +import glob +import subprocess +import json + +from python.pip_install.extract_wheels.lib import bazel, requirements, utilities +from python.pip_install.extract_wheels import configure_reproducible_wheels + + +def main() -> None: + parser = argparse.ArgumentParser( + description="Create rules to incrementally fetch needed dependencies \ +from a fully resolved requirements lock file." + ) + parser.add_argument( + "--requirement", + action="store", + required=True, + help="Path to fully resolved requirements.txt to use as the source of repos.", + ) + utilities.parse_common_args(parser) + args = parser.parse_args() + + configure_reproducible_wheels() + + pip_args = [sys.executable, "-m", "pip", "--isolated", "wheel", "--no-deps"] + if args.extra_pip_args: + pip_args += json.loads(args.extra_pip_args)["args"] + + pip_args.append(args.requirement) + + # Assumes any errors are logged by pip so do nothing. This command will fail if pip fails + subprocess.run(pip_args, check=True) + + name, extras_for_pkg = requirements._parse_requirement_for_extra(args.requirement) + extras = {name: extras_for_pkg} if extras_for_pkg else dict() + + if args.pip_data_exclude: + pip_data_exclude = json.loads(args.pip_data_exclude)["exclude"] + else: + pip_data_exclude = [] + + whl = next(iter(glob.glob("*.whl"))) + bazel.extract_wheel( + whl, + extras, + pip_data_exclude, + args.enable_implicit_namespace_pkgs, + incremental=True, + incremental_repo_prefix=bazel.create_incremental_repo_prefix(args.repo) + ) diff --git a/python/pip_install/create_incremental_repo/extract_single_wheel/__main__.py b/python/pip_install/create_incremental_repo/extract_single_wheel/__main__.py new file mode 100644 index 0000000000..2bfa1da065 --- /dev/null +++ b/python/pip_install/create_incremental_repo/extract_single_wheel/__main__.py @@ -0,0 +1,4 @@ +from python.pip_install.create_incremental_repo.extract_single_wheel import main + +if __name__ == "__main__": + main() diff --git a/python/pip_install/extract_wheels/__init__.py b/python/pip_install/extract_wheels/__init__.py index fe8b8ef7ea..d857441054 100644 --- a/python/pip_install/extract_wheels/__init__.py +++ b/python/pip_install/extract_wheels/__init__.py @@ -12,7 +12,7 @@ import sys import json -from python.pip_install.extract_wheels.lib import bazel, requirements +from python.pip_install.extract_wheels.lib import bazel, requirements, utilities def configure_reproducible_wheels() -> None: @@ -58,25 +58,7 @@ def main() -> None: required=True, help="Path to requirements.txt from where to install dependencies", ) - parser.add_argument( - "--repo", - action="store", - required=True, - help="The external repo name to install dependencies. In the format '@{REPO_NAME}'", - ) - parser.add_argument( - "--extra_pip_args", action="store", help="Extra arguments to pass down to pip.", - ) - parser.add_argument( - "--pip_data_exclude", - action="store", - help="Additional data exclusion parameters to add to the pip packages BUILD file.", - ) - parser.add_argument( - "--enable_implicit_namespace_pkgs", - action="store_true", - help="Disables conversion of implicit namespace packages into pkg-util style packages.", - ) + utilities.parse_common_args(parser) args = parser.parse_args() pip_args = [sys.executable, "-m", "pip", "--isolated", "wheel", "-r", args.requirements] @@ -93,10 +75,12 @@ def main() -> None: else: pip_data_exclude = [] + repo_label = "@%s" % args.repo + targets = [ '"%s%s"' % ( - args.repo, + repo_label, bazel.extract_wheel( whl, extras, pip_data_exclude, args.enable_implicit_namespace_pkgs ), @@ -106,5 +90,5 @@ def main() -> None: with open("requirements.bzl", "w") as requirement_file: requirement_file.write( - bazel.generate_requirements_file_contents(args.repo, targets) + bazel.generate_requirements_file_contents(repo_label, targets) ) diff --git a/python/pip_install/extract_wheels/lib/bazel.py b/python/pip_install/extract_wheels/lib/bazel.py index 0affa20b6d..df052ef3eb 100644 --- a/python/pip_install/extract_wheels/lib/bazel.py +++ b/python/pip_install/extract_wheels/lib/bazel.py @@ -2,13 +2,15 @@ import os import textwrap import json -from typing import Iterable, List, Dict, Set +from typing import Iterable, List, Dict, Set, Optional import shutil from python.pip_install.extract_wheels.lib import namespace_pkgs, wheel, purelib WHEEL_FILE_LABEL = "whl" +PY_LIBRARY_LABEL = "pkg" + def generate_build_file_contents( name: str, dependencies: List[str], whl_file_deps: List[str], pip_data_exclude: List[str], @@ -91,6 +93,9 @@ def requirement(name): def whl_requirement(name): return requirement(name) + ":whl" + + def install_deps(): + fail("install_deps() only works if you are creating an incremental repo. Did you mean to use pip_install_incremental()?") """.format( repo=repo_name, requirement_labels=requirement_labels, @@ -99,7 +104,17 @@ def whl_requirement(name): ) -def sanitise_name(name: str) -> str: +DEFAULT_PACKAGE_PREFIX = "pypi__" + + +def create_incremental_repo_prefix(parent_repo): + 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: """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: @@ -116,7 +131,7 @@ def sanitise_name(name: str) -> str: See: https://github.com/bazelbuild/bazel/issues/2636 """ - return "pypi__" + name.replace("-", "_").replace(".", "_").lower() + return prefix + name.replace("-", "_").replace(".", "_").lower() def setup_namespace_pkg_compatibility(wheel_dir: str) -> None: @@ -142,11 +157,33 @@ 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): + return '"//%s"' % sanitise_name(whl_name) + + +def sanitised_file_label(whl_name): + return '"//%s:%s"' % (sanitise_name(whl_name), WHEEL_FILE_LABEL) + + +def _whl_name_to_repo_root(whl_name, repo_prefix): + return f"@{sanitise_name(whl_name, prefix=repo_prefix)}//" + + +def sanitised_repo_library_label(whl_name, repo_prefix): + return f'"{_whl_name_to_repo_root(whl_name, repo_prefix)}:{PY_LIBRARY_LABEL}"' + + +def sanitised_repo_file_label(whl_name, repo_prefix): + return f'"{_whl_name_to_repo_root(whl_name, repo_prefix)}:{WHEEL_FILE_LABEL}"' + + def extract_wheel( wheel_file: str, extras: Dict[str, Set[str]], pip_data_exclude: List[str], enable_implicit_namespace_pkgs: bool, + incremental: bool = False, + incremental_repo_prefix: Optional[str] = None, ) -> str: """Extracts wheel into given directory and creates py_library and filegroup targets. @@ -155,17 +192,24 @@ def extract_wheel( extras: a list of extras to add as dependencies for the installed wheel pip_data_exclude: list of file patterns to exclude from the generated data section of the py_library 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'. """ whl = wheel.Wheel(wheel_file) - directory = sanitise_name(whl.name) - - os.mkdir(directory) - # copy the original wheel - shutil.copy(whl.path, directory) + if incremental: + directory = "." + else: + directory = sanitise_name(whl.name) + + os.mkdir(directory) + # copy the original wheel + shutil.copy(whl.path, directory) whl.unzip(directory) # Note: Order of operations matters here @@ -177,16 +221,27 @@ def extract_wheel( extras_requested = extras[whl.name] if whl.name in extras else set() whl_deps = sorted(whl.dependencies(extras_requested)) - sanitised_dependencies = [ - '"//%s"' % sanitise_name(d) for d in whl_deps - ] - sanitised_wheel_file_dependencies = [ - '"//%s:%s"' % (sanitise_name(d), WHEEL_FILE_LABEL) for d in whl_deps - ] + if incremental: + sanitised_dependencies = [ + sanitised_repo_library_label(d, repo_prefix=incremental_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 + ] + else: + sanitised_dependencies = [ + sanitised_library_label(d) for d in whl_deps + ] + sanitised_wheel_file_dependencies = [ + sanitised_file_label(d) for d in whl_deps + ] with open(os.path.join(directory, "BUILD.bazel"), "w") as build_file: contents = generate_build_file_contents( - sanitise_name(whl.name), sanitised_dependencies, sanitised_wheel_file_dependencies, pip_data_exclude + PY_LIBRARY_LABEL if incremental else sanitise_name(whl.name), + sanitised_dependencies, + sanitised_wheel_file_dependencies, + pip_data_exclude ) build_file.write(contents) diff --git a/python/pip_install/extract_wheels/lib/utilities.py b/python/pip_install/extract_wheels/lib/utilities.py new file mode 100644 index 0000000000..355d7abd3a --- /dev/null +++ b/python/pip_install/extract_wheels/lib/utilities.py @@ -0,0 +1,21 @@ +def parse_common_args(parser): + parser.add_argument( + "--repo", + action="store", + required=True, + help="The external repo name to install dependencies. In the format '@{REPO_NAME}'", + ) + parser.add_argument( + "--extra_pip_args", action="store", help="Extra arguments to pass down to pip.", + ) + parser.add_argument( + "--pip_data_exclude", + action="store", + help="Additional data exclusion parameters to add to the pip packages BUILD file.", + ) + parser.add_argument( + "--enable_implicit_namespace_pkgs", + action="store_true", + help="Disables conversion of implicit namespace packages into pkg-util style packages.", + ) + return parser diff --git a/python/pip_install/pip_repository.bzl b/python/pip_install/pip_repository.bzl index b2cb0a8446..ac8867cdc8 100644 --- a/python/pip_install/pip_repository.bzl +++ b/python/pip_install/pip_repository.bzl @@ -2,17 +2,8 @@ load("//python/pip_install:repositories.bzl", "all_requirements") -def _pip_repository_impl(rctx): - python_interpreter = rctx.attr.python_interpreter - if rctx.attr.python_interpreter_target != None: - target = rctx.attr.python_interpreter_target - python_interpreter = rctx.path(target) - else: - if "/" not in python_interpreter: - python_interpreter = rctx.which(python_interpreter) - if not python_interpreter: - fail("python interpreter not found") +def construct_pypath(rctx): rctx.file("BUILD", "") # Get the root directory of these rules @@ -24,17 +15,9 @@ def _pip_repository_impl(rctx): ] separator = ":" if not "windows" in rctx.os.name.lower() else ";" pypath = separator.join([str(p) for p in [rules_root] + thirdparty_roots]) + return pypath - args = [ - python_interpreter, - "-m", - "python.pip_install.extract_wheels", - "--requirements", - rctx.path(rctx.attr.requirements), - "--repo", - "@%s" % rctx.attr.name, - ] - +def parse_optional_attrs(rctx, args): if rctx.attr.extra_pip_args: args += [ "--extra_pip_args", @@ -50,6 +33,50 @@ def _pip_repository_impl(rctx): if rctx.attr.enable_implicit_namespace_pkgs: args.append("--enable_implicit_namespace_pkgs") + return args + + +def _pip_repository_impl(rctx): + python_interpreter = rctx.attr.python_interpreter + if rctx.attr.python_interpreter_target != None: + target = rctx.attr.python_interpreter_target + python_interpreter = rctx.path(target) + else: + if "/" not in python_interpreter: + python_interpreter = rctx.which(python_interpreter) + if not python_interpreter: + fail("python interpreter not found") + + if rctx.attr.incremental and not rctx.attr.requirements_lock: + fail("Incremental mode requires a requirements_lock attribute be specified.") + + pypath = construct_pypath(rctx) + + if rctx.attr.incremental: + args = [ + python_interpreter, + "-m", + "python.pip_install.create_incremental_repo", + "--requirements_lock", + rctx.path(rctx.attr.requirements_lock), + # pass quiet and timeout args through to child repos. + "--quiet", + str(rctx.attr.quiet), + "--timeout", + str(rctx.attr.timeout), + ] + else: + args = [ + python_interpreter, + "-m", + "python.pip_install.extract_wheels", + "--requirements", + rctx.path(rctx.attr.requirements), + ] + + args += ["--repo", rctx.attr.name] + args = parse_optional_attrs(rctx, args) + result = rctx.execute( args, environment = { @@ -59,51 +86,70 @@ def _pip_repository_impl(rctx): timeout = rctx.attr.timeout, quiet = rctx.attr.quiet, ) + if result.return_code: fail("rules_python_external failed: %s (%s)" % (result.stdout, result.stderr)) return -pip_repository = repository_rule( - attrs = { - "enable_implicit_namespace_pkgs": attr.bool( - default = False, - doc = """ + +common_attrs = { + "enable_implicit_namespace_pkgs": attr.bool( + default = False, + doc = """ If true, disables conversion of native namespace packages into pkg-util style namespace packages. When set all py_binary and py_test targets must specify either `legacy_create_init=False` or the global Bazel option `--incompatible_default_to_explicit_init_py` to prevent `__init__.py` being automatically generated in every directory. This option is required to support some packages which cannot handle the conversion to pkg-util style. """, - ), - "extra_pip_args": attr.string_list( - doc = "Extra arguments to pass on to pip. Must not contain spaces.", - ), - "pip_data_exclude": attr.string_list( - doc = "Additional data exclusion parameters to add to the pip packages BUILD file.", - ), - "python_interpreter": attr.string(default = "python3"), - "python_interpreter_target": attr.label(allow_single_file = True, doc = """ + ), + "extra_pip_args": attr.string_list( + doc = "Extra arguments to pass on to pip. Must not contain spaces.", + ), + "pip_data_exclude": attr.string_list( + doc = "Additional data exclusion parameters to add to the pip packages BUILD file.", + ), + "python_interpreter": attr.string(default = "python3"), + "python_interpreter_target": attr.label(allow_single_file = True, doc = """ If you are using a custom python interpreter built by another repository rule, use this attribute to specify its BUILD target. This allows pip_repository to invoke pip using the same interpreter as your toolchain. If set, takes precedence over python_interpreter. """), - "quiet": attr.bool( - default = True, - doc = "If True, suppress printing stdout and stderr output to the terminal.", - ), - "requirements": attr.label( - allow_single_file = True, - mandatory = True, - doc = "A 'requirements.txt' pip requirements file.", - ), - # 600 is documented as default here: https://docs.bazel.build/versions/master/skylark/lib/repository_ctx.html#execute - "timeout": attr.int( - default = 600, - doc = "Timeout (in seconds) on the rule's execution duration.", - ), - }, + "quiet": attr.bool( + default = True, + doc = "If True, suppress printing stdout and stderr output to the terminal.", + ), + # 600 is documented as default here: https://docs.bazel.build/versions/master/skylark/lib/repository_ctx.html#execute + "timeout": attr.int( + default = 600, + doc = "Timeout (in seconds) on the rule's execution duration.", + ), +} + +pip_repository_attrs = { + "requirements": attr.label( + allow_single_file = True, + doc = "A 'requirements.txt' pip requirements file.", + ), + "requirements_lock": attr.label( + allow_single_file = True, + doc = """ +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'. +"""), + "incremental": attr.bool( + default = False, + doc = "Create the repository in incremental form." + ) +} + +pip_repository_attrs.update(**common_attrs) + +pip_repository = repository_rule( + attrs = pip_repository_attrs, implementation = _pip_repository_impl, doc = """A rule for importing `requirements.txt` dependencies into Bazel. @@ -145,3 +191,49 @@ py_binary( ``` """, ) + +def _impl_whl_library(rctx): + # pointer to parent repo so these rules rerun if the definitions in requirements.bzl change. + _parent_repo_label = Label("@{parent}//:requirements.bzl".format(parent=rctx.attr.repo)) + pypath = construct_pypath(rctx) + args = [ + rctx.attr.python_interpreter, + "-m", + "python.pip_install.create_incremental_repo.extract_single_wheel", + "--requirement", + rctx.attr.requirement, + "--repo", + rctx.attr.repo, + ] + args = parse_optional_attrs(rctx, args) + result = rctx.execute( + args, + environment = { + # Manually construct the PYTHONPATH since we cannot use the toolchain here + "PYTHONPATH": pypath, + }, + quiet = rctx.attr.quiet, + timeout = rctx.attr.timeout, + ) + + if result.return_code: + fail("whl_library %s failed: %s (%s)" % (rctx.attr.name, result.stdout, result.stderr)) + + return + + +whl_library_attrs = { + "requirement": attr.string(mandatory=True, doc = "Python requirement string describing the package to make available"), + "repo": attr.string(mandatory=True, doc = "Pointer to parent repo name. Used to make these rules rerun if the parent repo changes.") +} + +whl_library_attrs.update(**common_attrs) + + +whl_library = repository_rule( + attrs = whl_library_attrs, + implementation = _impl_whl_library, + doc = """ +Download and extracts a single wheel based into a bazel repo based on the requirement string passed in. +Instantiated from pip_repository and inherits config options from there.""" +) From 66e28f6dbfd34c21e4d1c46f901e436a7060aeea Mon Sep 17 00:00:00 2001 From: Henry Fuller Date: Fri, 5 Mar 2021 12:33:28 -0800 Subject: [PATCH 02/13] Lint changes. --- .gitignore | 4 ++++ python/pip_install/extract_wheels/lib/bazel.py | 2 +- python/pip_install/extract_wheels/lib/purelib.py | 2 +- python/pip_install/pip_repository.bzl | 2 +- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index dcfa539a21..cc8decd9a1 100644 --- a/.gitignore +++ b/.gitignore @@ -37,3 +37,7 @@ bazel-bin bazel-genfiles bazel-out bazel-testlogs + +# vim swap files +*.swp +*.swo diff --git a/python/pip_install/extract_wheels/lib/bazel.py b/python/pip_install/extract_wheels/lib/bazel.py index df052ef3eb..d631d1b9b3 100644 --- a/python/pip_install/extract_wheels/lib/bazel.py +++ b/python/pip_install/extract_wheels/lib/bazel.py @@ -150,7 +150,7 @@ def setup_namespace_pkg_compatibility(wheel_dir: str) -> None: """ namespace_pkg_dirs = namespace_pkgs.implicit_namespace_packages( - wheel_dir, ignored_dirnames=["%s/bin" % wheel_dir,], + wheel_dir, ignored_dirnames=["%s/bin" % wheel_dir], ) for ns_pkg_dir in namespace_pkg_dirs: diff --git a/python/pip_install/extract_wheels/lib/purelib.py b/python/pip_install/extract_wheels/lib/purelib.py index 99f6299317..4e9eb3f3ef 100644 --- a/python/pip_install/extract_wheels/lib/purelib.py +++ b/python/pip_install/extract_wheels/lib/purelib.py @@ -27,7 +27,7 @@ def spread_purelib_into_root(wheel_dir: str) -> None: return dot_data_dir = wheel.get_dot_data_directory(wheel_dir) - # 'Root-Is-Purelib: false' is no guarantee a .date directory exists with + # 'Root-Is-Purelib: false' is no guarantee a .data directory exists with # package code in it. eg. the 'markupsafe' package. if not dot_data_dir: return diff --git a/python/pip_install/pip_repository.bzl b/python/pip_install/pip_repository.bzl index ac8867cdc8..3f246bf98e 100644 --- a/python/pip_install/pip_repository.bzl +++ b/python/pip_install/pip_repository.bzl @@ -88,7 +88,7 @@ def _pip_repository_impl(rctx): ) if result.return_code: - fail("rules_python_external failed: %s (%s)" % (result.stdout, result.stderr)) + fail("rules_python failed: %s (%s)" % (result.stdout, result.stderr)) return From 49be121b70c373a3e73d1a325882512f91ed5734 Mon Sep 17 00:00:00 2001 From: Henry Fuller Date: Fri, 5 Mar 2021 14:45:12 -0800 Subject: [PATCH 03/13] add mypy type annotations to new code --- .../create_incremental_repo/__init__.py | 10 ++++++---- .../extract_single_wheel/__init__.py | 2 +- python/pip_install/extract_wheels/lib/bazel.py | 15 +++++++++------ .../pip_install/extract_wheels/lib/utilities.py | 5 ++++- 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/python/pip_install/create_incremental_repo/__init__.py b/python/pip_install/create_incremental_repo/__init__.py index 17d725b052..349e6b10d8 100644 --- a/python/pip_install/create_incremental_repo/__init__.py +++ b/python/pip_install/create_incremental_repo/__init__.py @@ -1,20 +1,22 @@ import argparse import textwrap import sys +from typing import List, Tuple from python.pip_install.extract_wheels.lib import bazel, utilities from pip._internal.req import parse_requirements, constructors +from pip._internal.req.req_install import InstallRequirement from pip._internal.network.session import PipSession -def parse_install_requirements(requirements_lock): +def parse_install_requirements(requirements_lock: str) -> List[InstallRequirement]: return [ constructors.install_req_from_parsed_requirement(pr) for pr in parse_requirements(requirements_lock, session=PipSession()) ] -def repo_names_and_requirements(install_reqs, repo_prefix): +def repo_names_and_requirements(install_reqs: List[InstallRequirement], repo_prefix: str) -> List[Tuple[str, str]]: return [ ( bazel.sanitise_name(ir.name, prefix=repo_prefix), @@ -24,7 +26,7 @@ def repo_names_and_requirements(install_reqs, repo_prefix): ] -def generate_incremental_requirements_contents(all_args) -> str: +def generate_incremental_requirements_contents(all_args: argparse.Namespace) -> str: """ Parse each requirement from the requirements_lock file, and prepare arguments for each repository rule, which will represent the individual requirements. @@ -33,7 +35,7 @@ def generate_incremental_requirements_contents(all_args) -> str: a repository rule for each requirment in the lock file. """ - args = dict(all_args.__dict__) + args = dict(vars(all_args)) args.setdefault("python_interpreter", sys.executable) # Pop this off because it wont be used as a config argurment to thw whl_library rule. requirements_lock = args.pop("requirements_lock") diff --git a/python/pip_install/create_incremental_repo/extract_single_wheel/__init__.py b/python/pip_install/create_incremental_repo/extract_single_wheel/__init__.py index 8de8a7c0f3..4f9707ede4 100644 --- a/python/pip_install/create_incremental_repo/extract_single_wheel/__init__.py +++ b/python/pip_install/create_incremental_repo/extract_single_wheel/__init__.py @@ -34,7 +34,7 @@ def main() -> None: subprocess.run(pip_args, check=True) name, extras_for_pkg = requirements._parse_requirement_for_extra(args.requirement) - extras = {name: extras_for_pkg} if extras_for_pkg else dict() + extras = {name: extras_for_pkg} if extras_for_pkg and name else dict() if args.pip_data_exclude: pip_data_exclude = json.loads(args.pip_data_exclude)["exclude"] diff --git a/python/pip_install/extract_wheels/lib/bazel.py b/python/pip_install/extract_wheels/lib/bazel.py index d631d1b9b3..23587a3dc4 100644 --- a/python/pip_install/extract_wheels/lib/bazel.py +++ b/python/pip_install/extract_wheels/lib/bazel.py @@ -107,7 +107,7 @@ def install_deps(): DEFAULT_PACKAGE_PREFIX = "pypi__" -def create_incremental_repo_prefix(parent_repo): +def create_incremental_repo_prefix(parent_repo: str) -> str: return "{parent}_{default_package_prefix}".format( parent=parent_repo, default_package_prefix=DEFAULT_PACKAGE_PREFIX @@ -157,23 +157,23 @@ 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): +def sanitised_library_label(whl_name: str) -> str: return '"//%s"' % sanitise_name(whl_name) -def sanitised_file_label(whl_name): +def sanitised_file_label(whl_name: str) -> str: return '"//%s:%s"' % (sanitise_name(whl_name), WHEEL_FILE_LABEL) -def _whl_name_to_repo_root(whl_name, repo_prefix): +def _whl_name_to_repo_root(whl_name: str, repo_prefix: str) -> str: return f"@{sanitise_name(whl_name, prefix=repo_prefix)}//" -def sanitised_repo_library_label(whl_name, repo_prefix): +def sanitised_repo_library_label(whl_name: str, repo_prefix: str) -> str: return f'"{_whl_name_to_repo_root(whl_name, repo_prefix)}:{PY_LIBRARY_LABEL}"' -def sanitised_repo_file_label(whl_name, repo_prefix): +def sanitised_repo_file_label(whl_name: str, repo_prefix: str) -> str: return f'"{_whl_name_to_repo_root(whl_name, repo_prefix)}:{WHEEL_FILE_LABEL}"' @@ -222,6 +222,9 @@ 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 ] diff --git a/python/pip_install/extract_wheels/lib/utilities.py b/python/pip_install/extract_wheels/lib/utilities.py index 355d7abd3a..ee9a6491bc 100644 --- a/python/pip_install/extract_wheels/lib/utilities.py +++ b/python/pip_install/extract_wheels/lib/utilities.py @@ -1,4 +1,7 @@ -def parse_common_args(parser): +from argparse import ArgumentParser + + +def parse_common_args(parser: ArgumentParser) -> ArgumentParser: parser.add_argument( "--repo", action="store", From 0eae2176ec6ec9c15851f6f974895042d3a224e3 Mon Sep 17 00:00:00 2001 From: Henry Fuller Date: Sun, 7 Mar 2021 19:30:33 -0800 Subject: [PATCH 04/13] add tests for new incremental repo functions. --- .../pip_install/create_incremental_repo/BUILD | 20 +++++++++++ .../create_incremental_repo/__init__.py | 2 +- .../test_create_incremental_repo.py | 34 +++++++++++++++++++ python/pip_install/extract_wheels/lib/BUILD | 18 +++++++++- .../extract_wheels/lib/utilities_test.py | 23 +++++++++++++ 5 files changed, 95 insertions(+), 2 deletions(-) create mode 100644 python/pip_install/create_incremental_repo/test_create_incremental_repo.py create mode 100644 python/pip_install/extract_wheels/lib/utilities_test.py diff --git a/python/pip_install/create_incremental_repo/BUILD b/python/pip_install/create_incremental_repo/BUILD index e69de29bb2..3ba376e6ac 100644 --- a/python/pip_install/create_incremental_repo/BUILD +++ b/python/pip_install/create_incremental_repo/BUILD @@ -0,0 +1,20 @@ +load("//python/pip_install:repositories.bzl", "requirement") + +py_library( + name = "lib", + srcs = ["__init__.py"], + deps = [requirement("pip")] +) + +py_test( + name = "test_create_incremental_repo", + size = "small", + srcs = [ + "test_create_incremental_repo.py", + ], + tags = ["unit"], + deps = [ + ":lib", + "//python/pip_install/extract_wheels/lib" + ], +) diff --git a/python/pip_install/create_incremental_repo/__init__.py b/python/pip_install/create_incremental_repo/__init__.py index 349e6b10d8..413683d86b 100644 --- a/python/pip_install/create_incremental_repo/__init__.py +++ b/python/pip_install/create_incremental_repo/__init__.py @@ -37,7 +37,7 @@ def generate_incremental_requirements_contents(all_args: argparse.Namespace) -> args = dict(vars(all_args)) args.setdefault("python_interpreter", sys.executable) - # Pop this off because it wont be used as a config argurment to thw whl_library rule. + # 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.create_incremental_repo_prefix(args["repo"]) diff --git a/python/pip_install/create_incremental_repo/test_create_incremental_repo.py b/python/pip_install/create_incremental_repo/test_create_incremental_repo.py new file mode 100644 index 0000000000..16cc2f1d02 --- /dev/null +++ b/python/pip_install/create_incremental_repo/test_create_incremental_repo.py @@ -0,0 +1,34 @@ +import unittest +import argparse +from tempfile import NamedTemporaryFile + +from python.pip_install.create_incremental_repo import generate_incremental_requirements_contents +from python.pip_install.extract_wheels.lib.bazel import ( + sanitised_repo_library_label, + create_incremental_repo_prefix, + sanitised_repo_file_label +) + + +class TestGenerateRequirementsFileContents(unittest.TestCase): + + def test_incremental_requirements_bzl(self) -> None: + with NamedTemporaryFile() as requirements_lock: + requirement_string = "foo==0.0.0" + requirements_lock.write(bytes(requirement_string, encoding="utf-8")) + requirements_lock.flush() + args = argparse.Namespace() + args.requirements_lock = requirements_lock.name + args.repo = "pip_incremental" + contents = generate_incremental_requirements_contents(args) + library_target = sanitised_repo_library_label("foo", create_incremental_repo_prefix(args.repo)) + whl_target = sanitised_repo_file_label("foo", create_incremental_repo_prefix(args.repo)) + all_requirements = f'all_requirements = [{library_target}]' + all_whl_requirements = f'all_whl_requirements = [{whl_target}]' + self.assertIn(all_requirements, contents, contents) + self.assertIn(all_whl_requirements, contents, contents) + self.assertIn(requirement_string, contents, contents) + + +if __name__ == "__main__": + unittest.main() diff --git a/python/pip_install/extract_wheels/lib/BUILD b/python/pip_install/extract_wheels/lib/BUILD index 2a269856be..40d4413d4f 100644 --- a/python/pip_install/extract_wheels/lib/BUILD +++ b/python/pip_install/extract_wheels/lib/BUILD @@ -9,8 +9,12 @@ py_library( "purelib.py", "requirements.py", "wheel.py", + "utilities.py", + ], + visibility = [ + "//python/pip_install/extract_wheels:__subpackages__", + "//python/pip_install/create_incremental_repo:__pkg__" ], - visibility = ["//python/pip_install/extract_wheels:__subpackages__"], deps = [ requirement("pkginfo"), requirement("setuptools"), @@ -41,6 +45,18 @@ py_test( ], ) +py_test( + name = "utilities_test", + size = "small", + srcs = [ + "utilities_test.py", + ], + tags = ["unit"], + deps = [ + ":lib", + ], +) + py_test( name = "whl_filegroup_test", size = "small", diff --git a/python/pip_install/extract_wheels/lib/utilities_test.py b/python/pip_install/extract_wheels/lib/utilities_test.py new file mode 100644 index 0000000000..c92108fb39 --- /dev/null +++ b/python/pip_install/extract_wheels/lib/utilities_test.py @@ -0,0 +1,23 @@ +import unittest +import argparse + +from python.pip_install.extract_wheels.lib import utilities + + +class TestUtilities(unittest.TestCase): + def test_utilities(self) -> None: + parser = argparse.ArgumentParser() + parser = utilities.parse_common_args(parser) + repo_name = "foo" + index_url = "--index_url=pypi.org/simple" + args_dict = vars(parser.parse_args(args=["--repo", repo_name, f"--extra_pip_args={index_url}"])) + self.assertIn("repo", args_dict) + self.assertIn("extra_pip_args", args_dict) + self.assertEqual(args_dict["pip_data_exclude"], None) + self.assertEqual(args_dict["enable_implicit_namespace_pkgs"], False) + self.assertEqual(args_dict["repo"], repo_name) + self.assertEqual(args_dict["extra_pip_args"], index_url) + + +if __name__ == "__main__": + unittest.main() From 2257c3f68dd0dbbb8a8b541f3a67d82df3c75483 Mon Sep 17 00:00:00 2001 From: Henry Fuller Date: Sun, 7 Mar 2021 19:52:28 -0800 Subject: [PATCH 05/13] add example --- examples/pip_install_incremental/BUILD | 42 +++++++++++++++++++ examples/pip_install_incremental/WORKSPACE | 40 ++++++++++++++++++ examples/pip_install_incremental/main.py | 5 +++ .../pip_install_incremental/requirements.txt | 1 + .../requirements_lock.txt | 11 +++++ examples/pip_install_incremental/test.py | 11 +++++ 6 files changed, 110 insertions(+) create mode 100644 examples/pip_install_incremental/BUILD create mode 100644 examples/pip_install_incremental/WORKSPACE create mode 100644 examples/pip_install_incremental/main.py create mode 100644 examples/pip_install_incremental/requirements.txt create mode 100644 examples/pip_install_incremental/requirements_lock.txt create mode 100644 examples/pip_install_incremental/test.py diff --git a/examples/pip_install_incremental/BUILD b/examples/pip_install_incremental/BUILD new file mode 100644 index 0000000000..d32c94f665 --- /dev/null +++ b/examples/pip_install_incremental/BUILD @@ -0,0 +1,42 @@ +load("@pip//:requirements.bzl", "requirement") +load("@rules_python//python:defs.bzl", "py_binary", "py_test") + +# Toolchain setup, this is optional. +# Demonstrate that we can use the same python interpreter for the toolchain and executing pip in pip install (see WORKSPACE). +# +#load("@rules_python//python:defs.bzl", "py_runtime_pair") +# +#py_runtime( +# name = "python3_runtime", +# files = ["@python_interpreter//:files"], +# interpreter = "@python_interpreter//:python_bin", +# python_version = "PY3", +# visibility = ["//visibility:public"], +#) +# +#py_runtime_pair( +# name = "my_py_runtime_pair", +# py2_runtime = None, +# py3_runtime = ":python3_runtime", +#) +# +#toolchain( +# name = "my_py_toolchain", +# toolchain = ":my_py_runtime_pair", +# toolchain_type = "@bazel_tools//tools/python:toolchain_type", +#) +# End of toolchain setup. + +py_binary( + name = "main", + srcs = ["main.py"], + deps = [ + requirement("requests"), + ], +) + +py_test( + name = "test", + srcs = ["test.py"], + deps = [":main"], +) diff --git a/examples/pip_install_incremental/WORKSPACE b/examples/pip_install_incremental/WORKSPACE new file mode 100644 index 0000000000..5123e2a5a7 --- /dev/null +++ b/examples/pip_install_incremental/WORKSPACE @@ -0,0 +1,40 @@ +workspace(name = "example_repo") + +load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository") + +# TODO(hrfuller) change this to a released http_archive when a release exists. +git_repository( + name = "rules_python", + remote = "https://github.com/bazelbuild/rules_python.git", + commit = "57f13c054948679ced273d4ea9078ddb4e633f60" +) + +load("@rules_python//python:pip.bzl", "pip_install_incremental") + +pip_install_incremental( + # (Optional) You can provide extra parameters to pip. + # Here, make pip output verbose (this is usable with `quiet = False`). + #extra_pip_args = ["-v"], + + # (Optional) You can exclude custom elements in the data section of the generated BUILD files for pip packages. + # Exclude directories with spaces in their names in this example (avoids build errors if there are such directories). + #pip_data_exclude = ["**/* */**"], + + # (Optional) You can provide a python_interpreter (path) or a python_interpreter_target (a Bazel target, that + # acts as an executable). The latter can be anything that could be used as Python interpreter. E.g.: + # 1. Python interpreter that you compile in the build file (as above in @python_interpreter). + # 2. Pre-compiled python interpreter included with http_archive + # 3. Wrapper script, like in the autodetecting python toolchain. + #python_interpreter_target = "@python_interpreter//:python_bin", + + # (Optional) You can set quiet to False if you want to see pip output. + #quiet = False, + + # Uses the default repository name "pip_incremental" + requirements_lock = "//:requirements_lock.txt", +) + +load("@pip_incremental//:requirements.bzl", "install_deps") + +# Initialize repositories for all packages in requirements_lock.txt. +install_deps() diff --git a/examples/pip_install_incremental/main.py b/examples/pip_install_incremental/main.py new file mode 100644 index 0000000000..79e1c1219b --- /dev/null +++ b/examples/pip_install_incremental/main.py @@ -0,0 +1,5 @@ +import requests + + +def version(): + return requests.__version__ diff --git a/examples/pip_install_incremental/requirements.txt b/examples/pip_install_incremental/requirements.txt new file mode 100644 index 0000000000..4998f3f857 --- /dev/null +++ b/examples/pip_install_incremental/requirements.txt @@ -0,0 +1 @@ +requests[security]==2.24.0 diff --git a/examples/pip_install_incremental/requirements_lock.txt b/examples/pip_install_incremental/requirements_lock.txt new file mode 100644 index 0000000000..504891e7f2 --- /dev/null +++ b/examples/pip_install_incremental/requirements_lock.txt @@ -0,0 +1,11 @@ +# Transitive dependency set from :requirements.txt +certifi==2020.12.5 # via requests +cffi==1.14.5 # via cryptography +chardet==3.0.4 # via requests +cryptography==3.4.6 # via pyopenssl, requests +idna==2.10 # via requests +pycparser==2.20 # via cffi +pyopenssl==20.0.1 # via requests +requests[security]==2.24.0 # via -r /dev/fd/11 +six==1.15.0 # via pyopenssl +urllib3==1.25.11 # via requests diff --git a/examples/pip_install_incremental/test.py b/examples/pip_install_incremental/test.py new file mode 100644 index 0000000000..3cfb9bb91e --- /dev/null +++ b/examples/pip_install_incremental/test.py @@ -0,0 +1,11 @@ +import unittest +import main + + +class ExampleTest(unittest.TestCase): + def test_main(self): + self.assertEqual("2.24.0", main.version()) + + +if __name__ == '__main__': + unittest.main() From 4e1c72f0ed2b7e7426fb929f6f4137191090fd14 Mon Sep 17 00:00:00 2001 From: Henry Fuller Date: Sun, 7 Mar 2021 20:20:55 -0800 Subject: [PATCH 06/13] update readme --- .bazelrc | 4 +- README.md | 37 ++++++++++++++++++- examples/pip_install_incremental/BUILD | 2 +- .../pip_install/create_incremental_repo/BUILD | 1 + python/pip_install/pip_repository.bzl | 15 ++++++-- 5 files changed, 51 insertions(+), 8 deletions(-) diff --git a/.bazelrc b/.bazelrc index 1afaedbc24..12cf01c077 100644 --- a/.bazelrc +++ b/.bazelrc @@ -3,7 +3,7 @@ # This lets us glob() up all the files inside the examples to make them inputs to tests # (Note, we cannot use `common --deleted_packages` because the bazel version command doesn't support it) # To update these lines, run tools/bazel_integration_test/update_deleted_packages.sh -build --deleted_packages=examples/legacy_pip_import/boto,examples/legacy_pip_import/extras,examples/legacy_pip_import/helloworld,examples/pip_install -query --deleted_packages=examples/legacy_pip_import/boto,examples/legacy_pip_import/extras,examples/legacy_pip_import/helloworld,examples/pip_install +build --deleted_packages=examples/legacy_pip_import/boto,examples/legacy_pip_import/extras,examples/legacy_pip_import/helloworld,examples/pip_install,examples/pip_install_incremental +query --deleted_packages=examples/legacy_pip_import/boto,examples/legacy_pip_import/extras,examples/legacy_pip_import/helloworld,examples/pip_install,examples/pip_install_incrmental test --test_output=errors diff --git a/README.md b/README.md index 591c403554..43b4a65d7c 100644 --- a/README.md +++ b/README.md @@ -105,7 +105,7 @@ target in the appropriate wheel repo. ### Importing `pip` dependencies -To add pip dependencies to your `WORKSPACE` is you load +To add pip dependencies to your `WORKSPACE` load the `pip_install` function, and call it to create the individual wheel repos. @@ -136,6 +136,41 @@ re-executed in order to pick up a non-hermetic change to your environment (e.g., updating your system `python` interpreter), you can completely flush out your repo cache with `bazel clean --expunge`. +### Importing `pip` dependencies incrementally (experimental) + +One pain point with `pip_install` is the need to download all dependencies resolved by +your requirements.txt before the bazel analysis phase can start. For large python monorepos +this can take a long time, especially on slow connections. + +To download only the pip packages needed to build targets in the +subgraph of top level targets in your bazel invocation, you can experiment with using `pip_install_incremental`. +The interface of `pip_install_incremental` mirrors `pip_install` as closely as possible. + +The only user facing difference between `pip_install` and `pip_install_incremental` is that for the latter you need +to supply a fully resolved and pinned requirements_lock.txt file (named to distinguish it from requirments.txt +used in `pip_install`). The `requirements` attribute is replaced with a `requirements_lock` attribute to make it +clear that a fully pinned transitive resolve is needed. + +To add incremental pip dependencies to your `WORKSPACE` load +the `pip_install_incremental` function, and call it to create a main +repo which contains a macro called `install_deps()` which is used +to create child repos for each package in your requirements_lock.txt. + + +```python +load("@rules_python//python:pip.bzl", "pip_install_incremental") + +# Create a central repo that knows about the dependencies needed for +# requirements.txt. +pip_install( + name = "my_deps", + requirements_lock = "//path/to:requirements_lock.txt", +) + +load("@my_deps//:requirements.bzl", "install_deps") +install_deps() +``` + ### Importing `pip` dependencies with `pip_import` (legacy) The deprecated `pip_import` can still be used if needed. diff --git a/examples/pip_install_incremental/BUILD b/examples/pip_install_incremental/BUILD index d32c94f665..adc286ff9c 100644 --- a/examples/pip_install_incremental/BUILD +++ b/examples/pip_install_incremental/BUILD @@ -1,4 +1,4 @@ -load("@pip//:requirements.bzl", "requirement") +load("@pip_incremental//:requirements.bzl", "requirement") load("@rules_python//python:defs.bzl", "py_binary", "py_test") # Toolchain setup, this is optional. diff --git a/python/pip_install/create_incremental_repo/BUILD b/python/pip_install/create_incremental_repo/BUILD index 3ba376e6ac..0fa0bf24ad 100644 --- a/python/pip_install/create_incremental_repo/BUILD +++ b/python/pip_install/create_incremental_repo/BUILD @@ -1,3 +1,4 @@ +load("@rules_python//python:defs.bzl", "py_library", "py_test") load("//python/pip_install:repositories.bzl", "requirement") py_library( diff --git a/python/pip_install/pip_repository.bzl b/python/pip_install/pip_repository.bzl index 3f246bf98e..038a245e81 100644 --- a/python/pip_install/pip_repository.bzl +++ b/python/pip_install/pip_repository.bzl @@ -4,6 +4,10 @@ load("//python/pip_install:repositories.bzl", "all_requirements") def construct_pypath(rctx): + """Helper function to construct a PYTHONPATH containing entries for + code in this repo as well as packages downloaded from //python/pip_install:repositories.bzl. + This allows us to run python code inside repository rule implementations. + """ rctx.file("BUILD", "") # Get the root directory of these rules @@ -18,6 +22,9 @@ def construct_pypath(rctx): return pypath def parse_optional_attrs(rctx, args): + """Helper function to parse common attributes of pip_repository + and whl_library repository rules. + """ if rctx.attr.extra_pip_args: args += [ "--extra_pip_args", @@ -129,6 +136,10 @@ python_interpreter. } pip_repository_attrs = { + "incremental": attr.bool( + default = False, + doc = "Create the repository in incremental mode." + ) "requirements": attr.label( allow_single_file = True, doc = "A 'requirements.txt' pip requirements file.", @@ -140,10 +151,6 @@ A fully resolved 'requirements.txt' pip requirement file containing the transiti 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'. """), - "incremental": attr.bool( - default = False, - doc = "Create the repository in incremental form." - ) } pip_repository_attrs.update(**common_attrs) From 93301f2b8601c2c8c21101c48bf51dcf1a5f331d Mon Sep 17 00:00:00 2001 From: Henry Fuller Date: Sun, 7 Mar 2021 21:19:46 -0800 Subject: [PATCH 07/13] f-strings not allowed in CI. Fix syntax error introduced fixing linting. --- examples/pip_install_incremental/WORKSPACE | 2 +- .../pip_install/create_incremental_repo/__init__.py | 11 +++++++++-- .../test_create_incremental_repo.py | 4 ++-- python/pip_install/extract_wheels/lib/bazel.py | 6 +++--- python/pip_install/pip_repository.bzl | 4 ++-- 5 files changed, 17 insertions(+), 10 deletions(-) diff --git a/examples/pip_install_incremental/WORKSPACE b/examples/pip_install_incremental/WORKSPACE index 5123e2a5a7..b98d84521f 100644 --- a/examples/pip_install_incremental/WORKSPACE +++ b/examples/pip_install_incremental/WORKSPACE @@ -6,7 +6,7 @@ load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository") git_repository( name = "rules_python", remote = "https://github.com/bazelbuild/rules_python.git", - commit = "57f13c054948679ced273d4ea9078ddb4e633f60" + branch = "master" ) load("@rules_python//python:pip.bzl", "pip_install_incremental") diff --git a/python/pip_install/create_incremental_repo/__init__.py b/python/pip_install/create_incremental_repo/__init__.py index 413683d86b..157db62262 100644 --- a/python/pip_install/create_incremental_repo/__init__.py +++ b/python/pip_install/create_incremental_repo/__init__.py @@ -49,7 +49,7 @@ def generate_incremental_requirements_contents(all_args: argparse.Namespace) -> all_whl_requirements = ", ".join( [bazel.sanitised_repo_file_label(ir.name, repo_prefix=repo_prefix) for ir in install_reqs] ) - return textwrap.dedent(f"""\ + return textwrap.dedent("""\ load("@rules_python//python/pip_install:pip_repository.bzl", "whl_library") all_requirements = [{all_requirements}] @@ -75,7 +75,14 @@ def install_deps(): requirement = requirement, **_config, ) -""") + """.format( + all_requirements=all_requirements, + all_whl_requirements=all_whl_requirements, + repo_names_and_reqs=repo_names_and_reqs, + args=args, + repo_prefix=repo_prefix, + ) + ) def main() -> None: diff --git a/python/pip_install/create_incremental_repo/test_create_incremental_repo.py b/python/pip_install/create_incremental_repo/test_create_incremental_repo.py index 16cc2f1d02..a5f275e0a4 100644 --- a/python/pip_install/create_incremental_repo/test_create_incremental_repo.py +++ b/python/pip_install/create_incremental_repo/test_create_incremental_repo.py @@ -23,8 +23,8 @@ def test_incremental_requirements_bzl(self) -> None: contents = generate_incremental_requirements_contents(args) library_target = sanitised_repo_library_label("foo", create_incremental_repo_prefix(args.repo)) whl_target = sanitised_repo_file_label("foo", create_incremental_repo_prefix(args.repo)) - all_requirements = f'all_requirements = [{library_target}]' - all_whl_requirements = f'all_whl_requirements = [{whl_target}]' + all_requirements = 'all_requirements = [{library_target}]'.format(library_target=library_target) + all_whl_requirements = 'all_whl_requirements = [{whl_target}]'.format(whl_target=whl_target) self.assertIn(all_requirements, contents, contents) self.assertIn(all_whl_requirements, contents, contents) self.assertIn(requirement_string, contents, contents) diff --git a/python/pip_install/extract_wheels/lib/bazel.py b/python/pip_install/extract_wheels/lib/bazel.py index 23587a3dc4..fb910e2b58 100644 --- a/python/pip_install/extract_wheels/lib/bazel.py +++ b/python/pip_install/extract_wheels/lib/bazel.py @@ -166,15 +166,15 @@ def sanitised_file_label(whl_name: str) -> str: def _whl_name_to_repo_root(whl_name: str, repo_prefix: str) -> str: - return f"@{sanitise_name(whl_name, prefix=repo_prefix)}//" + return "@{}//".format(sanitise_name(whl_name, prefix=repo_prefix)) def sanitised_repo_library_label(whl_name: str, repo_prefix: str) -> str: - return f'"{_whl_name_to_repo_root(whl_name, repo_prefix)}:{PY_LIBRARY_LABEL}"' + return '"{}:{}"'.format(_whl_name_to_repo_root(whl_name, repo_prefix), PY_LIBRARY_LABEL) def sanitised_repo_file_label(whl_name: str, repo_prefix: str) -> str: - return f'"{_whl_name_to_repo_root(whl_name, repo_prefix)}:{WHEEL_FILE_LABEL}"' + return '"{}:{}"'.format(_whl_name_to_repo_root(whl_name, repo_prefix), WHEEL_FILE_LABEL) def extract_wheel( diff --git a/python/pip_install/pip_repository.bzl b/python/pip_install/pip_repository.bzl index 038a245e81..56f6d9d087 100644 --- a/python/pip_install/pip_repository.bzl +++ b/python/pip_install/pip_repository.bzl @@ -138,8 +138,8 @@ python_interpreter. pip_repository_attrs = { "incremental": attr.bool( default = False, - doc = "Create the repository in incremental mode." - ) + doc = "Create the repository in incremental mode.", + ), "requirements": attr.label( allow_single_file = True, doc = "A 'requirements.txt' pip requirements file.", From 06a2a6d6702eac8e8c1dc3e2be59f35d07b3496a Mon Sep 17 00:00:00 2001 From: Henry Fuller Date: Mon, 8 Mar 2021 18:02:42 -0800 Subject: [PATCH 08/13] fix one more f string and buildifier function docs --- .../extract_wheels/lib/utilities_test.py | 3 ++- python/pip_install/pip_repository.bzl | 23 ++++++++++++------- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/python/pip_install/extract_wheels/lib/utilities_test.py b/python/pip_install/extract_wheels/lib/utilities_test.py index c92108fb39..e99cd7b685 100644 --- a/python/pip_install/extract_wheels/lib/utilities_test.py +++ b/python/pip_install/extract_wheels/lib/utilities_test.py @@ -10,7 +10,8 @@ def test_utilities(self) -> None: parser = utilities.parse_common_args(parser) repo_name = "foo" index_url = "--index_url=pypi.org/simple" - args_dict = vars(parser.parse_args(args=["--repo", repo_name, f"--extra_pip_args={index_url}"])) + args_dict = vars(parser.parse_args( + args=["--repo", repo_name, "--extra_pip_args={index_url}".formt(index_url=index_url)])) self.assertIn("repo", args_dict) self.assertIn("extra_pip_args", args_dict) self.assertEqual(args_dict["pip_data_exclude"], None) diff --git a/python/pip_install/pip_repository.bzl b/python/pip_install/pip_repository.bzl index 56f6d9d087..75960a9d33 100644 --- a/python/pip_install/pip_repository.bzl +++ b/python/pip_install/pip_repository.bzl @@ -3,10 +3,13 @@ load("//python/pip_install:repositories.bzl", "all_requirements") -def construct_pypath(rctx): - """Helper function to construct a PYTHONPATH containing entries for - code in this repo as well as packages downloaded from //python/pip_install:repositories.bzl. +def _construct_pypath(rctx): + """Helper function to construct a PYTHONPATH. + Contains entries for code in this repo as well as packages downloaded from //python/pip_install:repositories.bzl. This allows us to run python code inside repository rule implementations. + Args: + rctx: Handle to the repository_context. + Returns: String of the PYTHONPATH. """ rctx.file("BUILD", "") @@ -21,9 +24,13 @@ def construct_pypath(rctx): pypath = separator.join([str(p) for p in [rules_root] + thirdparty_roots]) return pypath -def parse_optional_attrs(rctx, args): +def _parse_optional_attrs(rctx, args): """Helper function to parse common attributes of pip_repository and whl_library repository rules. + Args: + rctx: Handle to the rule repository context. + args: A list of parsed args for the rule. + Returns: Augmented args list. """ if rctx.attr.extra_pip_args: args += [ @@ -57,7 +64,7 @@ def _pip_repository_impl(rctx): if rctx.attr.incremental and not rctx.attr.requirements_lock: fail("Incremental mode requires a requirements_lock attribute be specified.") - pypath = construct_pypath(rctx) + pypath = _construct_pypath(rctx) if rctx.attr.incremental: args = [ @@ -82,7 +89,7 @@ def _pip_repository_impl(rctx): ] args += ["--repo", rctx.attr.name] - args = parse_optional_attrs(rctx, args) + args = _parse_optional_attrs(rctx, args) result = rctx.execute( args, @@ -202,7 +209,7 @@ py_binary( def _impl_whl_library(rctx): # pointer to parent repo so these rules rerun if the definitions in requirements.bzl change. _parent_repo_label = Label("@{parent}//:requirements.bzl".format(parent=rctx.attr.repo)) - pypath = construct_pypath(rctx) + pypath = _construct_pypath(rctx) args = [ rctx.attr.python_interpreter, "-m", @@ -212,7 +219,7 @@ def _impl_whl_library(rctx): "--repo", rctx.attr.repo, ] - args = parse_optional_attrs(rctx, args) + args = _parse_optional_attrs(rctx, args) result = rctx.execute( args, environment = { From 64e74c5744912bce3b1f30e7c1a934873eaad9ca Mon Sep 17 00:00:00 2001 From: Henry Fuller Date: Tue, 9 Mar 2021 09:59:27 -0800 Subject: [PATCH 09/13] Fix typo, and hopefully buildifier --- .../extract_wheels/lib/utilities_test.py | 2 +- python/pip_install/pip_repository.bzl | 37 +++++++++++-------- 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/python/pip_install/extract_wheels/lib/utilities_test.py b/python/pip_install/extract_wheels/lib/utilities_test.py index e99cd7b685..8ca7d62d28 100644 --- a/python/pip_install/extract_wheels/lib/utilities_test.py +++ b/python/pip_install/extract_wheels/lib/utilities_test.py @@ -11,7 +11,7 @@ def test_utilities(self) -> None: repo_name = "foo" index_url = "--index_url=pypi.org/simple" args_dict = vars(parser.parse_args( - args=["--repo", repo_name, "--extra_pip_args={index_url}".formt(index_url=index_url)])) + args=["--repo", repo_name, "--extra_pip_args={index_url}".format(index_url=index_url)])) self.assertIn("repo", args_dict) self.assertIn("extra_pip_args", args_dict) self.assertEqual(args_dict["pip_data_exclude"], None) diff --git a/python/pip_install/pip_repository.bzl b/python/pip_install/pip_repository.bzl index 75960a9d33..aadd70823a 100644 --- a/python/pip_install/pip_repository.bzl +++ b/python/pip_install/pip_repository.bzl @@ -2,11 +2,12 @@ load("//python/pip_install:repositories.bzl", "all_requirements") - def _construct_pypath(rctx): """Helper function to construct a PYTHONPATH. + Contains entries for code in this repo as well as packages downloaded from //python/pip_install:repositories.bzl. This allows us to run python code inside repository rule implementations. + Args: rctx: Handle to the repository_context. Returns: String of the PYTHONPATH. @@ -25,8 +26,8 @@ def _construct_pypath(rctx): return pypath def _parse_optional_attrs(rctx, args): - """Helper function to parse common attributes of pip_repository - and whl_library repository rules. + """Helper function to parse common attributes of pip_repository and whl_library repository rules. + Args: rctx: Handle to the rule repository context. args: A list of parsed args for the rule. @@ -49,7 +50,6 @@ def _parse_optional_attrs(rctx, args): return args - def _pip_repository_impl(rctx): python_interpreter = rctx.attr.python_interpreter if rctx.attr.python_interpreter_target != None: @@ -106,7 +106,6 @@ def _pip_repository_impl(rctx): return - common_attrs = { "enable_implicit_namespace_pkgs": attr.bool( default = False, @@ -125,12 +124,15 @@ This option is required to support some packages which cannot handle the convers doc = "Additional data exclusion parameters to add to the pip packages BUILD file.", ), "python_interpreter": attr.string(default = "python3"), - "python_interpreter_target": attr.label(allow_single_file = True, doc = """ + "python_interpreter_target": attr.label( + allow_single_file = True, + doc = """ If you are using a custom python interpreter built by another repository rule, use this attribute to specify its BUILD target. This allows pip_repository to invoke pip using the same interpreter as your toolchain. If set, takes precedence over python_interpreter. -"""), +""", + ), "quiet": attr.bool( default = True, doc = "If True, suppress printing stdout and stderr output to the terminal.", @@ -157,14 +159,14 @@ pip_repository_attrs = { 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'. -"""), +""", + ), } pip_repository_attrs.update(**common_attrs) pip_repository = repository_rule( attrs = pip_repository_attrs, - implementation = _pip_repository_impl, doc = """A rule for importing `requirements.txt` dependencies into Bazel. This rule imports a `requirements.txt` file and generates a new @@ -204,6 +206,7 @@ py_binary( ) ``` """, + implementation = _pip_repository_impl, ) def _impl_whl_library(rctx): @@ -235,19 +238,23 @@ def _impl_whl_library(rctx): return - whl_library_attrs = { - "requirement": attr.string(mandatory=True, doc = "Python requirement string describing the package to make available"), - "repo": attr.string(mandatory=True, doc = "Pointer to parent repo name. Used to make these rules rerun if the parent repo changes.") + "repo": attr.string( + mandatory = True, + doc = "Pointer to parent repo name. Used to make these rules rerun if the parent repo changes.", + ), + "requirement": attr.string( + mandatory = True, + doc = "Python requirement string describing the package to make available", + ), } whl_library_attrs.update(**common_attrs) - whl_library = repository_rule( attrs = whl_library_attrs, - implementation = _impl_whl_library, doc = """ Download and extracts a single wheel based into a bazel repo based on the requirement string passed in. -Instantiated from pip_repository and inherits config options from there.""" +Instantiated from pip_repository and inherits config options from there.""", + implementation = _impl_whl_library, ) From 01ba52586cbc887994beed606cf088ebd2428ca3 Mon Sep 17 00:00:00 2001 From: Henry Fuller Date: Mon, 15 Mar 2021 09:40:00 -0700 Subject: [PATCH 10/13] fix readme --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 43b4a65d7c..28b2e2096e 100644 --- a/README.md +++ b/README.md @@ -162,7 +162,7 @@ load("@rules_python//python:pip.bzl", "pip_install_incremental") # Create a central repo that knows about the dependencies needed for # requirements.txt. -pip_install( +pip_install_incremental( name = "my_deps", requirements_lock = "//path/to:requirements_lock.txt", ) From 432da9afaa0ddbf5958462ec63fe8a254d442775 Mon Sep 17 00:00:00 2001 From: Henry Fuller Date: Mon, 22 Mar 2021 00:33:55 -0700 Subject: [PATCH 11/13] Rename pip_install_incremental to pip_parse - Rename helpers and executables to match this new convention. - Make readme more readable. - Add integration test for the example. --- .bazelrc | 4 +- README.md | 33 ++++++++-------- examples/BUILD | 5 +++ .../BUILD | 2 +- .../WORKSPACE | 15 ++++--- .../main.py | 0 .../requirements.txt | 0 .../requirements_lock.txt | 1 + .../test.py | 0 python/pip.bzl | 2 +- python/pip_install/BUILD | 1 + .../pip_install/create_incremental_repo/BUILD | 21 ---------- .../extract_single_wheel/__main__.py | 4 -- python/pip_install/extract_wheels/__init__.py | 4 +- python/pip_install/extract_wheels/lib/BUILD | 8 ++-- .../lib/{utilities.py => arguments.py} | 0 .../{utilities_test.py => arguments_test.py} | 8 ++-- .../pip_install/extract_wheels/lib/bazel.py | 2 +- .../parse_requirements_to_bzl/BUILD | 39 +++++++++++++++++++ .../__init__.py | 10 ++--- .../__main__.py | 2 +- .../extract_single_wheel/BUILD | 8 ++++ .../extract_single_wheel/__init__.py | 11 +++--- .../extract_single_wheel/__main__.py | 4 ++ .../parse_requirements_to_bzl_test.py} | 16 ++++---- python/pip_install/pip_repository.bzl | 4 +- 26 files changed, 117 insertions(+), 87 deletions(-) rename examples/{pip_install_incremental => pip_parse}/BUILD (94%) rename examples/{pip_install_incremental => pip_parse}/WORKSPACE (76%) rename examples/{pip_install_incremental => pip_parse}/main.py (100%) rename examples/{pip_install_incremental => pip_parse}/requirements.txt (100%) rename examples/{pip_install_incremental => pip_parse}/requirements_lock.txt (86%) rename examples/{pip_install_incremental => pip_parse}/test.py (100%) delete mode 100644 python/pip_install/create_incremental_repo/BUILD delete mode 100644 python/pip_install/create_incremental_repo/extract_single_wheel/__main__.py rename python/pip_install/extract_wheels/lib/{utilities.py => arguments.py} (100%) rename python/pip_install/extract_wheels/lib/{utilities_test.py => arguments_test.py} (78%) create mode 100644 python/pip_install/parse_requirements_to_bzl/BUILD rename python/pip_install/{create_incremental_repo => parse_requirements_to_bzl}/__init__.py (92%) rename python/pip_install/{create_incremental_repo => parse_requirements_to_bzl}/__main__.py (50%) create mode 100644 python/pip_install/parse_requirements_to_bzl/extract_single_wheel/BUILD rename python/pip_install/{create_incremental_repo => parse_requirements_to_bzl}/extract_single_wheel/__init__.py (78%) create mode 100644 python/pip_install/parse_requirements_to_bzl/extract_single_wheel/__main__.py rename python/pip_install/{create_incremental_repo/test_create_incremental_repo.py => parse_requirements_to_bzl/parse_requirements_to_bzl_test.py} (56%) diff --git a/.bazelrc b/.bazelrc index 12cf01c077..ddba1f3b1a 100644 --- a/.bazelrc +++ b/.bazelrc @@ -3,7 +3,7 @@ # This lets us glob() up all the files inside the examples to make them inputs to tests # (Note, we cannot use `common --deleted_packages` because the bazel version command doesn't support it) # To update these lines, run tools/bazel_integration_test/update_deleted_packages.sh -build --deleted_packages=examples/legacy_pip_import/boto,examples/legacy_pip_import/extras,examples/legacy_pip_import/helloworld,examples/pip_install,examples/pip_install_incremental -query --deleted_packages=examples/legacy_pip_import/boto,examples/legacy_pip_import/extras,examples/legacy_pip_import/helloworld,examples/pip_install,examples/pip_install_incrmental +build --deleted_packages=examples/legacy_pip_import/boto,examples/legacy_pip_import/extras,examples/legacy_pip_import/helloworld,examples/pip_install,examples/pip_parse +query --deleted_packages=examples/legacy_pip_import/boto,examples/legacy_pip_import/extras,examples/legacy_pip_import/helloworld,examples/pip_install,examples/pip_parse test --test_output=errors diff --git a/README.md b/README.md index 28b2e2096e..85df2c9fcf 100644 --- a/README.md +++ b/README.md @@ -136,38 +136,37 @@ re-executed in order to pick up a non-hermetic change to your environment (e.g., updating your system `python` interpreter), you can completely flush out your repo cache with `bazel clean --expunge`. -### Importing `pip` dependencies incrementally (experimental) +### Fetch `pip` dependencies lazily (experimental) One pain point with `pip_install` is the need to download all dependencies resolved by your requirements.txt before the bazel analysis phase can start. For large python monorepos this can take a long time, especially on slow connections. -To download only the pip packages needed to build targets in the -subgraph of top level targets in your bazel invocation, you can experiment with using `pip_install_incremental`. -The interface of `pip_install_incremental` mirrors `pip_install` as closely as possible. +`pip_parse` provides a solution to this problem. If you can provide a lock +file of all your python dependencies `pip_parse` will translate each requirement into its own external repository. +Bazel will only fetch/build wheels for the requirements in the subgraph of your build target. -The only user facing difference between `pip_install` and `pip_install_incremental` is that for the latter you need -to supply a fully resolved and pinned requirements_lock.txt file (named to distinguish it from requirments.txt -used in `pip_install`). The `requirements` attribute is replaced with a `requirements_lock` attribute to make it -clear that a fully pinned transitive resolve is needed. - -To add incremental pip dependencies to your `WORKSPACE` load -the `pip_install_incremental` function, and call it to create a main -repo which contains a macro called `install_deps()` which is used -to create child repos for each package in your requirements_lock.txt. +There are API differences between `pip_parse` and `pip_install`: +1. `pip_parse` requires a fully resolved lock file of your python dependencies. You can generate this using + `pip-compile`, or a virtualenv and `pip freeze`. `pip_parse` uses a label argument called `requirements_lock` instead of `requirements` + to make this distinction clear. +2. `pip_parse` translates your requirements into a starlark macro called `install_deps`. You must call this macro in your WORKSPACE to + declare your dependencies. ```python -load("@rules_python//python:pip.bzl", "pip_install_incremental") +load("@rules_python//python:pip.bzl", "pip_parse") -# Create a central repo that knows about the dependencies needed for -# requirements.txt. -pip_install_incremental( +# Create a central repo that knows about the dependencies needed from +# requirements_lock.txt. +pip_parse( name = "my_deps", requirements_lock = "//path/to:requirements_lock.txt", ) +# Load the starlark macro which will define your dependencies. load("@my_deps//:requirements.bzl", "install_deps") +# Call it to define repos for your requirements. install_deps() ``` diff --git a/examples/BUILD b/examples/BUILD index 092ad40902..5b798d53a7 100644 --- a/examples/BUILD +++ b/examples/BUILD @@ -26,3 +26,8 @@ bazel_integration_test( name = "pip_install_example", timeout = "long", ) + +bazel_integration_test( + name = "pip_parse_example", + timeout = "long", +) diff --git a/examples/pip_install_incremental/BUILD b/examples/pip_parse/BUILD similarity index 94% rename from examples/pip_install_incremental/BUILD rename to examples/pip_parse/BUILD index adc286ff9c..ca56af9c0d 100644 --- a/examples/pip_install_incremental/BUILD +++ b/examples/pip_parse/BUILD @@ -1,4 +1,4 @@ -load("@pip_incremental//:requirements.bzl", "requirement") +load("@pip_parsed_deps//:requirements.bzl", "requirement") load("@rules_python//python:defs.bzl", "py_binary", "py_test") # Toolchain setup, this is optional. diff --git a/examples/pip_install_incremental/WORKSPACE b/examples/pip_parse/WORKSPACE similarity index 76% rename from examples/pip_install_incremental/WORKSPACE rename to examples/pip_parse/WORKSPACE index b98d84521f..4bd94c6c4d 100644 --- a/examples/pip_install_incremental/WORKSPACE +++ b/examples/pip_parse/WORKSPACE @@ -1,17 +1,16 @@ workspace(name = "example_repo") -load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository") +load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") -# TODO(hrfuller) change this to a released http_archive when a release exists. -git_repository( +http_archive( name = "rules_python", - remote = "https://github.com/bazelbuild/rules_python.git", - branch = "master" + url = "https://github.com/bazelbuild/rules_python/releases/download/0.1.0/rules_python-0.1.0.tar.gz", + sha256 = "b6d46438523a3ec0f3cead544190ee13223a52f6a6765a29eae7b7cc24cc83a0", ) -load("@rules_python//python:pip.bzl", "pip_install_incremental") +load("@rules_python//python:pip.bzl", "pip_parse") -pip_install_incremental( +pip_parse( # (Optional) You can provide extra parameters to pip. # Here, make pip output verbose (this is usable with `quiet = False`). #extra_pip_args = ["-v"], @@ -34,7 +33,7 @@ pip_install_incremental( requirements_lock = "//:requirements_lock.txt", ) -load("@pip_incremental//:requirements.bzl", "install_deps") +load("@pip_parsed_deps//:requirements.bzl", "install_deps") # Initialize repositories for all packages in requirements_lock.txt. install_deps() diff --git a/examples/pip_install_incremental/main.py b/examples/pip_parse/main.py similarity index 100% rename from examples/pip_install_incremental/main.py rename to examples/pip_parse/main.py diff --git a/examples/pip_install_incremental/requirements.txt b/examples/pip_parse/requirements.txt similarity index 100% rename from examples/pip_install_incremental/requirements.txt rename to examples/pip_parse/requirements.txt diff --git a/examples/pip_install_incremental/requirements_lock.txt b/examples/pip_parse/requirements_lock.txt similarity index 86% rename from examples/pip_install_incremental/requirements_lock.txt rename to examples/pip_parse/requirements_lock.txt index 504891e7f2..d237b1f0f5 100644 --- a/examples/pip_install_incremental/requirements_lock.txt +++ b/examples/pip_parse/requirements_lock.txt @@ -1,4 +1,5 @@ # Transitive dependency set from :requirements.txt +# Generated using `pip-compile requirements.txt -o requirements_lock.txt` certifi==2020.12.5 # via requests cffi==1.14.5 # via cryptography chardet==3.0.4 # via requests diff --git a/examples/pip_install_incremental/test.py b/examples/pip_parse/test.py similarity index 100% rename from examples/pip_install_incremental/test.py rename to examples/pip_parse/test.py diff --git a/python/pip.bzl b/python/pip.bzl index 7feee25bfd..32a8901856 100644 --- a/python/pip.bzl +++ b/python/pip.bzl @@ -56,7 +56,7 @@ def pip_install(requirements, name = "pip", **kwargs): **kwargs ) -def pip_install_incremental(requirements_lock, name = "pip_incremental", **kwargs): +def pip_parse(requirements_lock, name = "pip_parsed_deps", **kwargs): # Just in case our dependencies weren't already fetched pip_install_dependencies() diff --git a/python/pip_install/BUILD b/python/pip_install/BUILD index b37170eb53..bdbfe407e9 100644 --- a/python/pip_install/BUILD +++ b/python/pip_install/BUILD @@ -5,6 +5,7 @@ filegroup( srcs = glob(["*.bzl"]) + [ "BUILD", "//python/pip_install/extract_wheels:distribution", + "//python/pip_install/parse_requirements_to_bzl:distribution", ], visibility = ["//:__pkg__"], ) diff --git a/python/pip_install/create_incremental_repo/BUILD b/python/pip_install/create_incremental_repo/BUILD deleted file mode 100644 index 0fa0bf24ad..0000000000 --- a/python/pip_install/create_incremental_repo/BUILD +++ /dev/null @@ -1,21 +0,0 @@ -load("@rules_python//python:defs.bzl", "py_library", "py_test") -load("//python/pip_install:repositories.bzl", "requirement") - -py_library( - name = "lib", - srcs = ["__init__.py"], - deps = [requirement("pip")] -) - -py_test( - name = "test_create_incremental_repo", - size = "small", - srcs = [ - "test_create_incremental_repo.py", - ], - tags = ["unit"], - deps = [ - ":lib", - "//python/pip_install/extract_wheels/lib" - ], -) diff --git a/python/pip_install/create_incremental_repo/extract_single_wheel/__main__.py b/python/pip_install/create_incremental_repo/extract_single_wheel/__main__.py deleted file mode 100644 index 2bfa1da065..0000000000 --- a/python/pip_install/create_incremental_repo/extract_single_wheel/__main__.py +++ /dev/null @@ -1,4 +0,0 @@ -from python.pip_install.create_incremental_repo.extract_single_wheel import main - -if __name__ == "__main__": - main() diff --git a/python/pip_install/extract_wheels/__init__.py b/python/pip_install/extract_wheels/__init__.py index d857441054..96913cdad7 100644 --- a/python/pip_install/extract_wheels/__init__.py +++ b/python/pip_install/extract_wheels/__init__.py @@ -12,7 +12,7 @@ import sys import json -from python.pip_install.extract_wheels.lib import bazel, requirements, utilities +from python.pip_install.extract_wheels.lib import bazel, requirements, arguments def configure_reproducible_wheels() -> None: @@ -58,7 +58,7 @@ def main() -> None: required=True, help="Path to requirements.txt from where to install dependencies", ) - utilities.parse_common_args(parser) + arguments.parse_common_args(parser) args = parser.parse_args() pip_args = [sys.executable, "-m", "pip", "--isolated", "wheel", "-r", args.requirements] diff --git a/python/pip_install/extract_wheels/lib/BUILD b/python/pip_install/extract_wheels/lib/BUILD index 40d4413d4f..84cd4dc536 100644 --- a/python/pip_install/extract_wheels/lib/BUILD +++ b/python/pip_install/extract_wheels/lib/BUILD @@ -9,11 +9,11 @@ py_library( "purelib.py", "requirements.py", "wheel.py", - "utilities.py", + "arguments.py", ], visibility = [ "//python/pip_install/extract_wheels:__subpackages__", - "//python/pip_install/create_incremental_repo:__pkg__" + "//python/pip_install/parse_requirements_to_bzl:__subpackages__", ], deps = [ requirement("pkginfo"), @@ -46,10 +46,10 @@ py_test( ) py_test( - name = "utilities_test", + name = "arguments_test", size = "small", srcs = [ - "utilities_test.py", + "arguments_test.py", ], tags = ["unit"], deps = [ diff --git a/python/pip_install/extract_wheels/lib/utilities.py b/python/pip_install/extract_wheels/lib/arguments.py similarity index 100% rename from python/pip_install/extract_wheels/lib/utilities.py rename to python/pip_install/extract_wheels/lib/arguments.py diff --git a/python/pip_install/extract_wheels/lib/utilities_test.py b/python/pip_install/extract_wheels/lib/arguments_test.py similarity index 78% rename from python/pip_install/extract_wheels/lib/utilities_test.py rename to python/pip_install/extract_wheels/lib/arguments_test.py index 8ca7d62d28..a1bc51448c 100644 --- a/python/pip_install/extract_wheels/lib/utilities_test.py +++ b/python/pip_install/extract_wheels/lib/arguments_test.py @@ -1,13 +1,13 @@ import unittest import argparse -from python.pip_install.extract_wheels.lib import utilities +from python.pip_install.extract_wheels.lib import arguments -class TestUtilities(unittest.TestCase): - def test_utilities(self) -> None: +class ArguementsTestCase(unittest.TestCase): + def test_arguments(self) -> None: parser = argparse.ArgumentParser() - parser = utilities.parse_common_args(parser) + parser = arguments.parse_common_args(parser) repo_name = "foo" index_url = "--index_url=pypi.org/simple" args_dict = vars(parser.parse_args( diff --git a/python/pip_install/extract_wheels/lib/bazel.py b/python/pip_install/extract_wheels/lib/bazel.py index fb910e2b58..95e85531ba 100644 --- a/python/pip_install/extract_wheels/lib/bazel.py +++ b/python/pip_install/extract_wheels/lib/bazel.py @@ -107,7 +107,7 @@ def install_deps(): DEFAULT_PACKAGE_PREFIX = "pypi__" -def create_incremental_repo_prefix(parent_repo: str) -> str: +def whl_library_repo_prefix(parent_repo: str) -> str: return "{parent}_{default_package_prefix}".format( parent=parent_repo, default_package_prefix=DEFAULT_PACKAGE_PREFIX diff --git a/python/pip_install/parse_requirements_to_bzl/BUILD b/python/pip_install/parse_requirements_to_bzl/BUILD new file mode 100644 index 0000000000..0418461df5 --- /dev/null +++ b/python/pip_install/parse_requirements_to_bzl/BUILD @@ -0,0 +1,39 @@ +load("@rules_python//python:defs.bzl", "py_library", "py_test") +load("//python/pip_install:repositories.bzl", "requirement") + +py_binary( + name = "parse_requirements_to_bzl", + srcs = [ + "__init__.py", + "__main__.py", + ], + main = "__main__.py", + deps = ["//python/pip_install/extract_wheels/lib"], +) + +py_library( + name = "lib", + srcs = ["__init__.py"], + deps = [requirement("pip")] +) + +py_test( + name = "parse_requirements_to_bzl_test", + size = "small", + srcs = [ + "parse_requirements_to_bzl_test.py", + ], + tags = ["unit"], + deps = [ + ":lib", + "//python/pip_install/extract_wheels/lib" + ], +) + +filegroup( + name = "distribution", + srcs = glob(["*"], exclude = ["*_test.py"]) + [ + "//python/pip_install/parse_requirements_to_bzl/extract_single_wheel:distribution", + ], + visibility = ["//python/pip_install:__subpackages__"], +) diff --git a/python/pip_install/create_incremental_repo/__init__.py b/python/pip_install/parse_requirements_to_bzl/__init__.py similarity index 92% rename from python/pip_install/create_incremental_repo/__init__.py rename to python/pip_install/parse_requirements_to_bzl/__init__.py index 157db62262..605d2e31ee 100644 --- a/python/pip_install/create_incremental_repo/__init__.py +++ b/python/pip_install/parse_requirements_to_bzl/__init__.py @@ -3,7 +3,7 @@ import sys from typing import List, Tuple -from python.pip_install.extract_wheels.lib import bazel, utilities +from python.pip_install.extract_wheels.lib import bazel, arguments from pip._internal.req import parse_requirements, constructors from pip._internal.req.req_install import InstallRequirement from pip._internal.network.session import PipSession @@ -26,7 +26,7 @@ def repo_names_and_requirements(install_reqs: List[InstallRequirement], repo_pre ] -def generate_incremental_requirements_contents(all_args: argparse.Namespace) -> str: +def generate_parsed_requirements_contents(all_args: argparse.Namespace) -> str: """ Parse each requirement from the requirements_lock file, and prepare arguments for each repository rule, which will represent the individual requirements. @@ -39,7 +39,7 @@ def generate_incremental_requirements_contents(all_args: argparse.Namespace) -> 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.create_incremental_repo_prefix(args["repo"]) + repo_prefix = bazel.whl_library_repo_prefix(args["repo"]) install_reqs = parse_install_requirements(requirements_lock) repo_names_and_reqs = repo_names_and_requirements(install_reqs, repo_prefix) @@ -110,10 +110,10 @@ def main() -> None: required=True, help="timeout to use for pip operation.", ) - utilities.parse_common_args(parser) + arguments.parse_common_args(parser) args = parser.parse_args() with open("requirements.bzl", "w") as requirement_file: requirement_file.write( - generate_incremental_requirements_contents(args) + generate_parsed_requirements_contents(args) ) diff --git a/python/pip_install/create_incremental_repo/__main__.py b/python/pip_install/parse_requirements_to_bzl/__main__.py similarity index 50% rename from python/pip_install/create_incremental_repo/__main__.py rename to python/pip_install/parse_requirements_to_bzl/__main__.py index 23271a5cf9..89199612b5 100644 --- a/python/pip_install/create_incremental_repo/__main__.py +++ b/python/pip_install/parse_requirements_to_bzl/__main__.py @@ -1,5 +1,5 @@ """Main entry point.""" -from python.pip_install.create_incremental_repo import main +from python.pip_install.parse_requirements_to_bzl import main if __name__ == "__main__": main() diff --git a/python/pip_install/parse_requirements_to_bzl/extract_single_wheel/BUILD b/python/pip_install/parse_requirements_to_bzl/extract_single_wheel/BUILD new file mode 100644 index 0000000000..17bdfe75ce --- /dev/null +++ b/python/pip_install/parse_requirements_to_bzl/extract_single_wheel/BUILD @@ -0,0 +1,8 @@ +filegroup( + name = "distribution", + srcs = glob( + ["*"], + exclude = ["*_test.py"], + ), + visibility = ["//python/pip_install:__subpackages__"], +) diff --git a/python/pip_install/create_incremental_repo/extract_single_wheel/__init__.py b/python/pip_install/parse_requirements_to_bzl/extract_single_wheel/__init__.py similarity index 78% rename from python/pip_install/create_incremental_repo/extract_single_wheel/__init__.py rename to python/pip_install/parse_requirements_to_bzl/extract_single_wheel/__init__.py index 4f9707ede4..d2b9413029 100644 --- a/python/pip_install/create_incremental_repo/extract_single_wheel/__init__.py +++ b/python/pip_install/parse_requirements_to_bzl/extract_single_wheel/__init__.py @@ -4,22 +4,21 @@ import subprocess import json -from python.pip_install.extract_wheels.lib import bazel, requirements, utilities +from python.pip_install.extract_wheels.lib import bazel, requirements, arguments from python.pip_install.extract_wheels import configure_reproducible_wheels def main() -> None: parser = argparse.ArgumentParser( - description="Create rules to incrementally fetch needed dependencies \ -from a fully resolved requirements lock file." + description="Build and/or fetch a single wheel based on the requirement passed in" ) parser.add_argument( "--requirement", action="store", required=True, - help="Path to fully resolved requirements.txt to use as the source of repos.", + help="A single PEP508 requirement specifier string.", ) - utilities.parse_common_args(parser) + arguments.parse_common_args(parser) args = parser.parse_args() configure_reproducible_wheels() @@ -48,5 +47,5 @@ def main() -> None: pip_data_exclude, args.enable_implicit_namespace_pkgs, incremental=True, - incremental_repo_prefix=bazel.create_incremental_repo_prefix(args.repo) + incremental_repo_prefix=bazel.whl_library_repo_prefix(args.repo) ) diff --git a/python/pip_install/parse_requirements_to_bzl/extract_single_wheel/__main__.py b/python/pip_install/parse_requirements_to_bzl/extract_single_wheel/__main__.py new file mode 100644 index 0000000000..d45f90bbd1 --- /dev/null +++ b/python/pip_install/parse_requirements_to_bzl/extract_single_wheel/__main__.py @@ -0,0 +1,4 @@ +from python.pip_install.parse_requirements_to_bzl.extract_single_wheel import main + +if __name__ == "__main__": + main() diff --git a/python/pip_install/create_incremental_repo/test_create_incremental_repo.py b/python/pip_install/parse_requirements_to_bzl/parse_requirements_to_bzl_test.py similarity index 56% rename from python/pip_install/create_incremental_repo/test_create_incremental_repo.py rename to python/pip_install/parse_requirements_to_bzl/parse_requirements_to_bzl_test.py index a5f275e0a4..ae303cd193 100644 --- a/python/pip_install/create_incremental_repo/test_create_incremental_repo.py +++ b/python/pip_install/parse_requirements_to_bzl/parse_requirements_to_bzl_test.py @@ -2,10 +2,10 @@ import argparse from tempfile import NamedTemporaryFile -from python.pip_install.create_incremental_repo import generate_incremental_requirements_contents +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, - create_incremental_repo_prefix, + whl_library_repo_prefix, sanitised_repo_file_label ) @@ -19,12 +19,12 @@ def test_incremental_requirements_bzl(self) -> None: requirements_lock.flush() args = argparse.Namespace() args.requirements_lock = requirements_lock.name - args.repo = "pip_incremental" - contents = generate_incremental_requirements_contents(args) - library_target = sanitised_repo_library_label("foo", create_incremental_repo_prefix(args.repo)) - whl_target = sanitised_repo_file_label("foo", create_incremental_repo_prefix(args.repo)) - all_requirements = 'all_requirements = [{library_target}]'.format(library_target=library_target) - all_whl_requirements = 'all_whl_requirements = [{whl_target}]'.format(whl_target=whl_target) + args.repo = "pip_parsed_deps" + contents = generate_parsed_requirements_contents(args) + library_target = "@pip_parsed_deps_pypi__foo//:pkg" + whl_target = "@pip_parsed_deps_pypi__foo//:whl" + all_requirements = 'all_requirements = ["{library_target}"]'.format(library_target=library_target) + all_whl_requirements = 'all_whl_requirements = ["{whl_target}"]'.format(whl_target=whl_target) self.assertIn(all_requirements, contents, contents) self.assertIn(all_whl_requirements, contents, contents) self.assertIn(requirement_string, contents, contents) diff --git a/python/pip_install/pip_repository.bzl b/python/pip_install/pip_repository.bzl index aadd70823a..c07c9cdd86 100644 --- a/python/pip_install/pip_repository.bzl +++ b/python/pip_install/pip_repository.bzl @@ -70,7 +70,7 @@ def _pip_repository_impl(rctx): args = [ python_interpreter, "-m", - "python.pip_install.create_incremental_repo", + "python.pip_install.parse_requirements_to_bzl", "--requirements_lock", rctx.path(rctx.attr.requirements_lock), # pass quiet and timeout args through to child repos. @@ -216,7 +216,7 @@ def _impl_whl_library(rctx): args = [ rctx.attr.python_interpreter, "-m", - "python.pip_install.create_incremental_repo.extract_single_wheel", + "python.pip_install.parse_requirements_to_bzl.extract_single_wheel", "--requirement", rctx.attr.requirement, "--repo", From 23ed27b077d53cb3382102500588ca0c147583a6 Mon Sep 17 00:00:00 2001 From: Henry Fuller Date: Mon, 22 Mar 2021 00:48:53 -0700 Subject: [PATCH 12/13] Appease buildifier Use a requirements_lock.txt which is compatible with the cp3.5 platform. --- examples/pip_parse/requirements.txt | 2 +- examples/pip_parse/requirements_lock.txt | 28 +++++++++++-------- .../parse_requirements_to_bzl/BUILD | 2 +- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/examples/pip_parse/requirements.txt b/examples/pip_parse/requirements.txt index 4998f3f857..989b995c68 100644 --- a/examples/pip_parse/requirements.txt +++ b/examples/pip_parse/requirements.txt @@ -1 +1 @@ -requests[security]==2.24.0 +requests==2.24.0 diff --git a/examples/pip_parse/requirements_lock.txt b/examples/pip_parse/requirements_lock.txt index d237b1f0f5..b0d5b9ed51 100644 --- a/examples/pip_parse/requirements_lock.txt +++ b/examples/pip_parse/requirements_lock.txt @@ -1,12 +1,16 @@ -# Transitive dependency set from :requirements.txt -# Generated using `pip-compile requirements.txt -o requirements_lock.txt` -certifi==2020.12.5 # via requests -cffi==1.14.5 # via cryptography -chardet==3.0.4 # via requests -cryptography==3.4.6 # via pyopenssl, requests -idna==2.10 # via requests -pycparser==2.20 # via cffi -pyopenssl==20.0.1 # via requests -requests[security]==2.24.0 # via -r /dev/fd/11 -six==1.15.0 # via pyopenssl -urllib3==1.25.11 # via requests +# +# This file is autogenerated by pip-compile +# To update, run: +# +# pip-compile --output-file=requirements_lock.txt requirements.txt +# +certifi==2020.12.5 + # via requests +chardet==3.0.4 + # via requests +idna==2.10 + # via requests +requests==2.24.0 + # via -r requirements.txt +urllib3==1.25.11 + # via requests diff --git a/python/pip_install/parse_requirements_to_bzl/BUILD b/python/pip_install/parse_requirements_to_bzl/BUILD index 0418461df5..51c9176458 100644 --- a/python/pip_install/parse_requirements_to_bzl/BUILD +++ b/python/pip_install/parse_requirements_to_bzl/BUILD @@ -1,4 +1,4 @@ -load("@rules_python//python:defs.bzl", "py_library", "py_test") +load("@rules_python//python:defs.bzl", "py_binary", "py_library", "py_test") load("//python/pip_install:repositories.bzl", "requirement") py_binary( From 6f950b8699f6980ef1c4c6f56f55f41f241b4a7f Mon Sep 17 00:00:00 2001 From: Henry Fuller Date: Mon, 22 Mar 2021 02:11:49 -0700 Subject: [PATCH 13/13] Handle the serialized args passed from pip_resository rule. Don't write serialized json into the generated requirements.bzl file because whl_library doesn't understand it. --- examples/pip_parse/WORKSPACE | 2 +- python/pip_install/extract_wheels/lib/BUILD | 1 + .../extract_wheels/lib/arguments_test.py | 9 ++++++--- python/pip_install/parse_requirements_to_bzl/BUILD | 3 ++- .../parse_requirements_to_bzl/__init__.py | 13 +++++++++++++ .../parse_requirements_to_bzl_test.py | 9 +++++++-- 6 files changed, 30 insertions(+), 7 deletions(-) diff --git a/examples/pip_parse/WORKSPACE b/examples/pip_parse/WORKSPACE index 4bd94c6c4d..418e762d0b 100644 --- a/examples/pip_parse/WORKSPACE +++ b/examples/pip_parse/WORKSPACE @@ -13,7 +13,7 @@ load("@rules_python//python:pip.bzl", "pip_parse") pip_parse( # (Optional) You can provide extra parameters to pip. # Here, make pip output verbose (this is usable with `quiet = False`). - #extra_pip_args = ["-v"], + # extra_pip_args = ["-v"], # (Optional) You can exclude custom elements in the data section of the generated BUILD files for pip packages. # Exclude directories with spaces in their names in this example (avoids build errors if there are such directories). diff --git a/python/pip_install/extract_wheels/lib/BUILD b/python/pip_install/extract_wheels/lib/BUILD index 84cd4dc536..c23d8f376f 100644 --- a/python/pip_install/extract_wheels/lib/BUILD +++ b/python/pip_install/extract_wheels/lib/BUILD @@ -54,6 +54,7 @@ py_test( tags = ["unit"], deps = [ ":lib", + "//python/pip_install/parse_requirements_to_bzl:lib", ], ) diff --git a/python/pip_install/extract_wheels/lib/arguments_test.py b/python/pip_install/extract_wheels/lib/arguments_test.py index a1bc51448c..0d6a6af1fa 100644 --- a/python/pip_install/extract_wheels/lib/arguments_test.py +++ b/python/pip_install/extract_wheels/lib/arguments_test.py @@ -1,17 +1,20 @@ -import unittest import argparse +import json +import unittest from python.pip_install.extract_wheels.lib import arguments +from python.pip_install.parse_requirements_to_bzl import deserialize_structured_args -class ArguementsTestCase(unittest.TestCase): +class ArgumentsTestCase(unittest.TestCase): def test_arguments(self) -> None: parser = argparse.ArgumentParser() parser = arguments.parse_common_args(parser) repo_name = "foo" index_url = "--index_url=pypi.org/simple" args_dict = vars(parser.parse_args( - args=["--repo", repo_name, "--extra_pip_args={index_url}".format(index_url=index_url)])) + args=["--repo", repo_name, "--extra_pip_args={index_url}".format(index_url=json.dumps({"args": index_url}))])) + args_dict = deserialize_structured_args(args_dict) self.assertIn("repo", args_dict) self.assertIn("extra_pip_args", args_dict) self.assertEqual(args_dict["pip_data_exclude"], None) diff --git a/python/pip_install/parse_requirements_to_bzl/BUILD b/python/pip_install/parse_requirements_to_bzl/BUILD index 51c9176458..61bde474fc 100644 --- a/python/pip_install/parse_requirements_to_bzl/BUILD +++ b/python/pip_install/parse_requirements_to_bzl/BUILD @@ -14,7 +14,8 @@ py_binary( py_library( name = "lib", srcs = ["__init__.py"], - deps = [requirement("pip")] + deps = [requirement("pip")], + visibility = ["//python/pip_install/extract_wheels:__subpackages__"], ) py_test( diff --git a/python/pip_install/parse_requirements_to_bzl/__init__.py b/python/pip_install/parse_requirements_to_bzl/__init__.py index 605d2e31ee..e38f9b042b 100644 --- a/python/pip_install/parse_requirements_to_bzl/__init__.py +++ b/python/pip_install/parse_requirements_to_bzl/__init__.py @@ -1,4 +1,5 @@ import argparse +import json import textwrap import sys from typing import List, Tuple @@ -25,6 +26,17 @@ def repo_names_and_requirements(install_reqs: List[InstallRequirement], repo_pre for ir in install_reqs ] +def deserialize_structured_args(args): + """Deserialize structured arguments passed from the starlark rules. + Args: + args: dict of parsed command line arguments + """ + structured_args = ("extra_pip_args", "pip_data_exclude") + for arg_name in structured_args: + if args.get(arg_name) is not None: + args[arg_name] = json.loads(args[arg_name])["args"] + return args + def generate_parsed_requirements_contents(all_args: argparse.Namespace) -> str: """ @@ -36,6 +48,7 @@ def generate_parsed_requirements_contents(all_args: argparse.Namespace) -> str: """ args = dict(vars(all_args)) + args = deserialize_structured_args(args) 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") 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 ae303cd193..4b474d4f3c 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 @@ -1,5 +1,6 @@ import unittest import argparse +import json from tempfile import NamedTemporaryFile from python.pip_install.parse_requirements_to_bzl import generate_parsed_requirements_contents @@ -10,9 +11,9 @@ ) -class TestGenerateRequirementsFileContents(unittest.TestCase): +class TestParseRequirementsToBzl(unittest.TestCase): - def test_incremental_requirements_bzl(self) -> None: + def test_generated_requirements_bzl(self) -> None: with NamedTemporaryFile() as requirements_lock: requirement_string = "foo==0.0.0" requirements_lock.write(bytes(requirement_string, encoding="utf-8")) @@ -20,6 +21,8 @@ def test_incremental_requirements_bzl(self) -> None: args = argparse.Namespace() args.requirements_lock = requirements_lock.name args.repo = "pip_parsed_deps" + extra_pip_args = ["--index-url=pypi.org/simple"] + args.extra_pip_args = json.dumps({"args": extra_pip_args}) contents = generate_parsed_requirements_contents(args) library_target = "@pip_parsed_deps_pypi__foo//:pkg" whl_target = "@pip_parsed_deps_pypi__foo//:whl" @@ -28,6 +31,8 @@ def test_incremental_requirements_bzl(self) -> None: self.assertIn(all_requirements, contents, contents) self.assertIn(all_whl_requirements, contents, contents) self.assertIn(requirement_string, contents, contents) + self.assertIn(requirement_string, contents, contents) + self.assertIn("'extra_pip_args': {}".format(repr(extra_pip_args)), contents, contents) if __name__ == "__main__":