motiejus
/
jgit
1
Fork 0
Code Releases Activity

Fix IndexDiffs for git links 

After cloning a repo with a submodule, non-recursively, JGit would
encounter in its TreeWalk in IndexDiff:

* first, a missing gitlink (in index & HEAD, not in working tree)
* second, the untracked folder (not in index and head, in working tree)

As a result, it would report the submodule as missing. Canonical git
reports a clean workspace.

The root cause of this is that the path of a gitlink "x" did
not compare equal to the path of a tree "x" in JGit.

Correct Paths.compare() to account for that. If two paths are otherwise
equal, then let gitlinks match both trees and files. Matching trees
solves the bug. Matching files is necessary to handle the case where
the gitlink directory was replaced by a file; see the new test case
IndexDiffSubmoduleTest.testSubmoduleReplacedByFile(). Comparisons of
unequal paths are left untouched, so the sort order is unchanged.

After the fix, another bug(?) in WorkingTreeIterator became apparent:
with core.dirNoGitLinks = true, it was no longer possible to overwrite
a gitlink in the index. This is now fixed in WorkingTreeIterator.

Add new test cases for the bug itself and for some related cases
(submodule directory deleted or replaced by a file) in
IndexDiffSubmoduleTest. Add a test for missing files in IndexDiffTest,
and adapt the PathsTest to test matching gitlinks.

Bug: 467631
Change-Id: I0549d10d46b1858e5eec3cc15095aa9f1d5f5280
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
This commit is contained in:
Thomas Wolf 2018-08-25 01:58:09 +02:00
parent 3f745e40a1
commit 4678f4b735
7 changed files with 129 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
org.eclipse.jgit.test/tst/org/eclipse/jgit/api/AddCommandTest.java
View File

@ -1245,7 +1245,8 @@ public void testAddGitlinkDoesNotChange() throws Exception {
ConfigConstants.CONFIG_KEY_DIRNOGITLINKS, true);
config.save();
assert (db.getConfig().get(WorkingTreeOptions.KEY).isDirNoGitLinks());
assertTrue(
db.getConfig().get(WorkingTreeOptions.KEY).isDirNoGitLinks());
try (Git git = new Git(db)) {
git.add().addFilepattern("nested-repo").call();

21
org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CleanCommandTest.java
View File

@ -233,6 +233,27 @@ public void testCleanDirsWithDryRunAndNoIgnore()
assertTrue(cleanedFiles.contains("ignored-dir/"));
}
@Test
public void testCleanDirsWithPrefixFolder() throws Exception {
String path = "sub/foo.txt";
writeTrashFile(path, "sub is a prefix of sub-noclean");
git.add().addFilepattern(path).call();
Status beforeCleanStatus = git.status().call();
assertTrue(beforeCleanStatus.getAdded().contains(path));
Set<String> cleanedFiles = git.clean().setCleanDirectories(true).call();
// The "sub" directory should not be cleaned.
assertTrue(!cleanedFiles.contains(path + "/"));
assertTrue(cleanedFiles.contains("File2.txt"));
assertTrue(cleanedFiles.contains("File3.txt"));
assertTrue(!cleanedFiles.contains("sub-noclean/File1.txt"));
assertTrue(cleanedFiles.contains("sub-noclean/File2.txt"));
assertTrue(cleanedFiles.contains("sub-clean/"));
assertTrue(cleanedFiles.size() == 4);
}
@Test
public void testCleanDirsWithSubmodule() throws Exception {
SubmoduleAddCommand command = new SubmoduleAddCommand(db);

53
org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/IndexDiffSubmoduleTest.java
View File

@ -51,6 +51,7 @@
import java.io.IOException;
import java.util.Set;
import org.eclipse.jgit.api.CloneCommand;
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.errors.NoWorkTreeException;
@ -109,6 +110,58 @@ public void testInitiallyClean(IgnoreSubmoduleMode mode)
assertFalse(indexDiff.diff());
}
private Repository cloneWithoutCloningSubmodule() throws Exception {
File directory = createTempDirectory(
"testCloneWithoutCloningSubmodules");
CloneCommand clone = Git.cloneRepository();
clone.setDirectory(directory);
clone.setCloneSubmodules(false);
clone.setURI(db.getDirectory().toURI().toString());
Git git2 = clone.call();
addRepoToClose(git2.getRepository());
return git2.getRepository();
}
@Theory
public void testCleanAfterClone(IgnoreSubmoduleMode mode) throws Exception {
Repository db2 = cloneWithoutCloningSubmodule();
IndexDiff indexDiff = new IndexDiff(db2, Constants.HEAD,
new FileTreeIterator(db2));
indexDiff.setIgnoreSubmoduleMode(mode);
assertFalse(indexDiff.diff());
}
@Theory
public void testMissingIfDirectoryGone(IgnoreSubmoduleMode mode)
throws Exception {
recursiveDelete(submodule_trash);
IndexDiff indexDiff = new IndexDiff(db, Constants.HEAD,
new FileTreeIterator(db));
indexDiff.setIgnoreSubmoduleMode(mode);
boolean hasChanges = indexDiff.diff();
if (mode != IgnoreSubmoduleMode.ALL) {
assertTrue(hasChanges);
assertEquals("[modules/submodule]",
indexDiff.getMissing().toString());
} else {
assertFalse(hasChanges);
}
}
@Theory
public void testSubmoduleReplacedByFile(IgnoreSubmoduleMode mode)
throws Exception {
recursiveDelete(submodule_trash);
writeTrashFile("modules/submodule", "nonsense");
IndexDiff indexDiff = new IndexDiff(db, Constants.HEAD,
new FileTreeIterator(db));
indexDiff.setIgnoreSubmoduleMode(mode);
assertTrue(indexDiff.diff());
assertEquals("[]", indexDiff.getMissing().toString());
assertEquals("[]", indexDiff.getUntracked().toString());
assertEquals("[modules/submodule]", indexDiff.getModified().toString());
}
@Theory
public void testDirtyRootWorktree(IgnoreSubmoduleMode mode)
throws IOException {

22
org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/IndexDiffTest.java
View File

@ -118,6 +118,28 @@ public void testAdded() throws IOException {
assertEquals(Collections.EMPTY_SET, diff.getUntrackedFolders());
}
@Test
public void testMissing() throws Exception {
File file2 = writeTrashFile("file2", "file2");
File file3 = writeTrashFile("dir/file3", "dir/file3");
Git git = Git.wrap(db);
git.add().addFilepattern("file2").addFilepattern("dir/file3").call();
git.commit().setMessage("commit").call();
assertTrue(file2.delete());
assertTrue(file3.delete());
IndexDiff diff = new IndexDiff(db, Constants.HEAD,
new FileTreeIterator(db));
diff.diff();
assertEquals(2, diff.getMissing().size());
assertTrue(diff.getMissing().contains("file2"));
assertTrue(diff.getMissing().contains("dir/file3"));
assertEquals(0, diff.getChanged().size());
assertEquals(0, diff.getModified().size());
assertEquals(0, diff.getAdded().size());
assertEquals(0, diff.getRemoved().size());
assertEquals(Collections.EMPTY_SET, diff.getUntrackedFolders());
}
@Test
public void testRemoved() throws IOException {
writeTrashFile("file2", "file2");

18
org.eclipse.jgit.test/tst/org/eclipse/jgit/util/PathsTest.java
View File

@ -88,15 +88,30 @@ public void testPathCompare() {
assertEquals(0, compare(
a, 0, a.length, FileMode.TREE.getBits(),
b, 0, b.length, FileMode.TREE.getBits()));
assertEquals(0, compare(
a, 0, a.length, FileMode.TREE.getBits(),
b, 0, b.length, FileMode.GITLINK.getBits()));
assertEquals(0, compare(
a, 0, a.length, FileMode.GITLINK.getBits(),
b, 0, b.length, FileMode.GITLINK.getBits()));
assertEquals(0, compare(
a, 0, a.length, FileMode.GITLINK.getBits(),
b, 0, b.length, FileMode.TREE.getBits()));
assertEquals(0, compare(
a, 0, a.length, FileMode.REGULAR_FILE.getBits(),
b, 0, b.length, FileMode.REGULAR_FILE.getBits()));
assertEquals(-47, compare(
a, 0, a.length, FileMode.REGULAR_FILE.getBits(),
b, 0, b.length, FileMode.TREE.getBits()));
assertEquals(0, compare(
a, 0, a.length, FileMode.REGULAR_FILE.getBits(),
b, 0, b.length, FileMode.GITLINK.getBits()));
assertEquals(47, compare(
a, 0, a.length, FileMode.TREE.getBits(),
b, 0, b.length, FileMode.REGULAR_FILE.getBits()));
assertEquals(0, compare(
a, 0, a.length, FileMode.GITLINK.getBits(),
b, 0, b.length, FileMode.REGULAR_FILE.getBits()));
assertEquals(0, compareSameName(
a, 0, a.length,
@ -104,6 +119,9 @@ public void testPathCompare() {
assertEquals(0, compareSameName(
a, 0, a.length,
b, 0, b.length, FileMode.REGULAR_FILE.getBits()));
assertEquals(0, compareSameName(
a, 0, a.length,
b, 0, b.length, FileMode.GITLINK.getBits()));
a = Constants.encode("a.c");
b = Constants.encode("a");

2
org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java
View File

@ -1045,7 +1045,7 @@ public FileMode getIndexFileMode(DirCacheIterator indexIter) {
}
}
if (FileMode.GITLINK == iMode
&& FileMode.TREE == wtMode) {
&& FileMode.TREE == wtMode && !getOptions().isDirNoGitLinks()) {
return iMode;
}
if (FileMode.TREE == iMode

14
org.eclipse.jgit/src/org/eclipse/jgit/util/Paths.java
View File

@ -1,5 +1,5 @@
/*
* Copyright (C) 2016, Google Inc.
* Copyright (C) 2016, 2018 Google Inc.
* and other copyright owners as documented in the project's IP log.
*
* This program and the accompanying materials are made available
@ -43,6 +43,7 @@
package org.eclipse.jgit.util;
import static org.eclipse.jgit.lib.FileMode.TYPE_GITLINK;
import static org.eclipse.jgit.lib.FileMode.TYPE_MASK;
import static org.eclipse.jgit.lib.FileMode.TYPE_TREE;
@ -106,7 +107,7 @@ public static int compare(byte[] aPath, int aPos, int aEnd, int aMode,
aPath, aPos, aEnd, aMode,
bPath, bPos, bEnd, bMode);
if (cmp == 0) {
cmp = lastPathChar(aMode) - lastPathChar(bMode);
cmp = modeCompare(aMode, bMode);
}
return cmp;
}
@ -183,6 +184,15 @@ private static int lastPathChar(int mode) {
return 0;
}
private static int modeCompare(int aMode, int bMode) {
if ((aMode & TYPE_MASK) == TYPE_GITLINK
|| (bMode & TYPE_MASK) == TYPE_GITLINK) {
// Git links can be equal to files or folders
return 0;
}
return lastPathChar(aMode) - lastPathChar(bMode);
}
private Paths() {
}
}