feat(pip_repository): Support pip parse cycles

[arrdem]

This patch reworks the pip_repository machinery to allow users to manually annotate groups of libraries which form packaging cycles in PyPi and must be simultaneously installed.

The strategy here is to transform any dependencies A and B which have dependencies and are mutually dependent

graph LR;
    A-->B;
    A-->D;
    A-->E;
    B-->A;
    B-->F;
    B-->G;

Loading

into a new "dependency group" C which has A* and B* as dependencies, defined as A and B less any direct dependencies which are members of the group. This is viable for python because Python files just need to be emplaced into a runfiles directory for the interpreter. We don't actually have a true hard dependency between the build definition of A requiring the build product B be available which requires that the build product of A be available.

graph LR
     C-->A*;
     A*-->D;
     A*-->E;
     C-->B*;
     B*-->F;
     B*-->G;

Loading

This gets us most of the way there, as a user can now safely write requirement("A") and we can provide them with C, which has the desired effect of pulling in A, B and their respective transitives.

There is one remaining problem - a user writing deps = [requirement("A"), requirement("B")] will take a double direct dependency on C. So we need to insert a layer of indirection, generating C_A and C_B which serve only as unique aliases for C so that we can support the double dependency. Our final dependency graph then is as follows

graph LR
     C_A-->C;
     C_B-->C;
     C-->A*;
     A*-->D;
     A*-->E;
     C-->B*;
     B*-->F;
     B*-->G;

Loading

Addresses #1076, #1188

To do

• Get rebased
• Get re-validated manually
• Buildifier
• Get CI happy
• Update documentation
• Update changelog

[arrdem]

Hrm. Maybe I'm missing something here but it looks like bazel-under-bazel for the tests isn't happy with the two new templates. Not sure how to proceed with a fix.

[chrislovecnm]

@arrdem The change you made is causing

(19:31:08) ERROR: error loading package under directory '': no such package '@rules_pythonoverridepip~pip//': in call to file(), parameter 'content' got value of type 'Label', want 'string'

--
  | (19:31:08) ERROR: An error occurred during the fetch of repository 'rules_python~override~pip~pip':
  | Traceback (most recent call last):
  | File "/var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/31068de31d119d275d02c5ef2e251165/external/rules_python~override/python/pip_install/pip_repository.bzl", line 362, column 14, in _pip_repository_bzlmod_impl
  | rctx.file("BUILD.bazel", rctx.attr._build_template, substitutions = {
  | Error in file: in call to file(), parameter 'content' got value of type 'Label', want 'string'
  | (19:31:08) ERROR: <builtin>: fetching pip_repository_bzlmod rule @rules_python~override//python:rules_python~override~pip~pip: Traceback (most recent call last):
  | File "/var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/31068de31d119d275d02c5ef2e251165/external/rules_python~override/python/pip_install/pip_repository.bzl", line 362, column 14, in _pip_repository_bzlmod_impl
  | rctx.file("BUILD.bazel", rctx.attr._build_template, substitutions = {
  | Error in file: in call to file(), parameter 'content' got value of type 'Label', want 'string'
  | (19:31:08) INFO: Repository gazelle~0.29.0 instantiated at:
  | callstack not available

I'm seeing it here: https://buildkite.com/bazel/rules-python-python/builds/4591#018776ef-c472-4122-8b4b-2e30c38bee88

[rickeylev]

@groodt

[arrdem]

Okay - I think this is ready for a review. Includes test updates, passes the test updates. The bzlmod story for this is ... there isn't one yet. If there's a way to map this strategy onto bzlmod I'm all ears, I'm just not familiar with that machinery and opted to partition it and retain current main behavior.

[chrislovecnm]

I appreciate your work! We are having problems on windows with this PR. Please take a look at CI - https://buildkite.com/bazel/rules-python-python/builds/4896#01882c9e-c77f-4596-bfbc-8ca5b5c2931a

[chrislovecnm]

@arrdem please rebase.

[chrislovecnm]

First, thank you for the work. Second, I need your thoughts about how we will do multiple pip support in bzlmod. With these changes, you did you have knowledge that I need to learn about this code.

My overall comments:

• I love to see over-documented code. You can blame Tim Hockin for this. He taught me we cannot remember why we wrote code changes three months ago.
• Have we tested this with Gazelle? How does Gazelle interact with this new code? Hopefully, it ignores it, but we may want to document that Gazelle does not support this and file an issue.
• We have to have test coverage with bzlmod. Please update the existing examples to use this new code.

Kudos on the PR description. That is awesome.

[chrislovecnm]

Will this address #1188?

[chrislovecnm]

There is one remaining problem - a user writing deps = [requirement("A"), requirement("B")] will take a double direct dependency on C. So we need to insert a layer of indirection, generating C_A and C_B which serve only as shims to C so that we can support the double dependency.

If we are not addressing this problem, do we have an open issue?

[chrislovecnm]

@aignas, do you mind taking a look?

[arrdem]

Will this address #1188?

I believe it will.

There is one remaining problem - a user writing deps = [requirement("A"), requirement("B")] will take a double direct dependency on C. So we need to insert a layer of indirection, generating C_A and C_B which serve only as shims to C so that we can support the double dependency.

If we are not addressing this problem, do we have an open issue?

I'll edit the PR description a bit to clarify. This problem was present in the original PR I submitted and was fixed yesterday. Note that requirement is generating a reference to an alias rule to resolve this.

[arrdem]

Tightened this up a bunch. Removed all unneeded changes that impacted the bzlmod machinery. Backed out some of the formatting changes. Eliminated the generated lib.bzl file. Updated all the code, comments and attrs to consistently refer to requirement clusters.

As far as I can determine at this point the bzlmod CI failures on windows are unrelated. I don't have a windows test environment available and I've eliminated all the parts of this changeset which impact bzlmod machinery. Guidance on getting to green there would be appreciated.

[rickeylev]

This is some pretty solid work. Thanks, @arrdem!