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

pip_api: initial support for hashed requirements #126

Merged
merged 15 commits into from
Feb 3, 2022
Merged

pip_api: initial support for hashed requirements #126

merged 15 commits into from
Feb 3, 2022

Conversation

woodruffw
Copy link
Collaborator

@woodruffw woodruffw commented Jan 31, 2022

WIP.

Some things that need to be done:

  • For compatibility with pip, parse_requirments should go into a "strict" mode as soon as it sees a hashed requirement. In strict mode, any requirements that don't have a hash cause an error.
  • More tests.

Comment on lines 179 to 183
class Requirement(requirements.Requirement):
def __init__(self, *args, **kwargs):
self.hashes = kwargs.pop("hashes", None)

super().__init__(*args, **kwargs)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

N.B.: requirements.Requirement doesn't have any state/API for hashes, so I created a thin child wrapper instead. I figured this would be the least invasive approach, since it doesn't cause any API breakage, but let me know if there's a better/preferred alternative.

hashes_by_kind = defaultdict(list)
if known.hashes:
for hsh in known.hashes:
kind, hsh = hsh.split(":", 1)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure if it's worth it, but we could also promote any ValueError that happens here (due to a malformed --hash option) into a PipError.

@woodruffw
Copy link
Collaborator Author

cc @tetsuo-cpp

@di
Copy link
Owner

di commented Feb 1, 2022

@woodruffw I'm getting ready to release a new version with #127, should we try and include this as well?

@woodruffw
Copy link
Collaborator Author

Makes sense to me. I can have this ready in a moment.

@woodruffw woodruffw marked this pull request as ready for review February 1, 2022 15:46
README.md Show resolved Hide resolved
tests/test_parse_requirements.py Outdated Show resolved Hide resolved
@woodruffw
Copy link
Collaborator Author

pypa/pip-audit#229 should be good to go once this is in.

Copy link
Owner

@di di left a comment

Choose a reason for hiding this comment

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

LGTM otherwise

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Co-authored-by: Dustin Ingram <di@users.noreply.github.com>
@di di merged commit d08369f into di:master Feb 3, 2022
@woodruffw woodruffw deleted the ww/hash-support branch February 3, 2022 21:42
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.

3 participants