Misleading PathConflictException when Rebasing

Since Gerrit nowadays handles Path Conflicts and (depending
on the Project Settings) tries to resolve them with a content
merge, there will also be failing rebases due to content merge
conflicts.

A user getting an error message pointing towards a path
conflict could obviously be confused, since (s)he is used to
Gerrit normally handling them or since it might not even be a
path conflict that caused the error.

Change-Id: I8574a78efbf130abfe0da8fc6ae65decbf6872a1
This commit is contained in:
Gustaf Lundh
2014-12-10 17:03:02 +01:00
committed by David Pursehouse
parent 2c1928fa49
commit 54ac00d366
4 changed files with 15 additions and 32 deletions

View File

@@ -1,24 +0,0 @@
// Copyright (C) 2012 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.changedetail;
/** Indicates a path conflict during rebase or merge */
public class PathConflictException extends Exception {
private static final long serialVersionUID = 1L;
public PathConflictException(String msg) {
super(msg);
}
}

View File

@@ -31,6 +31,7 @@ import com.google.gerrit.server.change.PatchSetInserter;
import com.google.gerrit.server.change.PatchSetInserter.ValidatePolicy; import com.google.gerrit.server.change.PatchSetInserter.ValidatePolicy;
import com.google.gerrit.server.change.RevisionResource; import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeConflictException;
import com.google.gerrit.server.git.MergeUtil; import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.InvalidChangeOperationException;
@@ -138,7 +139,7 @@ public class RebaseChange {
uploader, baseCommit, mergeUtilFactory.create( uploader, baseCommit, mergeUtilFactory.create(
changeControl.getProjectControl().getProjectState(), true), changeControl.getProjectControl().getProjectState(), true),
committerIdent, true, true, ValidatePolicy.GERRIT); committerIdent, true, true, ValidatePolicy.GERRIT);
} catch (PathConflictException e) { } catch (MergeConflictException e) {
throw new IOException(e.getMessage()); throw new IOException(e.getMessage());
} finally { } finally {
if (inserter != null) { if (inserter != null) {
@@ -281,7 +282,7 @@ public class RebaseChange {
boolean sendMail, boolean runHooks, ValidatePolicy validate) boolean sendMail, boolean runHooks, ValidatePolicy validate)
throws NoSuchChangeException, throws NoSuchChangeException,
OrmException, IOException, InvalidChangeOperationException, OrmException, IOException, InvalidChangeOperationException,
PathConflictException { MergeConflictException {
if (!change.currentPatchSetId().equals(patchSetId)) { if (!change.currentPatchSetId().equals(patchSetId)) {
throw new InvalidChangeOperationException("patch set is not current"); throw new InvalidChangeOperationException("patch set is not current");
} }
@@ -338,7 +339,7 @@ public class RebaseChange {
final ObjectInserter inserter, final RevCommit original, final ObjectInserter inserter, final RevCommit original,
final RevCommit base, final MergeUtil mergeUtil, final RevCommit base, final MergeUtil mergeUtil,
final PersonIdent committerIdent) throws IOException, final PersonIdent committerIdent) throws IOException,
PathConflictException { MergeConflictException {
final RevCommit parentCommit = original.getParent(0); final RevCommit parentCommit = original.getParent(0);
@@ -351,8 +352,8 @@ public class RebaseChange {
merger.merge(original, base); merger.merge(original, base);
if (merger.getResultTreeId() == null) { if (merger.getResultTreeId() == null) {
throw new PathConflictException( throw new MergeConflictException(
"The change could not be rebased due to a path conflict during merge."); "The change could not be rebased due to a conflict during merge.");
} }
final CommitBuilder cb = new CommitBuilder(); final CommitBuilder cb = new CommitBuilder();

View File

@@ -32,6 +32,12 @@ public enum CommitMergeStatus {
+ "\n" + "\n"
+ "Please rebase the change locally and upload the rebased commit for review."), + "Please rebase the change locally and upload the rebased commit for review."),
/** */
REBASE_MERGE_CONFLICT(
"The change could not be merged due to a conflict.\n"
+ "\n"
+ "Please rebase the change locally and upload the rebased commit for review."),
/** */ /** */
MISSING_DEPENDENCY(""), MISSING_DEPENDENCY(""),

View File

@@ -20,10 +20,10 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.PatchSetInserter.ValidatePolicy; import com.google.gerrit.server.change.PatchSetInserter.ValidatePolicy;
import com.google.gerrit.server.changedetail.PathConflictException;
import com.google.gerrit.server.changedetail.RebaseChange; import com.google.gerrit.server.changedetail.RebaseChange;
import com.google.gerrit.server.git.CodeReviewCommit; import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CommitMergeStatus; import com.google.gerrit.server.git.CommitMergeStatus;
import com.google.gerrit.server.git.MergeConflictException;
import com.google.gerrit.server.git.MergeException; import com.google.gerrit.server.git.MergeException;
import com.google.gerrit.server.git.RebaseSorter; import com.google.gerrit.server.git.RebaseSorter;
import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.patch.PatchSetInfoFactory;
@@ -109,8 +109,8 @@ public class RebaseIfNecessary extends SubmitStrategy {
newMergeTip.setStatusCode(CommitMergeStatus.CLEAN_REBASE); newMergeTip.setStatusCode(CommitMergeStatus.CLEAN_REBASE);
newCommits.put(newPatchSet.getId().getParentKey(), newMergeTip); newCommits.put(newPatchSet.getId().getParentKey(), newMergeTip);
setRefLogIdent(args.mergeUtil.getSubmitter(n)); setRefLogIdent(args.mergeUtil.getSubmitter(n));
} catch (PathConflictException e) { } catch (MergeConflictException e) {
n.setStatusCode(CommitMergeStatus.PATH_CONFLICT); n.setStatusCode(CommitMergeStatus.REBASE_MERGE_CONFLICT);
} catch (NoSuchChangeException | OrmException | IOException } catch (NoSuchChangeException | OrmException | IOException
| InvalidChangeOperationException e) { | InvalidChangeOperationException e) {
throw new MergeException("Cannot rebase " + n.name(), e); throw new MergeException("Cannot rebase " + n.name(), e);