From 9dff074c73ba59f17d8e0430cae04b5a6d1ac32c Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Mon, 18 Nov 2013 16:03:09 +0100 Subject: [PATCH 1/2] Fix updating changes that fail to merge If a submitted change fails to merge due an error (e.g. inconsistent data or a pre merge validation found an issue) the change state should be set back to 'New', but it stays as 'Submitted, Merge Pending'. It should also display a proper error message to the user doing the submit and add a change message to the change that contains the error message. Both is currently not happening and the user has no idea why the change got stuck in 'Submitted, Merge Pending' state. This change ensures that all changes which cannot be merged due to an error are properly updated. Change-Id: Idd8da6a4cd089d2a7b09cbb60342612f1c5256e9 Signed-off-by: Edwin Kempin (cherry picked from commit 851f7017fd8728616406c605bd5b906706894bc7) --- .../java/com/google/gerrit/server/git/MergeOp.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java index aa334f38fa..cf8a16baa5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java @@ -27,6 +27,7 @@ import com.google.common.base.Objects; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.Iterables; import com.google.common.collect.ListMultimap; +import com.google.common.collect.Lists; import com.google.common.collect.Sets; import com.google.common.util.concurrent.CheckedFuture; import com.google.gerrit.common.ChangeHooks; @@ -144,6 +145,7 @@ public class MergeOp { private final ListMultimap toMerge; private final List potentiallyStillSubmittable; private final Map commits; + private final List toUpdate; private ReviewDb db; private Repository repo; private RevWalk rw; @@ -202,6 +204,7 @@ public class MergeOp { toMerge = ArrayListMultimap.create(); potentiallyStillSubmittable = new ArrayList(); commits = new HashMap(); + toUpdate = Lists.newArrayList(); } private void setDestProject() throws MergeException { @@ -269,6 +272,8 @@ public class MergeOp { toMerge.putAll(toMergeNextTurn); } + updateChangeStatus(toUpdate); + for (final CodeReviewCommit commit : potentiallyStillSubmittableOnNextRun) { final Capable capable = isSubmitStillPossible(commit); if (capable != Capable.OK) { @@ -447,6 +452,7 @@ public class MergeOp { if (chg.currentPatchSetId() == null) { commits.put(changeId, CodeReviewCommit .error(CommitMergeStatus.NO_PATCH_SET)); + toUpdate.add(chg); continue; } @@ -460,6 +466,7 @@ public class MergeOp { || ps.getRevision().get() == null) { commits.put(changeId, CodeReviewCommit .error(CommitMergeStatus.NO_PATCH_SET)); + toUpdate.add(chg); continue; } @@ -470,6 +477,7 @@ public class MergeOp { } catch (IllegalArgumentException iae) { commits.put(changeId, CodeReviewCommit .error(CommitMergeStatus.NO_PATCH_SET)); + toUpdate.add(chg); continue; } @@ -485,6 +493,7 @@ public class MergeOp { // commits.put(changeId, CodeReviewCommit .error(CommitMergeStatus.REVISION_GONE)); + toUpdate.add(chg); continue; } @@ -495,6 +504,7 @@ public class MergeOp { log.error("Invalid commit " + id.name() + " on " + chg.getKey(), e); commits.put(changeId, CodeReviewCommit .error(CommitMergeStatus.REVISION_GONE)); + toUpdate.add(chg); continue; } @@ -503,6 +513,7 @@ public class MergeOp { mergeValidators.validatePreMerge(repo, commit, destProject, destBranch, ps.getId()); } catch (MergeValidationException mve) { commits.put(changeId, CodeReviewCommit.error(mve.getStatus())); + toUpdate.add(chg); continue; } @@ -535,6 +546,7 @@ public class MergeOp { if (submitType == null) { commits.put(changeId, CodeReviewCommit.error(CommitMergeStatus.NO_SUBMIT_TYPE)); + toUpdate.add(chg); continue; } From 5cefcbfc68a8d7b49d1d98d36c5dfbb09de28923 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Mon, 18 Nov 2013 16:56:10 +0100 Subject: [PATCH 2/2] Make sure all exceptions during GC are caught and logged Change-Id: I607d60b938c57d84a3590af9bd779e22f028da16 Signed-off-by: Edwin Kempin (cherry picked from commit 4519a9054920cdb1b95c4c6e7e34211022846e3b) --- .../google/gerrit/server/git/GarbageCollection.java | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/GarbageCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/GarbageCollection.java index 685e87caa9..24af14d8d8 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/GarbageCollection.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/GarbageCollection.java @@ -21,8 +21,6 @@ import com.google.inject.Inject; import org.eclipse.jgit.api.GarbageCollectCommand; import org.eclipse.jgit.api.Git; -import org.eclipse.jgit.api.errors.GitAPIException; -import org.eclipse.jgit.api.errors.JGitInternalException; import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.ConfigConstants; @@ -33,7 +31,6 @@ import org.eclipse.jgit.storage.pack.PackConfig; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.IOException; import java.io.PrintWriter; import java.util.List; import java.util.Properties; @@ -91,15 +88,7 @@ public class GarbageCollection { result.addError(new GarbageCollectionResult.Error( GarbageCollectionResult.Error.Type.REPOSITORY_NOT_FOUND, p)); - } catch (IOException e) { - logGcError(writer, p, e); - result.addError(new GarbageCollectionResult.Error( - GarbageCollectionResult.Error.Type.GC_FAILED, p)); - } catch (GitAPIException e) { - logGcError(writer, p, e); - result.addError(new GarbageCollectionResult.Error( - GarbageCollectionResult.Error.Type.GC_FAILED, p)); - } catch (JGitInternalException e) { + } catch (Exception e) { logGcError(writer, p, e); result.addError(new GarbageCollectionResult.Error( GarbageCollectionResult.Error.Type.GC_FAILED, p));