diff options
5 files changed, 138 insertions, 35 deletions
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 73c230ac68..de768118bf 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.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()); + } + + } } 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 93ada4f301..fa7f5ab522 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 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()); + } + } 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 f78cbe2601..7542ec8910 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 class PathsTest { 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"); 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 94b9ddc188..f37c310752 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 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) { 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 be1b95e0d4..6be7ddbe12 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 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() { } } |