pypa / gh-action-pypi-publish

The blessed :octocat: GitHub Action, for publishing your :package: distribution files to PyPI: 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
919 stars 85 forks source link

Add major version at release #22

Open s-weigand opened 4 years ago

s-weigand commented 4 years ago

To make it easier for users to adapt to breaking changes in the action, having major version tags to rely on, instead of master, is a good thing for users and developers. I recently discovered this awesome action which does exactly this, fully automated on relese if you set you workflow up like this. Just wanted to share that info and action, to save you the research.

P.S.: Thanks for this action, it works like a charm 😄

webknjaz commented 4 years ago

fully automated on relese if you set you workflow up like this .

This sounds nice but is a security hazard (explained here why https://github.com/pypa/gh-action-pypi-publish/issues/27#issuecomment-596082795). I'm thinking about how to solve this best but don't have a timeline for completing such setup.

s-weigand commented 4 years ago

I see your point and thanks for sharing the post. But on the other hand, IMHO you have to trust some organizations like github or pypa, that the tools they publish aren't malicious. Actions from pypa aren't just 'random @github actions found on the markedplace' as mentioned in the post. In theory you would have the same security problem when updating twine.

To minimize the risk that a 3rd party action injects malicious code into your release, you could just fork release-github-actions. This way you have the control over what is done on release.

It might still be worth mentioning the security risks and encourage users to use the commit sha, since as the post stated patch version tags are as hazardous and major versions.

hugovk commented 4 years ago

A v1 tag is technically insecure, but so is v1.4.1.

A lack of v1 means people use master, arguably less secure than a tag.

Of course people should use a SHA or fork but they don't...

(The pro tip on the README recommends SHA or tags over master. It should recommend SHA/fork over tags and master.)

webknjaz commented 4 years ago

@hugovk fair enough! I guess I should find time to think through the options and finally solve this request once and for all...

pradyunsg commented 3 years ago

@webknjaz Have you thought through this?

I think it'd be good to have a v1 tag, following the model that actions/ repositories (as well as basically every other popular action that I know of) follow here.

webknjaz commented 3 years ago

@pradyunsg you are right. I think I want it to be introduced along with the detailed explanation of the security considerations. Not sure if I'll be up for it right now — I need to recover from the today's drama. But feel free to remind me in a day or two...

s-weigand commented 3 years ago

@webknjaz If you want I can make a PR with a workflow to generate/automate the major version tags and you can add a detailed explanation of the security considerations. Sharded works means less work for all. Also, good recovery 😄

pradyunsg commented 3 years ago

@s-weigand Please go ahead. I can't promise that @webknjaz would use it as is, but I'm certain that it'd be helpful to have it, even if it only serves as a starting platform! ^>^

webknjaz commented 3 years ago

Thanks, I'm honestly not sure what workflow I want but if you present me with options, I'll appreciate that. I recall that I looked at some automation in the past and didn't like something about it, can't remember what exactly, though.

webknjaz commented 3 years ago

Oh, I remember: haya14busa/action-update-semver that was submitted to another action I have doesn't support PEP440 versions, like .dev0 or .post0 suffixes that I like to use. I still feel that it's important to support it.

s-weigand commented 3 years ago

Oh, I remember: haya14busa/action-update-semver that was submitted to another action I have doesn't support PEP440 versions, like .dev0 or .post0 suffixes that I like to use. I still feel that it's important to support it.

Good to know, I will keep that in mind 😄 I guess .dev shouldn't trigger a major tag, while .post should.

webknjaz commented 3 years ago

Yep. I think JS folks who write these actions don't really know about anything other than semver. I think it should be fairly easy to come up with a workflow that is triggered on tag create events, installs packaging, analyzes the tag with Python and force-pushes a major version tag or bails.

s-weigand commented 3 years ago

Since this is a docker action and we save all the js bundeling stuff, the main points are parsing the version (e.g. using pkg_resources.parse_version and passing the tag as a CLI arg) and force pushing the tags if needed. I will give it a go tomorrow or so. 😄

webknjaz commented 3 years ago

But the pipeline for this specific action does not run it. We're talking about a separate pipeline that would just operate the GitHub repo, not the Action inside of it, right?

s-weigand commented 3 years ago

What I was talking about was just a python script, which decides whether to push minor/major tags, and does it if it should.

pradyunsg commented 3 years ago

pkg_resources.parse_version

Please use packaging.version instead -- https://packaging.pypa.io/en/latest/version/.

webknjaz commented 3 years ago

Yep! That's what I meant when I said install packaging earlier.

@s-weigand since it's supposed to be used within GHA only, I'd just embed that into YAML with shell: python YOLO style instead of adding an extra file :D

s-weigand commented 3 years ago

Sadly YOLO style doesn't work since shell: python is python 2.7 (RIP) and I want my f-strings 🤣

webknjaz commented 3 years ago

@s-weigand that is not entirely correct. shell: python is whatever points to python currently. If you setup-python before that, it can be 3.9 too. Example: https://github.com/webknjaz/intersphinx-untangled/blob/c9223eb/.github/workflows/build-gh-pages.yml#L16-L30. Also: https://github.com/sphinx-contrib/sphinxcontrib-towncrier/blob/413f87d/.github/workflows/tox-tests.yaml#L54 (yes, that is an f-string!).