Prevent duplicate commits in same project when uploading to refs/changes/n

Under certain circumstances, when pushing to 'refs/changes/n', the same
commit can be pushed onto multiple changes even if they are on the same
branch.

As a result of this, if the commit sha1 is used as change identifier
in the 'review' ssh command, it will fail with the error:

  'fatal: <SHA1> matches multiple patch sets.'

This commit performs the same check as when pushing to 'refs/for/branch'
to prevent this, and will show "commit already exists (in the project)"
error message to reject uploading a commit to an existing change via
`refs/changes/n` if the commit was already successfully pushed to a change
in project scope.

Bug: issue 2374
Change-Id: I1c000b2cc3e155617cdc9c295a46ce2107ec47ca
This commit is contained in:
Bruce Zu 2014-01-21 11:11:42 +08:00 committed by David Pursehouse
parent bf690c6c13
commit cd6572d435
3 changed files with 33 additions and 15 deletions
Documentation
gerrit-server/src/main/java/com/google/gerrit/server/git

@ -1,10 +1,19 @@
commit already exists
=====================
With this error message Gerrit rejects to push a commit to an
existing change via `refs/changes/n` if the commit was already
successfully pushed to the change. In this case there is no
new commit and consequently there is nothing for Gerrit to do.
With "commit already exists (as current patchset)" or
"commit already exists (in the change)" error message
Gerrit rejects to push a commit to an existing change via
`refs/changes/n` if the commit was already successfully
pushed to the change.
With "commit already exists (in the project)" error message
Gerrit rejects to push a commit to an existing change via
`refs/changes/n` if the commit was already successfully
pushed to a change in project scope.
In any above case there is no new commit and consequently
there is nothing for Gerrit to do.
For further information about how to resolve this error, please
refer to link:error-no-new-changes.html[no new changes].

@ -2,8 +2,9 @@ no new changes
==============
With this error message Gerrit rejects to push a commit if the pushed
commit was already successfully pushed to Gerrit. In this case there
is no new change and consequently there is nothing for Gerrit to do.
commit was already successfully pushed to Gerrit in project scope.
In this case there is no new change and consequently there is nothing
for Gerrit to do.
If your push is failing with this error message, you normally
don't have to do anything since the commit was already successfully
@ -31,7 +32,7 @@ means:
. you cannot reset a change to an old patch set by pushing the old
commit for this change again
. if a commit was pushed to one branch you cannot push this commit
to another branch
to another branch in project scope.
. if a commit was pushed directly to a branch (without going through
code review) you cannot push this commit once again for code
review (please note that in this case searching by the commit ID
@ -39,11 +40,11 @@ means:
If you need to re-push a commit you may rewrite this commit by
link:http://www.kernel.org/pub/software/scm/git/docs/git-commit.html[amending] it or doing an interactive link:http://www.kernel.org/pub/software/scm/git/docs/git-rebase.html[git rebase]. By rewriting the
commit you actually create a new commit (with a new commit ID) which
can then be pushed to Gerrit. If the old commit contains a Change-Id
in the commit message you also need to replace it with a new
Change-Id (case 1. and 3. above), otherwise the push will fail with
another error message.
commit you actually create a new commit (with a new commit ID in
project scope) which can then be pushed to Gerrit. If the old commit
contains a Change-Id in the commit message you also need to replace
it with a new Change-Id (case 1. and 3. above), otherwise the push
will fail with another error message.
GERRIT

@ -17,7 +17,6 @@ package com.google.gerrit.server.git;
import static com.google.gerrit.server.git.MultiProgressMonitor.UNKNOWN;
import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromApprovals;
import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromFooters;
import static org.eclipse.jgit.lib.Constants.R_HEADS;
import static org.eclipse.jgit.lib.RefDatabase.ALL;
import static org.eclipse.jgit.transport.ReceiveCommand.Result.NOT_ATTEMPTED;
@ -128,9 +127,9 @@ import org.eclipse.jgit.transport.AdvertiseRefsHook;
import org.eclipse.jgit.transport.AdvertiseRefsHookChain;
import org.eclipse.jgit.transport.BaseReceivePack;
import org.eclipse.jgit.transport.ReceiveCommand;
import org.eclipse.jgit.transport.RefFilter;
import org.eclipse.jgit.transport.ReceiveCommand.Result;
import org.eclipse.jgit.transport.ReceivePack;
import org.eclipse.jgit.transport.RefFilter;
import org.eclipse.jgit.transport.ServiceMayNotContinueException;
import org.eclipse.jgit.transport.UploadPack;
import org.kohsuke.args4j.CmdLineException;
@ -1694,6 +1693,7 @@ public class ReceiveCommits {
if (newCommit == priorCommit) {
// Ignore requests to make the change its current state.
skip = true;
reject(inputCommand, "commit already exists (as current patchset)");
return false;
}
@ -1705,10 +1705,18 @@ public class ReceiveCommits {
reject(inputCommand, "change " + ontoChange + " closed");
return false;
} else if (revisions.containsKey(newCommit)) {
reject(inputCommand, "commit already exists");
reject(inputCommand, "commit already exists (in the change)");
return false;
}
for (final Ref r : rp.getRepository().getRefDatabase()
.getRefs("refs/changes").values()) {
if (r.getObjectId().equals(inputCommand.getNewId())) {
reject(inputCommand, "commit already exists (in the project)");
return false;
}
}
for (RevCommit prior : revisions.keySet()) {
// Don't allow a change to directly depend upon itself. This is a
// very common error due to users making a new commit rather than