Skip to content

Fixing issue that tags are removed before they can be pushed#45

Merged
CasperWA merged 1 commit intoCasperWA:masterfrom
THuppke:master
Aug 4, 2021
Merged

Fixing issue that tags are removed before they can be pushed#45
CasperWA merged 1 commit intoCasperWA:masterfrom
THuppke:master

Conversation

@THuppke
Copy link
Contributor

@THuppke THuppke commented Jul 30, 2021

I ran into the issue that I created a new tag in my Github Action and when I want to push it (by specifying the tags: true) entry for your action, the line in entrypoint.sh that I edited in this PR fetches from origin and prunes all local tags that don't exist online as well. This kinda makes the whole option tags obsolete, as that seems to be overridden by this line.

If you find a nicer solution to actually be able to still push newly created tags, that's also fine, maybe by making this conditional on whether the tags flag was set or not, but this solution worked for me.

@CasperWA
Copy link
Owner

CasperWA commented Aug 4, 2021

I ran into the issue that I created a new tag in my Github Action and when I want to push it (by specifying the tags: true) entry for your action, the line in entrypoint.sh that I edited in this PR fetches from origin and prunes all local tags that don't exist online as well. (...)

Neat - and indeed, the -P option prunes all local tags not present on the remote.

(...) This kinda makes the whole option tags obsolete, as that seems to be overridden by this line.

I don't think this is the case, but I might definitely be wrong. The point of the tags option is to do an extra git push --tags command, explicitly pushing the tags, which I'm not sure would otherwise be pushed. However, if you have a way of testing this that would be amazing! I'll create another PR using your commit here, so that the CI works (my GH secrets needs to be used for the CI to work).

Edit: Looking into a bit of Git documentation (see "Sharing tags"), it seems the newly created tag shouldn't be pushed along with the commit, only if it's either explicitly pushed or --tags is used will it be pushed. But this should be tested, I think.

If you find a nicer solution to actually be able to still push newly created tags, that's also fine, maybe by making this conditional on whether the tags flag was set or not, but this solution worked for me.

The -P option needs to be removed regardless if you want to achieve your goal here and push newly created tags - so fine by me with this fix 👍

@CasperWA CasperWA mentioned this pull request Aug 4, 2021
@THuppke
Copy link
Contributor Author

THuppke commented Aug 4, 2021

(...) This kinda makes the whole option tags obsolete, as that seems to be overridden by this line.

I don't think this is the case, but I might definitely be wrong. The point of the tags option is to do an extra git push --tags command, explicitly pushing the tags, which I'm not sure would otherwise be pushed. However, if you have a way of testing this that would be amazing! I'll create another PR using your commit here, so that the CI works (my GH secrets needs to be used for the CI to work).

Oh sorry, maybe I was a bit unclear with my previous statement. I only meant that the tags option wouldn't push the newly created tags as long as the -P option was still in there. Other than that, the tags option is of course far from obsolete and makes perfect sense.

Edit: Looking into a bit of Git documentation (see "Sharing tags"), it seems the newly created tag shouldn't be pushed along with the commit, only if it's either explicitly pushed or --tags is used will it be pushed. But this should be tested, I think.

Yeah, I'm quite sure it works that way.

The -P option needs to be removed regardless if you want to achieve your goal here and push newly created tags - so fine by me with this fix +1

Nice, thanks for taking the time to get that included! 👍

@CasperWA CasperWA merged commit a98c952 into CasperWA:master Aug 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants