From e5e46bc53637f1e1227e81863d403e9fbb84aa19 Mon Sep 17 00:00:00 2001 From: Ronald Bhuleskar Date: Wed, 3 Aug 2022 17:16:34 -0400 Subject: [PATCH] Revert "Adds FilteredRevCommit that can overwrites its parents in the DAG." This reverts commit ceb51a5e0e9db166e572ea7cd362795b4662b0cd. Reason for revert: The change in https://git.eclipse.org/r/c/jgit/jgit/+/194354 broke the egit test [1]. Calling c.getShortMessage() causes an NPE. [1] https://ci.eclipse.org/egit/job/egit.gerrit/2711/ Change-Id: I411565b6eaa0bbb562cc1c8a355942ff09fd29bc --- .../org/eclipse/jgit/api/LogFilterTest.java | 7 - .../jgit/revwalk/FilteredRevCommitTest.java | 137 ------------------ .../jgit/revwalk/FilteredRevWalkTest.java | 117 --------------- .../jgit/revwalk/FirstParentRevWalkTest.java | 21 ++- .../jgit/revwalk/RevWalkPathFilter1Test.java | 16 +- .../jgit/revwalk/FilteredRevCommit.java | 115 --------------- .../jgit/revwalk/RewriteGenerator.java | 6 +- 7 files changed, 21 insertions(+), 398 deletions(-) delete mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FilteredRevCommitTest.java delete mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FilteredRevWalkTest.java delete mode 100644 org.eclipse.jgit/src/org/eclipse/jgit/revwalk/FilteredRevCommit.java diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/LogFilterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/LogFilterTest.java index 46095b3848..fa9c742f70 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/LogFilterTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/LogFilterTest.java @@ -20,7 +20,6 @@ import java.util.Iterator; import org.eclipse.jgit.junit.RepositoryTestCase; import org.eclipse.jgit.revwalk.RevCommit; -import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.util.FileUtils; import org.junit.After; import org.junit.Before; @@ -32,13 +31,10 @@ import org.junit.Test; public class LogFilterTest extends RepositoryTestCase { private Git git; - private RevWalk rw; - @Before public void setup() throws Exception { super.setUp(); git = new Git(db); - rw = new RevWalk(db); // create first file File file = new File(db.getWorkTree(), "a.txt"); @@ -102,7 +98,6 @@ public class LogFilterTest extends RepositoryTestCase { public void testLogWithFilterCanDistinguishFilesByPath() throws Exception { int count = 0; for (RevCommit c : git.log().addPath("a.txt").call()) { - rw.parseHeaders(c); assertEquals("commit1", c.getFullMessage()); count++; } @@ -110,7 +105,6 @@ public class LogFilterTest extends RepositoryTestCase { count = 0; for (RevCommit c : git.log().addPath("b.txt").call()) { - rw.parseHeaders(c); assertEquals("commit2", c.getFullMessage()); count++; } @@ -121,7 +115,6 @@ public class LogFilterTest extends RepositoryTestCase { public void testLogWithFilterCanIncludeFilesInDirectory() throws Exception { int count = 0; for (RevCommit c : git.log().addPath("subdir-include").call()) { - rw.parseHeaders(c); assertEquals("commit3", c.getFullMessage()); count++; } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FilteredRevCommitTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FilteredRevCommitTest.java deleted file mode 100644 index c8f3c0c7d8..0000000000 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FilteredRevCommitTest.java +++ /dev/null @@ -1,137 +0,0 @@ -/* - * Copyright (C) 2022, Google LLC. - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Distribution License v. 1.0 which is available at - * https://www.eclipse.org/org/documents/edl-v10.php. - * - * SPDX-License-Identifier: BSD-3-Clause - */ - -package org.eclipse.jgit.revwalk; - -import static java.nio.charset.StandardCharsets.UTF_8; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertSame; - -import java.util.Arrays; - -import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription; -import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; -import org.eclipse.jgit.junit.TestRepository; -import org.eclipse.jgit.lib.AnyObjectId; -import org.eclipse.jgit.lib.ObjectLoader; -import org.junit.Before; -import org.junit.Test; - -public class FilteredRevCommitTest { - private TestRepository tr; - - private RevWalk rw; - - @Before - public void setUp() throws Exception { - tr = new TestRepository<>( - new InMemoryRepository(new DfsRepositoryDescription("test"))); - rw = tr.getRevWalk(); - } - - @Test - public void testParseBody_noParent() throws Exception { - RevCommit root = tr.commit().add("todelete", "to be deleted").create(); - RevCommit orig = tr.commit().parent(root).rm("todelete") - .add("foo", "foo contents").add("bar", "bar contents") - .add("dir/baz", "baz contents").create(); - FilteredRevCommit filteredRevCommit = new FilteredRevCommit(orig); - filteredRevCommit.parseBody(rw); - tr.branch("master").update(filteredRevCommit); - assertEquals("foo contents", blobAsString(filteredRevCommit, "foo")); - assertEquals("bar contents", blobAsString(filteredRevCommit, "bar")); - assertEquals("baz contents", - blobAsString(filteredRevCommit, "dir/baz")); - } - - @Test - public void testParseBody_withParents() throws Exception { - RevCommit commit1 = tr.commit().add("foo", "foo contents\n").create(); - RevCommit commit2 = tr.commit().parent(commit1) - .message("original message").add("bar", "bar contents") - .create(); - RevCommit commit3 = tr.commit().parent(commit2).message("commit3") - .add("foo", "foo contents\n new line\n").create(); - - FilteredRevCommit filteredCommitHead = new FilteredRevCommit(commit3, - commit1); - - rw.parseBody(filteredCommitHead); - assertEquals(commit1, Arrays.stream(filteredCommitHead.getParents()) - .findFirst().get()); - assertEquals("commit3", filteredCommitHead.getFullMessage()); - assertEquals("foo contents\n new line\n", - blobAsString(filteredCommitHead, "foo")); - } - - @Test - public void testParseHeader_withParents() throws Exception { - RevCommit commit1 = tr.commit().add("foo", "foo contents\n").create(); - RevCommit commit2 = tr.commit().parent(commit1) - .message("original message").add("bar", "bar contents") - .create(); - RevCommit commit3 = tr.commit().parent(commit2).message("commit3") - .add("foo", "foo contents\n new line\n").create(); - - FilteredRevCommit filteredCommitHead = new FilteredRevCommit(commit3, - commit1); - - assertNull(filteredCommitHead.getTree()); - rw.parseHeaders(filteredCommitHead); - assertEquals(filteredCommitHead.getTree(), commit3.getTree()); - } - - @Test - public void testParseCommit_withParents_parsesRealParents() - throws Exception { - RevCommit commit1 = tr.commit().add("foo", "foo contents\n").create(); - RevCommit commit2 = tr.commit().parent(commit1) - .message("original message").add("bar", "bar contents") - .create(); - RevCommit commit3 = tr.commit().parent(commit2).message("commit3") - .add("foo", "foo contents\n new line\n").create(); - - FilteredRevCommit filteredCommitHead = new FilteredRevCommit(commit3, - commit1); - - RevCommit parsedCommit = rw.parseCommit(filteredCommitHead.getId()); - assertEquals(filteredCommitHead.getId(), commit3.getId()); - // This is an intended behavior as revWalk#parseCommit doesn't parse - // through the overridden parents rather uses the real parents. - assertNotEquals( - Arrays.stream(parsedCommit.getParents()).findFirst().get(), - Arrays.stream(filteredCommitHead.getParents()).findFirst() - .get()); - } - - @Test - public void testFlag() throws Exception { - RevCommit root = tr.commit().add("todelete", "to be deleted").create(); - RevCommit orig = tr.commit().parent(root).rm("todelete") - .add("foo", "foo contents").add("bar", "bar contents") - .add("dir/baz", "baz contents").create(); - - FilteredRevCommit filteredRevCommit = new FilteredRevCommit(orig, root); - assertEquals(RevObject.PARSED, orig.flags); - assertEquals(0, filteredRevCommit.flags); - filteredRevCommit.parseBody(rw); - assertEquals(RevObject.PARSED, filteredRevCommit.flags); - } - - private String blobAsString(AnyObjectId treeish, String path) - throws Exception { - RevObject obj = tr.get(rw.parseTree(treeish), path); - assertSame(RevBlob.class, obj.getClass()); - ObjectLoader loader = rw.getObjectReader().open(obj); - return new String(loader.getCachedBytes(), UTF_8); - } -} diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FilteredRevWalkTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FilteredRevWalkTest.java deleted file mode 100644 index 7c24898dad..0000000000 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FilteredRevWalkTest.java +++ /dev/null @@ -1,117 +0,0 @@ -/* - * Copyright (C) 2022, Google LLC. - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Distribution License v. 1.0 which is available at - * https://www.eclipse.org/org/documents/edl-v10.php. - * - * SPDX-License-Identifier: BSD-3-Clause - */ - -package org.eclipse.jgit.revwalk; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; - -import org.eclipse.jgit.internal.storage.file.FileRepository; -import org.eclipse.jgit.junit.TestRepository; -import org.junit.Before; -import org.junit.Test; - -public class FilteredRevWalkTest extends RevWalkTestCase { - private TestRepository repository; - - @Override - @Before - public void setUp() throws Exception { - super.setUp(); - repository = new TestRepository<>(db); - } - - @Test - public void testWalk() throws Exception { - writeTrashFile("a.txt", "content"); - repository.git().add().addFilepattern("a.txt").call(); - RevCommit c1 = repository.git().commit().setMessage("first commit") - .call(); - - writeTrashFile("b.txt", "new file added"); - repository.git().add().addFilepattern("b.txt").call(); - repository.git().commit().setMessage("second commit").call(); - - writeTrashFile("a.txt", "content added"); - repository.git().add().addFilepattern("a.txt").call(); - RevCommit c3 = repository.git().commit().setMessage("third commit") - .call(); - - RevWalk revWalk = repository.getRevWalk(); - FilteredRevCommit filteredRevCommit = new FilteredRevCommit(c3, c1); - - revWalk.markStart(filteredRevCommit); - assertEquals(c3, revWalk.next()); - assertEquals(c1, revWalk.next()); - } - - @Test - public void testParseBody() throws Exception { - writeTrashFile("a.txt", "content"); - repository.git().add().addFilepattern("a.txt").call(); - RevCommit c1 = repository.git().commit().setMessage("first commit") - .call(); - - writeTrashFile("b.txt", "new file added"); - repository.git().add().addFilepattern("b.txt").call(); - repository.git().commit().setMessage("second commit").call(); - - writeTrashFile("a.txt", "content added"); - repository.git().add().addFilepattern("a.txt").call(); - RevCommit c3 = repository.git().commit().setMessage("third commit") - .call(); - - RevWalk revWalk = repository.getRevWalk(); - FilteredRevCommit filteredRevCommit = new FilteredRevCommit(c3, c1); - - assertNull(filteredRevCommit.getTree()); - - revWalk.parseBody(filteredRevCommit); - assertNotNull(filteredRevCommit.getTree()); - assertNotNull(filteredRevCommit.getFullMessage()); - } - - /** - * Test that the uninteresting flag is carried over correctly. Every commit - * should have the uninteresting flag resulting in a RevWalk returning no - * commit. - * - * @throws Exception - */ - @Test - public void testRevWalkCarryUninteresting() throws Exception { - writeTrashFile("a.txt", "content"); - repository.git().add().addFilepattern("a.txt").call(); - RevCommit c1 = repository.git().commit().setMessage("first commit") - .call(); - - writeTrashFile("b.txt", "new file added"); - repository.git().add().addFilepattern("b.txt").call(); - RevCommit c2 = repository.git().commit().setMessage("second commit") - .call(); - - writeTrashFile("a.txt", "content added"); - repository.git().add().addFilepattern("a.txt").call(); - RevCommit c3 = repository.git().commit().setMessage("third commit") - .call(); - - RevWalk revWalk = repository.getRevWalk(); - FilteredRevCommit filteredCommit1 = new FilteredRevCommit(c1); - FilteredRevCommit filteredCommit2 = new FilteredRevCommit(c2, - filteredCommit1); - FilteredRevCommit filteredCommit3 = new FilteredRevCommit(c3, - filteredCommit2); - - revWalk.markStart(filteredCommit2); - markUninteresting(filteredCommit3); - assertNull("Found an unexpected commit", rw.next()); - } -} diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FirstParentRevWalkTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FirstParentRevWalkTest.java index d8d2c2d2b0..c8256b89c0 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FirstParentRevWalkTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FirstParentRevWalkTest.java @@ -423,10 +423,9 @@ public class FirstParentRevWalkTest extends RevWalkTestCase { rw.sort(RevSort.TOPO, true); rw.setTreeFilter(PathFilterGroup.createFromStrings("0")); markStart(d); - - assertEquals(d, rw.next()); - assertEquals(c, rw.next()); - assertEquals(b, rw.next()); + assertCommit(d, rw.next()); + assertCommit(c, rw.next()); + assertCommit(b, rw.next()); assertNull(rw.next()); } @@ -442,8 +441,8 @@ public class FirstParentRevWalkTest extends RevWalkTestCase { rw.sort(RevSort.TOPO, true); rw.setTreeFilter(PathFilterGroup.createFromStrings("0")); markStart(d); - assertEquals(d, rw.next()); - assertEquals(c, rw.next()); + assertCommit(d, rw.next()); + assertCommit(c, rw.next()); assertNull(rw.next()); } @@ -459,9 +458,9 @@ public class FirstParentRevWalkTest extends RevWalkTestCase { rw.sort(RevSort.TOPO_KEEP_BRANCH_TOGETHER, true); rw.setTreeFilter(PathFilterGroup.createFromStrings("0")); markStart(d); - assertEquals(d, rw.next()); - assertEquals(c, rw.next()); - assertEquals(b, rw.next()); + assertCommit(d, rw.next()); + assertCommit(c, rw.next()); + assertCommit(b, rw.next()); assertNull(rw.next()); } @@ -477,8 +476,8 @@ public class FirstParentRevWalkTest extends RevWalkTestCase { rw.sort(RevSort.TOPO_KEEP_BRANCH_TOGETHER, true); rw.setTreeFilter(PathFilterGroup.createFromStrings("0")); markStart(d); - assertEquals(d, rw.next()); - assertEquals(c, rw.next()); + assertCommit(d, rw.next()); + assertCommit(c, rw.next()); assertNull(rw.next()); } } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkPathFilter1Test.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkPathFilter1Test.java index 4bc7b0a2e4..5cce11aa1f 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkPathFilter1Test.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkPathFilter1Test.java @@ -23,8 +23,8 @@ import org.junit.Test; public class RevWalkPathFilter1Test extends RevWalkTestCase { protected void filter(String path) { - rw.setTreeFilter(AndTreeFilter.create( - PathFilterGroup.createFromStrings(Collections.singleton(path)), + rw.setTreeFilter(AndTreeFilter.create(PathFilterGroup + .createFromStrings(Collections.singleton(path)), TreeFilter.ANY_DIFF)); } @@ -87,7 +87,7 @@ public class RevWalkPathFilter1Test extends RevWalkTestCase { filter("d/f"); markStart(c); - assertEquals(c, rw.next()); + assertCommit(c, rw.next()); assertEquals(1, c.getParentCount()); assertCommit(a, c.getParent(0)); // b was skipped @@ -125,7 +125,7 @@ public class RevWalkPathFilter1Test extends RevWalkTestCase { markStart(d); // d was skipped - assertEquals(c, rw.next()); + assertCommit(c, rw.next()); assertEquals(1, c.getParentCount()); assertCommit(a, c.getParent(0)); // b was skipped @@ -136,7 +136,7 @@ public class RevWalkPathFilter1Test extends RevWalkTestCase { @Test public void testStringOfPearls_FilePath2_NoParentRewriting() - throws Exception { + throws Exception { final RevCommit a = commit(tree(file("d/f", blob("a")))); final RevCommit b = commit(tree(file("d/f", blob("a"))), a); final RevCommit c = commit(tree(file("d/f", blob("b"))), b); @@ -166,7 +166,7 @@ public class RevWalkPathFilter1Test extends RevWalkTestCase { markStart(d); // d was skipped - assertEquals(c, rw.next()); + assertCommit(c, rw.next()); assertEquals(1, c.getParentCount()); assertCommit(a, c.getParent(0)); // b was skipped @@ -211,11 +211,11 @@ public class RevWalkPathFilter1Test extends RevWalkTestCase { filter("d/f"); markStart(i); - assertEquals(i, rw.next()); + assertCommit(i, rw.next()); assertEquals(1, i.getParentCount()); assertCommit(c, i.getParent(0)); // h..d was skipped - assertEquals(c, rw.next()); + assertCommit(c, rw.next()); assertEquals(1, c.getParentCount()); assertCommit(a, c.getParent(0)); // b was skipped diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/FilteredRevCommit.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/FilteredRevCommit.java deleted file mode 100644 index be6e57fd5f..0000000000 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/FilteredRevCommit.java +++ /dev/null @@ -1,115 +0,0 @@ -/* - * Copyright (C) 2022, Google LLC. - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Distribution License v. 1.0 which is available at - * https://www.eclipse.org/org/documents/edl-v10.php. - * - * SPDX-License-Identifier: BSD-3-Clause - */ - -package org.eclipse.jgit.revwalk; - -import org.eclipse.jgit.lib.ObjectId; - -/** A filtered commit reference that overrides its parent in the DAG. */ -public class FilteredRevCommit extends RevCommit { - private RevCommit[] overriddenParents; - - /** - * Create a new commit reference for the given id. - * - * @param id - * object name for the commit. - */ - public FilteredRevCommit(ObjectId id) { - this(id, NO_PARENTS); - } - - /** - * Create a new commit reference wrapping an underlying commit reference. - * - * @param commit - * commit that is being wrapped - */ - public FilteredRevCommit(RevCommit commit) { - this(commit, NO_PARENTS); - } - - /** - * Create a new commit reference wrapping an underlying commit reference. - * - * @param commit - * commit that is being wrapped - * @param parents - * overridden parents for the commit - */ - public FilteredRevCommit(RevCommit commit, RevCommit... parents) { - this(commit.getId(), parents); - } - - /** - * Create a new commit reference wrapping an underlying commit reference. - * - * @param id - * object name for the commit. - * @param parents - * overridden parents for the commit - */ - public FilteredRevCommit(ObjectId id, RevCommit... parents) { - super(id); - this.overriddenParents = parents; - } - - /** - * Update parents on the commit - * - * @param overriddenParents - * parents to be overwritten - */ - public void setParents(RevCommit... overriddenParents) { - this.overriddenParents = overriddenParents; - } - - /** - * Get the number of parent commits listed in this commit. - * - * @return number of parents; always a positive value but can be 0 if it has - * no parents. - */ - @Override - public int getParentCount() { - return overriddenParents.length; - } - - /** - * Get the nth parent from this commit's parent list. - * - * @param nth - * parent index to obtain. Must be in the range 0 through - * {@link #getParentCount()}-1. - * @return the specified parent. - * @throws java.lang.ArrayIndexOutOfBoundsException - * an invalid parent index was specified. - */ - @Override - public RevCommit getParent(int nth) { - return overriddenParents[nth]; - } - - /** - * Obtain an array of all parents (NOTE - THIS IS NOT A COPY). - * - *

- * This method is exposed only to provide very fast, efficient access to - * this commit's parent list. Applications relying on this list should be - * very careful to ensure they do not modify its contents during their use - * of it. - * - * @return the array of parents. - */ - @Override - public RevCommit[] getParents() { - return overriddenParents; - } -} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RewriteGenerator.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RewriteGenerator.java index 3318a97a3f..2c88bb872e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RewriteGenerator.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RewriteGenerator.java @@ -79,9 +79,9 @@ class RewriteGenerator extends Generator { final RevCommit newp = rewrite(oldp); if (firstParent) { if (newp == null) { - c = new FilteredRevCommit(c.getId()); + c.parents = RevCommit.NO_PARENTS; } else { - c = new FilteredRevCommit(c.getId(), newp); + c.parents = new RevCommit[] { newp }; } return c; } @@ -91,7 +91,7 @@ class RewriteGenerator extends Generator { } } if (rewrote) { - c = new FilteredRevCommit(c.getId(), cleanup(pList)); + c.parents = cleanup(pList); } return c; } -- 2.39.5