(WIP) [JENKINS-20389] Merge commits trigger uneccessary builds when Include/Exclude regions are defined

[aelsabbahy]

Added a new method showChangedPaths.

This implementation uses git diff --name-only old...new instead of git log to identify changed paths more accurately.

Here's a good explanation on the difference between how git log and git diff work: http://stackoverflow.com/a/7256391

I'm hoping this method can be leveraged by the Jenkins git-plugin to filter included/excluded paths which would resolve:

• JENKINS-20389
• JENKINS-23606

Note: Originally, I was going to implement this using git log --first-parent, but @nwholloway pointed out that using --first-parent would cause issues when a user merges code into into a topic branch and pushes the results to master.

Note2: I don't develop in Java, so these changes probably need another set of eyes before they're merged.

[jenkinsadmin]

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

[bogdan-litescu]

When can I expect this change to be merged and available as updates?
Thanks

[MarkEWaite]

It can be as much as several months before a git client plugin change proposal is merged and released, particularly one that changes a fundamental technique like this one does.

If you want to run it sooner, you can fork a copy of the git client plugin repo, pull this change into it, and deploy it in your environment.

[MarkEWaite]

I've evaluated this pull request and provided a test which shows several areas where the JGit and command line git behaviors are different, and where the new API behaves unexpectedly.

Please refer to [https://github.com/MarkEWaite/git-client-plugin/commit/19874b752bf01149ba8e57b4926e929ada1d4c13] for the test source code.

Can you resolve those issues and the pull request can be evaluated again?

[pabloa]

Any news with this pull request? Do you have an ETA?

[MarkEWaite]

@pabloa Until the author of the pull request resolves the issues where the new API behaves differently between JGit and command line git, and until the author of the pull request resolves the places where the API behaves unexpectedly, this pull request won't be merged.

If you'd like to speed that process, you're welcome to resolve those issues and make those consistency changes. I felt that I already went far beyond the required by spending the time to write tests and use those tests to show issues with the proposed API.

Have you evaluated the pull request and found that it meets your needs? If so, can you describe those needs so that I understand better and can consider if I need to be the one to fix the problems in the proposed pull request (at the expense of other bug fixes and helping users).

[aelsabbahy]

I would like to apologize to everyone here for my extreme delay in responding. It was not my intent to abandon this pull request after creating it.

@MarkEWaite I've fixed some, but not all, of the inconsistencies you've pointed out.

What's fixed:
CLI:

• Duplicate entries
• Inconsistencies between first commit and others

JGit:

• Inverted arguments returning wrong response

What's not fixed:

• null pointer exception when second argument is null. I did not mean to allow the 2nd argument to be null in the Javadoc, any suggestions on what changes to make to the Javadoc to indicate otherwise?
• octopus merge, I'm kind of stuck on this one. RevCommit#getParents() is only returning two parents rather than all the parents involved in the octopus merge. I'm not sure how to correctly do this in JGit.

Also, I'm using mergeBase() but it's marked as deprecated, should I just remove the @Deprecated annotation?

[thorseye]

@aelsabbahy Do you know why this was never merged?

[aelsabbahy]

I think it was the octopus merge case. I couldn't figure out how to do it in JGit, not sure how the owner would like to proceed.

[jjh999]

Any update with this pull request?

[LuboVarga]

I have probably triggered probably issue JENKINS-38722 which this pull request solves. Imho semantically is git diff better for given purpose than git log. I would like to build job, when there is difference from last build and not if there were some changes, which were than removed and made thus "invisible" to building project. It would break only projects which somehow uses git history in build (for example adding changelogs or so). From my point of view it would be best if user can choose what change in repository means. If any change (also invisible ones) or change in actual codebase from last build (i.e. diff).

[antonsmyk]

I tried out this PR merging (resoving some trivial conflicts) this pull request in git-client-2.1.0 and using the patched plugin version. Alas, the problem is still there...

[antonsmyk]

In Git polling log I see that still git-log is used. Is there anything to activate in the plugin for the new approach (git-diff) to take place?

[neizmirasego]

I'm also affected with this issue. So +1 from me to speed up the fix

[MarkEWaite]

@neizmirasego, you're encouraged to checkout your own copy of the plugin and evaluate the pull request in your use case. @antonsmyk did that and found that it did not resolve his issue.

I am unlikely to work on this pull request for at least several months so that I can work on authentication related issues and submodule related issues which I believe are higher priority.

[fairmonk]

Hello to everyone and thanks for handling requests from large audience! I also have the same issue ( a merge will trigger build even if "Included Regions" doesn't contain the "touched path" ). This is very painful for me as I just moved from SVN to GitHub and it use to work in SVN. I need to make it work somehow in Git too.
I checked out code and have java 8 + mvn 3.3.9 installed on my workstation.
How is it possible to build a custom git-client plugin ( with changes on this Pull Request included; I want to give it a try, since I really need the fix ) and then upload it to jenkins master on top of existing git-plugin v 2.7.2 ?

[MarkEWaite]

@fairmonk if you want to experiment with this pull request, you will need to resolve the reported conflicts, compile a local version of the git client plugin, then adapt a private copy of the git plugin to use the new showChangedPath() API's added by this pull request.

I've attempted the conflict resolution in #326 if you want to use that as a starting point. When I executed the tests in that conflict resolution case, there were several failures in merge related areas. Since this pull request is making changes in merge related areas, test failures are a cause for concern.

Refer to my comments in #326 that this pull request will also require changes in the git plugin before you'll see a different behavior.

[fairmonk]

@MarkEWaite Thank you Mark. Meanwhile I opted for using excluded regions ( as a workaround ). The excluded regions doesn't seem to have this problem with merge commits from Pull Requests. Of course specifying "everything else" inside excluded regions requires some extra typing ( but since I generate my jobs via DSL plugin and the repository is not changing very often [ in terms of adding new paths ] - it was a matter of adding several if-else statements inside my job template. )

After thorough testing, my workaround is not valid. Merging Pull Requests still triggers build ( even if excluded regions are used and changes fall into excluded regions ). The Excluded/Included regions work fine only with normal commits for me.