From d4fab881128480e4c18fdf44eec757b0c24e7558 Mon Sep 17 00:00:00 2001 From: Alexandre Philbert Date: Tue, 19 Jan 2016 15:19:02 -0500 Subject: [PATCH] Fix pushing a first patchset does not trigger replication As mentioned in the replies of the issue[1]: Since change 71424[2], a call to setUpdateRef(false) was added on the ChangeInserter. BatchUpdate.addRefUpdate is what causes the batchRefUpdate member to be initialised, but when ChangeInserter.setUpdateRef is false, the ChangeInserter.updateRepo method does not call addRefUpdate. So when addRefUpdate is not called, batchRefUpdate remains null. And in BatchUpdate.execute, the call to gitRefUpdated.fire(project, batchRefUpdate) is only called when batchRefUpdate is not null. Result is that ref-updated events are not fired when a new change is uploaded. To clarify, updateRef needs to be set to true to keep the same flow as when the command is created on the spot. Or else we would need to get access to the RepoContext created in executeRefUpdates somehow, which would generate a special case flow. Simply changing to setUpdateRef(true) would result in a LOCK_FAILURE since it would create a duplicate to an already in progress command and they would be trying to access the same ref simultaneously. The idea of the fix is to set the command which was already completed beforehand instead of creating a duplicate just so the event gets fired properly when checking its status. [1] https://code.google.com/p/gerrit/issues/detail?id=3767 [2] https://gerrit-review.googlesource.com/#/c/71424/2 Bug: Issue 3767 Change-Id: I6674156e914b0c2c5d087c3229d81676daf8e13f --- .../gerrit/server/change/ChangeInserter.java | 14 ++++++++++++-- .../google/gerrit/server/git/ReceiveCommits.java | 3 ++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java index 3fff5bab3f..249b52260c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java @@ -108,6 +108,7 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp { private Set extraCC; private Map approvals; private RequestScopePropagator requestScopePropagator; + private ReceiveCommand updateRefCommand; private boolean runHooks; private boolean sendMail; private boolean updateRef; @@ -148,6 +149,7 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp { this.reviewers = Collections.emptySet(); this.extraCC = Collections.emptySet(); this.approvals = Collections.emptyMap(); + this.updateRefCommand = null; this.runHooks = true; this.sendMail = true; this.updateRef = true; @@ -258,6 +260,10 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp { return this; } + public void setUpdateRefCommand(ReceiveCommand cmd) { + updateRefCommand = cmd; + } + public PatchSet getPatchSet() { checkState(patchSet != null, "getPatchSet() only valid after creating change"); @@ -290,8 +296,12 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp { if (!updateRef) { return; } - ctx.addRefUpdate( - new ReceiveCommand(ObjectId.zeroId(), commit, psId.toRefName())); + if (updateRefCommand == null) { + ctx.addRefUpdate( + new ReceiveCommand(ObjectId.zeroId(), commit, psId.toRefName())); + } else { + ctx.addRefUpdate(updateRefCommand); + } } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index ef028555b1..c2c7a53d31 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -1753,6 +1753,7 @@ public class ReceiveCommits { .setValidatePolicy(CommitValidators.Policy.NONE); cmd = new ReceiveCommand(ObjectId.zeroId(), c, ins.getPatchSetId().toRefName()); + ins.setUpdateRefCommand(cmd); } CheckedFuture insertChange() { @@ -1800,7 +1801,7 @@ public class ReceiveCommits { .setMessage(msg) .setRequestScopePropagator(requestScopePropagator) .setSendMail(true) - .setUpdateRef(false)); + .setUpdateRef(true)); if (magicBranch != null) { bu.addOp( changeId,