From ee6334bccf6d81b1bf513c44897e2a63b6300400 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Tue, 6 Sep 2022 00:07:17 +0200 Subject: Revert "Option to pass start RevCommit to be blamed on to the BlameGenerator." This reverts commit 5747bba48b22a11beba8ebe0caf13a53d4ca96f2. This is done as a quick fix for the failure of egit tests caused by the introduction of FilteredRevCommit. Bug: 580690 Change-Id: Ia0178bc2de4fc825a81207bbd7979bf3a386c955 --- .../eclipse/jgit/api/blame/BlameGeneratorTest.java | 329 ++------------------- 1 file changed, 26 insertions(+), 303 deletions(-) (limited to 'org.eclipse.jgit.test/tst/org') diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/blame/BlameGeneratorTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/blame/BlameGeneratorTest.java index b175ead8ec..f47f447375 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/blame/BlameGeneratorTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/blame/BlameGeneratorTest.java @@ -13,330 +13,56 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import java.util.Iterator; - import org.eclipse.jgit.api.Git; import org.eclipse.jgit.blame.BlameGenerator; import org.eclipse.jgit.blame.BlameResult; import org.eclipse.jgit.junit.RepositoryTestCase; import org.eclipse.jgit.lib.Constants; -import org.eclipse.jgit.merge.MergeStrategy; -import org.eclipse.jgit.revwalk.FilteredRevCommit; import org.eclipse.jgit.revwalk.RevCommit; -import org.eclipse.jgit.revwalk.RevWalk; import org.junit.Test; /** Unit tests of {@link BlameGenerator}. */ public class BlameGeneratorTest extends RepositoryTestCase { - - public static final String OTHER_FILE = "other_file.txt"; - - public static final String INTERESTING_FILE = "interesting_file.txt"; - @Test - public void testSingleBlame() throws Exception { - - /** - *
-		 * (ts) 	OTHER_FILE			INTERESTING_FILE
-		 * 1 		a
-		 * 2	 	a, b
-		 * 3							1, 2				c1 <--
-		 * 4	 	a, b, c										 |
-		 * 5							1, 2, 3				c2---
-		 * 
- */ - try (Git git = new Git(db); - RevWalk revWalk = new RevWalk(git.getRepository())) { - writeTrashFile(OTHER_FILE, join("a")); - git.add().addFilepattern(OTHER_FILE).call(); - git.commit().setMessage("create file").call(); - - writeTrashFile(OTHER_FILE, join("a", "b")); - git.add().addFilepattern(OTHER_FILE).call(); - git.commit().setMessage("amend file").call(); - - writeTrashFile(INTERESTING_FILE, join("1", "2")); - git.add().addFilepattern(INTERESTING_FILE).call(); + public void testBoundLineDelete() throws Exception { + try (Git git = new Git(db)) { + String[] content1 = new String[] { "first", "second" }; + writeTrashFile("file.txt", join(content1)); + git.add().addFilepattern("file.txt").call(); RevCommit c1 = git.commit().setMessage("create file").call(); - writeTrashFile(OTHER_FILE, join("a", "b", "c")); - git.add().addFilepattern(OTHER_FILE).call(); - git.commit().setMessage("amend file").call(); - - writeTrashFile(INTERESTING_FILE, join("1", "2", "3")); - git.add().addFilepattern(INTERESTING_FILE).call(); - RevCommit c2 = git.commit().setMessage("amend file").call(); - - RevCommit filteredC1 = new FilteredRevCommit(c1); - RevCommit filteredC2 = new FilteredRevCommit(c2, filteredC1); - - revWalk.parseHeaders(filteredC2); + String[] content2 = new String[] { "third", "first", "second" }; + writeTrashFile("file.txt", join(content2)); + git.add().addFilepattern("file.txt").call(); + RevCommit c2 = git.commit().setMessage("create file").call(); - try (BlameGenerator generator = new BlameGenerator(db, - INTERESTING_FILE)) { - generator.push(filteredC2); + try (BlameGenerator generator = new BlameGenerator(db, "file.txt")) { + generator.push(null, db.resolve(Constants.HEAD)); assertEquals(3, generator.getResultContents().size()); assertTrue(generator.next()); assertEquals(c2, generator.getSourceCommit()); assertEquals(1, generator.getRegionLength()); - assertEquals(2, generator.getResultStart()); - assertEquals(3, generator.getResultEnd()); - assertEquals(2, generator.getSourceStart()); - assertEquals(3, generator.getSourceEnd()); - assertEquals(INTERESTING_FILE, generator.getSourcePath()); - - assertTrue(generator.next()); - assertEquals(c1, generator.getSourceCommit()); - assertEquals(2, generator.getRegionLength()); assertEquals(0, generator.getResultStart()); - assertEquals(2, generator.getResultEnd()); + assertEquals(1, generator.getResultEnd()); assertEquals(0, generator.getSourceStart()); - assertEquals(2, generator.getSourceEnd()); - assertEquals(INTERESTING_FILE, generator.getSourcePath()); - - assertFalse(generator.next()); - } - } - } - - @Test - public void testMergeSingleBlame() throws Exception { - try (Git git = new Git(db); - RevWalk revWalk = new RevWalk(git.getRepository())) { - - /** - * - * - *
-			 *  refs/heads/master
-			 *      A
-			 *     / \       		 refs/heads/side
-			 *    /   ---------------->  side
-			 *   /                        |
-			 *  merge <-------------------
-			 * 
- */ - - writeTrashFile(INTERESTING_FILE, join("1", "2")); - git.add().addFilepattern(INTERESTING_FILE).call(); - RevCommit c1 = git.commit().setMessage("create file").call(); - - createBranch(c1, "refs/heads/side"); - checkoutBranch("refs/heads/side"); - writeTrashFile(INTERESTING_FILE, join("1", "2", "3", "4")); - git.add().addFilepattern(INTERESTING_FILE).call(); - RevCommit sideCommit = git.commit() - .setMessage("amend file in another branch").call(); - - checkoutBranch("refs/heads/master"); - git.merge().setMessage("merge").include(sideCommit) - .setStrategy(MergeStrategy.RESOLVE).call(); - - Iterator it = git.log().call().iterator(); - RevCommit mergeCommit = it.next(); - - RevCommit filteredC1 = new FilteredRevCommit(c1); - RevCommit filteredSide = new FilteredRevCommit(sideCommit, - filteredC1); - RevCommit filteredMerge = new FilteredRevCommit(mergeCommit, - filteredSide, filteredC1); - - revWalk.parseHeaders(filteredMerge); - - try (BlameGenerator generator = new BlameGenerator(db, - INTERESTING_FILE)) { - generator.push(filteredMerge); - assertEquals(4, generator.getResultContents().size()); - - assertTrue(generator.next()); - assertEquals(mergeCommit, generator.getSourceCommit()); - assertEquals(2, generator.getRegionLength()); - assertEquals(2, generator.getResultStart()); - assertEquals(4, generator.getResultEnd()); - assertEquals(2, generator.getSourceStart()); - assertEquals(4, generator.getSourceEnd()); - assertEquals(INTERESTING_FILE, generator.getSourcePath()); + assertEquals(1, generator.getSourceEnd()); + assertEquals("file.txt", generator.getSourcePath()); assertTrue(generator.next()); - assertEquals(filteredC1, generator.getSourceCommit()); + assertEquals(c1, generator.getSourceCommit()); assertEquals(2, generator.getRegionLength()); - assertEquals(0, generator.getResultStart()); - assertEquals(2, generator.getResultEnd()); - assertEquals(0, generator.getSourceStart()); - assertEquals(2, generator.getSourceEnd()); - assertEquals(INTERESTING_FILE, generator.getSourcePath()); - - assertFalse(generator.next()); - } - } - } - - @Test - public void testMergeBlame() throws Exception { - try (Git git = new Git(db); - RevWalk revWalk = new RevWalk(git.getRepository())) { - - /** - * - * - *
-			 *  refs/heads/master
-			 *      A
-			 *     / \       		 refs/heads/side
-			 *    B   ---------------->  side
-			 *   /                        |
-			 *  merge <-------------------
-			 * 
- */ - writeTrashFile(INTERESTING_FILE, join("1", "2")); - git.add().addFilepattern(INTERESTING_FILE).call(); - RevCommit c1 = git.commit().setMessage("create file").call(); - - createBranch(c1, "refs/heads/side"); - checkoutBranch("refs/heads/side"); - writeTrashFile(INTERESTING_FILE, join("1", "2", "3")); - git.add().addFilepattern(INTERESTING_FILE).call(); - RevCommit sideCommit = git.commit().setMessage("amend file").call(); - - checkoutBranch("refs/heads/master"); - writeTrashFile(INTERESTING_FILE, join("1", "2", "4")); - git.add().addFilepattern(INTERESTING_FILE).call(); - RevCommit c2 = git.commit().setMessage("delete and amend file") - .call(); - - git.merge().setMessage("merge").include(sideCommit) - .setStrategy(MergeStrategy.RESOLVE).call(); - writeTrashFile(INTERESTING_FILE, join("1", "2", "3", "4")); - git.add().addFilepattern(INTERESTING_FILE).call(); - RevCommit mergeCommit = git.commit().setMessage("merge commit") - .call(); - - RevCommit filteredC1 = new FilteredRevCommit(c1); - RevCommit filteredSide = new FilteredRevCommit(sideCommit, - filteredC1); - RevCommit filteredC2 = new FilteredRevCommit(c2, filteredC1); - - RevCommit filteredMerge = new FilteredRevCommit(mergeCommit, - filteredSide, filteredC2); - - revWalk.parseHeaders(filteredMerge); - - try (BlameGenerator generator = new BlameGenerator(db, - INTERESTING_FILE)) { - generator.push(filteredMerge); - assertEquals(4, generator.getResultContents().size()); - - assertTrue(generator.next()); - assertEquals(filteredC2, generator.getSourceCommit()); - assertEquals(1, generator.getRegionLength()); - assertEquals(3, generator.getResultStart()); - assertEquals(4, generator.getResultEnd()); - assertEquals(2, generator.getSourceStart()); - assertEquals(3, generator.getSourceEnd()); - assertEquals(INTERESTING_FILE, generator.getSourcePath()); - - assertTrue(generator.next()); - assertEquals(filteredSide, generator.getSourceCommit()); - assertEquals(1, generator.getRegionLength()); - assertEquals(2, generator.getResultStart()); + assertEquals(1, generator.getResultStart()); assertEquals(3, generator.getResultEnd()); - assertEquals(2, generator.getSourceStart()); - assertEquals(3, generator.getSourceEnd()); - assertEquals(INTERESTING_FILE, generator.getSourcePath()); - - assertTrue(generator.next()); - assertEquals(filteredC1, generator.getSourceCommit()); - assertEquals(2, generator.getRegionLength()); - assertEquals(0, generator.getResultStart()); - assertEquals(2, generator.getResultEnd()); assertEquals(0, generator.getSourceStart()); assertEquals(2, generator.getSourceEnd()); - assertEquals(INTERESTING_FILE, generator.getSourcePath()); + assertEquals("file.txt", generator.getSourcePath()); assertFalse(generator.next()); } } } - @Test - public void testSingleBlame_compareWithWalk() throws Exception { - /** - *
-		 * (ts) 	OTHER_FILE			INTERESTING_FILE
-		 * 1 		a
-		 * 2	 	a, b
-		 * 3							1, 2				c1 <--
-		 * 4	 	a, b, c										 |
-		 * 6							3, 1, 2				c2---
-		 * 
- */ - try (Git git = new Git(db); - RevWalk revWalk = new RevWalk(git.getRepository())) { - writeTrashFile(OTHER_FILE, join("a")); - git.add().addFilepattern(OTHER_FILE).call(); - git.commit().setMessage("create file").call(); - - writeTrashFile(OTHER_FILE, join("a", "b")); - git.add().addFilepattern(OTHER_FILE).call(); - git.commit().setMessage("amend file").call(); - - writeTrashFile(INTERESTING_FILE, join("1", "2")); - git.add().addFilepattern(INTERESTING_FILE).call(); - RevCommit c1 = git.commit().setMessage("create file").call(); - - writeTrashFile(OTHER_FILE, join("a", "b", "c")); - git.add().addFilepattern(OTHER_FILE).call(); - git.commit().setMessage("amend file").call(); - - writeTrashFile(INTERESTING_FILE, join("3", "1", "2")); - git.add().addFilepattern(INTERESTING_FILE).call(); - RevCommit c2 = git.commit().setMessage("prepend").call(); - - RevCommit filteredC1 = new FilteredRevCommit(c1); - RevCommit filteredC2 = new FilteredRevCommit(c2, filteredC1); - - revWalk.parseHeaders(filteredC2); - - try (BlameGenerator g1 = new BlameGenerator(db, INTERESTING_FILE); - BlameGenerator g2 = new BlameGenerator(db, - INTERESTING_FILE)) { - g1.push(null, c2); - g2.push(null, filteredC2); - - assertEquals(g1.getResultContents().size(), - g2.getResultContents().size()); // 3 - - assertTrue(g1.next()); - assertTrue(g2.next()); - - assertEquals(g1.getSourceCommit(), g2.getSourceCommit()); // c2 - assertEquals(INTERESTING_FILE, g1.getSourcePath()); - assertEquals(g1.getRegionLength(), g2.getRegionLength()); // 1 - assertEquals(g1.getResultStart(), g2.getResultStart()); // 0 - assertEquals(g1.getResultEnd(), g2.getResultEnd()); // 1 - assertEquals(g1.getSourceStart(), g2.getSourceStart()); // 0 - assertEquals(g1.getSourceEnd(), g2.getSourceEnd()); // 1 - assertEquals(g1.getSourcePath(), g2.getSourcePath()); // INTERESTING_FILE - - assertTrue(g1.next()); - assertTrue(g2.next()); - - assertEquals(g1.getSourceCommit(), g2.getSourceCommit()); // c1 - assertEquals(g1.getRegionLength(), g2.getRegionLength()); // 2 - assertEquals(g1.getResultStart(), g2.getResultStart()); // 1 - assertEquals(g1.getResultEnd(), g2.getResultEnd()); // 3 - assertEquals(g1.getSourceStart(), g2.getSourceStart()); // 0 - assertEquals(g1.getSourceEnd(), g2.getSourceEnd()); // 2 - assertEquals(g1.getSourcePath(), g2.getSourcePath()); // INTERESTING_FILE - - assertFalse(g1.next()); - assertFalse(g2.next()); - } - } - } - @Test public void testRenamedBoundLineDelete() throws Exception { try (Git git = new Git(db)) { @@ -361,8 +87,7 @@ public class BlameGeneratorTest extends RepositoryTestCase { git.add().addFilepattern(FILENAME_2).call(); RevCommit c2 = git.commit().setMessage("change file2").call(); - try (BlameGenerator generator = new BlameGenerator(db, - FILENAME_2)) { + try (BlameGenerator generator = new BlameGenerator(db, FILENAME_2)) { generator.push(null, db.resolve(Constants.HEAD)); assertEquals(3, generator.getResultContents().size()); @@ -388,8 +113,7 @@ public class BlameGeneratorTest extends RepositoryTestCase { } // and test again with other BlameGenerator API: - try (BlameGenerator generator = new BlameGenerator(db, - FILENAME_2)) { + try (BlameGenerator generator = new BlameGenerator(db, FILENAME_2)) { generator.push(null, db.resolve(Constants.HEAD)); BlameResult result = generator.computeBlameResult(); @@ -412,22 +136,21 @@ public class BlameGeneratorTest extends RepositoryTestCase { try (Git git = new Git(db)) { String[] content1 = new String[] { "first", "second", "third" }; - writeTrashFile(INTERESTING_FILE, join(content1)); - git.add().addFilepattern(INTERESTING_FILE).call(); + writeTrashFile("file.txt", join(content1)); + git.add().addFilepattern("file.txt").call(); git.commit().setMessage("create file").call(); String[] content2 = new String[] { "" }; - writeTrashFile(INTERESTING_FILE, join(content2)); - git.add().addFilepattern(INTERESTING_FILE).call(); + writeTrashFile("file.txt", join(content2)); + git.add().addFilepattern("file.txt").call(); git.commit().setMessage("create file").call(); - writeTrashFile(INTERESTING_FILE, join(content1)); - git.add().addFilepattern(INTERESTING_FILE).call(); + writeTrashFile("file.txt", join(content1)); + git.add().addFilepattern("file.txt").call(); RevCommit c3 = git.commit().setMessage("create file").call(); - try (BlameGenerator generator = new BlameGenerator(db, - INTERESTING_FILE)) { + try (BlameGenerator generator = new BlameGenerator(db, "file.txt")) { generator.push(null, db.resolve(Constants.HEAD)); assertEquals(3, generator.getResultContents().size()); -- cgit v1.2.3 From 7c4a5421ccd16b84b1ea4f3c1e566bfdf9c0e30d Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Tue, 6 Sep 2022 00:06:04 +0200 Subject: Revert "Adds FilteredRevCommit that can overwrites its parents in the DAG." This reverts commit 6297491e8adb85e43d60ffe75fb71f335e733449. This is done as a quick fix for the failure of egit tests caused by the introduction of FilteredRevCommit. Bug: 580690 Change-Id: Ia6b651dd11b0a4b02d5e52247eb4bf13adf94e27 --- .../jgit/revwalk/FilteredRevCommitTest.java | 135 --------------------- .../eclipse/jgit/revwalk/FilteredRevWalkTest.java | 121 ------------------ .../jgit/revwalk/FirstParentRevWalkTest.java | 53 ++------ .../jgit/revwalk/RevWalkFollowFilterTest.java | 15 ++- .../jgit/revwalk/RevWalkPathFilter1Test.java | 102 +++++----------- .../eclipse/jgit/revwalk/FilteredRevCommit.java | 95 --------------- .../src/org/eclipse/jgit/revwalk/RevCommit.java | 18 +-- .../org/eclipse/jgit/revwalk/RewriteGenerator.java | 37 ++---- 8 files changed, 53 insertions(+), 523 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 (limited to 'org.eclipse.jgit.test/tst/org') 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 49ce47ef42..0000000000 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FilteredRevCommitTest.java +++ /dev/null @@ -1,135 +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 - * - * @since 6.3 - */ -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.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 testParseHeaders_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.parseHeaders(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 testParents() 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); - - assertEquals(commit1, Arrays.stream(filteredCommitHead.getParents()) - .findFirst().get()); - assertEquals("commit3", filteredCommitHead.getFullMessage()); - assertEquals("foo contents\n new line\n", - blobAsString(filteredCommitHead, "foo")); - assertEquals(filteredCommitHead.getTree(), commit3.getTree()); - - } - - @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(RevObject.PARSED, filteredRevCommit.flags); - } - - @Test - public void testCommitState() 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(filteredRevCommit.getParentCount(), 1); - assertSame(filteredRevCommit.getRawBuffer(), orig.getRawBuffer()); - assertSame(filteredRevCommit.getTree(), orig.getTree()); - assertEquals(filteredRevCommit.getFullMessage(), orig.getFullMessage()); - assertEquals(filteredRevCommit.commitTime, orig.commitTime); - assertSame(filteredRevCommit.parents, RevCommit.NO_PARENTS); - } - - @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()); - } - - 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 b1f8c0c0e9..0000000000 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FilteredRevWalkTest.java +++ /dev/null @@ -1,121 +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.assertNull; -import static org.junit.Assert.assertSame; - -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(); - - FilteredRevCommit filteredRevCommit = new FilteredRevCommit(c3, c1); - filteredRevCommit.disposeBody(); - - RevWalk revWalk = repository.getRevWalk(); - - revWalk.parseBody(filteredRevCommit); - assertEquals(filteredRevCommit.getFullMessage(), c3.getFullMessage()); - assertEquals(filteredRevCommit.getShortMessage(), c3.getShortMessage()); - assertEquals(filteredRevCommit.commitTime, c3.commitTime); - assertSame(filteredRevCommit.getTree(), c3.getTree()); - assertSame(filteredRevCommit.parents, RevCommit.NO_PARENTS); - - } - - /** - * 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 146d16953c..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 @@ -12,7 +12,6 @@ package org.eclipse.jgit.revwalk; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.revwalk.filter.MessageRevFilter; @@ -424,41 +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()); - assertNull(rw.next()); - } - - @Test - public void testWithTopoSortAndTreeFilter_shouldUseFilteredRevCommits() - throws Exception { - RevCommit a = commit(); - RevCommit b = commit(tree(file("0", blob("b"))), a); - RevCommit c = commit(tree(file("0", blob("c"))), b, a); - RevCommit d = commit(tree(file("0", blob("d"))), c); - - rw.reset(); - rw.setFirstParent(true); - rw.sort(RevSort.TOPO, true); - rw.setTreeFilter(PathFilterGroup.createFromStrings("0")); - markStart(d); - - RevCommit x = rw.next(); - assertTrue(x instanceof FilteredRevCommit); - assertEquals(1, x.getParentCount()); - assertEquals(c, x.getParent(0)); - - RevCommit y = rw.next(); - assertTrue(y instanceof FilteredRevCommit); - assertEquals(1, y.getParentCount()); - assertEquals(b, y.getParent(0)); - - RevCommit z = rw.next(); - assertTrue(z instanceof FilteredRevCommit); - assertEquals(0, z.getParentCount()); - + assertCommit(d, rw.next()); + assertCommit(c, rw.next()); + assertCommit(b, rw.next()); assertNull(rw.next()); } @@ -474,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()); } @@ -491,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()); } @@ -509,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/RevWalkFollowFilterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkFollowFilterTest.java index 20478ef709..c62136e64d 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkFollowFilterTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkFollowFilterTest.java @@ -9,7 +9,6 @@ */ package org.eclipse.jgit.revwalk; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import java.util.ArrayList; @@ -56,7 +55,7 @@ public class RevWalkFollowFilterTest extends RevWalkTestCase { final RevCommit a = commit(tree(file("0", blob("0")))); follow("0"); markStart(a); - assertEquals(a, rw.next()); + assertCommit(a, rw.next()); assertNull(rw.next()); assertNoRenames(); @@ -73,8 +72,8 @@ public class RevWalkFollowFilterTest extends RevWalkTestCase { follow("b"); markStart(renameCommit); - assertEquals(renameCommit, rw.next()); - assertEquals(a, rw.next()); + assertCommit(renameCommit, rw.next()); + assertCommit(a, rw.next()); assertNull(rw.next()); assertRenames("a->b"); @@ -102,10 +101,10 @@ public class RevWalkFollowFilterTest extends RevWalkTestCase { follow("a"); markStart(renameCommit3); - assertEquals(renameCommit3, rw.next()); - assertEquals(renameCommit2, rw.next()); - assertEquals(renameCommit1, rw.next()); - assertEquals(a, rw.next()); + assertCommit(renameCommit3, rw.next()); + assertCommit(renameCommit2, rw.next()); + assertCommit(renameCommit1, rw.next()); + assertCommit(a, rw.next()); assertNull(rw.next()); assertRenames("c->a", "b->c", "a->b"); 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 d933a6fc72..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 @@ -11,7 +11,6 @@ package org.eclipse.jgit.revwalk; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import java.util.Collections; @@ -24,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)); } @@ -50,7 +49,7 @@ public class RevWalkPathFilter1Test extends RevWalkTestCase { final RevCommit a = commit(tree(file("0", blob("0")))); filter("0"); markStart(a); - assertEquals(a, rw.next()); + assertCommit(a, rw.next()); assertNull(rw.next()); } @@ -73,10 +72,10 @@ public class RevWalkPathFilter1Test extends RevWalkTestCase { final RevCommit d = commit(tree(file("0", blob("d"))), c); filter("0"); markStart(d); - assertEquals(d, rw.next()); - assertEquals(c, rw.next()); - assertEquals(b, rw.next()); - assertEquals(a, rw.next()); + assertCommit(d, rw.next()); + assertCommit(c, rw.next()); + assertCommit(b, rw.next()); + assertCommit(a, rw.next()); assertNull(rw.next()); } @@ -88,11 +87,11 @@ public class RevWalkPathFilter1Test extends RevWalkTestCase { filter("d/f"); markStart(c); - assertEquals(c, rw.next()); + assertCommit(c, rw.next()); assertEquals(1, c.getParentCount()); - assertEquals(a, c.getParent(0)); // b was skipped + assertCommit(a, c.getParent(0)); // b was skipped - assertEquals(a, rw.next()); + assertCommit(a, rw.next()); assertEquals(0, a.getParentCount()); assertNull(rw.next()); } @@ -107,11 +106,11 @@ public class RevWalkPathFilter1Test extends RevWalkTestCase { markStart(c); rw.setRewriteParents(false); - assertEquals(c, rw.next()); + assertCommit(c, rw.next()); assertEquals(1, c.getParentCount()); - assertEquals(b, c.getParent(0)); + assertCommit(b, c.getParent(0)); - assertEquals(a, rw.next()); // b was skipped + assertCommit(a, rw.next()); // b was skipped assertEquals(0, a.getParentCount()); assertNull(rw.next()); } @@ -126,18 +125,18 @@ public class RevWalkPathFilter1Test extends RevWalkTestCase { markStart(d); // d was skipped - assertEquals(c, rw.next()); + assertCommit(c, rw.next()); assertEquals(1, c.getParentCount()); - assertEquals(a, c.getParent(0)); // b was skipped + assertCommit(a, c.getParent(0)); // b was skipped - assertEquals(a, rw.next()); + assertCommit(a, rw.next()); assertEquals(0, a.getParentCount()); assertNull(rw.next()); } @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); @@ -147,12 +146,12 @@ public class RevWalkPathFilter1Test extends RevWalkTestCase { rw.setRewriteParents(false); // d was skipped - assertEquals(c, rw.next()); + assertCommit(c, rw.next()); assertEquals(1, c.getParentCount()); - assertEquals(b, c.getParent(0)); + assertCommit(b, c.getParent(0)); // b was skipped - assertEquals(a, rw.next()); + assertCommit(a, rw.next()); assertEquals(0, a.getParentCount()); assertNull(rw.next()); } @@ -167,11 +166,11 @@ public class RevWalkPathFilter1Test extends RevWalkTestCase { markStart(d); // d was skipped - assertEquals(c, rw.next()); + assertCommit(c, rw.next()); assertEquals(1, c.getParentCount()); - assertEquals(a, c.getParent(0)); // b was skipped + assertCommit(a, c.getParent(0)); // b was skipped - assertEquals(a, rw.next()); + assertCommit(a, rw.next()); assertEquals(0, a.getParentCount()); assertNull(rw.next()); } @@ -212,15 +211,15 @@ public class RevWalkPathFilter1Test extends RevWalkTestCase { filter("d/f"); markStart(i); - assertEquals(i, rw.next()); + assertCommit(i, rw.next()); assertEquals(1, i.getParentCount()); - assertEquals(c, i.getParent(0)); // h..d was skipped + assertCommit(c, i.getParent(0)); // h..d was skipped - assertEquals(c, rw.next()); + assertCommit(c, rw.next()); assertEquals(1, c.getParentCount()); - assertEquals(a, c.getParent(0)); // b was skipped + assertCommit(a, c.getParent(0)); // b was skipped - assertEquals(a, rw.next()); + assertCommit(a, rw.next()); assertEquals(0, a.getParentCount()); assertNull(rw.next()); } @@ -274,49 +273,4 @@ public class RevWalkPathFilter1Test extends RevWalkTestCase { assertCommit(b, rw.next()); assertCommit(a, rw.next()); } - - @Test - public void testCommitHeaders_rewrittenParents() 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); - filter("d/f"); - markStart(c); - - RevCommit cBar = rw.next(); - assertNotNull(cBar.getShortMessage()); - assertEquals(cBar.getCommitTime(), c.getCommitTime()); - - RevCommit aBar = rw.next(); - assertNotNull(aBar.getShortMessage()); - assertEquals(aBar.getCommitTime(), a.getCommitTime()); - - assertNull(rw.next()); - } - - @Test - public void testFlags_rewrittenParents() 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); - - final RevFlag flag1 = rw.newFlag("flag1"); - final RevFlag flag2 = rw.newFlag("flag2"); - - a.add(flag1); - c.add(flag2); - - filter("d/f"); - markStart(c); - - RevCommit cBar = rw.next(); - assertEquals(cBar.flags & RevObject.PARSED, 1); - assertEquals(cBar.flags & flag2.mask, flag2.mask); - - RevCommit aBar = rw.next(); - assertEquals(aBar.flags & RevObject.PARSED, 1); - assertEquals(aBar.flags & flag1.mask, flag1.mask); - - assertNull(rw.next()); - } } 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 16beac3903..0000000000 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/FilteredRevCommit.java +++ /dev/null @@ -1,95 +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; - -/** - * A filtered commit reference that overrides its parent in the DAG. - * - * @since 6.3 - */ -public class FilteredRevCommit extends RevCommit { - private RevCommit[] overriddenParents; - - /** - * 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) { - super(commit); - this.overriddenParents = parents; - this.parents = NO_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/RevCommit.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java index a7c21e3f13..6b644cef90 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java @@ -114,7 +114,7 @@ public class RevCommit extends RevObject { * * @since 6.3 */ - RevCommit[] parents; + protected RevCommit[] parents; int commitTime; // An int here for performance, overflows in 2038 @@ -132,22 +132,6 @@ public class RevCommit extends RevObject { super(id); } - /** - * Create a new commit reference. - * - * @param orig - * commit to be copied from. - */ - RevCommit(RevCommit orig) { - super(orig.getId()); - this.buffer = orig.buffer; - this.commitTime = orig.commitTime; - this.flags = orig.flags; - this.parents = orig.parents; - this.tree = orig.tree; - this.inDegree = orig.inDegree; - } - @Override void parseHeaders(RevWalk walk) throws MissingObjectException, IncorrectObjectTypeException, IOException { 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 9ec331b697..2c88bb872e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RewriteGenerator.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RewriteGenerator.java @@ -11,8 +11,6 @@ package org.eclipse.jgit.revwalk; import java.io.IOException; -import java.util.HashMap; -import java.util.Map; import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.errors.MissingObjectException; @@ -40,13 +38,10 @@ class RewriteGenerator extends Generator { private final FIFORevQueue pending; - private final Map transformedCommits; - RewriteGenerator(Generator s) { super(s.firstParent); source = s; pending = new FIFORevQueue(s.firstParent); - transformedCommits = new HashMap<>(); } @Override @@ -63,10 +58,10 @@ class RewriteGenerator extends Generator { @Override RevCommit next() throws MissingObjectException, IncorrectObjectTypeException, IOException { - FilteredRevCommit c = (FilteredRevCommit) pending.next(); + RevCommit c = pending.next(); if (c == null) { - c = transform(source.next()); + c = source.next(); if (c == null) { // We are done: Both the source generator and our internal list // are completely exhausted. @@ -84,9 +79,9 @@ class RewriteGenerator extends Generator { final RevCommit newp = rewrite(oldp); if (firstParent) { if (newp == null) { - c.setParents(RevCommit.NO_PARENTS); + c.parents = RevCommit.NO_PARENTS; } else { - c.setParents(newp); + c.parents = new RevCommit[] { newp }; } return c; } @@ -96,7 +91,7 @@ class RewriteGenerator extends Generator { } } if (rewrote) { - c.setParents(cleanup(pList)); + c.parents = cleanup(pList); } return c; } @@ -116,7 +111,7 @@ class RewriteGenerator extends Generator { for (RevCommit parent : c.getParents()) { while ((parent.flags & RevWalk.TREE_REV_FILTER_APPLIED) == 0) { - FilteredRevCommit n = transform(source.next()); + RevCommit n = source.next(); if (n != null) { pending.add(n); @@ -135,8 +130,6 @@ class RewriteGenerator extends Generator { IncorrectObjectTypeException, IOException { for (;;) { - p = transform(p); - if (p.getParentCount() > 1) { // This parent is a merge, so keep it. // @@ -165,25 +158,9 @@ class RewriteGenerator extends Generator { } applyFilterToParents(p.getParent(0)); - p = transform(p.getParent(0)); - - } - } + p = p.getParent(0); - private FilteredRevCommit transform(RevCommit c) { - if (c == null) { - return null; } - - if (c instanceof FilteredRevCommit) { - return (FilteredRevCommit) c; - } - - if (!transformedCommits.containsKey(c)) { - transformedCommits.put(c, new FilteredRevCommit(c, c.getParents())); - } - - return transformedCommits.get(c); } private RevCommit[] cleanup(RevCommit[] oldList) { -- cgit v1.2.3 From a8e683fef6acf3e9f00ac2648fff60b13a28fb13 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Tue, 16 Aug 2022 01:02:21 +0200 Subject: [merge] Fix merge conflicts with symlinks Previous code would do a content merge on symlinks, and write the merge result to the working tree as a file. C git doesn't do this; it leaves a symlink in the working tree unchanged, or in a delete-modify conflict it would check out "theirs". Moreover, previous code would write the merge result to the link target, not to the link. This would overwrite an existing link target, or fail if the link pointed to a directory. In link/file conflicts or file/link conflicts, C git always puts the file into the working tree. Change conflict handling accordingly. Add tests for all the conflict cases. Bug: 580347 Change-Id: I3cffcb4bcf8e336a85186031fff23f0c4b6ee19d Signed-off-by: Thomas Wolf --- .../src/org/eclipse/jgit/junit/TestRepository.java | 18 ++ .../org/eclipse/jgit/merge/SymlinkMergeTest.java | 296 +++++++++++++++++++++ .../src/org/eclipse/jgit/merge/ResolveMerger.java | 168 ++++++++---- 3 files changed, 429 insertions(+), 53 deletions(-) create mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/SymlinkMergeTest.java (limited to 'org.eclipse.jgit.test/tst/org') diff --git a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java index 54e4a09ee5..483b9a7c81 100644 --- a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java +++ b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java @@ -275,6 +275,24 @@ public class TestRepository implements AutoCloseable { return e; } + /** + * Construct a symlink mode tree entry. + * + * @param path + * path of the symlink. + * @param blob + * a blob, previously constructed in the repository. + * @return the entry. + * @throws Exception + * @since 6.3 + */ + public DirCacheEntry link(String path, RevBlob blob) throws Exception { + DirCacheEntry e = new DirCacheEntry(path); + e.setFileMode(FileMode.SYMLINK); + e.setObjectId(blob); + return e; + } + /** * Construct a tree from a specific listing of file entries. * diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/SymlinkMergeTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/SymlinkMergeTest.java new file mode 100644 index 0000000000..3cdc8da34e --- /dev/null +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/SymlinkMergeTest.java @@ -0,0 +1,296 @@ +/* + * Copyright (C) 2022 Thomas Wolf and others + * + * 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.merge; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeTrue; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.LinkOption; + +import org.eclipse.jgit.api.Git; +import org.eclipse.jgit.api.MergeResult; +import org.eclipse.jgit.api.MergeResult.MergeStatus; +import org.eclipse.jgit.api.ResetCommand.ResetType; +import org.eclipse.jgit.junit.RepositoryTestCase; +import org.eclipse.jgit.junit.TestRepository; +import org.eclipse.jgit.lib.ConfigConstants; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.lib.StoredConfig; +import org.eclipse.jgit.revwalk.RevCommit; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; + +/** + * Tests for merges involving symlinks. + */ +@RunWith(Parameterized.class) +public class SymlinkMergeTest extends RepositoryTestCase { + + @Parameters(name = "target={0}, core.symlinks={1}") + public static Object[][] parameters() { + return new Object[][] { + { Target.NONE, Boolean.TRUE }, + { Target.FILE, Boolean.TRUE }, + { Target.DIRECTORY, Boolean.TRUE }, + { Target.NONE, Boolean.FALSE }, + { Target.FILE, Boolean.FALSE }, + { Target.DIRECTORY, Boolean.FALSE }, + }; + } + + public enum Target { + NONE, FILE, DIRECTORY + } + + @Parameter(0) + public Target target; + + @Parameter(1) + public boolean useSymLinks; + + private void setTargets() throws IOException { + switch (target) { + case DIRECTORY: + assertTrue(new File(trash, "target").mkdir()); + assertTrue(new File(trash, "target1").mkdir()); + assertTrue(new File(trash, "target2").mkdir()); + break; + case FILE: + writeTrashFile("target", "t"); + writeTrashFile("target1", "t1"); + writeTrashFile("target2", "t2"); + break; + default: + break; + } + } + + private void checkTargets() throws IOException { + File t = new File(trash, "target"); + File t1 = new File(trash, "target1"); + File t2 = new File(trash, "target2"); + switch (target) { + case DIRECTORY: + assertTrue(t.isDirectory()); + assertTrue(t1.isDirectory()); + assertTrue(t2.isDirectory()); + break; + case FILE: + checkFile(t, "t"); + checkFile(t1, "t1"); + checkFile(t2, "t2"); + break; + default: + assertFalse(Files.exists(t.toPath(), LinkOption.NOFOLLOW_LINKS)); + assertFalse(Files.exists(t1.toPath(), LinkOption.NOFOLLOW_LINKS)); + assertFalse(Files.exists(t2.toPath(), LinkOption.NOFOLLOW_LINKS)); + break; + } + } + + private void assertSymLink(File link, String content) throws Exception { + if (useSymLinks) { + assertTrue(Files.isSymbolicLink(link.toPath())); + assertEquals(content, db.getFS().readSymLink(link)); + } else { + assertFalse(Files.isSymbolicLink(link.toPath())); + assertTrue(link.isFile()); + checkFile(link, content); + } + } + + // Link/link conflict: C git records the conflict but leaves the link in the + // working tree unchanged. + + @Test + public void mergeWithSymlinkConflict() throws Exception { + assumeTrue(db.getFS().supportsSymlinks() || !useSymLinks); + StoredConfig config = db.getConfig(); + config.setBoolean(ConfigConstants.CONFIG_CORE_SECTION, null, + ConfigConstants.CONFIG_KEY_SYMLINKS, useSymLinks); + config.save(); + try (TestRepository repo = new TestRepository<>(db)) { + db.incrementOpen(); + // Create the links directly in the git repo, then use a hard reset + // to get them into the workspace. This enables us to run these + // tests also with core.symLinks = false. + RevCommit base = repo + .commit(repo.tree(repo.link("link", repo.blob("target")))); + RevCommit side = repo.commit( + repo.tree(repo.link("link", repo.blob("target1"))), base); + RevCommit head = repo.commit( + repo.tree(repo.link("link", repo.blob("target2"))), base); + try (Git git = new Git(db)) { + setTargets(); + git.reset().setMode(ResetType.HARD).setRef(head.name()).call(); + File link = new File(trash, "link"); + assertSymLink(link, "target2"); + MergeResult result = git.merge().include(side) + .setMessage("merged").call(); + assertEquals(MergeStatus.CONFLICTING, result.getMergeStatus()); + // Link should be unmodified + assertSymLink(link, "target2"); + checkTargets(); + assertEquals("[link, mode:120000, stage:1, content:target]" + + "[link, mode:120000, stage:2, content:target2]" + + "[link, mode:120000, stage:3, content:target1]", + indexState(CONTENT)); + } + } + } + + // In file/link conflicts, C git never does a content merge. It records the + // stages in the index, and always puts the file into the workspace. + + @Test + public void mergeWithFileSymlinkConflict() throws Exception { + assumeTrue(db.getFS().supportsSymlinks() || !useSymLinks); + StoredConfig config = db.getConfig(); + config.setBoolean(ConfigConstants.CONFIG_CORE_SECTION, null, + ConfigConstants.CONFIG_KEY_SYMLINKS, useSymLinks); + config.save(); + try (TestRepository repo = new TestRepository<>(db)) { + db.incrementOpen(); + RevCommit base = repo.commit(repo.tree()); + RevCommit side = repo.commit( + repo.tree(repo.link("link", repo.blob("target1"))), base); + RevCommit head = repo.commit( + repo.tree(repo.file("link", repo.blob("not a link"))), + base); + try (Git git = new Git(db)) { + setTargets(); + git.reset().setMode(ResetType.HARD).setRef(head.name()).call(); + File link = new File(trash, "link"); + assertFalse(Files.isSymbolicLink(link.toPath())); + checkFile(link, "not a link"); + MergeResult result = git.merge().include(side) + .setMessage("merged").call(); + assertEquals(MergeStatus.CONFLICTING, result.getMergeStatus()); + // File should be unmodified + assertFalse(Files.isSymbolicLink(link.toPath())); + checkFile(link, "not a link"); + checkTargets(); + assertEquals("[link, mode:100644, stage:2, content:not a link]" + + "[link, mode:120000, stage:3, content:target1]", + indexState(CONTENT)); + } + } + } + + @Test + public void mergeWithSymlinkFileConflict() throws Exception { + assumeTrue(db.getFS().supportsSymlinks() || !useSymLinks); + StoredConfig config = db.getConfig(); + config.setBoolean(ConfigConstants.CONFIG_CORE_SECTION, null, + ConfigConstants.CONFIG_KEY_SYMLINKS, useSymLinks); + config.save(); + try (TestRepository repo = new TestRepository<>(db)) { + db.incrementOpen(); + RevCommit base = repo.commit(repo.tree()); + RevCommit side = repo.commit( + repo.tree(repo.file("link", repo.blob("not a link"))), + base); + RevCommit head = repo.commit( + repo.tree(repo.link("link", repo.blob("target2"))), base); + try (Git git = new Git(db)) { + setTargets(); + git.reset().setMode(ResetType.HARD).setRef(head.name()).call(); + File link = new File(trash, "link"); + assertSymLink(link, "target2"); + MergeResult result = git.merge().include(side) + .setMessage("merged").call(); + assertEquals(MergeStatus.CONFLICTING, result.getMergeStatus()); + // Should now be a file! + assertFalse(Files.isSymbolicLink(link.toPath())); + checkFile(link, "not a link"); + checkTargets(); + assertEquals("[link, mode:120000, stage:2, content:target2]" + + "[link, mode:100644, stage:3, content:not a link]", + indexState(CONTENT)); + } + } + } + + // In Delete/modify conflicts with the non-deleted side a link, C git puts + // the link into the working tree. + + @Test + public void mergeWithSymlinkDeleteModify() throws Exception { + assumeTrue(db.getFS().supportsSymlinks() || !useSymLinks); + StoredConfig config = db.getConfig(); + config.setBoolean(ConfigConstants.CONFIG_CORE_SECTION, null, + ConfigConstants.CONFIG_KEY_SYMLINKS, useSymLinks); + config.save(); + try (TestRepository repo = new TestRepository<>(db)) { + db.incrementOpen(); + RevCommit base = repo + .commit(repo.tree(repo.link("link", repo.blob("target")))); + RevCommit side = repo.commit( + repo.tree(repo.link("link", repo.blob("target1"))), base); + RevCommit head = repo.commit(repo.tree(), base); + try (Git git = new Git(db)) { + setTargets(); + git.reset().setMode(ResetType.HARD).setRef(head.name()).call(); + File link = new File(trash, "link"); + assertFalse( + Files.exists(link.toPath(), LinkOption.NOFOLLOW_LINKS)); + MergeResult result = git.merge().include(side) + .setMessage("merged").call(); + assertEquals(MergeStatus.CONFLICTING, result.getMergeStatus()); + // Link should have the content from side + assertSymLink(link, "target1"); + checkTargets(); + assertEquals("[link, mode:120000, stage:1, content:target]" + + "[link, mode:120000, stage:3, content:target1]", + indexState(CONTENT)); + } + } + } + + @Test + public void mergeWithSymlinkModifyDelete() throws Exception { + assumeTrue(db.getFS().supportsSymlinks() || !useSymLinks); + StoredConfig config = db.getConfig(); + config.setBoolean(ConfigConstants.CONFIG_CORE_SECTION, null, + ConfigConstants.CONFIG_KEY_SYMLINKS, useSymLinks); + config.save(); + try (TestRepository repo = new TestRepository<>(db)) { + db.incrementOpen(); + RevCommit base = repo + .commit(repo.tree(repo.link("link", repo.blob("target")))); + RevCommit side = repo.commit(repo.tree(), base); + RevCommit head = repo.commit( + repo.tree(repo.link("link", repo.blob("target2"))), base); + try (Git git = new Git(db)) { + setTargets(); + git.reset().setMode(ResetType.HARD).setRef(head.name()).call(); + File link = new File(trash, "link"); + assertSymLink(link, "target2"); + MergeResult result = git.merge().include(side) + .setMessage("merged").call(); + assertEquals(MergeStatus.CONFLICTING, result.getMergeStatus()); + // Link should be unmodified + assertSymLink(link, "target2"); + checkTargets(); + assertEquals("[link, mode:120000, stage:1, content:target]" + + "[link, mode:120000, stage:2, content:target2]", + indexState(CONTENT)); + } + } + } +} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java index 23f8e4a5d5..8b9b569c38 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java @@ -3,7 +3,7 @@ * Copyright (C) 2010-2012, Matthias Sohn * Copyright (C) 2012, Research In Motion Limited * Copyright (C) 2017, Obeo (mathieu.cartaud@obeo.fr) - * Copyright (C) 2018, 2022 Thomas Wolf and others + * Copyright (C) 2018, 2022 Thomas Wolf and others * * This program and the accompanying materials are made available under the * terms of the Eclipse Distribution License v. 1.0 which is available at @@ -320,6 +320,25 @@ public class ResolveMerger extends ThreeWayMerger { return null; } + /** + * Adds the conflict stages for the current path of {@link #tw} to the index + * builder and returns the "theirs" stage; if present. + * + * @param base + * of the conflict + * @param ours + * of the conflict + * @param theirs + * of the conflict + * @return the {@link DirCacheEntry} for the "theirs" stage, or {@code null} + */ + private DirCacheEntry addConflict(CanonicalTreeParser base, + CanonicalTreeParser ours, CanonicalTreeParser theirs) { + add(tw.getRawPath(), base, DirCacheEntry.STAGE_1, EPOCH, 0); + add(tw.getRawPath(), ours, DirCacheEntry.STAGE_2, EPOCH, 0); + return add(tw.getRawPath(), theirs, DirCacheEntry.STAGE_3, EPOCH, 0); + } + /** * adds a entry to the index builder which is a copy of the specified * DirCacheEntry @@ -501,9 +520,7 @@ public class ResolveMerger extends ThreeWayMerger { // length. // This path can be skipped on ignoreConflicts, so the caller // could use virtual commit. - add(tw.getRawPath(), base, DirCacheEntry.STAGE_1, EPOCH, 0); - add(tw.getRawPath(), ours, DirCacheEntry.STAGE_2, EPOCH, 0); - add(tw.getRawPath(), theirs, DirCacheEntry.STAGE_3, EPOCH, 0); + addConflict(base, ours, theirs); unmergedPaths.add(tw.getPathString()); mergeResults.put(tw.getPathString(), new MergeResult<>(Collections.emptyList())); @@ -608,9 +625,7 @@ public class ResolveMerger extends ThreeWayMerger { add(tw.getRawPath(), ours, DirCacheEntry.STAGE_0, EPOCH, 0); return true; } else if (gitLinkMerging) { - add(tw.getRawPath(), base, DirCacheEntry.STAGE_1, EPOCH, 0); - add(tw.getRawPath(), ours, DirCacheEntry.STAGE_2, EPOCH, 0); - add(tw.getRawPath(), theirs, DirCacheEntry.STAGE_3, EPOCH, 0); + addConflict(base, ours, theirs); MergeResult result = createGitLinksMergeResult( base, ours, theirs); result.setContainsConflicts(true); @@ -631,9 +646,7 @@ public class ResolveMerger extends ThreeWayMerger { default: break; } - add(tw.getRawPath(), base, DirCacheEntry.STAGE_1, EPOCH, 0); - add(tw.getRawPath(), ours, DirCacheEntry.STAGE_2, EPOCH, 0); - add(tw.getRawPath(), theirs, DirCacheEntry.STAGE_3, EPOCH, 0); + addConflict(base, ours, theirs); // attribute merge issues are conflicts but not failures unmergedPaths.add(tw.getPathString()); @@ -646,30 +659,61 @@ public class ResolveMerger extends ThreeWayMerger { } MergeResult result = null; - try { - result = contentMerge(base, ours, theirs, attributes, - getContentMergeStrategy()); - } catch (BinaryBlobException e) { + boolean hasSymlink = FileMode.SYMLINK.equals(modeO) + || FileMode.SYMLINK.equals(modeT); + if (!hasSymlink) { + try { + result = contentMerge(base, ours, theirs, attributes, + getContentMergeStrategy()); + } catch (BinaryBlobException e) { + // result == null + } + } + if (result == null) { switch (getContentMergeStrategy()) { - case OURS: - keep(ourDce); - return true; - case THEIRS: - DirCacheEntry theirEntry = add(tw.getRawPath(), theirs, - DirCacheEntry.STAGE_0, EPOCH, 0); - addToCheckout(tw.getPathString(), theirEntry, attributes); - return true; - default: - result = new MergeResult<>(Collections.emptyList()); - result.setContainsConflicts(true); - break; + case OURS: + keep(ourDce); + return true; + case THEIRS: + DirCacheEntry e = add(tw.getRawPath(), theirs, + DirCacheEntry.STAGE_0, EPOCH, 0); + if (e != null) { + addToCheckout(tw.getPathString(), e, attributes); + } + return true; + default: + result = new MergeResult<>(Collections.emptyList()); + result.setContainsConflicts(true); + break; } } if (ignoreConflicts) { result.setContainsConflicts(false); } - updateIndex(base, ours, theirs, result, attributes[T_OURS]); String currentPath = tw.getPathString(); + if (hasSymlink) { + if (ignoreConflicts) { + if (((modeT & FileMode.TYPE_MASK) == FileMode.TYPE_FILE)) { + DirCacheEntry e = add(tw.getRawPath(), theirs, + DirCacheEntry.STAGE_0, EPOCH, 0); + addToCheckout(currentPath, e, attributes); + } else { + keep(ourDce); + } + } else { + // Record the conflict + DirCacheEntry e = addConflict(base, ours, theirs); + mergeResults.put(currentPath, result); + // If theirs is a file, check it out. In link/file + // conflicts, C git prefers the file. + if (((modeT & FileMode.TYPE_MASK) == FileMode.TYPE_FILE) + && e != null) { + addToCheckout(currentPath, e, attributes); + } + } + } else { + updateIndex(base, ours, theirs, result, attributes[T_OURS]); + } if (result.containsConflicts() && !ignoreConflicts) { unmergedPaths.add(currentPath); } @@ -683,40 +727,58 @@ public class ResolveMerger extends ThreeWayMerger { if (gitLinkMerging && ignoreConflicts) { add(tw.getRawPath(), ours, DirCacheEntry.STAGE_0, EPOCH, 0); } else if (gitLinkMerging) { - add(tw.getRawPath(), base, DirCacheEntry.STAGE_1, EPOCH, 0); - add(tw.getRawPath(), ours, DirCacheEntry.STAGE_2, EPOCH, 0); - add(tw.getRawPath(), theirs, DirCacheEntry.STAGE_3, EPOCH, 0); + addConflict(base, ours, theirs); MergeResult result = createGitLinksMergeResult( base, ours, theirs); result.setContainsConflicts(true); mergeResults.put(tw.getPathString(), result); unmergedPaths.add(tw.getPathString()); } else { + boolean isSymLink = ((modeO | modeT) + & FileMode.TYPE_MASK) == FileMode.TYPE_SYMLINK; // Content merge strategy does not apply to delete-modify // conflicts! MergeResult result; - try { - result = contentMerge(base, ours, theirs, attributes, - ContentMergeStrategy.CONFLICT); - } catch (BinaryBlobException e) { + if (isSymLink) { + // No need to do a content merge result = new MergeResult<>(Collections.emptyList()); result.setContainsConflicts(true); + } else { + try { + result = contentMerge(base, ours, theirs, + attributes, ContentMergeStrategy.CONFLICT); + } catch (BinaryBlobException e) { + result = new MergeResult<>(Collections.emptyList()); + result.setContainsConflicts(true); + } } if (ignoreConflicts) { - // In case a conflict is detected the working tree file - // is again filled with new content (containing conflict - // markers). But also stage 0 of the index is filled - // with that content. result.setContainsConflicts(false); - updateIndex(base, ours, theirs, result, - attributes[T_OURS]); + if (isSymLink) { + if (modeO != 0) { + keep(ourDce); + } else { + // Check out theirs + if (isWorktreeDirty(work, ourDce)) { + return false; + } + DirCacheEntry e = add(tw.getRawPath(), theirs, + DirCacheEntry.STAGE_0, EPOCH, 0); + if (e != null) { + addToCheckout(tw.getPathString(), e, + attributes); + } + } + } else { + // In case a conflict is detected the working tree + // file is again filled with new content (containing + // conflict markers). But also stage 0 of the index + // is filled with that content. + updateIndex(base, ours, theirs, result, + attributes[T_OURS]); + } } else { - add(tw.getRawPath(), base, DirCacheEntry.STAGE_1, EPOCH, - 0); - add(tw.getRawPath(), ours, DirCacheEntry.STAGE_2, EPOCH, - 0); - DirCacheEntry e = add(tw.getRawPath(), theirs, - DirCacheEntry.STAGE_3, EPOCH, 0); + DirCacheEntry e = addConflict(base, ours, theirs); // OURS was deleted checkout THEIRS if (modeO == 0) { @@ -862,25 +924,25 @@ public class ResolveMerger extends ThreeWayMerger { // A conflict occurred, the file will contain conflict markers // the index will be populated with the three stages and the // workdir (if used) contains the halfway merged content. - add(tw.getRawPath(), base, DirCacheEntry.STAGE_1, EPOCH, 0); - add(tw.getRawPath(), ours, DirCacheEntry.STAGE_2, EPOCH, 0); - add(tw.getRawPath(), theirs, DirCacheEntry.STAGE_3, EPOCH, 0); + addConflict(base, ours, theirs); mergeResults.put(tw.getPathString(), result); return; } // No conflict occurred, the file will contain fully merged content. // The index will be populated with the new merged version. - Instant lastModified = - mergedFile == null ? null : nonNullRepo().getFS().lastModifiedInstant(mergedFile); + Instant lastModified = mergedFile == null ? null + : nonNullRepo().getFS().lastModifiedInstant(mergedFile); // Set the mode for the new content. Fall back to REGULAR_FILE if // we can't merge modes of OURS and THEIRS. int newMode = mergeFileModes(tw.getRawMode(0), tw.getRawMode(1), tw.getRawMode(2)); FileMode mode = newMode == FileMode.MISSING.getBits() ? FileMode.REGULAR_FILE : FileMode.fromBits(newMode); - workTreeUpdater.insertToIndex(rawMerged.openInputStream(), tw.getPathString().getBytes(UTF_8), mode, - DirCacheEntry.STAGE_0, lastModified, (int) rawMerged.length(), + workTreeUpdater.insertToIndex(rawMerged.openInputStream(), + tw.getPathString().getBytes(UTF_8), mode, + DirCacheEntry.STAGE_0, lastModified, + (int) rawMerged.length(), attributes.get(Constants.ATTR_MERGE)); } finally { if (rawMerged != null) { -- cgit v1.2.3