ad-m / github-push-action

GitHub actions to push back to repository eg. updated code
MIT License
1.21k stars 230 forks source link

Add atomic push #138

Closed sleepypikachu closed 2 years ago

sleepypikachu commented 2 years ago

Since time elapses between the checkout the github workflow performs and the eventual push this action invokes the remote HEAD may have changed. If this is the case the HEAD update will be rejected but any tag (and their commits) will be pushed.

In general I think this operation should be atomic, either we push everything or we push nothing.

Force pushes still work the way you would expect (i.e. if we force the HEAD update with --atomic everything is still pushed)

This also protects from the situation where someone else has seized your tag name in the meantime but not updated your HEAD.

See git docs for more information - https://git-scm.com/docs/git-push#Documentation/git-push.txt---no-atomic

ZPascal commented 2 years ago

Hi @sleepypikachu, in general it looks good to me. Do you've tested it?

sleepypikachu commented 2 years ago

Yes, it is working well for us. We're using this behaviour from my fork :-)

ZPascal commented 2 years ago

Hi @sleepypikachu, thanks for the update. I'm currently thinking about the case, if the server side do not support the function, will the complete call fail or only the beforehand executed pull? Maybe we're on a safer side to handle it with an automatic enabled default parameter, but with opportunity to disable it.

sleepypikachu commented 2 years ago

Hmmm. This is a github action and github does support this feature. So it's hard to imagine what would cause someone to unset this flag.

If you feel strongly I'm happy to implement. Just want to understand the motivation.

ZPascal commented 2 years ago

I'm thinking of a case for GitHub Enterprise support. I'm not sure if there's a possibility to disable the function inside a GTE system.

sleepypikachu commented 2 years ago

Gotcha, no problem. Will implement.

ZPascal commented 2 years ago

@sleepypikachu Thanks for the contribution! I'll merge the PR.