gh-action-pypi-publish icon indicating copy to clipboard operation
gh-action-pypi-publish copied to clipboard

Add major version at release

Open s-weigand opened this issue 5 years ago • 19 comments
trafficstars

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 😄

s-weigand avatar Dec 19 '19 00:12 s-weigand

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.

webknjaz avatar Apr 02 '20 23:04 webknjaz

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.

s-weigand avatar Apr 03 '20 04:04 s-weigand

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.)

hugovk avatar Oct 07 '20 09:10 hugovk

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

webknjaz avatar Oct 07 '20 15:10 webknjaz

@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.

pradyunsg avatar Oct 31 '20 13:10 pradyunsg

@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...

webknjaz avatar Nov 01 '20 18:11 webknjaz

@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 😄

s-weigand avatar Nov 01 '20 18:11 s-weigand

@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! ^>^

pradyunsg avatar Nov 01 '20 19:11 pradyunsg

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 avatar Nov 01 '20 19:11 webknjaz

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.

webknjaz avatar Nov 01 '20 19:11 webknjaz

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.

s-weigand avatar Nov 01 '20 20:11 s-weigand

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.

webknjaz avatar Nov 01 '20 20:11 webknjaz

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. 😄

s-weigand avatar Nov 01 '20 20:11 s-weigand

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?

webknjaz avatar Nov 01 '20 20:11 webknjaz

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

s-weigand avatar Nov 01 '20 21:11 s-weigand

pkg_resources.parse_version

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

pradyunsg avatar Nov 01 '20 21:11 pradyunsg

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

webknjaz avatar Nov 01 '20 21:11 webknjaz

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

s-weigand avatar Nov 03 '20 15:11 s-weigand

@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!).

webknjaz avatar Nov 03 '20 21:11 webknjaz