Merge changes Ia2cc9a7b,I7c1d0a3e * changes: Pass commit-graph positions to TagSet and ChangeNotes commits Use the commit-graph for mergeability checks
diff --git a/java/com/google/gerrit/server/change/MergeabilityCacheImpl.java b/java/com/google/gerrit/server/change/MergeabilityCacheImpl.java index 8ffd40a..79618bf 100644 --- a/java/com/google/gerrit/server/change/MergeabilityCacheImpl.java +++ b/java/com/google/gerrit/server/change/MergeabilityCacheImpl.java
@@ -199,6 +199,8 @@ return true; // Assume yes on new branch. } try (CodeReviewRevWalk rw = CodeReviewCommit.newRevWalk(repo)) { + // Dry runs do not need message bodies. + rw.setRetainBody(false); return submitDryRun.run( null, key.submitType,
diff --git a/java/com/google/gerrit/server/git/CodeReviewCommit.java b/java/com/google/gerrit/server/git/CodeReviewCommit.java index 9c05763..2074863 100644 --- a/java/com/google/gerrit/server/git/CodeReviewCommit.java +++ b/java/com/google/gerrit/server/git/CodeReviewCommit.java
@@ -77,8 +77,8 @@ } @Override - protected CodeReviewCommit createCommit(AnyObjectId id) { - return new CodeReviewCommit(id); + protected CodeReviewCommit createCommit(AnyObjectId id, int graphPos) { + return new CodeReviewCommit(id, graphPos); } @Override @@ -153,6 +153,10 @@ super(id); } + private CodeReviewCommit(AnyObjectId id, int graphPosition) { + super(id, graphPosition); + } + public ChangeNotes notes() { return notes; }
diff --git a/java/com/google/gerrit/server/git/TagSet.java b/java/com/google/gerrit/server/git/TagSet.java index 24a2dec..6ffb370 100644 --- a/java/com/google/gerrit/server/git/TagSet.java +++ b/java/com/google/gerrit/server/git/TagSet.java
@@ -509,8 +509,8 @@ } @Override - protected TagCommit createCommit(AnyObjectId id) { - return new TagCommit(id); + protected TagCommit createCommit(AnyObjectId id, int graphPos) { + return new TagCommit(id, graphPos); } } @@ -520,8 +520,8 @@ /** CachedRef.flag => isVisible, indicating if this commit is reachable from the ref. */ RoaringBitmap refFlags; - TagCommit(AnyObjectId id) { - super(id); + TagCommit(AnyObjectId id, int graphPos) { + super(id, graphPos); } /**
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesCommit.java b/java/com/google/gerrit/server/notedb/ChangeNotesCommit.java index df60789..a7fac73 100644 --- a/java/com/google/gerrit/server/notedb/ChangeNotesCommit.java +++ b/java/com/google/gerrit/server/notedb/ChangeNotesCommit.java
@@ -75,8 +75,8 @@ } @Override - protected ChangeNotesCommit createCommit(AnyObjectId id) { - return new ChangeNotesCommit(id); + protected ChangeNotesCommit createCommit(AnyObjectId id, int graphPos) { + return new ChangeNotesCommit(id, graphPos); } @Override @@ -117,6 +117,10 @@ super(id); } + private ChangeNotesCommit(AnyObjectId id, int graphPosition) { + super(id, graphPosition); + } + private void initFooterLines() { if (footerLines == null) { footerLines = getFooterLines();
diff --git a/java/com/google/gerrit/server/query/change/ConflictsPredicate.java b/java/com/google/gerrit/server/query/change/ConflictsPredicate.java index 9676a03..8dda1c4 100644 --- a/java/com/google/gerrit/server/query/change/ConflictsPredicate.java +++ b/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
@@ -214,6 +214,8 @@ ObjectId other = changeData.currentPatchSet().commitId(); try (Repository repo = args.repoManager.openRepository(otherChange.getProject()); CodeReviewCommit.CodeReviewRevWalk rw = CodeReviewCommit.newRevWalk(repo)) { + // Dry runs do not need message bodies. + rw.setRetainBody(false); // Only check whether the other change itself conflicts with the tested change. Whether the // other change's own dependencies are open or already merged is irrelevant for this, so // treat its parents as already accepted. Since all ancestors of already accepted commits
diff --git a/javatests/com/google/gerrit/server/git/CodeReviewCommitGraphTest.java b/javatests/com/google/gerrit/server/git/CodeReviewCommitGraphTest.java new file mode 100644 index 0000000..32d3e5b --- /dev/null +++ b/javatests/com/google/gerrit/server/git/CodeReviewCommitGraphTest.java
@@ -0,0 +1,119 @@ +// Copyright (C) 2026 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.git; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk; +import java.io.IOException; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicInteger; +import org.eclipse.jgit.internal.storage.commitgraph.CommitGraph; +import org.eclipse.jgit.internal.storage.file.FileRepository; +import org.eclipse.jgit.internal.storage.file.GC; +import org.eclipse.jgit.junit.TestRepository; +import org.eclipse.jgit.lib.AnyObjectId; +import org.eclipse.jgit.lib.ConfigConstants; +import org.eclipse.jgit.lib.ObjectLoader; +import org.eclipse.jgit.lib.ObjectReader; +import org.eclipse.jgit.lib.StoredConfig; +import org.eclipse.jgit.revwalk.RevCommit; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +/** Tests that {@link CodeReviewRevWalk} parses commits from the commit-graph when it can. */ +public class CodeReviewCommitGraphTest { + @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + private FileRepository repo; + private TestRepository<FileRepository> tr; + private RevCommit tip; + + @Before + public void setUp() throws Exception { + repo = new FileRepository(temporaryFolder.newFolder("repo.git")); + repo.create(true); + tr = new TestRepository<>(repo); + RevCommit c1 = tr.commit().message("c1").create(); + RevCommit c2 = tr.commit().parent(c1).message("c2").create(); + tip = tr.commit().parent(c2).message("c3").create(); + tr.update("refs/heads/master", tip); + } + + @Test + public void commitsAreParsedFromCommitGraph() throws Exception { + writeCommitGraph(); + assertThat(objectsOpenedWalkingFromTip()).isEqualTo(0); + } + + @Test + public void commitsAreParsedWithoutCommitGraph() throws Exception { + assertThat(objectsOpenedWalkingFromTip()).isEqualTo(3); + } + + private int objectsOpenedWalkingFromTip() throws Exception { + CountingReader reader = new CountingReader(repo.newObjectReader()); + try (CodeReviewRevWalk rw = CodeReviewCommit.newRevWalk(reader)) { + rw.setRetainBody(false); + rw.markStart(rw.lookupCommit(tip)); + int commits = 0; + while (rw.next() != null) { + commits++; + } + assertThat(commits).isEqualTo(3); + return reader.opens.get(); + } + } + + private void writeCommitGraph() throws Exception { + StoredConfig cfg = repo.getConfig(); + cfg.setBoolean(ConfigConstants.CONFIG_CORE_SECTION, null, "commitGraph", true); + cfg.setBoolean( + ConfigConstants.CONFIG_GC_SECTION, + null, + ConfigConstants.CONFIG_KEY_WRITE_COMMIT_GRAPH, + true); + cfg.save(); + new GC(repo).gc().get(); + } + + /** Reader that counts object loads and keeps exposing the commit-graph of the wrapped reader. */ + private static class CountingReader extends ObjectReader.Filter { + final AtomicInteger opens = new AtomicInteger(); + private final ObjectReader delegate; + + CountingReader(ObjectReader delegate) { + this.delegate = delegate; + } + + @Override + protected ObjectReader delegate() { + return delegate; + } + + @Override + public ObjectLoader open(AnyObjectId objectId, int typeHint) throws IOException { + opens.incrementAndGet(); + return super.open(objectId, typeHint); + } + + @Override + public Optional<CommitGraph> getCommitGraph() throws IOException { + return delegate.getCommitGraph(); + } + } +}
diff --git a/javatests/com/google/gerrit/server/git/TagSetTest.java b/javatests/com/google/gerrit/server/git/TagSetTest.java index ed68a3f..9402b45 100644 --- a/javatests/com/google/gerrit/server/git/TagSetTest.java +++ b/javatests/com/google/gerrit/server/git/TagSetTest.java
@@ -42,22 +42,32 @@ import java.util.Comparator; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import org.eclipse.jgit.internal.storage.commitgraph.CommitGraph; import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; +import org.eclipse.jgit.internal.storage.file.FileRepository; +import org.eclipse.jgit.internal.storage.file.GC; import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.lib.AnyObjectId; +import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectIdOwnerMap; import org.eclipse.jgit.lib.ObjectLoader; import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.revwalk.RevCommit; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; import org.roaringbitmap.RoaringBitmap; public class TagSetTest { + @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); + @Test public void roundTripToProto() { HashMap<String, CachedRef> refs = new HashMap<>(); @@ -128,6 +138,61 @@ } @Test + public void buildParsesCommitsFromCommitGraph() throws Exception { + AtomicInteger reads = new AtomicInteger(); + FileRepository repo = + new FileRepository(temporaryFolder.newFolder("project.git")) { + @Override + public ObjectReader newObjectReader() { + ObjectReader delegate = super.newObjectReader(); + return new ObjectReader.Filter() { + @Override + protected ObjectReader delegate() { + return delegate; + } + + @Override + public ObjectLoader open(AnyObjectId objectId) throws IOException { + return open(objectId, OBJ_ANY); + } + + @Override + public ObjectLoader open(AnyObjectId objectId, int typeHint) throws IOException { + reads.incrementAndGet(); + return super.open(objectId, typeHint); + } + + @Override + public Optional<CommitGraph> getCommitGraph() throws IOException { + return delegate.getCommitGraph(); + } + }; + } + }; + repo.create(true); + try (TestRepository<FileRepository> tr = new TestRepository<>(repo)) { + RevCommit c1 = tr.commit().create(); + RevCommit c2 = tr.commit().parent(c1).create(); + tr.update("refs/heads/master", tr.commit().parent(c2).create()); + tr.update("refs/tags/v1", c1); + } + StoredConfig cfg = repo.getConfig(); + cfg.setBoolean(ConfigConstants.CONFIG_CORE_SECTION, null, "commitGraph", true); + cfg.setBoolean( + ConfigConstants.CONFIG_GC_SECTION, + null, + ConfigConstants.CONFIG_KEY_WRITE_COMMIT_GRAPH, + true); + cfg.save(); + new GC(repo).gc().get(); + + reads.set(0); + assertThat(new TagSet(Project.nameKey("project")).build(repo, null, null)).isTrue(); + // Only the branch and tag tips are read; the commit between them comes from the graph. + assertThat(reads.get()).isEqualTo(2); + } + + @Test public void buildStopsReadingObjectsWhenInterrupted() throws Exception { AtomicInteger interruptedReads = new AtomicInteger(); // Mimics JGit's file-based object reader, which throws InterruptedIOException for every