]> source.dussan.org Git - jgit.git/commitdiff
Revert "Adds FilteredRevCommit that can overwrites its parents in the DAG." 57/194257/1
authorRonald Bhuleskar <funronald@google.com>
Wed, 3 Aug 2022 21:16:34 +0000 (17:16 -0400)
committerRonald Bhuleskar <funronald@google.com>
Wed, 3 Aug 2022 21:16:34 +0000 (17:16 -0400)
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.test/tst/org/eclipse/jgit/api/LogFilterTest.java
org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FilteredRevCommitTest.java [deleted file]
org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FilteredRevWalkTest.java [deleted file]
org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FirstParentRevWalkTest.java
org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkPathFilter1Test.java
org.eclipse.jgit/src/org/eclipse/jgit/revwalk/FilteredRevCommit.java [deleted file]
org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RewriteGenerator.java

index 46095b384860e21e6d0e19684ece85f4f7d1466b..fa9c742f70deb5ef8532f6a59f53835306b6f65f 100644 (file)
@@ -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 (file)
index c8f3c0c..0000000
+++ /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<InMemoryRepository> 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 (file)
index 7c24898..0000000
+++ /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<FileRepository> 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());
-       }
-}
index d8d2c2d2b056057b3338f164727d959eb440e111..c8256b89c0af091a421560acca98b41247abafd2 100644 (file)
@@ -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());
        }
 }
index 4bc7b0a2e4f85f02010132c091e52ec21a26c997..5cce11aa1f7561b62f1c99c0aa41976d5cad1db1 100644 (file)
@@ -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 (file)
index be6e57f..0000000
+++ /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 (<b>NOTE - THIS IS NOT A COPY</b>).
-        *
-        * <p>
-        * 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;
-       }
-}
index 3318a97a3f63ca2b6522028a59ac29f5fd606f97..2c88bb872e9c3c4f201641fb52664fe7738a4f75 100644 (file)
@@ -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;
        }