CandyShop / gerrit

Automatically exported from code.google.com/p/gerrit
Apache License 2.0
1 stars 0 forks source link

auto closing change after pushing to any branch #1142

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Affected Version: 2.2.1

What steps will reproduce the problem?

After submit for review, i push the commit to a sandbox branch for
backup. This causes the change closes unexpectedly and i can't submit
for another iteration of review.

Can gerrit be more clever? For example, it closes the change only when
the code is pushed to the corresponding review branch.

Following is my workflow, any suggestion?

1. commit and submit for review (for master)

$ git commit -a -m "Test change close"
$ git push origin HEAD:refs/for/master
...
remote: New Changes:
remote:   http://bear/gerrit/996
...

2. backup code to server (causing change closed)

$ git push origin HEAD:sandbox/backup

3. amend the change and can't submit review again

# make some changes
$ git commit --amend -C HEAD
$ git push origin HEAD:refs/for/master
...
! [remote rejected] HEAD -> refs/for/master (change 996 closed)

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

The changes shouldn't be closed if pushing to a non-target branch.

This is the same with issue 635. 635 was fixed in 2.1.7, but it seems that this 
issue reopens in 2.2.1

Original issue reported on code.google.com by pkufra...@gmail.com on 28 Sep 2011 at 5:45

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Hi Ping Yin
I want confirm with you about this issue in detail.
I triggered this issue today by below steps. will please review my comments in 
it?
if my understanding is right. I will begin trying to fix it.

 2548  ssh  bruce.zu@localhost  -p 29418  gerrit create-project   --name  project-test  --empty-commit
 2549  git clone ssh://bruce.zu@localhost:29418/project-test
 2550  cd project-test/
 2551  echo "a" > f.txt
 2552  git add f.txt
 2553  git commit  -m "a------base commit on the empty commit "
 2554  git checkout -b local-branch
 2555  git push ssh://bruce.zu@localhost:29418/project-test HEAD:refs/heads/master
 2556  git push ssh://bruce.zu@localhost:29418/project-test HEAD:refs/heads/sandbox/backup
      [comments:
       above steps are about background. am I right?
       now begin test as your steps. 
      ]
 2557  vi f.txt
 2558  git commit -a -m "Test change close"
 2559  git push origin HEAD:refs/for/master
 2560  git push origin HEAD:sandbox/backup
      [comments:
       here I watch the Gerrit UI page and find the change is in "merged" status.
       so I think here is the place where this issue happens. am I right?
      ]
 2561  vi f.txt
 2562  git commit --amend -C HEAD
 2563  git push origin HEAD:refs/for/master
      [comments:
       no error here, this command create a new change.
      ]
 2564  git push origin HEAD:refs/changes/master
      [comments:
       find error here. as the this issue has happened and the change has been in 'merged' status .
      ]
 2565  history

Original comment by zu.bruce.china@gmail.com on 28 Oct 2011 at 11:21

GoogleCodeExporter commented 9 years ago
Hi zu.bruce,

I have the same problem. Here's my review for your steps:

> 2559  git push origin HEAD:refs/for/master
(Gerrit create a new change for this new commit)

> 2556  git push ssh://bruce.zu@localhost:29418/project-test 
HEAD:refs/heads/sandbox/backup
>      [comments:
>       above steps are about background. am I right?
>       now begin test as your steps. 
>      ]
(when you push that commit into repo directly, the corresponding change will be 
closed accidentally, and that's the bug)

Original comment by leem...@gmail.com on 28 Oct 2011 at 5:09

GoogleCodeExporter commented 9 years ago
hi, zu.bruce, leem...

>> 2556  git push ssh://bruce.zu@localhost:29418/project-test 
HEAD:refs/heads/sandbox/backup
>>      [comments:
>>       above steps are about background. am I right?
>>       now begin test as your steps. 
>>      ]
> (when you push that commit into repo directly, the corresponding change will 
be closed accidentally, and that's the bug)

Yes, that's the bug.

The triggering steps are right.

Original comment by pkufra...@gmail.com on 29 Oct 2011 at 2:33

GoogleCodeExporter commented 9 years ago
Hi, Leem
 in your feedback, you use line 2556 to replace line 2560 in my steps.
 careless error or intentional? 
 make me a little confused.

 if I am not wrong, the expected result is:
 after line 2560  "git push origin HEAD:sandbox/backup"
 on the Gerrit UI page the change(on master branch in my case) should not be changed to "merged" status/ your words "be closed".

Original comment by zu.bruce.china@gmail.com on 30 Oct 2011 at 10:44

GoogleCodeExporter commented 9 years ago
Hi zu.bruce,

it's my careless mistake. I wanted to use the 2555 line instead of 2559 line. 
in fact, the bug should have been  triggered after 2556 line.

Original comment by leem...@gmail.com on 30 Oct 2011 at 11:09

GoogleCodeExporter commented 9 years ago
BTW, the W/A is to recreate the commits with git rebase

Original comment by egrumbach on 6 Nov 2011 at 7:52

GoogleCodeExporter commented 9 years ago
Hi Egrumbach
what is the meaning of "the W/A is to recreate the commits"? 
I just fixed this issue and the commit is under review now. 

Original comment by bruce.zu@sonymobile.com on 8 Nov 2011 at 8:25

GoogleCodeExporter commented 9 years ago
Hi,

by "recreate the commit", I mean git rebase --no-ff.
I must admit I have no cycles to build gerrit by myself, so I only use 
releases. I know that a lot of people hack on the code to have the features / 
bug fixes they need, but my employer expects me to develop other things and not 
gerrit :-)
If 2.2.2 will fix this issue, I will more than happy :-)

Original comment by egrumbach on 8 Nov 2011 at 8:29

GoogleCodeExporter commented 9 years ago
The attached patch aims to resolve this issue. It closes the change if and only 
if the branch names are the same.

Original comment by de...@spotify.com on 8 Nov 2011 at 12:29

Attachments:

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
thanks sharing this patch. I have fixed this issue also in my patch which is 
not same as the above one which I have test and find :

1> 'push ... refs/for/reviewbranch'
ok

2>'push ... refs/heads/sandbox'
ok

3> merge the change from WebUI.

not ok .
WebUI: merged but in pending status
Gerrit server log: find errors.

Original comment by bruce.zu@sonymobile.com on 11 Nov 2011 at 6:35

GoogleCodeExporter commented 9 years ago
Thanks for letting me know the bug Bruce. It is now fixed, the updated patch is 
attached.

Original comment by de...@spotify.com on 25 Dec 2011 at 9:36

Attachments:

GoogleCodeExporter commented 9 years ago
Are there any plans to merge a fix for this issue in a future release ?

Original comment by egrumbach on 14 Feb 2012 at 10:56

GoogleCodeExporter commented 9 years ago
I have tested this latest patch with v2.2.2 it works fine for me.  Thanks, and 
+1.

Now I'm working on finding all the "lost" patchsets that got dropped from the 
review queue when they were "merged" to sandboxes in the past.  Looks like we 
have several in our repo.

Original comment by phil.hord on 18 Feb 2012 at 5:19

GoogleCodeExporter commented 9 years ago
Deniz Türkoglu just uploaded a fix:
https://gerrit-review.googlesource.com/#/c/32881/

Original comment by egrumbach on 20 Feb 2012 at 7:21

GoogleCodeExporter commented 9 years ago
The code in MergeOp.java breaks review submit/merges by marking 
non-desination-target branches as unacceptable, which in turn hides previously 
merged dependencies and lists them as missing.

Original comment by georgey21 on 23 Feb 2012 at 4:49

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
In 2.4.2 this behaviour is particularly inconsistent: One can submit a change 
to multiple branches with the same change-id, but this is not possible anymore 
once it is merged in any of them

Original comment by bjoern.m...@gmail.com on 15 Jul 2012 at 11:40

GoogleCodeExporter commented 9 years ago
http://code.google.com/p/gerrit/issues/detail?id=648
http://code.google.com/p/gerrit/issues/detail?id=1153
http://code.google.com/p/gerrit/issues/detail?id=1329

all seem related ...

Original comment by bjoern.m...@gmail.com on 19 Jul 2012 at 3:24

GoogleCodeExporter commented 9 years ago
Issue 1329 has been merged into this issue.

Original comment by bklarson@gmail.com on 26 Nov 2012 at 4:51

GoogleCodeExporter commented 9 years ago
I think this still exists somehow or at least very similar error.

gerrit version 2.5.2-1-g0b0dbb0
git version git version 1.8.1.msysgit.1

what i do is this. 
git checkout develop
echo "Fish, Water, gravel, plants, water" >fishtank
git add fishtank
git commit -m "My brand new fishtank"
git push origin HEAD:refs/for/develop

Then i do 
git push origin HEAD:refs/heads/topic/fishtank
Then on the open change this comments pops up 

Change has been successfully pushed into branch topic/fishtank.
and status has changed to merged. 

My problem is that i cannot from the webui, merge it to develop at all, as the 
status is merged.

/Rasmus Voss

Original comment by rasmus.v...@live.dk on 1 Mar 2013 at 2:22

GoogleCodeExporter commented 9 years ago
I agree this problem still exists, at least as of v2.5.1. That's why it is 
still marked "New" instead of Closed/Resolved.

Original comment by phil.hord on 4 Mar 2013 at 5:09

GoogleCodeExporter commented 9 years ago
This is a serious issue for us, the work around would be to modify the 
changeids for all commits which have been merged into the other branch, and 
re-commit them to the branch they were 'stolen' from.

I think the changeId should be unique to that branch, it shouldn't be possible 
to switch the branch of a changeId once created.

Original comment by duncan.a...@gmail.com on 15 Apr 2013 at 11:18

GoogleCodeExporter commented 9 years ago
Still seeing this on 2.5.2

Original comment by cunm...@lytro.com on 16 May 2013 at 3:51

GoogleCodeExporter commented 9 years ago
still on 2.5.1, I will upgrade tomorrow and let you know.

Original comment by duncan.a...@gmail.com on 16 May 2013 at 5:17

GoogleCodeExporter commented 9 years ago
I am seeing the same issue on 2.6.1. Below my question posted on the 
repo-discuss group.

---8<---------------------------------------------------------------------------
----

We use gerrit on a daily basis and just recently I noticed something
unexpected. In our gerrit project we have a scratch/sandbox where
users can push their branches (for backup or whatever). So here is
what happened to me (A=Action, R=Result):

A1. I pushed a change for review: git push gerrit HEAD:refs/for/target-branch
  R1: A new change X is added for branch 'target-branch'
A2: push change to scratch area after rework: git push gerrit
HEAD:refs/heads/users/tmp
  R2: I have a users/tmp branch on the server
A3: update the change with rework patch: git push gerrit HEAD:refs/changes/X
  R3: change X state changed to Merged with comment:
      "Change has been successfully pushed into branch users/tmp"

R3 is unexpected and the comment added to the change gives the
motivation but I would expect it only to change to Merged when the
change ends up in the 'target-branch' indicated in A1. For my use-case
this looks like a bug, but there may be other reasons for this
behaviour.

Regards,
Arend

Original comment by aspr...@gmail.com on 10 Jan 2014 at 1:57

GoogleCodeExporter commented 9 years ago
Any updates on this issue?

Original comment by tobiassc...@googlemail.com on 7 Oct 2014 at 3:54

GoogleCodeExporter commented 9 years ago
#30: https://code.google.com/p/gerrit/issues/detail?id=1195

Original comment by gustaf.l...@sonyericsson.com on 20 Oct 2014 at 3:03

GoogleCodeExporter commented 9 years ago
See also https://code.google.com/p/gerrit/issues/detail?id=1107 

Original comment by ocroque...@free.fr on 1 Feb 2015 at 7:27

GoogleCodeExporter commented 9 years ago
anyone working on this issue? 

Original comment by altmann....@gmail.com on 7 Apr 2015 at 3:34

GoogleCodeExporter commented 9 years ago
This issue is a serious problem for us. Please raise its priority!

Original comment by crlf0...@gmail.com on 10 Jun 2015 at 7:18