pypa / gh-action-pypi-publish

The blessed :octocat: GitHub Action, for publishing your :package: distribution files to PyPI, the tokenless way: https://github.com/marketplace/actions/pypi-publish
https://packaging.python.org/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows/
BSD 3-Clause "New" or "Revised" License
929 stars 87 forks source link

build(deps): bump `pkginfo` version to support `Metadata-version=2.3` #219

Closed SigureMo closed 8 months ago

SigureMo commented 8 months ago

pkginfo released a new version (1.10.0) to support Metadata-version=2.3 (see https://pypi.org/project/pkginfo/ and https://github.com/pypa/twine/issues/1059). So I re-generate the requirements by run pip-compile.

webknjaz commented 8 months ago

What command did you use? If you want to update one package, use -P pkginfo to have smaller diff. Additionally, the same Python version must be used (3.11). And it looks like the pip-tools version was < 7 to generate the original file. I'd rather have smaller controlled updates of each of these instead of a huge bump of everything, to be on the safe side.

SigureMo commented 8 months ago

What command did you use? If you want to update one package, use -P pkginfo to have smaller diff. Additionally, the same Python version must be used (3.11). And it looks like the pip-tools version was < 7 to generate the original file. I'd rather have smaller controlled updates of each of these instead of a huge bump of everything, to be on the safe side.

Thanks for the suggestion, I've re-generated the requirements using pip-tools<7, python 3.11.

❯ pip-compile --version            
pip-compile, version 6.14.0

❯ pip-compile --allow-unsafe --output-file=requirements/runtime.txt --resolver=backtracking --strip-extras requirements/runtime.in -P pkginfo
#
# This file is autogenerated by pip-compile with Python 3.11
# by the following command:
#
#    pip-compile --allow-unsafe --output-file=requirements/runtime.txt --resolver=backtracking --strip-extras requirements/runtime.in
#
annotated-types==0.5.0
    # via pydantic
bleach==6.0.0
    # via readme-renderer
certifi==2023.7.22
    # via requests
charset-normalizer==3.2.0
    # via requests
docutils==0.20.1
    # via readme-renderer
id==1.0.0
    # via -r requirements/runtime.in
idna==3.4
    # via requests
importlib-metadata==6.8.0
    # via
    #   keyring
    #   twine
jaraco-classes==3.3.0
    # via keyring
keyring==24.2.0
    # via twine
markdown-it-py==3.0.0
    # via rich
mdurl==0.1.2
    # via markdown-it-py
more-itertools==9.1.0
    # via jaraco-classes
pkginfo==1.10.0
    # via twine
pydantic==2.0.2
    # via id
pydantic-core==2.1.2
    # via pydantic
pygments==2.15.1
    # via
    #   readme-renderer
    #   rich
readme-renderer==40.0
    # via twine
requests==2.31.0
    # via
    #   -r requirements/runtime.in
    #   id
    #   requests-toolbelt
    #   twine
requests-toolbelt==1.0.0
    # via twine
rfc3986==2.0.0
    # via twine
rich==13.4.2
    # via twine
six==1.16.0
    # via bleach
twine==4.0.2
    # via -r requirements/runtime.in
typing-extensions==4.7.1
    # via
    #   pydantic
    #   pydantic-core
urllib3==2.0.7
    # via
    #   requests
    #   twine
webencodings==0.5.1
    # via bleach
zipp==3.16.0
    # via importlib-metadata
webknjaz commented 8 months ago

Looks like your command is executed from the repo root while dependabot changes dir to requirements. This should also be reproduced to avoid pointless changes in comments, back and forth.

webknjaz commented 8 months ago

🤔 I wonder why a whole bunch of crypto-related things is getting removed..

SigureMo commented 8 months ago

🤔 I wonder why a whole bunch of crypto-related things is getting removed..

Oh, my mistake, I was running it on mac before, after re-running it on Linux they came back.

webknjaz commented 8 months ago

Did you instruct it to update one specific dep or all? I believe it shouldn't have touched others, looking at where they come from.. If we want to bump several deps at a time, that should probably be reflected in the title.

SigureMo commented 8 months ago

I thought I only needed to update pkginfo. To avoid a possible dependency conflict due to manual updating, I ran the command to update it, and what's strange is that it updates several other deps at the same time. I've reverted the other changes and just updated pkginfo manually.

webknjaz commented 8 months ago

Okay, thanks. I think there's another CLI option for updating everything. I figured maybe you ended up using it somehow..

FWIW updating the entire tree is useful from time to time. But I'm not chasing the latest versions when what we have works. I normally accept what dependabot offers. But if you want to, feel free to send bumps for other bits separately. Additionally, we'll need to switch to Python 3.12 at some point, although it shouldn't change anything for us really. And maybe, starting to use newer pip-tools would be a good idea. With its new config file support, especially. But each of these should go in separately, for transparency. Another good idea, if you're looking to contribute would be making a pair of in+txt files for pip-tools itself so that it's documented what the pins are made with. And that could be wrapped with tox or nox as a follow-up.

webknjaz commented 8 months ago

@alex I noticed your approval — are you waiting for a new release or is there no rush?

alex commented 8 months ago

I don't use this directly, but it's a blocker for something else I forget :-)

On Wed, Mar 6, 2024, 1:17 PM Sviatoslav Sydorenko (Святослав Сидоренко) < @.***> wrote:

@alex https://github.com/alex I noticed your approval — are you waiting for a new release or is there no rush?

— Reply to this email directly, view it on GitHub https://github.com/pypa/gh-action-pypi-publish/pull/219#issuecomment-1981514129, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAGBHO6J3DGSL7YB2E7W3YW5MVJAVCNFSM6AAAAABEGNY6H6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBRGUYTIMJSHE . You are receiving this because you were mentioned.Message ID: @.***>

stinodego commented 8 months ago

It seems like this issue impacts our release process. The metadata check fails, while the wheels are actually correct (I downloaded the wheels to make sure, twine check passes locally.

Could a patch release be issued that includes this fix?

EDIT: We pinned the action to this commit and our workflow is saved. So thanks for the fix! Since others are likely also impacted, a new release is probably still a good idea.

webknjaz commented 8 months ago

Ah, I see it's also backreferenced from uv. @charliermarsh you should've just dropped a note so I'd notice faster that it needs to be prioritized... Regular dependency pins are not significant and can live in the repo unreleased for months.

charliermarsh commented 8 months ago

Thanks @webknjaz, I appreciate it! I didn't want to bother you since we had a workaround but grateful for the fast release.

webknjaz commented 8 months ago

@charliermarsh thanks for being considerate! I just needed to know that this actually affects somebody. Otherwise, the perception is that this is something insignificant and can be postponed indefinitely. The PR description wasn't explicit about the motivation and I initially looked at the PR from phone, not following external references so I didn't have that context.

webknjaz commented 8 months ago

FTR I ended up making two separate runtime-related bumps (just in case):