[JENKINS-40529] Prune tags on fetch

[nfalco79]

JENKINS-40529 - Trait to prune stale tags on fetch

I our infrastructure we use pipeline to perform releases. Releases are performed in Maven, Lerna (NodeJS) and fastlane (IOS). All tools requires that release tag does not in the current git repo. When a releases fails for some reason we fix and re-run the process that will fail again because the tag exists locally. This force the DevOps teams manually remove tags from workspaces in Jenkins slaves. The trait in this PR always prune tags that do not exists on remote.

Checklist

• I have read the CONTRIBUTING doc
• I have referenced the Jira issue related to my changes in one or more commit messages
• I have added tests that verify my changes
• Unit tests pass locally with my changes
• I have added documentation as necessary
• No Javadoc warnings were introduced with my changes
• No spotbugs warnings were introduced with my changes
• I have interactively tested my changes
• Any dependent changes have been merged and published in upstream modules (like git-client-plugin)

Types of changes

• New feature (non-breaking change which adds functionality)

[nfalco79]

I missed a class in the previous commit

[nfalco79]

@MarkEWaite could you review these changes?

[nfalco79]

Rebased against latest commit on master and update javadoc @since from 4.2.0 to 4.2.1

[MarkEWaite]

@nfalco79 it will be a while before I have time to review this pull request. There is a proposal to improve fetch performance significantly on large repositories that needs review before this one.

@rsandell or @fcojfernandez might be able to review it before I can.

[fcojfernandez]

I'm travelling this week, so I will auto-request a review to do it during the next week

[MarkEWaite]

This behavior may be quite difficult to maintain across different versions of command line git. Command line git 2.20 and later intentionally changed their behavior in handling tags that are updated on remote repositories. See JENKINS-55284 for more details on the change.

I realize that the use case here is intended to discard a tag that only exists in the workspace repository and not on the remote. The reviewer of this pull request will need to check the behavior in cases like:

Local tag	Remote tag
exists - same	exists - same
not exists	not exists
exists	not exists
not exists	exists
exists - differ	exists - differ

Some of those cases may be irrelevant. Others are likely relevant and should be expressed in automated tests so that they will be evaluated on the different versions of CLI git that are used in git plugin automated testing.

[nfalco79]

@MarkEWaite I know this command is valid only for git command line version >= 1.7.8. Should I check if the git command is a GitCliCommand implementation and get it's version. For JGit should not be an issue, I do not know any different behaviour.

[nfalco79]

I will perform a manual test for git version >= 2.20.1 is requires a --force parameter on fetch or the prune parameters already acts like force

[MarkEWaite]

I know this command is valid only for git command line version >= 1.7.8. Should I check if the git command is a GitCliCommand implementation and get it's version. For JGit should not be an issue, I do not know any different behaviour.

If you know that it is valid for command line git versions 1.7.8 and newer, then there is no official need to check the version because the Jenkins git plugin officially supports git 1.7.10 and newer. I still run tests on CentOS 6 with its command line git version that is older than 1.7.10, but those tests are run only as a "best effort" to not break CentOS 6 git any worse than it is already broken.

If I find surprises when I run this on CentOS 6, then I'll either add the version check or exclude the tests from running on CentOS 6. The command line git version on CentOS 6 is ancient and there are many alternatives for CentOS 6 users to install a more modern version of command line git.

The command line git version on CentOS 7 (1.8.3) also has its share of issues, but is still officially supported by the git plugin. The day will come when that old CLI git version will no longer be supported, but not yet.

[nfalco79]

I got the information on a stackoverflow response.
Starting from 2.17 version the --prune-tags option also do the work, it's an alias for refs/tags/*:refs/tags/*. The command I had choose is much compatible with more versions. From (I do not know the version) I could use the git config fetch.pruneTags true to make this automatic on fetch.

Which way do you suggest?

[MarkEWaite]

Needs a test of command line git, not just JGit implementation. Otherwise, the git platform configurations won't help us detect problems.

[MarkEWaite]

I got the information on a stackoverflow response.
Starting from 2.17 version the --prune-tags option also do the work, it's an alias for refs/tags/*:refs/tags/*. The command I had choose is much compatible with more versions. From (I do not know the version) I could use the git config fetch.pruneTags true to make this automatic on fetch.

Which way do you suggest?

I prefer the implementation you used. It is compatible with all the command line git versions supported by the git plugin (1.7.10 and newer).

[MarkEWaite]

Thanks very much for adding the broader tests and for testing with command line git!

I'm traveling currently and unable to spend the time in the IDE that will be needed to adequately review the latest changes. I'll rely on a review from @fcojfernandez or will review it after I return from traveling.

Thanks again for your effort on the pull request!

[nfalco79]

If you noted I had change the trait implementation because I did suppose that git fetch --prune.. also download new tags. Since could be a lot of remotes (for example in case of PR with forks) download tags from all remotes may heavily affect the performance.
New implementation use ls-remote and than locally remove tags that do not exist in any remote. This is the faster way.

I miss to fix the constructor suggest because is not clear to me if you would have two contructors, one annotated DataBoundConstructor and other one that takes a boolean to skip any logic.

[fcojfernandez]

thanks @nfalco79 ! Overall looks well, but I'd like to have more context/details, so I've added some questions in order to feel more confortable with my review

[nfalco79]

Comments fixed.
More details provided.

[nfalco79]

rebase on master

[fcojfernandez]

Sorry for the delay. These days have been a bit craazy. The code looks good to me. @MarkEWaite , @nfalco79 has included the tests you requested. Do you want to review them in depth or are you fine with me merging this PR?

[MarkEWaite]

Approving to unblock the merge

[MarkEWaite]

@fcojfernandez since you reviewed the code in detail, I'd love to have you merge the code .

[nfalco79]

Than I can proceed with a squash commit

[fcojfernandez]

The CI has failed after the last commit. Closing and re-opening to relaunch it (the failure seems to be infra related)

[MarkEWaite]

@nfalco79 thanks for the pull request and @fcojfernandez thanks for merging it.

I was performing some interactive testing today to prepare for a beta release of git plugin 4.3.0 As part of that, I discovered JENKINS-61869.

I think that will need to be fixed before we allow this feature to reach users.

I'll report additional bugs as I learn more from my interactive testing.

If you'd like a Docker image definition that includes a Freestyle job that I was using for experiments, refer to the lts-with-plugin branch of my docker-lfs repository.

[MarkEWaite]

I've also submitted JENKINS-61871 to describe the cases I'm seeing where stale tags are not pruned by fetch into the workspace.

[nfalco79]

it's the same bug. The suggested property in code review to enable/disable the extensions must be true by default. That option causes all these issues.

[MarkEWaite]

it's the same bug. The suggested property in code review to enable/disable the extensions must be true by default. That option causes all these issues.

That's good to hear. Will you submit a pull request to resolve it?

Will the change then alter the default behavior by enabling tag pruning by default? I hope not, since that is not the way the git plugin handles compatibility in general. We try to retain behavioral compatibility so that users are not surprised by changes in default behavior.

[nfalco79]

Already done #867