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

Show hash values of files to be uploaded #87

Merged
merged 11 commits into from Jan 9, 2022

Conversation

dukecat0
Copy link
Member

@dukecat0 dukecat0 commented Jan 7, 2022

twine-upload.sh Outdated Show resolved Hide resolved
@dukecat0 dukecat0 marked this pull request as draft January 8, 2022 04:08
@dukecat0 dukecat0 marked this pull request as ready for review January 8, 2022 04:28
@dukecat0 dukecat0 requested a review from webknjaz January 8, 2022 04:28
@dukecat0 dukecat0 changed the title Show hash values of files uploaded Show hash values of files to be uploaded Jan 8, 2022
@dukecat0
Copy link
Member Author

dukecat0 commented Jan 8, 2022

@webknjaz It should work properly now.

twine-upload.sh Outdated Show resolved Hide resolved
twine-upload.sh Outdated Show resolved Hide resolved
print-hash.py Outdated Show resolved Hide resolved
print-hash.py Outdated Show resolved Hide resolved
print-hash.py Outdated Show resolved Hide resolved
print-hash.py Outdated Show resolved Hide resolved
print-hash.py Outdated Show resolved Hide resolved
print-hash.py Outdated Show resolved Hide resolved
Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

@meowmeowmeowcat thanks! I've applied some refactoring through the web UI without testing it — could you verify that it doesn't error out before I merge this?

@webknjaz
Copy link
Member

webknjaz commented Jan 8, 2022

Preview: meowmeowmeowcat/test-repo/runs/4746294453?check_suite_focus=true#step:5:16

I noticed you use the main PyPI in tests. It's recommended to use the test instance (TestPyPI) instead: https://test.pypi.org. FYI.

@dukecat0
Copy link
Member Author

dukecat0 commented Jan 9, 2022

could you verify that it doesn't error out before I merge this?

It doesn't error out and generates the correct hash values. 👍

Final test for it: https://github.com/meowmeowmeowcat/test-repo/runs/4752022357?check_suite_focus=true#step:6:1

EDIT: I've fixed the bug below.

However, the hash values generated by the script are very strange.

When I use Github Action to generate the tar.gz and wheel files by running python -m build, the hash values of the wheel file are not the same as the one on PyPI.

GHA link: https://github.com/meowmeowmeowcat/test-repo/runs/4751716715?check_suite_focus=true#step:6:22
PyPI link: https://pypi.org/project/my-test-repo/0.0.16/#files

If I generate the tar.gz and wheel files locally by running the same command as above and commit them, the hash values of the tar.gz file are not the same as the one on PyPI.

GHA link: https://github.com/meowmeowmeowcat/test-repo/runs/4751560961?check_suite_focus=true#step:5:15
PyPI link: https://pypi.org/project/my-test-repo/0.0.13/#files

@webknjaz webknjaz added the enhancement New feature or request label Jan 9, 2022
@webknjaz webknjaz merged commit 4992a00 into pypa:master Jan 9, 2022
@@ -30,6 +30,10 @@ inputs:
description: Show verbose output.
required: false
default: false
print_hash:
Copy link
Member

Choose a reason for hiding this comment

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

I just realized that maybe it's better to make this plural. But it's too late now so I'll keep it as is.

Copy link
Member

Choose a reason for hiding this comment

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

This would be a breaking change of the public API. We'd have to wait until v2 or even v3 depending on how we decide to do deprecation. There are other things, I'd change like using kebab-case for the inputs but not today.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, and I just realized that.

Copy link

Choose a reason for hiding this comment

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

You may add a print_hashes, and treat the current name as an undocumented alias.

@webknjaz
Copy link
Member

webknjaz commented Jan 9, 2022

Released https://github.com/pypa/gh-action-pypi-publish/releases/tag/v1.5.0

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

Successfully merging this pull request may close these issues.

Print hash values of uploaded files
2 participants