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

Generate a single hash #1406

Closed
wants to merge 28 commits into from
Closed

Generate a single hash #1406

wants to merge 28 commits into from

Conversation

plannigan
Copy link
Contributor

@plannigan plannigan commented May 31, 2021

Changelog-friendly one-liner: Option to restrict generated hashes to the single best matching file

Closes #1330

Contributor checklist
  • Provided the tests for the changes.
  • Gave a clear one-line description in the PR (that the maintainers can add to CHANGELOG.md on release).
  • Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

@plannigan plannigan changed the title Single hash Generate a single hash May 31, 2021
@plannigan
Copy link
Contributor Author

The mypy failures in the pre-commit.ci look like they are legitimate, but not related to my changes. I am getting the same errors when running checkqa locally on the default branch.

@plannigan
Copy link
Contributor Author

Pulled in the latest changes to the default branch and now mypy is passing. The rest of the CI is waiting for a maintainer to approve it to run.

@codecov

This comment has been minimized.

plannigan added 2 commits June 5, 2021 13:52
Add typing info the nullcontext to help mypy know it is a generic context manager
@plannigan plannigan mentioned this pull request Jun 9, 2021
3 tasks
@plannigan
Copy link
Contributor Author

I switched to the compatibility implementation of nullcontext() which will resolve the previous failure on python 3.6.

@ssbarnea ssbarnea requested review from atugushev and webknjaz June 22, 2021 08:00
@ssbarnea ssbarnea added bug fix enhancement Improvements to functionality and removed bug fix labels Jun 22, 2021
@ssbarnea ssbarnea requested a review from jdufresne June 22, 2021 08:02
@ssbarnea ssbarnea added this to the 6.2.0 milestone Jun 22, 2021
Copy link
Member

@ssbarnea ssbarnea left a comment

Choose a reason for hiding this comment

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

While I do not see myself using it, it does still LGTM, especially as the number of hashes
can become quite large for some packages.

Still, I hope others will review the change better and note if there are reasons for not doing this change.

@atugushev
Copy link
Member

Can we postpone this to 6.3.0? There is no rush and i'd like to take a look at this later.

@ssbarnea ssbarnea modified the milestones: 6.2.0, 6.3.0 Jun 22, 2021
@atugushev
Copy link
Member

It looks like there is an issue with existing hashes in requirements.txt. How to reproduce:

  1. Generate hashes first:
$ pip-compile --generate-hashes
#
# This file is autogenerated by pip-compile with python 3.8
# To update, run:
#
#    pip-compile --generate-hashes
#
psycopg2==2.9.1 \
    --hash=sha256:079d97fc22de90da1d370c90583659a9f9a6ee4007355f5825e5f1c70dffc1fa \
    --hash=sha256:2087013c159a73e09713294a44d0c8008204d06326006b7f652bef5ace66eebb \
    --hash=sha256:2c992196719fadda59f72d44603ee1a2fdcc67de097eea38d41c7ad9ad246e62 \
    --hash=sha256:7640e1e4d72444ef012e275e7b53204d7fab341fb22bc76057ede22fe6860b25 \
    --hash=sha256:7f91312f065df517187134cce8e395ab37f5b601a42446bdc0f0d51773621854 \
    --hash=sha256:830c8e8dddab6b6716a4bf73a09910c7954a92f40cf1d1e702fb93c8a919cc56 \
    --hash=sha256:89409d369f4882c47f7ea20c42c5046879ce22c1e4ea20ef3b00a4dfc0a7f188 \
    --hash=sha256:bf35a25f1aaa8a3781195595577fcbb59934856ee46b4f252f56ad12b8043bcf \
    --hash=sha256:de5303a6f1d0a7a34b9d40e4d3bef684ccc44a49bbe3eb85e3c0bffb4a131b7c
    # via -r requirements.in
  1. Then run pip-compile again with the single hash option:
$ pip-compile --generate-hashes --single-hash
#
# This file is autogenerated by pip-compile with python 3.8
# To update, run:
#
#    pip-compile --generate-hashes --single-hash
#
psycopg2==2.9.1 \
    --hash=sha256:079d97fc22de90da1d370c90583659a9f9a6ee4007355f5825e5f1c70dffc1fa \
    --hash=sha256:2087013c159a73e09713294a44d0c8008204d06326006b7f652bef5ace66eebb \
    --hash=sha256:2c992196719fadda59f72d44603ee1a2fdcc67de097eea38d41c7ad9ad246e62 \
    --hash=sha256:7640e1e4d72444ef012e275e7b53204d7fab341fb22bc76057ede22fe6860b25 \
    --hash=sha256:7f91312f065df517187134cce8e395ab37f5b601a42446bdc0f0d51773621854 \
    --hash=sha256:830c8e8dddab6b6716a4bf73a09910c7954a92f40cf1d1e702fb93c8a919cc56 \
    --hash=sha256:89409d369f4882c47f7ea20c42c5046879ce22c1e4ea20ef3b00a4dfc0a7f188 \
    --hash=sha256:bf35a25f1aaa8a3781195595577fcbb59934856ee46b4f252f56ad12b8043bcf \
    --hash=sha256:de5303a6f1d0a7a34b9d40e4d3bef684ccc44a49bbe3eb85e3c0bffb4a131b7c
    # via -r requirements.in

Note that --no-reuse-hashes helps to generate single hash output:

$ pip-compile --generate-hashes --no-reuse-hashes --single-hash
#
# This file is autogenerated by pip-compile with python 3.8
# To update, run:
#
#    pip-compile --generate-hashes --no-reuse-hashes --single-hash
#
psycopg2==2.9.1 \
    --hash=sha256:de5303a6f1d0a7a34b9d40e4d3bef684ccc44a49bbe3eb85e3c0bffb4a131b7c
    # via -r requirements.in

@plannigan
Copy link
Contributor Author

Ok, that makes sense. The local repository checks the cache before calling get_hashes() for the PyPI repository. I'll have some time to circle back on this in a few days.

@plannigan
Copy link
Contributor Author

Thanks for tracking down the hash reuse issue. The fix was a simple check to see if there were more than one existing hash.

@plannigan plannigan requested a review from atugushev July 30, 2021 13:36
@atugushev atugushev removed this from the 6.3.0 milestone Sep 20, 2021
Copy link
Contributor

@AndydeCleyre AndydeCleyre left a comment

Choose a reason for hiding this comment

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

Small English changes in the readme, please:

  • "give" to "given"
  • "match" to "matching"

Thanks!

@plannigan plannigan requested a review from AndydeCleyre October 2, 2021 13:26
@plannigan
Copy link
Contributor Author

Fixed the grammar in a few more locations in docstrings. Should be good to go now.

@plannigan plannigan requested a review from AndydeCleyre October 4, 2021 13:17
@jdufresne jdufresne removed their request for review September 27, 2022 12:43
@ssbarnea
Copy link
Member

Closing this PR as it seems outdated. Please try to reopen it once these problems are sorted. Small PRs that are easy to review are likely to be merged quickly as they impose less risks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements to functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide a way to limit to a single hash per requirement when generating hashes
4 participants