Introduce new suppressMergeCommitDiff parameter in showRevision

[anatoly-spb]

trafficstars

The -m flag breaks polling changes mechanism, for example:

https://issues.jenkins.io/browse/JENKINS-20389 https://issues.jenkins.io/browse/JENKINS-23606 https://issues.jenkins.io/browse/JENKINS-36180 https://issues.jenkins.io/browse/JENKINS-65932

This pull request is the first step to fix above issues. The next step is to call showRevision in https://github.com/jenkinsci/git-plugin/blob/master/src/main/java/hudson/plugins/git/GitSCM.java#L2063 with suppressMergeCommitDiff=true to suppress merge commit diff in change log.

Checklist

• [x] I have read the CONTRIBUTING doc
• [x] I have referenced the Jira issue related to my changes in one or more commit messages
• [ ] I have added tests that verify my changes
• [x] Unit tests pass locally with my changes
• [x] I have added documentation as necessary
• [x] No Javadoc warnings were introduced with my changes
• [x] No spotbugs warnings were introduced with my changes
• [x] I have interactively tested my changes

Types of changes

• [x] Bug fix (non-breaking change which fixes an issue)

[MarkEWaite]

The change breaks compatibility as shown by the automated test that fails. It will need to be a flag that is passed into the API rather than an "always on" change. The default of the flag will need to be "off" so that we don't break compatibility for users.

[anatoly-spb]

This will break compatibility with existing use cases. It needs to be only enabled when a new argument is toggled to enable the new behavior.

Thank you @MarkEWaite. May be you know where is polling change mechanism implemented to call showRevision with additional argument? Who produce the following:

Started on 18.06.2021 21:18:59
Polling SCM changes on master
Using strategy: Default
[poll] Last Built Revision: Revision 28adb19c518b2d32ab79980e8c234465e59f9d04 (refs/remotes/origin/main)
The recommended git tool is: NONE
No credentials specified
 > git rev-parse --resolve-git-dir D:\Jenkins\jobs\jenkins-git-plugin-problem-with-polling\workspace\.git # timeout=10
Fetching changes from the remote Git repositories
 > git config remote.origin.url https://gitlab.com/anatoly.shirokov/jenkins-git-plugin-problem.git # timeout=10
Fetching upstream changes from https://gitlab.com/anatoly.shirokov/jenkins-git-plugin-problem.git
 > git --version # timeout=10
 > git --version # 'git version 2.31.1.windows.1'
 > git fetch --tags --force --progress -- https://gitlab.com/anatoly.shirokov/jenkins-git-plugin-problem.git +refs/heads/*:refs/remotes/origin/* # timeout=10
Polling for changes in
 > git rev-parse "refs/remotes/origin/main^{commit}" # timeout=10
 > git log --full-history --no-abbrev --format=raw -M -m --raw 28adb19c518b2d32ab79980e8c234465e59f9d04..b97c302743fe36e6e596a207397b93af2f0ab425 # timeout=10
Ignored commit b97c302743fe36e6e596a207397b93af2f0ab425: No paths matched included region whitelist
Done. Took 0,95 sec
Changes found 

?

Updated: it seems to me I have found https://github.com/jenkinsci/git-plugin/blob/master/src/main/java/hudson/plugins/git/GitSCM.java#L2063

@MarkEWaite So at this point we will call showRevision with new argument "suppressMergeCommitDiff" = true, which remove -m flag from git call. What do you think about it?

[MarkEWaite]

Yes, that sounds about right. Include tests to confirm the old behavior and the new behavior

[anatoly-spb]

@MarkEWaite are you going to merge this request or what should I do to push this work will be done?

[MarkEWaite]

... Include tests to confirm the old behavior and the new behavior

@anatoly-spb as far as I can tell, you have not created any tests to confirm the new behavior is what you expect. The existing behavior might be covered by existing tests, but the new behavior is not covered by tests. Without tests, it is far too easy for a later change to undo or damage the behavior that you want. Please provide tests

[MarkEWaite]

Closing because tests have not been added to confirm the new behavior works as expected.

[anatoly-spb]

Closing because tests have not been added to confirm the new behavior works as expected.

Could you help me to add such tests?

We have introduced the suppressMergeCommitDiff flag, so the old behavior will not changed, because the suppressMergeCommitDiff flag equals to false for default. If we run current tests they will be passed as expected without any changed. Is it not enough?

As for me this is important flag which must be repaired old age bugs. But I am not familiar with Jenkins Development Environment and I need the help to finish this job.

[murerfel]

Any chances this will get re-opened and re-evaluated? It seems it would fix a problem that has been plaguing us for years now (https://issues.jenkins.io/browse/JENKINS-23606)

[MarkEWaite]

Any chances this will get re-opened and re-evaluated? It seems it would fix a problem that has been plaguing us for years now (https://issues.jenkins.io/browse/JENKINS-23606)

With my current workload, there is no chance that I will reopen it and re-evaluate it. Others are welcome to reopen it and re-evaluate it, but my workload does not allow me to do the additional work needed on this pull request.

@anatoly-spb indicated that new tests have not been added because help is needed to create those new tests. I don't have capacity right now to create those new tests. If others would like to create those new tests and report that they have deployed the incremental build of the plugin into production, that would might persuade me or other maintainers of the plugin to evaluate the changes.

@murerfel are you willing to add the new tests, merge the latest changes from the master branch, deploy an incremental build into your environment, and report the results here?

[murerfel]

Thank you, @MarkEWaite, the quick response is appreciated. Unfortunately, I have neither the required expertise nor the resources for such an endeavor at this point. So we'll just have to be patient for now.