Browse Source

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>
stable-5.2
Thomas Wolf 6 years ago committed by Matthias Sohn
parent
commit
8277f62897
  1. 63
      org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffEntryTest.java
  2. 34
      org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/IndexDiffSubmoduleTest.java
  3. 18
      org.eclipse.jgit.test/tst/org/eclipse/jgit/util/PathsTest.java
  4. 44
      org.eclipse.jgit/src/org/eclipse/jgit/lib/IndexDiff.java
  5. 14
      org.eclipse.jgit/src/org/eclipse/jgit/util/Paths.java

63
org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffEntryTest.java

@ -59,9 +59,12 @@ import org.eclipse.jgit.diff.DiffEntry.ChangeType;
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 class DiffEntryTest extends RepositoryTestCase {
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());
}
}
}

34
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 class IndexDiffSubmoduleTest extends RepositoryTestCase {
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 class IndexDiffSubmoduleTest extends RepositoryTestCase {
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());
}
}

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

@ -88,30 +88,15 @@ public class PathsTest {
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,
@ -119,9 +104,6 @@ public class PathsTest {
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");

44
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 class IndexDiff {
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 class IndexDiff {
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 class IndexDiff {
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 class IndexDiff {
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 class IndexDiff {
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) {

14
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 class Paths {
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 @@ public class Paths {
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() {
}
}

Loading…
Cancel
Save