My favorites | Sign in
Project Home Downloads Wiki Issues Source
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 635: Direct push to a dev branch causes a pending review change to become submitted in master
14 people starred this issue and may be notified of changes. Back to list
Status:  Released
Owner:  ----
Closed:  May 2011


Sign in to add a comment
 
Reported by jjhel...@gmail.com, Jul 26, 2010
Affected Version: 2.1.2.5

What steps will reproduce the problem?
1. Create a change in the "master" branch

git co master
echo "foo" >> readme.txt
git commit -am "test gerrit"
# Git commit runs the commit-msg hook and inserts the appropriate Change-Id line into the commit msg
git push HEAD:refs/for/master

2. Switch to a dev branch dev/test and cherry pick the above change (along with its Change-Id line)

git co -b dev/test
git cherry-pick c0f119b143a9f47875b4ee504e2b4bc6c9021860 # this is the above change that was pushed for review)

3. Push the change directly to dev/test branch 

  git push origin HEAD:dev/test

What is the expected output? What do you see instead?

I would expect "master" branch to remain unaffected.  Instead, what happens is that Gerrit claims the change pending in Gerrit to be merged into dev/test "Change has been successfully pushed into branch dev/test".

I wouldn't expect the original change in "master" to be affected by any changes going into another branch.

I suppose this happens because the Change-Id is the same in both master and the dev branch.  I think this causes a lot of confusion among users as they'd expect to be able to cherry pick changes from one branch to another without worrying about hand-editing the Change-Id line.

This has been discussed earlier on the repo discuss mailing list.  I think pushes to branch 'a' shouldn't affect branch 'b' even if the Change-Id is the same.  I suppose the Change-Id would need to be a 2-tuple (branch_name,sha1).

Aug 2, 2010
#1 mhanw...@gmail.com
This is something we have encountered with multiple integration branches too. If a commit is for master, but merged into next, I would not expect the proposed change to be marked as merged until it is merged into master. We use merge commits rather than cherry picking, and so the SHA-1s match too, and so this happens even without the Change-Id lines.
Aug 2, 2010
#2 nas...@chromium.org
I saw this the other day on our 2.1.3 server and it caused quite a headache (especially since it added the new commit as a patchset onto the existing change).
Labels: -Priority-Minor Priority-Major
Aug 2, 2010
Project Member #3 bklarson@gmail.com
We've seen this on occasion as well.  We don't use Change-Id lines... I wonder if uploading a commit to review on one branch and then direct-pushing the same commit to a separate branch might also cause this problem?
Aug 2, 2010
#4 mhanw...@gmail.com
I can confirm that "uploading a commit to review on one branch and then direct-pushing the same commit to a separate branch" does cause this problem.
Aug 20, 2010
#5 maria.gu...@gmail.com
We also see this happen when we push changes which have a Change-Id line from another server, like kernel.org, up to our gerrit server.

In addition to closing some changes which weren't actually merged, this causes folks to get very confusing emails where the commit message and commit contents are from the kernel change which was pushed but things like change owner match their gerrit change on a totally different repository on the server.
Apr 20, 2011
#6 jaanek...@gmail.com
Is there any way to overcome this problem with current versions of Gerrit? A known solution for example? We use 2.1.6.1 version of Gerrit.
It does cause a lot of problems for us also.
Apr 21, 2011
#7 darksk...@gmail.com
I've been forced into setting up a mirror repository and keep dev branches out of gerrit.

Apr 21, 2011
#8 jaanek...@gmail.com
This thread http://goo.gl/gXhjG mentions that it might not be hard to patch the Gerrit to accept same Change-Ids for different branches. As I'm not (yet) taken a look into the source code myself then I'm not sure thought that is so.
Apr 21, 2011
#9 jaanek...@gmail.com
Did you set up your mirror using the approach described in this link: http://goo.gl/g8vZZ ?
May 16, 2011
#10 sop@google.com
(No comment was entered for this change.)
Status: Submitted
Labels: FixedIn-2.1.7
May 31, 2011
#11 sop@google.com
(No comment was entered for this change.)
Status: Released
Sign in to add a comment

Powered by Google Project Hosting