diff --git a/java/com/google/gerrit/server/git/MergeUtil.java b/java/com/google/gerrit/server/git/MergeUtil.java index c2e147b317..016415cf9c 100644 --- a/java/com/google/gerrit/server/git/MergeUtil.java +++ b/java/com/google/gerrit/server/git/MergeUtil.java @@ -216,7 +216,7 @@ public class MergeUtil { List result = new ArrayList<>(); try { result.addAll(mergeSorter.sort(toSort)); - } catch (IOException e) { + } catch (IOException | OrmException e) { throw new IntegrationException("Branch head sorting failed", e); } result.sort(CodeReviewCommit.ORDER); @@ -566,7 +566,7 @@ public class MergeUtil { throws IntegrationException { try { return !mergeSorter.sort(Collections.singleton(toMerge)).contains(toMerge); - } catch (IOException e) { + } catch (IOException | OrmException e) { throw new IntegrationException("Branch head sorting failed", e); } } diff --git a/java/com/google/gerrit/server/submit/CommitMergeStatus.java b/java/com/google/gerrit/server/submit/CommitMergeStatus.java index 7f4f4af0e3..202fab7a06 100644 --- a/java/com/google/gerrit/server/submit/CommitMergeStatus.java +++ b/java/com/google/gerrit/server/submit/CommitMergeStatus.java @@ -14,6 +14,14 @@ package com.google.gerrit.server.submit; +import static java.util.stream.Collectors.toSet; + +import com.google.gerrit.server.query.change.ChangeData; +import com.google.gerrit.server.query.change.InternalChangeQuery; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Provider; +import java.util.List; + /** * Status codes set on {@link com.google.gerrit.server.git.CodeReviewCommit}s by {@link * SubmitStrategy} implementations. @@ -76,4 +84,23 @@ public enum CommitMergeStatus { public String getDescription() { return description; } + + public static String createMissingDependencyMessage( + Provider queryProvider, String commit, String otherCommit) + throws OrmException { + List changes = queryProvider.get().enforceVisibility(true).byCommit(otherCommit); + + if (changes.isEmpty()) { + return String.format( + "Commit %s depends on commit %s which cannot be merged.", commit, otherCommit); + } else if (changes.size() == 1) { + return String.format( + "Commit %s depends on commit %s of change %d which cannot be merged.", + commit, otherCommit, changes.get(0).getId().get()); + } else { + return String.format( + "Commit %s depends on commit %s of changes %s which cannot be merged.", + commit, otherCommit, changes.stream().map(cd -> cd.getId().get()).collect(toSet())); + } + } } diff --git a/java/com/google/gerrit/server/submit/MergeSorter.java b/java/com/google/gerrit/server/submit/MergeSorter.java index fa5435bc44..6dbec325bd 100644 --- a/java/com/google/gerrit/server/submit/MergeSorter.java +++ b/java/com/google/gerrit/server/submit/MergeSorter.java @@ -16,6 +16,9 @@ package com.google.gerrit.server.submit; import com.google.gerrit.server.git.CodeReviewCommit; import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk; +import com.google.gerrit.server.query.change.InternalChangeQuery; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Provider; import java.io.IOException; import java.util.Collection; import java.util.HashSet; @@ -29,21 +32,24 @@ public class MergeSorter { private final CodeReviewRevWalk rw; private final RevFlag canMergeFlag; private final Set accepted; + private final Provider queryProvider; private final Set incoming; public MergeSorter( CodeReviewRevWalk rw, Set alreadyAccepted, RevFlag canMergeFlag, + Provider queryProvider, Set incoming) { this.rw = rw; this.canMergeFlag = canMergeFlag; this.accepted = alreadyAccepted; + this.queryProvider = queryProvider; this.incoming = incoming; } public Collection sort(Collection toMerge) - throws IOException { + throws IOException, OrmException { final Set heads = new HashSet<>(); final Set sort = new HashSet<>(toMerge); while (!sort.isEmpty()) { @@ -64,8 +70,7 @@ public class MergeSorter { // n.setStatusCode(CommitMergeStatus.MISSING_DEPENDENCY); n.setStatusMessage( - String.format( - "Commit %s depends on commit %s which cannot be merged.", n.name(), c.name())); + CommitMergeStatus.createMissingDependencyMessage(queryProvider, n.name(), c.name())); break; } contents.add(c); diff --git a/java/com/google/gerrit/server/submit/RebaseSorter.java b/java/com/google/gerrit/server/submit/RebaseSorter.java index bd4f7eafad..32f35580ba 100644 --- a/java/com/google/gerrit/server/submit/RebaseSorter.java +++ b/java/com/google/gerrit/server/submit/RebaseSorter.java @@ -59,7 +59,8 @@ public class RebaseSorter { this.incoming = incoming; } - public List sort(Collection toSort) throws IOException { + public List sort(Collection toSort) + throws IOException, OrmException { final List sorted = new ArrayList<>(); final Set sort = new HashSet<>(toSort); while (!sort.isEmpty()) { @@ -83,8 +84,8 @@ public class RebaseSorter { // n.setStatusCode(CommitMergeStatus.MISSING_DEPENDENCY); n.setStatusMessage( - String.format( - "Commit %s depends on commit %s which cannot be merged.", n.name(), c.name())); + CommitMergeStatus.createMissingDependencyMessage( + queryProvider, n.name(), c.name())); } // Stop RevWalk because c is either a merged commit or a missing // dependency. Not need to walk further. diff --git a/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java b/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java index b2b54aeb18..fa0fddead4 100644 --- a/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java +++ b/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java @@ -59,7 +59,7 @@ public class RebaseSubmitStrategy extends SubmitStrategy { List sorted; try { sorted = args.rebaseSorter.sort(toMerge); - } catch (IOException e) { + } catch (IOException | OrmException e) { throw new IntegrationException("Commit sorting failed", e); } List ops = new ArrayList<>(sorted.size()); diff --git a/java/com/google/gerrit/server/submit/SubmitDryRun.java b/java/com/google/gerrit/server/submit/SubmitDryRun.java index ed9bfeb2fd..e273652d9d 100644 --- a/java/com/google/gerrit/server/submit/SubmitDryRun.java +++ b/java/com/google/gerrit/server/submit/SubmitDryRun.java @@ -27,7 +27,9 @@ import com.google.gerrit.server.git.MergeUtil; import com.google.gerrit.server.project.NoSuchProjectException; import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectState; +import com.google.gerrit.server.query.change.InternalChangeQuery; import com.google.inject.Inject; +import com.google.inject.Provider; import java.io.IOException; import java.util.Collection; import java.util.HashSet; @@ -91,11 +93,16 @@ public class SubmitDryRun { private final ProjectCache projectCache; private final MergeUtil.Factory mergeUtilFactory; + private final Provider queryProvider; @Inject - SubmitDryRun(ProjectCache projectCache, MergeUtil.Factory mergeUtilFactory) { + SubmitDryRun( + ProjectCache projectCache, + MergeUtil.Factory mergeUtilFactory, + Provider queryProvider) { this.projectCache = projectCache; this.mergeUtilFactory = mergeUtilFactory; + this.queryProvider = queryProvider; } public boolean run( @@ -116,7 +123,8 @@ public class SubmitDryRun { repo, rw, mergeUtilFactory.create(getProject(destBranch)), - new MergeSorter(rw, alreadyAccepted, canMerge, ImmutableSet.of(toMergeCommit))); + new MergeSorter( + rw, alreadyAccepted, canMerge, queryProvider, ImmutableSet.of(toMergeCommit))); switch (submitType) { case CHERRY_PICK: diff --git a/java/com/google/gerrit/server/submit/SubmitStrategy.java b/java/com/google/gerrit/server/submit/SubmitStrategy.java index 6e3e0b8fc0..2b45cd9dd6 100644 --- a/java/com/google/gerrit/server/submit/SubmitStrategy.java +++ b/java/com/google/gerrit/server/submit/SubmitStrategy.java @@ -204,7 +204,8 @@ public abstract class SubmitStrategy { projectCache.get(destBranch.getParentKey()), "project not found: %s", destBranch.getParentKey()); - this.mergeSorter = new MergeSorter(rw, alreadyAccepted, canMergeFlag, incoming); + this.mergeSorter = + new MergeSorter(rw, alreadyAccepted, canMergeFlag, queryProvider, incoming); this.rebaseSorter = new RebaseSorter( rw, mergeTip.getInitialTip(), alreadyAccepted, canMergeFlag, queryProvider, incoming); diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java index c606bb026c..0da5e5919b 100644 --- a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java @@ -408,6 +408,8 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge { + change3a.getCommit().name() + " depends on commit " + change2.getCommit().name() + + " of change " + + change2.getChange().getId() + " which cannot be merged."); RevCommit tipbranch = getRemoteLog(project, "branch").get(0); @@ -511,6 +513,8 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge { + change3.getCommit().name() + " depends on commit " + change2result.getCommit().name() + + " of change " + + change2result.getChange().getId() + " which cannot be merged."); assertRefUpdatedEvents();