662 duplicates are not supported in requirements.txt when run with disable pip #749
Conversation
|
Thanks for the patch @mathbou! I'll review this today. |
| req_specifiers: dict[str, SpecifierSet] = dict() | ||
|
|
||
| for req in reqs: | ||
| if ( | ||
| isinstance(req, InstallRequirement) | ||
| and (req.marker is None or req.marker.evaluate()) | ||
| and req.req is not None | ||
| ): | ||
| if req.name in req_names: | ||
| duplicate_req_specifier = req_specifiers.get(req.name) | ||
|
|
||
| if not duplicate_req_specifier: | ||
| req_specifiers[req.name] = req.specifier | ||
|
|
||
| elif duplicate_req_specifier != req.specifier: |
There was a problem hiding this comment.
Just making sure I understand the behavior here: rather than deduping on just the requirement's name (i.e. package name), we're now deduping on different specifiers for duplicated names, right?
IIUC that means that we'll still have some false positives here (since SpecifierSet != SpecifierSet does not imply that the two are incompatible), right? If so that's fine, just making sure I understand 🙂
There was a problem hiding this comment.
rather than deduping on just the requirement's name (i.e. package name), we're now deduping on different specifiers for duplicated names
Yep, that's the idea. As long as each package sharing the same name, also share the same specifier, it is considered identical.
IIUC that means that we'll still have some false positives here (since SpecifierSet != SpecifierSet does not imply that the two are incompatible), right? If so that's fine, just making sure I understand 🙂
I'm not sure how we could still get false positives here 🤔 Have you a specific case in mind ?
Unless you modify the resolved requirements, it is very unlikely to get different SpecifierSet for the package and its extras.
There was a problem hiding this comment.
I'm not sure how we could still get false positives here 🤔 Have you a specific case in mind ?
Unless you modify the resolved requirements, it is very unlikely to get different SpecifierSet for the package and its extras.
What I'm thinking of is something like this:
example >= 1.0
example >= 1.0, < 3.0
In this case, SpecifierSet(">= 1.0") != SpecifierSet(">= 1.0, < 3.0"), but the two are not incompatible (all versions in 1.x and 2.x satisfy both sets). Unless I'm misunderstanding, I believe this change will still incorrectly reject this as a "duplicate" dependency, when one is a superset of the other.
There was a problem hiding this comment.
You are correct, it this case it will be rejected.
Maybe it would be more appropriate to reject them as incompatible instead of duplicate, what do you think ?
…uirements.txt-when-run-with---disable-pip
Recently, I run in the same problem described in #662. To avoid this, I propose a finer check for duplicates based on both name and specifier.
As stated in the issue, when the
--disable-pipflag is used, we could consider that a full requirement resolution has been made. Knowing that, as long as specifiers matches, having duplicates is not a problem. If they don't match, we raise an error like before.On the side, I also add a small fix for stdout/stderr reading in
pip_audit/_subprocess.py. I don't know if it's specific to windows, but the fact that a size was specified, I had the process hanging indefinitely.