My favorites | Sign in
Project Home Downloads Wiki Issues Source
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 254: Pushing an old commit to a change marks the change as merged
2 people starred this issue and may be notified of changes. Back to list
Status:  Accepted
Owner:  sop@google.com
Cc:  mpoole...@gmail.com


Sign in to add a comment
 
Reported by code-rev...@gtempaccount.com, Sep 24, 2009
Reported by Michael Poole <mdpoole@troilus.org> on Tue Jul 28 11:18:29 PDT 2009
Source: JIRA GERRIT-255
Affected Version: 2.0.17

Gerrit allows an already-submitted commit to be pushed as a new change.  This
shows up as a new patch set for the commit, which is less than optimal, but
also marks the patch as "Merged" -- which is even less desirable.

To reproduce:
git checkout master
# Make and locally commit a change.
git push origin HEAD:refs/for/master
# Assume that the change number is 1234.
git checkout origin/master
# Make a different change, but forget to commit it.
git push origin HEAD:refs/patches/1234
# Now change 1234 has moved to "Merged".
Sep 24, 2009
#1 code-rev...@gtempaccount.com
Comment by Shawn Pearce <sop@google.com> on Tue Jul 28 11:22:07 PDT 2009

This is a feature.  You asked Gerrit to replace the change with HEAD.  You are
telling Gerrit that HEAD is the replacement for this change.  Gerrit detects
the replacement has already been submitted to the branch, and thus the change
is submitted, because its new replacement is already submitted.

See GERRIT-54 for the feature requesting this behavior.
Sep 24, 2009
#2 code-rev...@gtempaccount.com
Update by Shawn Pearce <sop@google.com> on Tue Jul 28 11:22:07 PDT 2009
Status: WontFix
Sep 24, 2009
#3 code-rev...@gtempaccount.com
Comment by Michael Poole <mdpoole@troilus.org> on Tue Jul 28 12:50:18 PDT 2009

My concern -- the mistake that I made -- is that the user can accidentally
replace the change with an ancestor of the existing change.  This seems like
an awkward way to abandon the change.  Could Gerrit reject that specific
(attempted) operation without affecting the feature from GERRIT-54?
Sep 24, 2009
#4 code-rev...@gtempaccount.com
Comment by Shawn Pearce <sop@google.com> on Tue Jul 28 12:55:35 PDT 2009

Oh, that's a good point.  Replacing a change with an ancestor of an existing
patch set is likely to be an error if the replacement commit is already merged
into the branch.  I'll reopen this and look at implementing that sanity check.
Sep 24, 2009
#5 code-rev...@gtempaccount.com
Update by Shawn Pearce <sop@google.com> on Tue Jul 28 12:55:38 PDT 2009
Status: New
Sep 24, 2009
#6 sop+code@google.com
(No comment was entered for this change.)
Owner: s...@google.com
Sep 24, 2009
#7 sop+code@google.com
(No comment was entered for this change.)
Status: Accepted
Cc: mdpo...@troilus.org
Nov 21, 2009
#8 sop@google.com
(No comment was entered for this change.)
Owner: s...@google.com
Sign in to add a comment

Powered by Google Project Hosting