Revert 4678f4b and provide another solution for bug 467631
Making gitlinks and folders match in a tree walk was the wrong
approach to fix bug 467631. The problem is that in such a conflict
the tree walk may then not descend into the folder.
Revert the changes to Paths.java and PathsTest.java from commit
4678f4b. Instead test for the problem case from bug 467631 explicitly
in IndexDiff. Add Daniel's test case from bug 545162, and add yet
another test case for DiffEntry.scan() that covers the problem
originally reported in bug 545162.
Bug: 545162
Change-Id: Ie2214c5d5ee32ac6596b621f0f1c7b86d38fa9b7
Also-by: Daniel Veihelmann <daniel.veihelmann@gmail.com>
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffEntryTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffEntryTest.java
index 73c230a..de76811 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffEntryTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffEntryTest.java
@@ -59,9 +59,12 @@
import org.eclipse.jgit.dircache.DirCache;
import org.eclipse.jgit.dircache.DirCacheEditor;
import org.eclipse.jgit.dircache.DirCacheEditor.PathEdit;
+import org.eclipse.jgit.internal.storage.file.FileRepository;
import org.eclipse.jgit.dircache.DirCacheEntry;
+import org.eclipse.jgit.junit.JGitTestUtil;
import org.eclipse.jgit.junit.RepositoryTestCase;
import org.eclipse.jgit.lib.FileMode;
+import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.treewalk.EmptyTreeIterator;
import org.eclipse.jgit.treewalk.FileTreeIterator;
@@ -417,4 +420,64 @@ public void apply(DirCacheEntry ent) {
assertEquals(FileMode.REGULAR_FILE, diff.getOldMode());
}
}
+
+ @Test
+ public void shouldReportSubmoduleReplacedByFileMove() throws Exception {
+ // Create a submodule
+ FileRepository submoduleStandalone = createWorkRepository();
+ JGitTestUtil.writeTrashFile(submoduleStandalone, "fileInSubmodule",
+ "submodule");
+ Git submoduleStandaloneGit = Git.wrap(submoduleStandalone);
+ submoduleStandaloneGit.add().addFilepattern("fileInSubmodule").call();
+ submoduleStandaloneGit.commit().setMessage("add file to submodule")
+ .call();
+
+ Repository submodule_db = Git.wrap(db).submoduleAdd()
+ .setPath("modules/submodule")
+ .setURI(submoduleStandalone.getDirectory().toURI().toString())
+ .call();
+ File submodule_trash = submodule_db.getWorkTree();
+ addRepoToClose(submodule_db);
+ writeTrashFile("fileInRoot", "root");
+ Git rootGit = Git.wrap(db);
+ rootGit.add().addFilepattern("fileInRoot").call();
+ rootGit.commit().setMessage("add submodule and root file").call();
+ // Dummy change on fileInRoot
+ writeTrashFile("fileInRoot", "changed");
+ rootGit.add().addFilepattern("fileInRoot").call();
+ RevCommit firstCommit = rootGit.commit().setMessage("change root file")
+ .call();
+ // Remove the submodule again and move fileInRoot into that subfolder
+ rootGit.rm().setCached(true).addFilepattern("modules/submodule").call();
+ recursiveDelete(submodule_trash);
+ JGitTestUtil.deleteTrashFile(db, "fileInRoot");
+ // Move the fileInRoot file
+ writeTrashFile("modules/submodule/fileInRoot", "changed");
+ rootGit.rm().addFilepattern("fileInRoot").addFilepattern("modules/")
+ .call();
+ rootGit.add().addFilepattern("modules/").call();
+ RevCommit secondCommit = rootGit.commit()
+ .setMessage("remove submodule and move root file")
+ .call();
+ // Diff should report submodule having been deleted and file moved
+ // (deleted and added)
+ try (TreeWalk walk = new TreeWalk(db)) {
+ walk.addTree(firstCommit.getTree());
+ walk.addTree(secondCommit.getTree());
+ walk.setRecursive(true);
+ List<DiffEntry> diffs = DiffEntry.scan(walk);
+ assertEquals(3, diffs.size());
+ DiffEntry e = diffs.get(0);
+ assertEquals(DiffEntry.ChangeType.DELETE, e.getChangeType());
+ assertEquals("fileInRoot", e.getOldPath());
+ e = diffs.get(1);
+ assertEquals(DiffEntry.ChangeType.DELETE, e.getChangeType());
+ assertEquals("modules/submodule", e.getOldPath());
+ assertEquals(FileMode.GITLINK, e.getOldMode());
+ e = diffs.get(2);
+ assertEquals(DiffEntry.ChangeType.ADD, e.getChangeType());
+ assertEquals("modules/submodule/fileInRoot", e.getNewPath());
+ }
+
+ }
}
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/IndexDiffSubmoduleTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/IndexDiffSubmoduleTest.java
index 93ada4f..fa7f5ab 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/IndexDiffSubmoduleTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/IndexDiffSubmoduleTest.java
@@ -43,12 +43,14 @@
package org.eclipse.jgit.lib;
+import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import java.io.File;
import java.io.IOException;
+import java.util.Arrays;
import java.util.Set;
import org.eclipse.jgit.api.CloneCommand;
@@ -128,7 +130,8 @@ public void testCleanAfterClone(IgnoreSubmoduleMode mode) throws Exception {
IndexDiff indexDiff = new IndexDiff(db2, Constants.HEAD,
new FileTreeIterator(db2));
indexDiff.setIgnoreSubmoduleMode(mode);
- assertFalse(indexDiff.diff());
+ boolean changed = indexDiff.diff();
+ assertFalse(changed);
}
@Theory
@@ -263,4 +266,33 @@ public void testDirtySubmoduleWorktreeUntracked(IgnoreSubmoduleMode mode)
assertDiff(indexDiff, mode, IgnoreSubmoduleMode.ALL,
IgnoreSubmoduleMode.DIRTY, IgnoreSubmoduleMode.UNTRACKED);
}
+
+ @Theory
+ public void testSubmoduleReplacedByMovedFile(IgnoreSubmoduleMode mode)
+ throws Exception {
+ Git git = Git.wrap(db);
+ git.rm().setCached(true).addFilepattern("modules/submodule").call();
+ recursiveDelete(submodule_trash);
+ JGitTestUtil.deleteTrashFile(db, "fileInRoot");
+ // Move the fileInRoot file
+ writeTrashFile("modules/submodule/fileInRoot", "root");
+ git.rm().addFilepattern("fileInRoot").addFilepattern("modules/").call();
+ git.add().addFilepattern("modules/").call();
+ IndexDiff indexDiff = new IndexDiff(db, Constants.HEAD,
+ new FileTreeIterator(db));
+ indexDiff.setIgnoreSubmoduleMode(mode);
+ assertTrue(indexDiff.diff());
+ String[] removed = indexDiff.getRemoved().toArray(new String[0]);
+ Arrays.sort(removed);
+ if (IgnoreSubmoduleMode.ALL.equals(mode)) {
+ assertArrayEquals(new String[] { "fileInRoot" }, removed);
+ } else {
+ assertArrayEquals(
+ new String[] { "fileInRoot", "modules/submodule" },
+ removed);
+ }
+ assertEquals("[modules/submodule/fileInRoot]",
+ indexDiff.getAdded().toString());
+ }
+
}
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/PathsTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/PathsTest.java
index f78cbe2..7542ec8 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/PathsTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/PathsTest.java
@@ -89,29 +89,14 @@ public void testPathCompare() {
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,
@@ -119,9 +104,6 @@ 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");
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/IndexDiff.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/IndexDiff.java
index 94b9ddc..f37c310 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/IndexDiff.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/IndexDiff.java
@@ -47,7 +47,11 @@
package org.eclipse.jgit.lib;
+import java.io.File;
import java.io.IOException;
+import java.nio.file.DirectoryIteratorException;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.Collection;
@@ -261,6 +265,8 @@ public TreeFilter clone() {
private Set<String> missing = new HashSet<>();
+ private Set<String> missingSubmodules = new HashSet<>();
+
private Set<String> modified = new HashSet<>();
private Set<String> untracked = new HashSet<>();
@@ -501,9 +507,15 @@ public boolean diff(final ProgressMonitor monitor, int estWorkTreeSize,
if (dirCacheIterator != null) {
if (workingTreeIterator == null) {
// in index, not in workdir => missing
- if (!isEntryGitLink(dirCacheIterator)
- || ignoreSubmoduleMode != IgnoreSubmoduleMode.ALL)
- missing.add(treeWalk.getPathString());
+ boolean isGitLink = isEntryGitLink(dirCacheIterator);
+ if (!isGitLink
+ || ignoreSubmoduleMode != IgnoreSubmoduleMode.ALL) {
+ String path = treeWalk.getPathString();
+ missing.add(path);
+ if (isGitLink) {
+ missingSubmodules.add(path);
+ }
+ }
} else {
if (workingTreeIterator.isModified(
dirCacheIterator.getDirCacheEntry(), true,
@@ -543,8 +555,8 @@ public boolean diff(final ProgressMonitor monitor, int estWorkTreeSize,
smw.getPath()), e);
}
try (Repository subRepo = smw.getRepository()) {
+ String subRepoPath = smw.getPath();
if (subRepo != null) {
- String subRepoPath = smw.getPath();
ObjectId subHead = subRepo.resolve("HEAD"); //$NON-NLS-1$
if (subHead != null
&& !subHead.equals(smw.getObjectId())) {
@@ -573,6 +585,21 @@ public boolean diff(final ProgressMonitor monitor, int estWorkTreeSize,
recordFileMode(subRepoPath, FileMode.GITLINK);
}
}
+ } else if (missingSubmodules.remove(subRepoPath)) {
+ // If the directory is there and empty but the submodule
+ // repository in .git/modules doesn't exist yet it isn't
+ // "missing".
+ File gitDir = new File(
+ new File(repository.getDirectory(),
+ Constants.MODULES),
+ subRepoPath);
+ if (!gitDir.isDirectory()) {
+ File dir = SubmoduleWalk.getSubmoduleDirectory(
+ repository, subRepoPath);
+ if (dir.isDirectory() && !hasFiles(dir)) {
+ missing.remove(subRepoPath);
+ }
+ }
}
}
}
@@ -592,6 +619,15 @@ public boolean diff(final ProgressMonitor monitor, int estWorkTreeSize,
return true;
}
+ private boolean hasFiles(File directory) {
+ try (DirectoryStream<java.nio.file.Path> dir = Files
+ .newDirectoryStream(directory.toPath())) {
+ return dir.iterator().hasNext();
+ } catch (DirectoryIteratorException | IOException e) {
+ return false;
+ }
+ }
+
private void recordFileMode(String path, FileMode mode) {
Set<String> values = fileModes.get(mode);
if (path != null) {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/Paths.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/Paths.java
index be1b95e..6be7ddb 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/Paths.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/Paths.java
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2016, 2018 Google Inc.
+ * Copyright (C) 2016, Google Inc.
* and other copyright owners as documented in the project's IP log.
*
* This program and the accompanying materials are made available
@@ -43,7 +43,6 @@
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;
@@ -107,7 +106,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 = modeCompare(aMode, bMode);
+ cmp = lastPathChar(aMode) - lastPathChar(bMode);
}
return cmp;
}
@@ -184,15 +183,6 @@ 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() {
}
}
