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 pex3 lock export handling of exotic reqs. #2423

Merged
merged 2 commits into from
Jun 9, 2024

Conversation

jsirois
Copy link
Member

@jsirois jsirois commented Jun 8, 2024

Although Pex supports locking VCS requirements and local project
requirements, it does so with a be-spoke system for fingerprinting
each; as such, the --hashes emitted when exporting lock files
containing these types of requirements are not actually useable in
practice. Continue to support exporting this class of lock file, but
warn of the potential problems and offer a new --format pip-no-hash
mode for the daring. In addition, change the requirements output for
this class of lock file to match the input requirement for best
fidelity when actually attempting to use the resulting exported
requirement file without --hashes.

Fixes #2416

Although Pex supports locking VCS requirements and local project
requirements, it does so with a be-spoke system for fingerprinting
each; as such, the `--hash`es emitted when exporting lock files
containing these types of requirements are not actually useable in
practice. Continue to support exporting this class of lock file, but
warn of the potential problems and offer a new `--format pip-no-hash`
mode for the daring. In addition, change the requirements output for
this class of lock file to match the input requirement for best
fidelity when actually attempting to use the resulting exported
requirement file without `--hash`es.

Fixes pex-tool#2416
@jsirois jsirois requested review from zmanji, benjyw and huonw June 8, 2024 01:55
@jsirois
Copy link
Member Author

jsirois commented Jun 8, 2024

I appreciate any and all reviews as always. That said, in this case, I'll be blocking on your opinion @huonw. There are some eye-of-the-beholder decisions here that I'd like your second eye on.

_
............................................________
....................................,.-'"...................``~.,
.............................,.-"..................................."-.,
.........................,/...............................................":,
.....................,?......................................................,
.................../...........................................................,}
................./......................................................,:`^`..}
.............../...................................................,:"........./
..............?.....__.........................................:`.........../
............./__.(....."~-,_..............................,:`........../
.........../(_...."~,_........"~,_....................,:`........_/
..........{.._$;_......"=,_......."-,_.......,.-~-,},.~";/....}
...........((.....*~_......."=-._......";,,./`..../"............../
...,,,___.`~,......"~.,....................`.....}............../
............(....`=-,,.......`........................(......;_,,-"
............/.`~,......`-...................................../
.............`~.*-,.....................................|,./.....,__
,,_..........}.>-._...................................|..............`=~-,
.....`=~-,__......`,.................................
...................`=~-,,.,...............................
................................`:,,...........................`..............__
.....................................`=-,...................,%`>--==``
........................................_..........._,-%.......`
...................................,
@huonw
Copy link
Collaborator

huonw commented Jun 8, 2024

I've only had a moment for a quick skim but looks reasonable: nice approach with the warning message, and the introduction of --format pip-no-hash seems sensible (and the name looks good: obviously preserving --format pip for backwards compatibility, but even if starting from scratch, seems better to have the "more secure" version as the easier default).

Could you indicate which parts you're thinking are worth particular attention?

@jsirois
Copy link
Member Author

jsirois commented Jun 8, 2024

That's it, you covered it all in that paragraph.

Copy link
Collaborator

@benjyw benjyw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation looks good to me, so if @huonw is happy with the functionality...

@jsirois jsirois merged commit cf1336d into pex-tool:main Jun 9, 2024
26 checks passed
@jsirois jsirois deleted the issues/2416 branch June 9, 2024 20:38
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.

pex3 lock export can turn VCS requirements into PyPI ones, resulting in hash failures
3 participants