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

allow downloading yanked sdists for pip dependencies #506

Merged
merged 1 commit into from
Apr 9, 2024
Merged

allow downloading yanked sdists for pip dependencies #506

merged 1 commit into from
Apr 9, 2024

Conversation

slimreaper35
Copy link
Collaborator

@slimreaper35 slimreaper35 commented Mar 22, 2024

Maintainers will complete the following section

  • Commit messages are descriptive enough
  • Code coverage from testing does not decrease and new code is covered
  • [n/a] Docs updated (if applicable)
  • [n/a] Docs links in the code are still valid (if docs were updated)

Note: if the contribution is external (not from an organization member), the CI
pipeline will not run automatically. After verifying that the CI is safe to run:

@slimreaper35
Copy link
Collaborator Author

slimreaper35 commented Mar 22, 2024

This is just a small update on @ben-alkov recent pull request. It was "hidden" deep in the backlog.

@ben-alkov
Copy link
Member

The original story is a little weird, because I have definitely seen pip refuse to install anything at all if the only matching download has been yanked.

It's possible that the deps in question weren't pinned, though (e.g. '==' or '==='), which seems to be prereq according to the applicable PEP.

@ben-alkov
Copy link
Member

@slimreaper35; Overall, LGTM, but I feel like we should have a little more warning info for end-users.

Copy link
Contributor

@taylormadore taylormadore left a comment

Choose a reason for hiding this comment

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

LGTM, but do we want to have integration test coverage for this too?

@ben-alkov
Copy link
Member

LGTM, but do we want to have integration test coverage for this too?

Yes

cachi2 requires all dependencies to be pinned to a specific version

A couple of notes from PEP 592:
- "Yanking a file allows authors to effectively delete a file, without
   breaking things for people who have pinned to exactly a specific version"
- "Yanked files are always ignored, unless they are the only file
   that matches a version specifier that 'pins' to an exact version"
- "Regardless of the specific strategy that an installer chooses
   for deciding when to install yanked files, an installer SHOULD emit
   a warning when it does decide to install a yanked file"

Signed-off-by: Michal Šoltis <msoltis@redhat.com>
@eskultety eskultety added this pull request to the merge queue Apr 9, 2024
Merged via the queue into containerbuildsystem:main with commit 67ca8c3 Apr 9, 2024
15 checks passed
@slimreaper35 slimreaper35 deleted the pip branch April 9, 2024 15:36
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.

5 participants