update `compile_pip_requirements` to support multiple input files

[cj81499]

PR Checklist

Please check if your PR fulfills the following requirements:

• [x] Tests for the changes have been added (for bug fixes / features)
• [x] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• [ ] Bugfix
• [x] Feature (please, look at the "Scope of the project" section in the README.md file)
• [ ] Code style update (formatting, local variables)
• [ ] Refactoring (no functional changes, no api changes)
• [ ] Build related changes
• [ ] CI related changes
• [ ] Documentation content changes
• [ ] Other... Please describe:

What is the current behavior?

Issue Number: N/A

pip-compile can compile multiple input files into a single output file, but rules_python's compile_pip_requirements doesn't currently support this.

With this change, the requirements_in argument to compile_pip_requirements can now accept a list of strings (in addition to the previously accepted argument types).

In order to support a variable number of input files, my coworker (@lpulley) and I updated dependency_resolver.py to use the click CLI library. We felt this was acceptable since pip-compile already requires click to run, so we're not adding a new dependency.

We also made changes to the script to avoid mutating sys.argv, instead opting to build a new list (argv) from scratch that'll be passed to the pip-compile CLI. While subjective, I feel this improves readability, since it's not immediately obvious what's in sys.argv, but it's clear that argv begins empty, and is added to over the course of the program's execution.

What is the new behavior?

In addition to supporting a single requirements_in file, a list of requirements input files can be specified and will be passed to be pip-compile.

Does this PR introduce a breaking change?

• [ ] Yes
• [x] No

Other information

I'm still awaiting CLA signature from my company, so I'll leave this as a draft for now. I'm hoping to have it signed by end of week, but don't have an exact ETA.

[google-cla[bot]]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[f0rmiga]

Thanks for this. How about we split this into 3 PRs? The first would be the changes to pre-commit, the second using click, and the third adding the new functionality.

[aignas]

FYI, right now this usecase might be supported by having three files:

# requirements.in
-r requirements_1.in
-r requirements_2.in

# requirements_1.in
numpy == 1.20.0

# requirements_2.in
boto3

And then use the data attribute to add dependencies to requirements.in compilation:

compile_pip_requirements(
    name = "pip_dependencies",
    data = [
        "requirements_1.in",
        "requirements_2.in",
    ],
    extra_args = [
        "--allow-unsafe",
        "--resolver=backtracking",
    ],
    requirements_in = "requirements.in",
    requirements_txt = "requirements_lock.txt",
)

FYI: This got implemented in #909.

[cj81499]

I split this PR into 3 as requested by @f0rmiga (#1070, #1071, and this one (which is blocked by #1071)). I expect it'll be easier to review now, so I really appreciate the suggestion!

[cj81499]

@aignas I've actually tried doing exactly that! Unfortunately, I couldn't quite get it working right. We're using generated requirements.in files and I couldn't figure out the right path to put in the "root" requirements.in file (the one that is given directly to compile_pip_requirements as requirements_in). I even tried generating the "root" requirements.in file using $(location //xxx:requirements.in) (and the other path related make variable substitution options), but I couldn't get it to work correctly.

Regardless, I feel that accepting multiple input files is simpler than generating an additional "root" requirements.in file.

[f0rmiga]

I believe my last fix on https://github.com/bazelbuild/rules_python/pull/1053 will cover your use case with what @aignas described. I merged it yesterday. I'm still open to reviewing your PR, though.

[cj81499]

I still think this would be a nice feature to have (since I'm currently using https://github.com/cj81499/rules_python/commit/bc4569464ee62898b6eeaaa8bf7c98f42ef89e39 and it's working for my project), but I'll try out the latest commit from main to see if it's fixed later today.

[cj81499]

@f0rmiga using 767b050e45c49e63cfe103e6b97f7e2fd9be5e2e (most recent commit on main at time of writing) and the -r approach, //python:requirements_test fails with:

Checking python/requirements.txt
Traceback (most recent call last):
  File "/home/<username>/.cache/bazel/_bazel_<username>/<hash>/sandbox/linux-sandbox/40/execroot/bazel-playground/bazel-out/k8-fastbuild/bin/python/requirements_test.runfiles/pypi__pip/pip/_internal/req/req_file.py", line 540, in get_file_content
    with open(url, "rb") as f:
         ^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: 'python/python/<name_of_project>/requirements.in'

Notice the repeated "python" in the last line of output.

The requirements file is being generated such that the line -r $(rootpath //python/<name_of_project>:requirements.in) will be included in the file (after make variable substitution of course). I've tried using location, rlocationpath, etc too, but none work (rootpath is closest to the expected path of python/<name_of_project>/requirements.in).

It seems that compile_pip_requirements is adding a prefix to the path for some reason?

My best guess is that this is b/c compile_pip_requirements is used from python/BUILD.bazel, rather than at WORKSPACE root (which seems to be the typical case based on the examples provided in this repo).

[cj81499]

Just an update, getting the CLA signed is taking (far) longer than originally expected. I'm still working on it!

[cj81499]

I believe the test failure is an unrelated (flaky) failure.

2023/11/02 23:20:29 could not download Bazel: could not copy from https://releases.bazel.build/6.4.0/release/bazel-6.4.0-linux-x86_64 to /var/lib/buildkite-agent/.cache/bazelisk/downloads/bazelbuild/bazel-6.4.0-linux-x86_64/bin/download461043671: stream error: stream ID 1; INTERNAL_ERROR

[cj81499]

Resolved the conflicts.

CI failure seems unrelated to my changes. My theory is it was caused by GitHub's ~~partial~~ outage today. Would appreciate a retry!

[cj81499]

@aignas Good catch! I updated the changelog. Thanks for taking a look :)

[cj81499]

Spent some time investigating the Windows-specific failures. I'm having a difficult time sorting it out independently and could use some help. I pushed a commit (make sure to read the commit message!) to a separate branch with my findings so far. https://github.com/cj81499/rules_python/commit/b843fd650f270a9309f5babd4ac22f7c7c6f852b

Note that the relevant tests do pass (locally) with the changes in that commit, but I don't think it ought to be merged as-is...

[aignas]

~I think these are just flakes, restarted the jobs.~

EDIT: I could be wrong here, as they failed twice.

EDIT2: I've read the linked commit message and it is really interesting. On the other hand, adding hatch related lines seems a little bit weird to me. Maybe rules_python should have it's own build-backend that does almost nothing?

[lpulley]

(I work with @cj81499.) Any chance this will pass CI now? Maybe there have been some improvements? We've been using this PR as a patch for 14 months now since we're not sure what's wrong with the CI here.

We did take a crack at it on Windows a few months ago, as @cj81499 mentioned: https://github.com/cj81499/rules_python/commit/b843fd650f270a9309f5babd4ac22f7c7c6f852b

[cj81499]

@rickeylev @aignas I (and my coworkers) would still quite like to make this contribution. Can you provide any guidance on the failures we saw?

[cj81499]

Disabling the failing test on windows would be one way to make CI happy.

Is that acceptable...? If so, how would I go about doing that?

[cj81499]

Massive shout-out to my coworker @lpulley for pairing with me to root-cause and fix the failures on Windows!

We managed to implement a bit of workaround/hack:

https://github.com/cj81499/rules_python/blob/9a5653ca7f32fbf698dfb1ba1b7c425dc521c7d5/python/private/pypi/pip_compile.bzl#L160-L164

I think the idea that "Maybe rules_python should have it's own build-backend that does almost nothing?" might be less hacky, but I suspect a lot of work to implement and maintain (and certainly not something I have bandwidth to contribute). Hopefully the workaround/hack is sufficient.

[cj81499]

FYI: I've opened https://github.com/pypa/distutils/pull/278. I figure it's best to handle this upstream of rules_python if possible. I don't think the workaround is too nasty to merge in the meantime, but hopefully it can be removed if/when that PR is merged to distutils (and vendored by setuptools, and released, and rules_python upgrades to a newer version of setuptools that include the fix).

[aignas]

FYI: I've opened pypa/distutils#278. I figure it's best to handle this upstream of rules_python if possible. I don't think the workaround is too nasty to merge in the meantime, but hopefully it can be removed if/when that PR is merged to distutils (and vendored by setuptools, and released, and rules_python upgrades to a newer version of setuptools that include the fix).

I am wondering if we should instead open a ticket in setuptools because distutils is deprecated (PEP632) and you may have a hard time getting response from them.

I think I'll just raise a separate issue in rules_python to track this issue, let's continue conversation there (#2121).

[cj81499]

I am wondering if we should instead open a ticket in setuptools because distutils is deprecated (PEP632) and you may have a hard time getting response from them.

Maybe? https://github.com/pypa/distutils has been updated recently (within the last month or so), so I someone must still be looking at it. You're more than welcome to open an issue on https://github.com/pypa/setuptools to try to draw attention to the issue/my PR https://github.com/pypa/distutils/pull/278 if you think that's worthwhile.