Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(pypi): Fix use_hub_alias_dependencies with WORKSPACE #2504

Merged
merged 1 commit into from
Dec 15, 2024

Conversation

gholms
Copy link
Contributor

@gholms gholms commented Dec 14, 2024

The code path pip_parse follows when using a WORKSPACE file with
use_hub_alias_dependencies enabled forgets to pass requirement cycles
along to alias creation, leading to the _groups package never being
created and aliases skipping them. Requirement cycles are just ignored
entirely. In this patch we attempt to fix that so grouping works more
or less the same way as it does under bzlmod with that flag enabled.

The code path pip_parse follows when using a WORKSPACE file with
use_hub_alias_dependencies enabled forgets to pass requirement cycles
along to alias creation, leading to the _groups package never being
created and aliases skipping them.  Requirement cycles are just ignored
entirely.  In this patch we attempt to fix that so grouping works more
or less the same way as it does under bzlmod with that flag enabled.
@aignas
Copy link
Collaborator

aignas commented Dec 15, 2024

Thanks, tested in the examples folder by building sphinx with the following patch:

$ gd
diff --git a/examples/pip_parse/WORKSPACE b/examples/pip_parse/WORKSPACE
index bb4714d9..17b1288d 100644
--- a/examples/pip_parse/WORKSPACE
+++ b/examples/pip_parse/WORKSPACE
@@ -24,6 +24,8 @@ pip_parse(
     # environment = {"HTTPS_PROXY": "http://my.proxy.fun/"},
     name = "pypi",

+    use_hub_alias_dependencies = False,
+
     # Requirement groups allow Bazel to tolerate PyPi cycles by putting dependencies
     # which are known to form cycles into groups together.
     experimental_requirement_cycles = {

It seems that is working well in both cases, when I am using hub aliases dependencies or not, so this is a great fix to have.

@aignas aignas added this pull request to the merge queue Dec 15, 2024
Merged via the queue into bazelbuild:main with commit 727ab43 Dec 15, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants