Merge remote-tracking branches 'origin/fix-isort-pre-commit' and 'origin/multiple-requirements-in' into merged

cj81499 · cj81499 · commit bc4569464ee6 · 2023-02-14T17:47:35.000Z
diff --git a/docs/pip.md b/docs/pip.md
diff --git a/python/pip_install/requirements.bzl b/python/pip_install/requirements.bzl
@@ -48,7 +48,7 @@ def compile_pip_requirements(
         extra_deps: extra dependencies passed to pip-compile.
         py_binary: the py_binary rule to be used.
         py_test: the py_test rule to be used.
-        requirements_in: file expressing desired dependencies.
+        requirements_in: file(s) expressing desired dependencies.
         requirements_txt: result of "compiling" the requirements.in file.
         requirements_linux: File of linux specific resolve output to check validate if requirement.in has changes.
         requirements_darwin: File of darwin specific resolve output to check validate if requirement.in has changes.
@@ -69,23 +69,31 @@ def compile_pip_requirements(
         visibility = visibility,
     )
 
-    data = [name, requirements_in, requirements_txt] + [f for f in (requirements_linux, requirements_darwin, requirements_windows) if f != None]
+    if type(requirements_in) == "string":
+        requirements_in = [requirements_in]
+    data = [name] + requirements_in + [requirements_txt] + [f for f in (requirements_linux, requirements_darwin, requirements_windows) if f != None]
 
     # Use the Label constructor so this is expanded in the context of the file
     # where it appears, which is to say, in @rules_python
     pip_compile = Label("//python/pip_install/tools/dependency_resolver:dependency_resolver.py")
 
     loc = "$(rootpath {})"
 
-    args = [
-        loc.format(requirements_in),
+    args = ["--in={}".format(loc.format(f)) for f in requirements_in] + [
         loc.format(requirements_txt),
-        # String None is a placeholder for argv ordering.
-        loc.format(requirements_linux) if requirements_linux else "None",
-        loc.format(requirements_darwin) if requirements_darwin else "None",
-        loc.format(requirements_windows) if requirements_windows else "None",
         "//%s:%s.update" % (native.package_name(), name),
-    ] + extra_args
+    ]
+
+    if requirements_linux:
+        args.append("--requirements-linux={}".format(loc.format(requirements_linux)))
+
+    if requirements_darwin:
+        args.append("--requirements-darwin={}".format(loc.format(requirements_darwin)))
+
+    if requirements_windows:
+        args.append("--requirements-windows={}".format(loc.format(requirements_windows)))
+
+    args.extend(extra_args)
 
     deps = [
         requirement("build"),
diff --git a/python/pip_install/tools/dependency_resolver/dependency_resolver.py b/python/pip_install/tools/dependency_resolver/dependency_resolver.py
@@ -19,7 +19,9 @@
 import shutil
 import sys
 from pathlib import Path
+from typing import List, Optional, Tuple
 
+import click
 import piptools.writer as piptools_writer
 from piptools.scripts.compile import cli
 
@@ -66,27 +68,27 @@ def _select_golden_requirements_file(
         return requirements_txt
 
 
-if __name__ == "__main__":
-    if len(sys.argv) < 4:
-        print(
-            "Expected at least two arguments: requirements_in requirements_out",
-            file=sys.stderr,
-        )
-        sys.exit(1)
-
-    parse_str_none = lambda s: None if s == "None" else s
-
-    requirements_in = sys.argv.pop(1)
-    requirements_txt = sys.argv.pop(1)
-    requirements_linux = parse_str_none(sys.argv.pop(1))
-    requirements_darwin = parse_str_none(sys.argv.pop(1))
-    requirements_windows = parse_str_none(sys.argv.pop(1))
-    update_target_label = sys.argv.pop(1)
-
+@click.command(context_settings={"ignore_unknown_options": True})
+@click.option("--in", "requirements_ins", multiple=True, required=True)
+@click.argument("requirements_txt")
+@click.argument("update_target_label")
+@click.option("--requirements-linux")
+@click.option("--requirements-darwin")
+@click.option("--requirements-windows")
+@click.argument("extra_args", nargs=-1, type=click.UNPROCESSED)
+def main(
+    requirements_ins: Tuple[str, ...],
+    requirements_txt: str,
+    update_target_label: str,
+    requirements_linux: Optional[str],
+    requirements_darwin: Optional[str],
+    requirements_windows: Optional[str],
+    extra_args: Tuple[str, ...],
+) -> None:
     # The requirements_in file could be generated, so we will need to remove the
     # absolute prefixes in the locked requirements output file.
-    requirements_in_path = Path(requirements_in)
-    resolved_requirements_in = str(requirements_in_path.resolve())
+    requirements_in_paths = [Path(f) for f in requirements_ins]
+    resolved_requirements_ins = [str(p.resolve()) for p in requirements_in_paths]
 
     # Before loading click, set the locale for its parser.
     # If it leaks through to the system setting, it may fail:
@@ -96,6 +98,8 @@ def _select_golden_requirements_file(
     os.environ["LC_ALL"] = "C.UTF-8"
     os.environ["LANG"] = "C.UTF-8"
 
+    argv = []
+
     UPDATE = True
     # Detect if we are running under `bazel test`.
     if "TEST_TMPDIR" in os.environ:
@@ -104,8 +108,7 @@ def _select_golden_requirements_file(
         # to the real user cache, Bazel sandboxing makes the file read-only
         # and we fail.
         # In theory this makes the test more hermetic as well.
-        sys.argv.append("--cache-dir")
-        sys.argv.append(os.environ["TEST_TMPDIR"])
+        argv.append(f"--cache-dir={os.environ['TEST_TMPDIR']}")
         # Make a copy for pip-compile to read and mutate.
         requirements_out = os.path.join(
             os.environ["TEST_TMPDIR"], os.path.basename(requirements_txt) + ".out"
@@ -121,12 +124,17 @@ def _select_golden_requirements_file(
     os.environ["CUSTOM_COMPILE_COMMAND"] = update_command
     os.environ["PIP_CONFIG_FILE"] = os.getenv("PIP_CONFIG_FILE") or os.devnull
 
-    sys.argv.append("--generate-hashes")
-    sys.argv.append("--output-file")
-    sys.argv.append(requirements_txt if UPDATE else requirements_out)
-    sys.argv.append(
-        requirements_in if requirements_in_path.exists() else resolved_requirements_in
+    argv.append("--generate-hashes")
+    argv.append(f"--output-file={requirements_txt if UPDATE else requirements_out}")
+    argv.extend(
+        (req_in if req_in_path.exists() else resolved_req_in)
+        for req_in, req_in_path, resolved_req_in in zip(
+            requirements_ins, requirements_in_paths, resolved_requirements_ins
+        )
     )
+    argv.extend(extra_args)
+
+    argv.extend(extra_args)
 
     if UPDATE:
         print("Updating " + requirements_txt)
@@ -140,20 +148,20 @@ def _select_golden_requirements_file(
                 atexit.register(
                     lambda: shutil.copy(requirements_txt, requirements_txt_tree)
                 )
-        cli()
+        cli(argv)
     else:
         # cli will exit(0) on success
         try:
             print("Checking " + requirements_txt)
-            cli()
+            cli(argv)
             print("cli() should exit", file=sys.stderr)
             sys.exit(1)
         except SystemExit as e:
             if e.code == 2:
                 print(
                     "pip-compile exited with code 2. This means that pip-compile found "
                     "incompatible requirements or could not find a version that matches "
-                    f"the install requirement in {requirements_in}.",
+                    f"the install requirement in one of {requirements_ins}.",
                     file=sys.stderr,
                 )
                 sys.exit(1)
@@ -184,3 +192,7 @@ def _select_golden_requirements_file(
                     file=sys.stderr,
                 )
                 sys.exit(1)
+
+
+if __name__ == "__main__":
+    main()
diff --git a/tests/multiple_requirements_in/BUILD.bazel b/tests/multiple_requirements_in/BUILD.bazel
@@ -0,0 +1,14 @@
+load("@rules_python//python:pip.bzl", "compile_pip_requirements")
+
+compile_pip_requirements(
+    name = "requirements",
+    extra_args = [
+        "--allow-unsafe",
+        "--resolver=backtracking",
+    ],
+    requirements_in = [
+        "requirements_1.in",
+        "requirements_2.in",
+    ],
+    requirements_txt = "requirements.txt",
+)
diff --git a/tests/multiple_requirements_in/README.md b/tests/multiple_requirements_in/README.md
@@ -0,0 +1,2 @@
+# ignore_root_user_errors
+There are cases when we have to run Python targets with root, e.g., in Docker containers, requiring setting `ignore_root_user_error = True` when registering Python toolchain. This test makes sure that rules_python works in this case.
diff --git a/tests/multiple_requirements_in/WORKSPACE b/tests/multiple_requirements_in/WORKSPACE
@@ -0,0 +1,19 @@
+workspace(name = "multiple_requirements_in")
+
+local_repository(
+    name = "rules_python",
+    path = "../..",
+)
+
+load("@rules_python//python:repositories.bzl", "py_repositories", "python_register_toolchains")
+
+py_repositories()
+
+load("@rules_python//python/pip_install:repositories.bzl", "pip_install_dependencies")
+
+pip_install_dependencies()
+
+python_register_toolchains(
+    name = "python39",
+    python_version = "3.9",
+)
diff --git a/tests/multiple_requirements_in/requirements.txt b/tests/multiple_requirements_in/requirements.txt
@@ -0,0 +1,14 @@
+#
+# This file is autogenerated by pip-compile with Python 3.11
+# by the following command:
+#
+#    bazel run //tests/multiple_requirements_in:requirements.update
+#
+attrs==22.2.0 \
+    --hash=sha256:29e95c7f6778868dbd49170f98f8818f78f3dc5e0e37c0b1f474e3561b240836 \
+    --hash=sha256:c9227bfc2f01993c03f68db37d1d15c9690188323c067c641f1a35ca58185f99
+    # via -r tests/multiple_requirements_in/requirements_2.in
+urllib3==1.26.14 \
+    --hash=sha256:076907bf8fd355cde77728471316625a4d2f7e713c125f51953bb5b3eecf4f72 \
+    --hash=sha256:75edcdc2f7d85b137124a6c3c9fc3933cdeaa12ecb9a6a959f22797a0feca7e1
+    # via -r tests/multiple_requirements_in/requirements_1.in
diff --git a/tests/multiple_requirements_in/requirements_1.in b/tests/multiple_requirements_in/requirements_1.in
@@ -0,0 +1 @@
+urllib3 ~= 1.26
diff --git a/tests/multiple_requirements_in/requirements_2.in b/tests/multiple_requirements_in/requirements_2.in
@@ -0,0 +1 @@
+attrs ~= 22.2

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+# ignore_root_user_errors`
	`2`	+There are cases when we have to run Python targets with root, e.g., in Docker containers, requiring setting `ignore_root_user_error = True` when registering Python toolchain. This test makes sure that rules_python works in this case.