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

e3-pypi-closure: Add yanked management #667

Merged

Conversation

leocardao
Copy link
Contributor

@leocardao leocardao commented Jan 17, 2024

As stated in PEP_592, a yanked package should not be installed unless it is the only one matching a version specifier.

It used to be possible for e3-pypi-closure to choose a yanked package, if it was simply the most recent version of the package.

With this patch, no yanked package will be installed without explicit use of the --allow-yanked option.

Also add minor improvement to e3-pypi-closure --help.

As stated in PEP_592, a yanked package should not be installed unless it
is the only one matching a version specifier.

It used to be possible for e3-pypi-closure to choose a yanked package, if
it was simply the most recent version of the package.

With this patch, no yanked package will be installed without explicit use
of the --allow-yanked option.
@leocardao leocardao self-assigned this Jan 17, 2024
@leocardao leocardao force-pushed the mr/cardao/e3-pypi-closure-manage-yanked branch from 90d76b8 to 7cc70c6 Compare January 17, 2024 17:16
@grouigrokon
Copy link
Contributor

No additional tests ?
Not sure how easy it is to create a yanked package, but I think that simply editing its metadata would do ?

@leocardao
Copy link
Contributor Author

@grouigrokon I don't know if this is feasible, but in any case, the current code doesn't seem to allow it easily.

Currently, in the tests, we generate a setup.py for this, but I haven't found any information to mark a package "yanked" in this file.

So we'd have to create a fake pypi server. If you think it's worth it, I'll do it, but this PR is expected by other teams so I suggest you do it in another PR.

Do you agree?

@grouigrokon
Copy link
Contributor

I agree that it would be a bit of work. Let's keep it for later.
My idea was to:

  • create a wheel
  • unzip
  • update the METADATA file to mark it as yanked
  • rezip it (wheel file are zip files)

@leocardao
Copy link
Contributor Author

I agree that it would be a bit of work. Let's keep it for later. My idea was to:

  • create a wheel
  • unzip
  • update the METADATA file to mark it as yanked
  • rezip it (wheel file are zip files)

Perfect, I'll open the issue and merge this PR. Thank you.

@leocardao leocardao merged commit 24f081f into AdaCore:master Jan 18, 2024
9 checks passed
@leocardao
Copy link
Contributor Author

For the record: @grouigrokon your method doesn't seem to work. I just downloaded the setuptools-scm wheel version 8.0.0, which is a yanked package. There is unfortunately no trace of "yanked", in the metadata. I think it's pypi who says if something is yanked, not the wheels. It is therefore necessary to simulate pypi.

I put the issue with the label good first issue (even if I had planned to do it ) ... I think that ultimately it is not a good issue to enter the code.

@grouigrokon
Copy link
Contributor

Ok, this information seems to be coming from the pypi server :'(

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