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

Remove --hash from version in requirements.txt #379

Merged
merged 4 commits into from
May 15, 2023

Conversation

robotdana
Copy link
Contributor

This removes everything after the whitespace of the version to also
catch the other per-requirement options --global-option and
--config-settings and any future options that may be added

https://pip.pypa.io/en/stable/reference/requirements-file-format/#per-requirement-options

fixes: #369

While addressing this i noticed the pip documentation example for --hash
used line continuations which weren't currently supported by this parser
so i've added support for this

This removes everything after the whitespace of the version to also
catch the other per-requirement options --global-option and
--config-settings and any future options that may be added

https://pip.pypa.io/en/stable/reference/requirements-file-format/#per-requirement-options

fixes: google#369

# Conflicts:
#	pkg/lockfile/parse-requirements-txt_test.go
@another-rex another-rex requested review from G-Rath and another-rex May 12, 2023 01:10
robotdana added 2 commits May 12, 2023 13:13
While addressing google#369 i noticed the pip documentation example for --hash
used line continuations which weren't currently supported by this parser

so i've added support for line continuations
@robotdana robotdana force-pushed the requirements_text_version_meta branch from 7f238ce to 1623b97 Compare May 12, 2023 01:13
Copy link
Collaborator

@G-Rath G-Rath left a comment

Choose a reason for hiding this comment

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

nice stuff! I think it would be good to add another line to the with-hash fixture that uses either --global-option or --config-settings just to show that it's not specifically supporting just --hash, but that's not blocking

Copy link
Collaborator

@another-rex another-rex left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@another-rex another-rex merged commit 50d5e07 into google:main May 15, 2023
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.

requirements.txt mis-parses lines that contain --hash
3 participants