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

Restrict unused version pins in URL/VCS requirements? #11567

Closed
1 task done
woodruffw opened this issue Oct 31, 2022 · 5 comments · Fixed by #11617
Closed
1 task done

Restrict unused version pins in URL/VCS requirements? #11567

woodruffw opened this issue Oct 31, 2022 · 5 comments · Fixed by #11617
Labels
type: feature request Request for a new feature

Comments

@woodruffw
Copy link
Member

What's the problem this feature will solve?

I noticed this behavior while debugging/repro-ing pypa/pip-audit#382: pip install -r requirements.txt will happily install a VCS or other URL dependency that contains a fragment string like this:

hypothesis @ git+https://github.com/HypothesisWorks/hypothesis.git@bb6b55ad8d#egg=hypothesis==9.9.9&subdirectory=hypothesis-python

The egg=hypothesis==9.9.9 fragment implies that hypothesis==9.9.9 is being installed from this URL, but that version specifier is actually ignored and the real version at that VCS ref (6.56.3) is installed instead.

AFAICT this version pin was never supported in the first place (#5384 says that the egg fragment has never supported 508-style specs), but it's a little surprising (as an end user) for it to silently be ignored rather than producing a warning or requirements parsing error.

Describe the solution you'd like

Ideally, pip would produce a hard error (or at least a warning) here, since the supplied version specifier is (1) ineffective and (2) indicates user confusion about what they're asking for.

Alternative Solutions

No alternative solution is necessary, since this isn't broken per se.

Additional context

pip-audit context: pypa/pip-audit#382

This issue also manifests in third-party requirements-file parsers, like pip-audit and pip-requirements-parser. If this behavior is changed in pip itself, I can submit patches there as well.

Code of Conduct

@woodruffw woodruffw added S: needs triage Issues/PRs that need to be triaged type: feature request Request for a new feature labels Oct 31, 2022
@woodruffw
Copy link
Member Author

Also: I'm happy to submit the patch for this! I just wanted to file it as an issue first, to register interest/awareness.

@pfmoore
Copy link
Member

pfmoore commented Oct 31, 2022

+1 on giving an error for anything other than #egg=name where name follows the specification for a project name.

@woodruffw
Copy link
Member Author

+1 on giving an error for anything other than #egg=name where name follows the specification for a project name.

Awesome! I'll take a stab at this tomorrow.

woodruffw added a commit to trail-of-forks/pip that referenced this issue Nov 22, 2022
This should help reduce user confusion about what can go in a URI's
egg fragment.

Fixes pypa#11567.

Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw
Copy link
Member Author

#11617 is a first stab at this, but it's revealed some ambiguities in how #egg= fragments are defined: pip's own documentation says that the egg fragment is nothing except for a project name (implied to be PEP 508), while pip's testsuite includes tests for egg fragments that contain extras (e.g. #egg=foo[bar]).

@uranusjr
Copy link
Member

If I remember the history correctly, originally #egg was meant to be just the project name (designed by setuptools), which pip copied. It was later extended to be able to take extras (otherwise there’s no way to specify extras for a URL) and pip’s documentation was probably never rewritten to reflect the fact. And then PEP 508 came out and should have made everything irrelavant, but people still continue using #egg to this day.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 28, 2023
@pradyunsg pradyunsg removed the S: needs triage Issues/PRs that need to be triaged label Jan 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: feature request Request for a new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants