-
Bug
-
Resolution: Fixed
-
Major
-
None
-
Jenkins 1.509.4, Git plugin 2.0, Git client plugin 1.4.6, Debian Linux 7.2 ("Wheezy"), JDK 1.7.0u45 x64
If I use the jgit implementation in a job which should merge from multiple branches, the merge result is not pushed back to origin. Merge result is pushed if I use the command line implementation in the same job.
Steps to duplicate the problem (with apologies for the number of steps)
- Create a bare git repo on the master node as Jenkins user
- rm -rf /tmp/changes.git /tmp/changes
- git init --bare /tmp/changes.git
- echo "#! /bin/sh" > /tmp/changes.git/hooks/post-receive
- echo "curl --silent http://127.0.0.1:8080/git/notifyCommit?url=/tmp/changes.git" >> /tmp/changes.git/hooks/post-receive
- chmod a+x /tmp/changes.git/hooks/post-receive
- cd /tmp
- git clone changes.git
- cd changes
- echo "Git plugin 2.0 no longer shows changes from merges" > README
- git add README
- git commit -m "Add README to describe this repository" README
- git push origin master
- Create a Jenkins merge job using that bare git repo
- Create a new free-style software project "merge-changes"
- Restrict where this project can be run
- Label Expression: master
- Git source code management settings
- Use jgit rather than default
- Repository URL: /tmp/changes.git
- Branches to build: */master*
- Additional behaviors: Merge before build
- Name of repository: origin
- (error on merge if this is blank - help says should use default, but didn't)
- Branch to merge to: master
- Name of repository: origin
- Poll SCM (for convenience with the git hook installed earlier)
- H * * * *
- Add post-build action: Git Publisher
- Git Publisher settings
- Merge results: Yes
- Branches:
- Branches to push: master
- Target remote name: origin
- Git Publisher settings
- Save the job
- Build Jenkins job
- curl --silent http://127.0.0.1:8080/job/merge-changes/build?delay=0sec
- Create new master-add-timestamp branch and commit from it
- git checkout -b master-add-timestamp
- date >> timestamp
- git add timestamp
- git commit -m "Add timestamp" timestamp
- git push origin master-add-timestamp
- Jenkins job runs
- Confirm master branch did not receive the merge result
- git checkout master
- git pull
- git log
- is duplicated by
-
-
- Closed
-
-
-
- Closed
-
[JENKINS-20393] Git plugin 2.0 JGit implementation does not publish merge result
Kristian Kraljic
added a comment - Understood and agreed! 
I'll try to find a semi-elegant way to fix it in JGitAPIImpl.
Thanks a lot for your support so far! I'll post a pull request to git-client-plugin some minutes then.
Kristian Kraljic
added a comment - Understood and agreed! I'll try to find a semi-elegant way to fix it in JGitAPIImpl .
Thanks a lot for your support so far! I'll post a pull request to git-client-plugin some minutes then. Kristian Kraljic
added a comment - Hey Mark, as suggested I have added a fixRefSpec() to JGitAPIImpl, which gets called in the execute method of PushCommand. I have opened a new pull request to git-client-plugin.
fixRefSpec() should fix the "most common" refSpecs for JGit. I decided to simply map "HEAD" to "refs/heads/master" or if "HEAD:branch" is given to "refs/heads/branch". This should do the trick for 90% of the use-cases hopefully. A quick collection of fixes:
: fixed to: :refs/heads/master master fixed to: refs/heads/master:refs/heads/master :master fixed to: :refs/heads/master HEAD fixed to: refs/heads/master:refs/heads/master HEAD:master fixed to: refs/heads/master:refs/heads/master HEAD:branch fixed to: refs/heads/branch:refs/heads/branch master:master fixed to: refs/heads/master:refs/heads/master branch:branch fixed to: refs/heads/branch:refs/heads/branch master:branch fixed to: refs/heads/master:refs/heads/branch :refs/heads/master fixed to: :refs/heads/master refs/heads/master fixed to: refs/heads/master:refs/heads/master refs/heads/master:master fixed to: refs/heads/master:refs/heads/master master:refs/heads/master fixed to: refs/heads/master:refs/heads/master tags/tag fixed to: refs/tags/tag:refs/tags/tag /tags/tag fixed to: refs/tags/tag:refs/tags/tag
I tested the "fixing" with the original version of git-plugin and it worked flawless! So I think is solution is also "quite nice", I guess. Plus not having to fear 60k installations broken, is a releief for my first contribution to the Jenkins community! 
Could you try to run your new test case on it, to see if it's working propertly? Thanks again and have a great weekend.
Kristian Kraljic
added a comment - Hey Mark, as suggested I have added a fixRefSpec() to JGitAPIImpl , which gets called in the execute method of PushCommand . I have opened a new pull request to git-client-plugin.
fixRefSpec() should fix the "most common" refSpecs for JGit. I decided to simply map "HEAD" to "refs/heads/master" or if "HEAD:branch" is given to "refs/heads/branch". This should do the trick for 90% of the use-cases hopefully. A quick collection of fixes:
: fixed to: :refs/heads/master
master fixed to: refs/heads/master:refs/heads/master
:master fixed to: :refs/heads/master
HEAD fixed to: refs/heads/master:refs/heads/master
HEAD:master fixed to: refs/heads/master:refs/heads/master
HEAD:branch fixed to: refs/heads/branch:refs/heads/branch
master:master fixed to: refs/heads/master:refs/heads/master
branch:branch fixed to: refs/heads/branch:refs/heads/branch
master:branch fixed to: refs/heads/master:refs/heads/branch
:refs/heads/master fixed to: :refs/heads/master
refs/heads/master fixed to: refs/heads/master:refs/heads/master
refs/heads/master:master fixed to: refs/heads/master:refs/heads/master
master:refs/heads/master fixed to: refs/heads/master:refs/heads/master
tags/tag fixed to: refs/tags/tag:refs/tags/tag
/tags/tag fixed to: refs/tags/tag:refs/tags/tag
I tested the "fixing" with the original version of git-plugin and it worked flawless! So I think is solution is also "quite nice", I guess. Plus not having to fear 60k installations broken, is a releief for my first contribution to the Jenkins community! Could you try to run your new test case on it, to see if it's working propertly? Thanks again and have a great weekend. I sure can. I think I'll take your table of transformations and create a parameterized unit test to exercise each of those cases.
Kristian Kraljic
added a comment - Please do so, I still havn't set-up my IDE to work with Jenkins. So I couldn't run the changes locally and had to package/upload the plugin to test. Writing and testing the test cases would be very tedious right now. Thanks in advance!
Kristian Kraljic
added a comment - Please do so, I still havn't set-up my IDE to work with Jenkins. So I couldn't run the changes locally and had to package/upload the plugin to test. Writing and testing the test cases would be very tedious right now. Thanks in advance! Kristian Kraljic
added a comment - Just one more thing. The JGitAPIImplTest.test_fetch_needs_preceding_prune test fails at line 1050:
w.git.push("origin", "parent/a");
I assumed that branches may not contain any "/". I fixed the fixRefSpec implementation.
Kristian Kraljic
added a comment - Just one more thing. The JGitAPIImplTest.test_fetch_needs_preceding_prune test fails at line 1050:
w.git.push( "origin" , "parent/a" );
I assumed that branches may not contain any "/". I fixed the fixRefSpec implementation.
An initial idea for the test is evolving as PushTest.java.
It is currently missing tests for branches other than master and for branch names which contain embedded slashes. I'm confident there are many other relevant cases which it is missing. Could you review it and provide your feedback on GitHub?
Kristian Kraljic
added a comment - Done so . Very nice test, I like it! Code changed in jenkins
User: Kristian Kraljic
Path:
src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java
http://jenkins-ci.org/commit/git-client-plugin/c9e96d48062e6a8ed11857f0f00a00c86ec76c98
Log:
[Fix JENKINS-20393] Allow push for JGit repositories
kriskra I'm biased against a change in the refspec in the GitPublisher because that changes the behavior for both CLI git and JGit to disallow use of "refs/heads/" as a prefix to the branch name. I believe there are other places in the plugin where refs/heads/ is allowed as the prefix to a branch name.
My rationale for the change being inside the JGit implementation (even if it is far less elegant than changing GitPublisher) is that the JGit implementation has never been able to push unless a user specifically used refs/heads/master as their branch name. If the change breaks JGit push, it will not be any more broken than it is today. If the GitPublisher change inadvertently breaks a use case for 1% of the 60 000+ installations of the plugin, there will be 600 installations broken.
I'm glad the change in GitPublisher worked for you. That's a good result. Unfortunately, there are so many diverse use cases of the git plugin and the git client plugin that I'm unlikely to allow that type of change in GitPublisher.