-
Notifications
You must be signed in to change notification settings - Fork 544
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
pip.parse does not accept lock files with duplicated dependencies #1615
Comments
So this appears to be a bzlmod exclusive behavior? We can see this issue with bzlmod starting from 0.15.0, but if we use |
Alright, I've now created a repo which shows this is exclusively an issue with The |
That behavior differs between bzlmod and workspace here is a bit strange. The two implementation re-use a lot of the lower level pieces. The thing that sticks out to me is both use the same function to parse the requirements. That parsing ignores the |
Uhhh, huh. So the workspace code path isn't de-duplicating the names. It is calling In this case, the last one wins. The last one is the one whose requirement line sorts last (there's an earlier sorted() call on the list of requirement lines). In practice, I think this works out OK? Because the requirements file expands out extras into their actual packages? I'm not too familiar with how extras work. If so, then an earlier line being ignored doesn't matter. It should be easy to mimic this behavior in bzlmod. We just have to dedupe the names, giving preference to the last one. |
@rickeylev I'm happy to submit a PR if you can point me in a direction. Unless this is one of those "it'd take the same time to explain as it would to implement" sort of things. |
When a package has extras, the requirements parsing discards the extra portion of the name and just returns the base name. When a repository is created for each entry, this means the same name is used for multiple repositories. Under WORKSPACE builds, duplicate repository names aren't an error. It appears the last defined repo takes affect. Under bzlmod, duplicate repo names are an error. To fix, mimic the last-defined-wins behavior in bzlmod by using a map to dedupe the package names. Fixes bazelbuild#1615
I've got a fix, so now its just about constructing a test case. Is there a trick to generating a requirements file with both |
When a package has extras, the requirements parsing discards the extra portion of the name and just returns the base name. When a repository is created for each entry, this means the same name is used for multiple repositories. Under WORKSPACE builds, duplicate repository names aren't an error. It appears the last defined repo takes affect. Under bzlmod, duplicate repo names are an error. To fix, mimic the last-defined-wins behavior in bzlmod by using a map to dedupe the package names. Fixes bazelbuild#1615
When a package has extras, the requirements parsing discards the extra portion of the name and just returns the base name. When a repository is created for each entry, this means the same name is used for multiple repositories. Under WORKSPACE builds, duplicate repository names aren't an error. It appears the last defined repo takes affect. Under bzlmod, duplicate repo names are an error. To fix, mimic the last-defined-wins behavior in bzlmod by using a map to dedupe the package names. Fixes bazelbuild#1615
When a package has extras, the requirements parsing discards the extra portion of the name and just returns the base name. When a repository is created for each entry, this means the same name is used for multiple repositories. Under WORKSPACE builds, duplicate repository names aren't an error. It appears the last defined repo takes affect. Under bzlmod, duplicate repo names are an error. To fix, mimic the last-defined-wins behavior in bzlmod by using a map to dedupe the package names. Fixes bazelbuild#1615
It's a quirk with
and run |
The relevant poetry behavior is discussed a bit more in this issue: python-poetry/poetry#5688 It's not so much an issue with extras as it is with constraints in general. |
When a package has extras, the requirements parsing discards the extra portion of the name and just returns the base name. When a repository is created for each entry, this means the same name is used for multiple repositories. Under WORKSPACE builds, duplicate repository names aren't an error. It appears the last defined repo takes affect. Under bzlmod, duplicate repo names are an error. To fix, mimic the last-defined-wins behavior in bzlmod by using a map to dedupe the package names. Fixes bazelbuild#1615
#1620) Requirements files are permitted to have duplicate lines for the same package. An example of this is having separate lines for a package and its extras. When we parse requirements, the parser discards the "extra" portion of the entry and returns a list of all the packages as-is. When a repository is created for each entry, this means the same name is used for multiple repositories. Under WORKSPACE builds, duplicate repository names aren't an error. It appears that last defined repo takes affect. Under bzlmod, duplicate repo names are an error. To fix, mimic the last-defined-wins behavior in bzlmod by using a map to dedupe the package names. Fixes #1615
🐞 bug report
Affected Rule
pip.parse
Is this a regression?
Yes, this behavior is only present when using
bzlmod
Description
When using
pip.parse
, if a dependency is repeated in your requirements lock, you'll get an error. This is handled just fine by pip and is necessary for tools likepoetry
to export this way because it's possible for the same dependency to have different constraints in different parts of the dependency tree.Ref: python-poetry/poetry#5688
🔬 Minimal Reproduction
Repo with repro here: https://github.com/AndrewGuenther/rules_python_1615_repro
The
main
branch usesbzlmod
andworkspace
usesWORKSPACE
. Theworkspace
branch builds fine, butmain
does not. Same requirements on both branches, the only difference isbzlmod
.🔥 Exception or Error
If you try and build the example above, you'll get an error to the following effect:
🌍 Your Environment
Operating System:
Output of
bazel version
:Rules_python version:
Anything else relevant?
The text was updated successfully, but these errors were encountered: