Unable to push commit whose parent is not the latest commit for the parent patch
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Daimona
	Jul 10 2025, 8:30 PM

Description

I've recently come across the following problem with chained commits in gerrit. To reproduce:

Clone a test repository (tested with sandbox and gerrit-ping).
Make a test patch that will be used as parent patch:
- git checkout -b parent && echo "foo" > testparent && git add -A && git commit -m "Parent patch" && git review
Make another patch that depends on the previous one:
- git checkout -b child && echo "foo" > testchild && git add -A && git commit -m "Child patch" && git review
- Note the message that you're submitting multiple commits, type 'yes' and submit
Go to the gerrit interface for the parent patch, make any edit using the UI (for example, change the testparent file from before), publish
Locally, make sure you still have the child patch checked out, and that you do NOT have the newer version of the parent
Make any change to that patch and try to push it:
- echo "bar" > testchild && git add -A && git commit --amend --no-edit && git review
- Note the message about submitting multiple commits, which has the older commit for the parent patch. Type 'yes' and submit.

This will fail with an error like:

The outstanding commits are:

95e5c35 (HEAD -> child) Child patch
97cd163 (parent) Parent patch

Do you really want to submit the above commits?
Type 'yes' to confirm, other to cancel: yes
remote: 
remote: Processing changes: refs: 1        
remote: Processing changes: refs: 1        
remote: Processing changes: refs: 1, done            
To ssh://gerrit.wikimedia.org:29418/test/gerrit-ping
 ! [remote rejected] HEAD -> refs/for/master%topic=child (commit 97cd163e26 already exists in change 1167928)
error: failed to push some refs to 'ssh://gerrit.wikimedia.org:29418/test/gerrit-ping'

I'm 100% sure this used to work before. I think previously it just detected that the parent commit is not the latest, and push the patch with an outdated dependency that I could then fix later. Since the gerrit-ping repo is currently set to use the "Merge If Necessary" strategy, this cannot be caused by the recent change to the default strategy. Thinking that it could be something on my local, I did a quick check and sure enough:

$ apt list --installed 2>/dev/null | fgrep git/
git/noble-updates,noble-security,now 1:2.43.0-1ubuntu7.3 amd64 [installed]

$ fgrep "upgrade git:" /var/log/dpkg.log
2025-07-09 12:59:42 upgrade git:amd64 1:2.43.0-1ubuntu7.2 1:2.43.0-1ubuntu7.3

This looks suspicious, but checking the changelog, I don't immediately see anything that would make me suspect (unintended) consequences of a security fix. Also, I originally thought I'd seen this issue for more than just 24 hours (maybe like a week or so), but now I'm not so sure about it. At any rate, I tried downgrading to 1:2.43.0-1ubuntu7 and the problem is still there, so most likely it isn't that.

Also, if anyone else is having this issue: the workaround is to first download the parent patch locally, then re-do the push for the child patch. For just two patches it's probably OK, but for longer chains it can be quite annoying.

Details

Related Changes in Gerrit:

	Subject	Repo	Branch	Lines +/-
	Disable receive.createNewChangeForAllInTarget again	All-Projects	refs/meta/config	+1 -1

Customize query in gerrit

Related Objects

Mentioned In: T400918: Zuul does not support Gerrit rebase submit strategy correctly, may merge deterministically broken code (breaking all subsequent CI) or report spurious merge conflicts
Mentioned Here: T390719: Change Gerrit default submit strategy to 'Rebase if Necessary' and allowing content merge
T390666: Upgrade to Gerrit 3.10.6

Event Timeline

Daimona created this task.Jul 10 2025, 8:30 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 10 2025, 8:30 PM

The message comes from java/com/google/gerrit/server/git/receive/ReceiveCommits.java, in stable-3.10:

} else if (revisions.containsKey(newCommit)) {
  reject(
      inputCommand,
      String.format(
          "commit %s already exists in change %s",
          newCommit.name().substring(0, 10), change.getId()));
  return false;
}

The list of commits apparently comes from queueSuccessMessages.

/** Appends messages for successful change creation/updates. */
private void queueSuccessMessages(List<CreateRequest> newChanges) {
  // adjacency list for commit => parent
  Map<String, String> adjList = new HashMap<>();
  for (CreateRequest cr : newChanges) {
    String parent = cr.commit.getParentCount() == 0 ? null : cr.commit.getParent(0).name();
    adjList.put(cr.commit.name(), parent);
  }
  for (ReplaceRequest rr : replaceByChange.values()) {
    String parent = null;
    if (rr.revCommit != null) {
      parent = rr.revCommit.getParentCount() == 0 ? null : rr.revCommit.getParent(0).name();
    }
    adjList.put(rr.newCommitId.name(), parent);
  }

  // get commits that are not parents
  Set<String> leafs = new TreeSet<>(adjList.keySet());
  leafs.removeAll(adjList.values());
  // go backwards from the last commit to its parent(s)
  Set<String> ordered = new LinkedHashSet<>();
  for (String leaf : leafs) {
    if (ordered.contains(leaf)) {
      continue;
    }
    while (leaf != null) {
      if (!ordered.contains(leaf)) {
        ordered.add(leaf);
      }
      leaf = adjList.get(leaf);
    }
  }
  // reverse the order to start with earliest commit
  List<String> orderedCommits = new ArrayList<>(ordered);
  Collections.reverse(orderedCommits);

Fun times.

Would that mean this was caused by a gerrit upgrade? The last one was in T390666 a few months ago AFAICT, but I've only had this issue for the last few days. I could check for other local changes (like upgrades and such), but I'm not sure what else to look for.

It is possible but unlikely imho. I think it never worked and that looks very similar to https://issues.gerritcodereview.com/issues/40011702 That describes how editing a patch prevents child from being pushed because the parent is no more up-to-date. The workaround is to use the base push option: https://gerrit.wikimedia.org/r/Documentation/user-upload.html#base

I'd need to write a reproducible test case and then send it to upstream for their consideration :)

Repro which does not edit the UI

git clone https://gerrit.wikimedia.org/r/test/gerrit-ping
cd gerrit-ping

Create two commits and push for review, saving the commit sha1:

git commit --allow-empty -m 'Parent'
parent=$(git rev-parse HEAD)
git commit --allow-empty -m 'Child'
child=$(git rev-parse HEAD)

git push origin HEAD:refs/for/master%topic=T399241
# https://gerrit.wikimedia.org/r/c/test/gerrit-ping/+/1168606/1
# https://gerrit.wikimedia.org/r/c/test/gerrit-ping/+/1168607/1

Amend parent change:

git reset --hard $parent
git commit --allow-empty --amend --no-edit
git push origin HEAD:refs/for/master

Head back to original child, amend it and send:

git reset --hard $child
git commit --allow-empty --amend --no-edit
git push origin HEAD:refs/for/master

! [remote rejected] HEAD -> refs/for/master (commit 2539245b54 already exists in change 1168606)

2539245b54 is the original parent and is patchset 1168606,1 but that change has received a new patchset 2.

I have tried to reproduce by writing at test which was unexpectedly working, or at least not reproducing the issue:

javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java

public abstract class AbstractPushForReview extends AbstractDaemonTest {
  @Test
  public void pushForReviewWhenParentChangeGotUpdated() throws Exception {
    PushOneCommit.Result rParent = pushTo("refs/for/master");
    rParent.assertOkStatus();
    ObjectId idParent = rParent.getCommit().copy();
  
    PushOneCommit.Result rChild = pushTo("refs/for/master");
    ObjectId childCommit  = rChild.getCommit().copy();
  
    // Amend parent locally and update its change
    RevCommit amendedParentCommit = testRepo.amend(idParent).create();
    assertThat(amendedParentCommit).isNotEqualTo(idParent);
    testRepo.reset(amendedParentCommit);
    assertPushOk(pushHead(testRepo, "refs/for/master", false), "refs/for/master");
    
    // Amend child and update its change
    testRepo.reset(childCommit);
    RevCommit amendedChildCommit = testRepo.amend(childCommit).create();
    assertThat(amendedChildCommit).isNotEqualTo(childCommit);
    testRepo.reset(amendedChildCommit);

    assertPushRejected(
        pushHead(testRepo, "refs/for/master"),
        "refs/for/master",
        "foo error");

    /**
    ObjectId initialHead = testRepo.getRepository().resolve("HEAD");
    List <RevCommit> commits = createChanges(2, "refs/for/master");

    // Amend parent
    amendChanges(initialHead, List.of(commits.get(0)), "refs/for/master");

    // Amend child which has previous parent
    amendChanges(commits.get(0).getId(), List.of(commits.get(1)), "refs/for/master");
    */
  }

Same for master or stable-3.10. I was questionning the behavior of the test suite and then went to use the raw git commands against a booted daemon and, to my surprise: the change can be sent just fine.

So it must be an issue with our configuration. I have compared our gerrit config and All-Projects config and eventually I found out we have:

[receive]
  createNewChangeForAllNotInTarget = true

That parameter is flipped true/false in some tests of AbstractPushForReview.

receive.createNewChangeForAllNotInTarget

This option provides a convenience for selecting the merge base by setting it automatically to the target branch’s tip so you can create new changes for all commits not in the target branch.

This option is disabled if the tip of the push is a merge commit.

This option also only works if there are no merge commits in the commit chain, in such cases it fails warning the user that such pushes can only be performed by manually specifying bases

This option is useful if you want to push a change to your personal branch first and for review to another branch for example. Or in cases where a commit is already merged into a branch and you want to create a new open change for that commit on another branch.

When I have changed the default submit strategy to rebase if necessary (T390719), I also turned on receive.createNewChangeForAllNotInTarget which led to this issue.

$ git show -U0 30f68346884142412db1e6ee56b06b45dd768724
commit 30f68346884142412db1e6ee56b06b45dd768724
Author: Hashar <hashar@free.fr>
Date:   Fri Jul 4 13:17:15 2025 +0000

    Modified project settings

diff --git a/project.config b/project.config
index b04c394..c66ec85 100644
--- a/project.config
+++ b/project.config
@@ -46 +46 @@
-       createNewChangeForAllNotInTarget = false
+       createNewChangeForAllNotInTarget = true
@@ -49,2 +49,2 @@
-       mergeContent = false
-       action = fast forward only
+       mergeContent = true
+       action = rebase if necessary

Change #1168621 had a related patch set uploaded (by Hashar; author: Hashar):

[All-Projects@refs/meta/config] Disable receive.createNewChangeForAllInTarget again

https://gerrit.wikimedia.org/r/1168621

Change #1168621 merged by Hashar:

[All-Projects@refs/meta/config] Disable receive.createNewChangeForAllInTarget again

https://gerrit.wikimedia.org/r/1168621

Maintenance_bot removed a project: Patch-For-Review.Jul 13 2025, 3:31 PM

I have confirmed the fix with the chain of commits I did on test/gerrit-ping. I have managed to send a patchset 2 on https://gerrit.wikimedia.org/r/c/test/gerrit-ping/+/1168607/ :)

@Daimona you were right that it started happening when I have changed the submit strategy to rebase if necessary. I have changed another setting while doing it and that caused the trouble!

Thank you so much for having reported it. I'll look at polishing my test and see with upstream whether that is a desired behavior.

https://gerrit-review.googlesource.com/c/gerrit/+/489941 Test review rejects when parent is obsolete

From a thread on upstream discord https://discord.com/channels/775374026587373568/1019975766756311130/1394287901197734033:

The feature was created in 2014 by someone at Spotify https://gerrit-review.googlesource.com/c/gerrit/+/59909 which was with Gerrit 2.2.1. I do not think it is needed anymore.

The issue was https://issues.gerritcodereview.com/issues/40001323 :
A) create a commit against master
B) send it to your personal sandbox refs/sandbox/john/issue1234 (that can be used to share code, save it outside of laptop etc)
C) send the same commit (same sha1/change-Id) for review: git push origin refs/for/master

The issue was that Gerrit rejected it with "no new changes". I have tried it with a Gerrit 3.10.6 and that is no more an issue.

I am tempted to remove the feature createNewChangeForAllNotInTarget

Thanks for investigating and fixing!

In T399241#10998344, @hashar wrote:

@Daimona you were right that it started happening when I have changed the submit strategy to rebase if necessary. I have changed another setting while doing it and that caused the trouble!

Just an example of being right by pure chance ;)

Just an example of being right by pure chance ;)

Yes it works sometime!! :)

Meanwhile, I have learned that createNewChangeForAllNotInTarget was a workaround for an issue in older Gerrit. Nowadays, it does not seem to apply anymore and the feature should probably be dropped. I have reached out to upstream about it. But at least the original issue is fixed now :)

thcipriani added a project: Essential-Work.Jul 18 2025, 8:00 PM

Umherirrender mentioned this in T400918: Zuul does not support Gerrit rebase submit strategy correctly, may merge deterministically broken code (breaking all subsequent CI) or report spurious merge conflicts.Aug 1 2025, 1:26 PM

Unable to push commit whose parent is not the latest commit for the parent patchClosed, ResolvedPublicActions

Description

Details

Related Objects

Event Timeline

Unable to push commit whose parent is not the latest commit for the parent patch
Closed, ResolvedPublic
Actions