Let users know which change prevents submit in case of MISSING_DEPENDENCY
If a change cannot be submitted because it depends on a commit that
cannot be merged the user got the following message:
Depends on change that was not submitted. Commit <commit> depends on
commit <other-commit> which cannot be merged.
To find the change to which <other-commit> belongs the user must do a
change query. Do this for the user and return the change ID too.
Now we return:
Depends on change that was not submitted. Commit <commit> depends on
commit <other-commit> of change <other-change> which cannot be merged.
Returning the commit SHA1 of <other-commit> is still useful since the
commit that prevents submit may be an outdated patch set and this can be
found easier if the commit SHA1 is known.
Change-Id: I1368af204702a06d01ee6f812a5da5ac419b89d7
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/java/com/google/gerrit/server/git/MergeUtil.java b/java/com/google/gerrit/server/git/MergeUtil.java
index c2e147b..016415c 100644
--- a/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/java/com/google/gerrit/server/git/MergeUtil.java
@@ -216,7 +216,7 @@
List<CodeReviewCommit> 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 @@
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 7f4f4af..202fab7 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 String getDescription() {
return description;
}
+
+ public static String createMissingDependencyMessage(
+ Provider<InternalChangeQuery> queryProvider, String commit, String otherCommit)
+ throws OrmException {
+ List<ChangeData> 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 fa5435b..6dbec32 100644
--- a/java/com/google/gerrit/server/submit/MergeSorter.java
+++ b/java/com/google/gerrit/server/submit/MergeSorter.java
@@ -16,6 +16,9 @@
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 @@
private final CodeReviewRevWalk rw;
private final RevFlag canMergeFlag;
private final Set<RevCommit> accepted;
+ private final Provider<InternalChangeQuery> queryProvider;
private final Set<CodeReviewCommit> incoming;
public MergeSorter(
CodeReviewRevWalk rw,
Set<RevCommit> alreadyAccepted,
RevFlag canMergeFlag,
+ Provider<InternalChangeQuery> queryProvider,
Set<CodeReviewCommit> incoming) {
this.rw = rw;
this.canMergeFlag = canMergeFlag;
this.accepted = alreadyAccepted;
+ this.queryProvider = queryProvider;
this.incoming = incoming;
}
public Collection<CodeReviewCommit> sort(Collection<CodeReviewCommit> toMerge)
- throws IOException {
+ throws IOException, OrmException {
final Set<CodeReviewCommit> heads = new HashSet<>();
final Set<CodeReviewCommit> sort = new HashSet<>(toMerge);
while (!sort.isEmpty()) {
@@ -64,8 +70,7 @@
//
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 bd4f7ea..32f3558 100644
--- a/java/com/google/gerrit/server/submit/RebaseSorter.java
+++ b/java/com/google/gerrit/server/submit/RebaseSorter.java
@@ -59,7 +59,8 @@
this.incoming = incoming;
}
- public List<CodeReviewCommit> sort(Collection<CodeReviewCommit> toSort) throws IOException {
+ public List<CodeReviewCommit> sort(Collection<CodeReviewCommit> toSort)
+ throws IOException, OrmException {
final List<CodeReviewCommit> sorted = new ArrayList<>();
final Set<CodeReviewCommit> sort = new HashSet<>(toSort);
while (!sort.isEmpty()) {
@@ -83,8 +84,8 @@
//
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 b2b54ae..fa0fdde 100644
--- a/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java
+++ b/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java
@@ -59,7 +59,7 @@
List<CodeReviewCommit> sorted;
try {
sorted = args.rebaseSorter.sort(toMerge);
- } catch (IOException e) {
+ } catch (IOException | OrmException e) {
throw new IntegrationException("Commit sorting failed", e);
}
List<SubmitStrategyOp> 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 ed9bfeb..e273652 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.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 @@
private final ProjectCache projectCache;
private final MergeUtil.Factory mergeUtilFactory;
+ private final Provider<InternalChangeQuery> queryProvider;
@Inject
- SubmitDryRun(ProjectCache projectCache, MergeUtil.Factory mergeUtilFactory) {
+ SubmitDryRun(
+ ProjectCache projectCache,
+ MergeUtil.Factory mergeUtilFactory,
+ Provider<InternalChangeQuery> queryProvider) {
this.projectCache = projectCache;
this.mergeUtilFactory = mergeUtilFactory;
+ this.queryProvider = queryProvider;
}
public boolean run(
@@ -116,7 +123,8 @@
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 6e3e0b8..2b45cd9d 100644
--- a/java/com/google/gerrit/server/submit/SubmitStrategy.java
+++ b/java/com/google/gerrit/server/submit/SubmitStrategy.java
@@ -204,7 +204,8 @@
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 c606bb0..0da5e59 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 @@
+ 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 @@
+ change3.getCommit().name()
+ " depends on commit "
+ change2result.getCommit().name()
+ + " of change "
+ + change2result.getChange().getId()
+ " which cannot be merged.");
assertRefUpdatedEvents();
