Merge "Add ChangeKind for trivial rebase with commit message update."
diff --git a/Documentation/config-labels.txt b/Documentation/config-labels.txt
index 29d1b85..857725d 100644
--- a/Documentation/config-labels.txt
+++ b/Documentation/config-labels.txt
@@ -339,7 +339,7 @@
Gerrit currently supports the following predicates:
[[changekind]]
-==== changekind:{NO_CHANGE,NO_CODE_CHANGE,MERGE_FIRST_PARENT_UPDATE,REWORK,TRIVIAL_REBASE}
+==== changekind:{NO_CHANGE,NO_CODE_CHANGE,MERGE_FIRST_PARENT_UPDATE,REWORK,TRIVIAL_REBASE,TRIVIAL_REBASE_WITH_MESSAGE_UPDATE}
Matches if the diff between two patch sets was of a certain change kind:
@@ -410,6 +410,10 @@
For the pre-installed Code-Review label this predicate is used by
default.
+* [[trivial_rebase_with_message_update]]`TRIVIAL_REBASE_WITH_MESSAGE_UPDATE`:
++
+Same as TRIVIAL_REBASE, but commit message can be different.
+
* [[rework]]`REWORK`:
+
Matches all kind of change kinds because any other change kind
diff --git a/Documentation/json.txt b/Documentation/json.txt
index dc82ad1..94f9de9 100644
--- a/Documentation/json.txt
+++ b/Documentation/json.txt
@@ -118,7 +118,11 @@
REWORK;; Nontrivial content changes.
- TRIVIAL_REBASE;; Conflict-free merge between the new parent and the prior patch set.
+ TRIVIAL_REBASE;; Conflict-free merge between the new parent and the prior patch set; same commit
+ message.
+
+ TRIVIAL_REBASE_WITH_MESSAGE_UPDATE;; Conflict-free merge between the new parent and the prior
+ patch set.
MERGE_FIRST_PARENT_UPDATE;; Conflict-free change of first (left) parent of a merge commit.
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 6e14415..c6e5623 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -8900,7 +8900,8 @@
|===========================
|Field Name ||Description
|`kind` ||The change kind. Valid values are `REWORK`, `TRIVIAL_REBASE`,
-`MERGE_FIRST_PARENT_UPDATE`, `NO_CODE_CHANGE`, and `NO_CHANGE`.
+`TRIVIAL_REBASE_WITH_MESSAGE_UPDATE`, `MERGE_FIRST_PARENT_UPDATE`,
+`NO_CODE_CHANGE`, and `NO_CHANGE`.
|`_number` ||The patch set number, or `edit` if the patch set is an edit.
|`created` ||
The link:rest-api.html#timestamp[timestamp] of when the patch set was
diff --git a/java/com/google/gerrit/extensions/client/ChangeKind.java b/java/com/google/gerrit/extensions/client/ChangeKind.java
index 6240bba..6c6069d 100644
--- a/java/com/google/gerrit/extensions/client/ChangeKind.java
+++ b/java/com/google/gerrit/extensions/client/ChangeKind.java
@@ -22,6 +22,12 @@
/** Conflict-free merge between the new parent and the prior patch set. */
TRIVIAL_REBASE,
+ /**
+ * Conflict-free merge between the new parent and the prior patch set, accompanied with a change
+ * to commit message.
+ */
+ TRIVIAL_REBASE_WITH_MESSAGE_UPDATE,
+
/** Conflict-free change of first (left) parent of a merge commit. */
MERGE_FIRST_PARENT_UPDATE,
@@ -39,6 +45,8 @@
return true;
case TRIVIAL_REBASE:
return isTrivialRebase();
+ case TRIVIAL_REBASE_WITH_MESSAGE_UPDATE:
+ return isTrivialRebaseWithMessageUpdate();
case MERGE_FIRST_PARENT_UPDATE:
return isMergeFirstParentUpdate(isMerge);
case NO_CHANGE:
@@ -59,6 +67,14 @@
return this == NO_CHANGE || this == TRIVIAL_REBASE;
}
+ public boolean isTrivialRebaseWithMessageUpdate() {
+ // TRIVIAL_REBASE is more strict condition and hence matched as well
+ return this == NO_CHANGE
+ || this == NO_CODE_CHANGE
+ || this == TRIVIAL_REBASE
+ || this == TRIVIAL_REBASE_WITH_MESSAGE_UPDATE;
+ }
+
public boolean isMergeFirstParentUpdate(boolean isMerge) {
if (!isMerge) {
return false;
diff --git a/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java b/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java
index 74aa373..90752c0 100644
--- a/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java
+++ b/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java
@@ -193,7 +193,7 @@
@Override
public ChangeKind call() throws IOException {
if (Objects.equals(key.prior(), key.next())) {
- return ChangeKind.NO_CODE_CHANGE;
+ return ChangeKind.NO_CHANGE;
}
RevWalk rw = alreadyOpenRw;
@@ -210,15 +210,10 @@
RevCommit next = rw.parseCommit(key.next());
rw.parseBody(next);
- if (!next.getFullMessage().equals(prior.getFullMessage())) {
- if (isSameDeltaAndTree(rw, prior, next)) {
- return ChangeKind.NO_CODE_CHANGE;
- }
- return ChangeKind.REWORK;
- }
+ boolean commitMessageChanged = !next.getFullMessage().equals(prior.getFullMessage());
if (isSameDeltaAndTree(rw, prior, next)) {
- return ChangeKind.NO_CHANGE;
+ return commitMessageChanged ? ChangeKind.NO_CODE_CHANGE : ChangeKind.NO_CHANGE;
}
if (prior.getParentCount() == 0 || next.getParentCount() == 0) {
@@ -243,9 +238,11 @@
if (merger.merge(next.getParent(0), prior)
&& merger.getResultTreeId().equals(next.getTree())) {
if (prior.getParentCount() == 1) {
- return ChangeKind.TRIVIAL_REBASE;
+ return commitMessageChanged
+ ? ChangeKind.TRIVIAL_REBASE_WITH_MESSAGE_UPDATE
+ : ChangeKind.TRIVIAL_REBASE;
}
- return ChangeKind.MERGE_FIRST_PARENT_UPDATE;
+ return commitMessageChanged ? ChangeKind.REWORK : ChangeKind.MERGE_FIRST_PARENT_UPDATE;
}
} catch (LargeObjectException e) {
// Some object is too large for the merge attempt to succeed. Assume
diff --git a/java/com/google/gerrit/server/git/receive/ReplaceOp.java b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
index c0ffde3..2036078 100644
--- a/java/com/google/gerrit/server/git/receive/ReplaceOp.java
+++ b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
@@ -453,6 +453,8 @@
+ ".";
case TRIVIAL_REBASE:
return ": Patch Set " + priorPatchSetId.get() + " was rebased.";
+ case TRIVIAL_REBASE_WITH_MESSAGE_UPDATE:
+ return ": Patch Set " + priorPatchSetId.get() + " was rebased. Commit message was updated.";
case NO_CHANGE:
return ": New patch set was added with same tree, parent "
+ (commit.getParentCount() != 1 ? "trees" : "tree")
diff --git a/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java b/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java
index 01537e0..0c451ac 100644
--- a/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java
+++ b/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java
@@ -20,13 +20,33 @@
import static com.google.gerrit.server.cache.testing.CacheSerializerTestUtil.byteString;
import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.client.ChangeKind;
import com.google.gerrit.proto.testing.SerializedClassSubject;
import com.google.gerrit.server.cache.proto.Cache.ChangeKindKeyProto;
import com.google.gerrit.server.cache.serialize.CacheSerializer;
+import com.google.gerrit.server.change.ChangeKindCacheImpl.NoCache;
+import com.google.gerrit.testing.InMemoryRepositoryManager;
+import com.google.gerrit.testing.InMemoryRepositoryManager.Repo;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.junit.Before;
import org.junit.Test;
public class ChangeKindCacheImplTest {
+ private InMemoryRepositoryManager repoManager;
+ private ChangeKindCache changeKindCache;
+
+ @Before
+ public void setUp() {
+ repoManager = new InMemoryRepositoryManager();
+ // For simplicity, we use non-caching version, and as long as we call the method that doesn't
+ // use ChangeData, we can provide null instead of constructing a factory.
+ changeKindCache = new NoCache(new Config(), null, repoManager);
+ }
+
@Test
public void keySerializer() throws Exception {
ChangeKindCacheImpl.Key key =
@@ -57,4 +77,134 @@
ImmutableMap.of(
"prior", ObjectId.class, "next", ObjectId.class, "strategyName", String.class));
}
+
+ @Test
+ public void commitMessageChanged() throws Exception {
+ TestRepository<Repo> p = newRepo("p");
+ RevCommit root = p.commit().create();
+ RevCommit firstRev = p.commit().parent(root).message("Commit message").create();
+ RevCommit secondRev = p.commit().parent(root).message("Commit message update").create();
+
+ assertThat(
+ changeKindCache.getChangeKind(
+ p.getRepository().getDescription().getProject(), null, null, firstRev, secondRev))
+ .isEqualTo(ChangeKind.NO_CODE_CHANGE);
+ }
+
+ @Test
+ public void sameObject_noChange() throws Exception {
+ TestRepository<Repo> p = newRepo("p");
+ RevCommit root = p.commit().create();
+ RevCommit rev =
+ p.commit().parent(root).message("Commit message").add("test.md", "Hello").create();
+
+ assertThat(
+ changeKindCache.getChangeKind(
+ p.getRepository().getDescription().getProject(), null, null, rev, rev))
+ .isEqualTo(ChangeKind.NO_CHANGE);
+ }
+
+ @Test
+ public void sameContent_noChange() throws Exception {
+ TestRepository<Repo> p = newRepo("p");
+ RevCommit root = p.commit().create();
+ RevCommit firstRev =
+ p.commit().parent(root).message("Commit message").add("test.md", "Hello").create();
+ RevCommit secondRev =
+ p.commit().parent(root).message("Commit message").add("test.md", "Hello").create();
+
+ assertThat(
+ changeKindCache.getChangeKind(
+ p.getRepository().getDescription().getProject(), null, null, firstRev, secondRev))
+ .isEqualTo(ChangeKind.NO_CHANGE);
+ }
+
+ @Test
+ public void contentChanged_rework() throws Exception {
+ TestRepository<Repo> p = newRepo("p");
+ RevCommit root = p.commit().create();
+ RevCommit firstRev =
+ p.commit().parent(root).message("Commit message").add("test.md", "Hello").create();
+ RevCommit secondRev =
+ p.commit().parent(root).message("Commit message").add("test.md", "Goodbye").create();
+
+ assertThat(
+ changeKindCache.getChangeKind(
+ p.getRepository().getDescription().getProject(), null, null, firstRev, secondRev))
+ .isEqualTo(ChangeKind.REWORK);
+ }
+
+ @Test
+ public void mergeConflict_rework() throws Exception {
+ // Delete a change in one of the parents
+ TestRepository<Repo> p = newRepo("p");
+ RevCommit root = p.commit().add("foo", "foo-text").create();
+ RevCommit firstRev =
+ p.commit().parent(root).message("Commit message").add("foo", "bar-text").create();
+ // File was deleted, but the commit is still writing new content to it.
+ RevCommit newRoot = p.commit().parent(root).rm("foo").create();
+ RevCommit secondRev =
+ p.commit().parent(newRoot).message("Commit message").add("foo", "bar-text").create();
+
+ assertThat(
+ changeKindCache.getChangeKind(
+ p.getRepository().getDescription().getProject(), null, null, firstRev, secondRev))
+ .isEqualTo(ChangeKind.REWORK);
+ }
+
+ @Test
+ public void rebaseThenEdit_rework() throws Exception {
+ // Delete a change in one of the parents
+ TestRepository<Repo> p = newRepo("p");
+ RevCommit root = p.commit().add("foo", "foo-text").create();
+ RevCommit firstRev =
+ p.commit().parent(root).message("Commit message").add("foo", "bar-text").create();
+ // Unrelated file was added.
+ RevCommit newRoot = p.commit().parent(root).add("baz", "baz-text").create();
+ RevCommit secondRev =
+ p.commit().parent(newRoot).message("Commit message").add("foo", "foobar-text").create();
+
+ assertThat(
+ changeKindCache.getChangeKind(
+ p.getRepository().getDescription().getProject(), null, null, firstRev, secondRev))
+ .isEqualTo(ChangeKind.REWORK);
+ }
+
+ @Test
+ public void trivialRebase() throws Exception {
+ TestRepository<Repo> p = newRepo("p");
+ RevCommit root = p.commit().add("foo", "foo-text").create();
+ RevCommit firstRev =
+ p.commit().parent(root).message("Commit message").add("foo", "bar-text").create();
+ // Unrelated file was added.
+ RevCommit newRoot = p.commit().parent(root).add("baz", "baz-text").create();
+ RevCommit secondRev =
+ p.commit().parent(newRoot).message("Commit message").add("foo", "bar-text").create();
+
+ assertThat(
+ changeKindCache.getChangeKind(
+ p.getRepository().getDescription().getProject(), null, null, firstRev, secondRev))
+ .isEqualTo(ChangeKind.TRIVIAL_REBASE);
+ }
+
+ @Test
+ public void trivialRebaseCommitMessage() throws Exception {
+ TestRepository<Repo> p = newRepo("p");
+ RevCommit root = p.commit().add("foo", "foo-text").create();
+ RevCommit firstRev =
+ p.commit().parent(root).message("Commit message").add("foo", "bar-text").create();
+ // Unrelated file was added.
+ RevCommit newRoot = p.commit().parent(root).add("baz", "baz-text").create();
+ RevCommit secondRev =
+ p.commit().parent(newRoot).message("Commit subject").add("foo", "bar-text").create();
+
+ assertThat(
+ changeKindCache.getChangeKind(
+ p.getRepository().getDescription().getProject(), null, null, firstRev, secondRev))
+ .isEqualTo(ChangeKind.TRIVIAL_REBASE_WITH_MESSAGE_UPDATE);
+ }
+
+ private TestRepository<Repo> newRepo(String name) throws Exception {
+ return new TestRepository<>(repoManager.createRepository(Project.nameKey(name)));
+ }
}
diff --git a/polygerrit-ui/app/api/rest-api.ts b/polygerrit-ui/app/api/rest-api.ts
index 134cdd3..c354d2e 100644
--- a/polygerrit-ui/app/api/rest-api.ts
+++ b/polygerrit-ui/app/api/rest-api.ts
@@ -174,6 +174,7 @@
export enum RevisionKind {
REWORK = 'REWORK',
TRIVIAL_REBASE = 'TRIVIAL_REBASE',
+ TRIVIAL_REBASE_WITH_MESSAGE_UPDATE = 'TRIVIAL_REBASE_WITH_MESSAGE_UPDATE',
MERGE_FIRST_PARENT_UPDATE = 'MERGE_FIRST_PARENT_UPDATE',
NO_CODE_CHANGE = 'NO_CODE_CHANGE',
NO_CHANGE = 'NO_CHANGE',
