From ed7b322186a872bdd4f41a1a31554a7e90d74de6 Mon Sep 17 00:00:00 2001 From: Christian Halstrick Date: Mon, 13 Feb 2012 09:39:05 +0100 Subject: [PATCH] Generate conflicts and index updates on file mode changes Handle more cases for file mode changes. Especially make sure that the following cases are handled correctly. Case 1) An entry in the working tree, HEAD tree, and merge tree have different modes and different content. Prior Outcome: Dirty working tree content is replaced and file mode changes are lost. New Outcome: Conflict is generated. Case 2) An entry in the index and merge tree have the same content but different modes but both modes are file type modes. Prior Outcome: File mode in working tree is not updated and the working directory is dirty. New Outcome: Index is updated and the working directory is clean. Bug: 363772 Change-Id: I224602d68228eb419813986807f1eeab77e9c302 Signed-off-by: Christian Halstrick Also-by: Kevin Sawicki Signed-off-by: Matthias Sohn --- .../jgit/lib/DirCacheCheckoutTest.java | 203 +++++++++++++++++- .../jgit/dircache/DirCacheCheckout.java | 41 +++- 2 files changed, 235 insertions(+), 9 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/DirCacheCheckoutTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/DirCacheCheckoutTest.java index fafd745b5..efb686da5 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/DirCacheCheckoutTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/DirCacheCheckoutTest.java @@ -55,22 +55,27 @@ import java.util.List; import java.util.Map; +import org.eclipse.jgit.api.CheckoutCommand; +import org.eclipse.jgit.api.CheckoutResult; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.MergeResult.MergeStatus; import org.eclipse.jgit.api.ResetCommand.ResetType; +import org.eclipse.jgit.api.Status; import org.eclipse.jgit.api.errors.GitAPIException; +import org.eclipse.jgit.api.errors.JGitInternalException; import org.eclipse.jgit.api.errors.NoFilepatternException; import org.eclipse.jgit.dircache.DirCache; import org.eclipse.jgit.dircache.DirCacheCheckout; import org.eclipse.jgit.dircache.DirCacheEditor; -import org.eclipse.jgit.dircache.DirCacheEntry; import org.eclipse.jgit.dircache.DirCacheEditor.PathEdit; +import org.eclipse.jgit.dircache.DirCacheEntry; import org.eclipse.jgit.errors.CheckoutConflictException; import org.eclipse.jgit.errors.CorruptObjectException; import org.eclipse.jgit.errors.NoWorkTreeException; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.treewalk.FileTreeIterator; import org.eclipse.jgit.treewalk.TreeWalk; +import org.eclipse.jgit.util.FS; import org.junit.Test; public class DirCacheCheckoutTest extends RepositoryTestCase { @@ -939,6 +944,202 @@ public void testDontOverwriteDirtyFile() throws IOException { } } + @Test + public void testFileModeChangeWithNoContentChangeUpdate() throws Exception { + if (!FS.DETECTED.supportsExecute()) + return; + + Git git = Git.wrap(db); + + // Add non-executable file + File file = writeTrashFile("file.txt", "a"); + git.add().addFilepattern("file.txt").call(); + git.commit().setMessage("commit1").call(); + assertFalse(db.getFS().canExecute(file)); + + // Create branch + git.branchCreate().setName("b1").call(); + + // Make file executable + db.getFS().setExecute(file, true); + git.add().addFilepattern("file.txt").call(); + git.commit().setMessage("commit2").call(); + + // Verify executable and working directory is clean + Status status = git.status().call(); + assertTrue(status.getModified().isEmpty()); + assertTrue(status.getChanged().isEmpty()); + assertTrue(db.getFS().canExecute(file)); + + // Switch branches + git.checkout().setName("b1").call(); + + // Verify not executable and working directory is clean + status = git.status().call(); + assertTrue(status.getModified().isEmpty()); + assertTrue(status.getChanged().isEmpty()); + assertFalse(db.getFS().canExecute(file)); + } + + @Test + public void testFileModeChangeAndContentChangeConflict() throws Exception { + if (!FS.DETECTED.supportsExecute()) + return; + + Git git = Git.wrap(db); + + // Add non-executable file + File file = writeTrashFile("file.txt", "a"); + git.add().addFilepattern("file.txt").call(); + git.commit().setMessage("commit1").call(); + assertFalse(db.getFS().canExecute(file)); + + // Create branch + git.branchCreate().setName("b1").call(); + + // Make file executable + db.getFS().setExecute(file, true); + git.add().addFilepattern("file.txt").call(); + git.commit().setMessage("commit2").call(); + + // Verify executable and working directory is clean + Status status = git.status().call(); + assertTrue(status.getModified().isEmpty()); + assertTrue(status.getChanged().isEmpty()); + assertTrue(db.getFS().canExecute(file)); + + writeTrashFile("file.txt", "b"); + + // Switch branches + CheckoutCommand checkout = git.checkout().setName("b1"); + try { + checkout.call(); + fail("Checkout exception not thrown"); + } catch (JGitInternalException e) { + CheckoutResult result = checkout.getResult(); + assertNotNull(result); + assertNotNull(result.getConflictList()); + assertEquals(1, result.getConflictList().size()); + assertTrue(result.getConflictList().contains("file.txt")); + } + } + + @Test + public void testDirtyFileModeEqualHeadMerge() + throws Exception { + if (!FS.DETECTED.supportsExecute()) + return; + + Git git = Git.wrap(db); + + // Add non-executable file + File file = writeTrashFile("file.txt", "a"); + git.add().addFilepattern("file.txt").call(); + git.commit().setMessage("commit1").call(); + assertFalse(db.getFS().canExecute(file)); + + // Create branch + git.branchCreate().setName("b1").call(); + + // Create second commit and don't touch file + writeTrashFile("file2.txt", ""); + git.add().addFilepattern("file2.txt").call(); + git.commit().setMessage("commit2").call(); + + // stage a mode change + writeTrashFile("file.txt", "a"); + db.getFS().setExecute(file, true); + git.add().addFilepattern("file.txt").call(); + + // dirty the file + writeTrashFile("file.txt", "b"); + + assertEquals( + "[file.txt, mode:100755, content:a][file2.txt, mode:100644, content:]", + indexState(CONTENT)); + assertWorkDir(mkmap("file.txt", "b", "file2.txt", "")); + + // Switch branches and check that the dirty file survived in worktree + // and index + git.checkout().setName("b1").call(); + assertEquals("[file.txt, mode:100755, content:a]", indexState(CONTENT)); + assertWorkDir(mkmap("file.txt", "b")); + } + + @Test + public void testDirtyFileModeEqualIndexMerge() + throws Exception { + if (!FS.DETECTED.supportsExecute()) + return; + + Git git = Git.wrap(db); + + // Add non-executable file + File file = writeTrashFile("file.txt", "a"); + git.add().addFilepattern("file.txt").call(); + git.commit().setMessage("commit1").call(); + assertFalse(db.getFS().canExecute(file)); + + // Create branch + git.branchCreate().setName("b1").call(); + + // Create second commit with executable file + file = writeTrashFile("file.txt", "b"); + db.getFS().setExecute(file, true); + git.add().addFilepattern("file.txt").call(); + git.commit().setMessage("commit2").call(); + + // stage the same content as in the branch we want to switch to + writeTrashFile("file.txt", "a"); + db.getFS().setExecute(file, false); + git.add().addFilepattern("file.txt").call(); + + // dirty the file + writeTrashFile("file.txt", "c"); + db.getFS().setExecute(file, true); + + assertEquals("[file.txt, mode:100644, content:a]", indexState(CONTENT)); + assertWorkDir(mkmap("file.txt", "c")); + + // Switch branches and check that the dirty file survived in worktree + // and index + git.checkout().setName("b1").call(); + assertEquals("[file.txt, mode:100644, content:a]", indexState(CONTENT)); + assertWorkDir(mkmap("file.txt", "c")); + } + + @Test + public void testFileModeChangeAndContentChangeNoConflict() throws Exception { + if (!FS.DETECTED.supportsExecute()) + return; + + Git git = Git.wrap(db); + + // Add first file + File file1 = writeTrashFile("file1.txt", "a"); + git.add().addFilepattern("file1.txt").call(); + git.commit().setMessage("commit1").call(); + assertFalse(db.getFS().canExecute(file1)); + + // Add second file + File file2 = writeTrashFile("file2.txt", "b"); + git.add().addFilepattern("file2.txt").call(); + git.commit().setMessage("commit2").call(); + assertFalse(db.getFS().canExecute(file2)); + + // Create branch from first commit + assertNotNull(git.checkout().setCreateBranch(true).setName("b1") + .setStartPoint(Constants.HEAD + "~1").call()); + + // Change content and file mode in working directory and index + file1 = writeTrashFile("file1.txt", "c"); + db.getFS().setExecute(file1, true); + git.add().addFilepattern("file1.txt").call(); + + // Switch back to 'master' + assertNotNull(git.checkout().setName(Constants.MASTER).call()); + } + public void assertWorkDir(HashMap i) throws CorruptObjectException, IOException { TreeWalk walk = new TreeWalk(db); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java index 9b397e88c..58b691228 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java @@ -472,6 +472,23 @@ private void removeEmptyParents(File f) { } } + /** + * Compares whether two pairs of ObjectId and FileMode are equal. + * + * @param id1 + * @param mode1 + * @param id2 + * @param mode2 + * @return true if FileModes and ObjectIds are equal. + * false otherwise + */ + private boolean equalIdAndMode(ObjectId id1, FileMode mode1, ObjectId id2, + FileMode mode2) { + if (!mode1.equals(mode2)) + return false; + return id1 != null ? id1.equals(id2) : id2 == null; + } + /** * Here the main work is done. This method is called for each existing path * in head, index and merge. This method decides what to do with the @@ -508,6 +525,9 @@ void processEntry(AbstractTreeIterator h, AbstractTreeIterator m, ObjectId iId = (i == null ? null : i.getEntryObjectId()); ObjectId mId = (m == null ? null : m.getEntryObjectId()); ObjectId hId = (h == null ? null : h.getEntryObjectId()); + FileMode iMode = (i == null ? null : i.getEntryFileMode()); + FileMode mMode = (m == null ? null : m.getEntryFileMode()); + FileMode hMode = (h == null ? null : h.getEntryFileMode()); /** * 
@@ -596,7 +616,7 @@ void processEntry(AbstractTreeIterator h, AbstractTreeIterator m,
 			case 0xFDD: // 10 11
 				// TODO: make use of tree extension as soon as available in jgit
 				// we would like to do something like
-				// if (!iId.equals(mId))
+				// if (!equalIdAndMode(iId, iMode, mId, mMode)
 				//   conflict(name, i.getDirCacheEntry(), h, m);
 				// But since we don't know the id of a tree in the index we do
 				// nothing here and wait that conflicts between index and merge
@@ -610,7 +630,7 @@ void processEntry(AbstractTreeIterator h, AbstractTreeIterator m,
 				conflict(name, (i != null) ? i.getDirCacheEntry() : null, h, m);
 				break;
 			case 0xFDF: // 7 8 9
-				if (hId.equals(mId)) {
+				if (equalIdAndMode(hId, hMode, mId, mMode)) {
 					if (isModified(name))
 						conflict(name, i.getDirCacheEntry(), h, m); // 8
 					else
@@ -625,7 +645,7 @@ void processEntry(AbstractTreeIterator h, AbstractTreeIterator m,
 				keep(i.getDirCacheEntry());
 				break;
 			case 0xFFD: // 12 13 14
-				if (hId.equals(iId)) {
+				if (equalIdAndMode(hId, hMode, iId, iMode)) {
 					dce = i.getDirCacheEntry();
 					if (f == null || f.isModified(dce, true))
 						conflict(name, dce, h, m);
@@ -662,7 +682,9 @@ void processEntry(AbstractTreeIterator h, AbstractTreeIterator m,
 				if (!FileMode.GITLINK.equals(m.getEntryFileMode())) {
 					// a dirty worktree: the index is empty but we have a
 					// workingtree-file
-					if (mId == null || !mId.equals(f.getEntryObjectId())) {
+					if (mId == null
+							|| !equalIdAndMode(mId, mMode,
+									f.getEntryObjectId(), f.getEntryFileMode())) {
 						conflict(name, null, h, m);
 						return;
 					}
@@ -703,7 +725,7 @@ else if (m == null)
 				 *
*/ - if (m == null || mId.equals(iId)) { + if (m == null || equalIdAndMode(mId, mMode, iId, iMode)) { if (m==null && walk.isDirectoryFileConflict()) { if (dce != null && (f == null || f.isModified(dce, true))) @@ -732,7 +754,7 @@ else if (m == null) // be removed from the index, but not deleted from disk. remove(name); } else { - if (hId.equals(iId)) { + if (equalIdAndMode(hId, hMode, iId, iMode)) { if (f == null || f.isModified(dce, true)) conflict(name, dce, h, m); else @@ -741,9 +763,12 @@ else if (m == null) conflict(name, dce, h, m); } } else { - if (!hId.equals(mId) && !hId.equals(iId) && !mId.equals(iId)) + if (!equalIdAndMode(hId, hMode, mId, mMode) + && !equalIdAndMode(hId, hMode, iId, iMode) + && !equalIdAndMode(mId, mMode, iId, iMode)) conflict(name, dce, h, m); - else if (hId.equals(iId) && !mId.equals(iId)) { + else if (equalIdAndMode(hId, hMode, iId, iMode) + && !equalIdAndMode(mId, mMode, iId, iMode)) { // For submodules just update the index with the new SHA-1 if (dce != null && FileMode.GITLINK.equals(dce.getFileMode())) {