Make sure not to overwrite untracked not-ignored files

When DirCacheCheckout was checking out it was silently
overwriting untracked files. This is only ok if the
files are also ignored. Untracked and not ignored files
should not be overwritten. This fix adds checks for
this situation.
Because this change in the behaviour also broke tests
which expected that a checkout will overwrite untracked
files (PullCommandTest) these tests have to be modified
also.

Bug: 333093
Change-Id: I26806d2108ceb64c51abaa877e11b584bf527fc9
Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>
Signed-off-by: Chris Aniszczyk <caniszczyk@gmail.com>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PullCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PullCommandTest.java
index ed5650461d0c1eb1d4f6400d33324bfce4180349..04f666cc7bc31b89f9f91dbc74d15e96de44357b 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PullCommandTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PullCommandTest.java
@@ -56,10 +56,8 @@ import java.io.IOException;
 import org.eclipse.jgit.api.CreateBranchCommand.SetupUpstreamMode;
 import org.eclipse.jgit.api.MergeResult.MergeStatus;
 import org.eclipse.jgit.lib.Constants;
-import org.eclipse.jgit.lib.RefUpdate;
 import org.eclipse.jgit.lib.RepositoryTestCase;
 import org.eclipse.jgit.lib.StoredConfig;
-import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.storage.file.FileRepository;
 import org.eclipse.jgit.transport.RefSpec;
 import org.eclipse.jgit.transport.RemoteConfig;
@@ -190,13 +188,7 @@ public class PullCommandTest extends RepositoryTestCase {
                writeToFile(sourceFile, "Hello world");
                // and commit it
                source.add().addFilepattern("SomeFile.txt").call();
-               RevCommit commit = source.commit().setMessage(
-                               "Initial commit for source").call();
-
-               // point the master branch to the new commit
-               RefUpdate upd = dbTarget.updateRef("refs/heads/master");
-               upd.setNewObjectId(commit.getId());
-               upd.update();
+               source.commit().setMessage("Initial commit for source").call();
 
                // configure the target repo to connect to the source via "origin"
                StoredConfig targetConfig = dbTarget.getConfig();
@@ -214,9 +206,9 @@ public class PullCommandTest extends RepositoryTestCase {
                targetConfig.save();
 
                targetFile = new File(dbTarget.getWorkTree(), "SomeFile.txt");
-               writeToFile(targetFile, "Hello world");
                // make sure we have the same content
                target.pull().call();
+               assertFileContentsEqual(targetFile, "Hello world");
        }
 
        private void writeToFile(File actFile, String string) throws IOException {

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ReadTreeTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ReadTreeTest.java
index 001967dd6815e3cf67b745caea59836b702996cd..4f6e2b85b2c0fa254f2235126e15fb3991eed381 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ReadTreeTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ReadTreeTest.java
@@ -571,6 +571,34 @@ public abstract class ReadTreeTest extends RepositoryTestCase {
                writeTrashFile("foo", "foo");
                go();
 
+               // test that we don't overwrite untracked files when there is a HEAD
+               recursiveDelete(new File(trash, "foo"));
+               setupCase(mk("other"), mkmap("other", "other", "foo", "foo"),
+                               mk("other"));
+               writeTrashFile("foo", "bar");
+               try {
+                       checkout();
+                       fail("didn't get the expected exception");
+               } catch (CheckoutConflictException e) {
+                       assertConflict("foo");
+                       assertWorkDir(mkmap("foo", "bar", "other", "other"));
+                       assertIndex(mk("other"));
+               }
+
+               // test that we don't overwrite untracked files when there is no HEAD
+               recursiveDelete(new File(trash, "other"));
+               recursiveDelete(new File(trash, "foo"));
+               setupCase(null, mk("foo"), null);
+               writeTrashFile("foo", "bar");
+               try {
+                       checkout();
+                       fail("didn't get the expected exception");
+               } catch (CheckoutConflictException e) {
+                       assertConflict("foo");
+                       assertWorkDir(mkmap("foo", "bar"));
+                       assertIndex(mkmap());
+               }
+
                // TODO: Why should we expect conflicts here?
                // H and M are emtpy and according to rule #5 of
                // the carry-over rules a dirty index is no reason
@@ -581,6 +609,7 @@ public abstract class ReadTreeTest extends RepositoryTestCase {
                // assertConflict("foo");
 
                recursiveDelete(new File(trash, "foo"));
+               recursiveDelete(new File(trash, "other"));
                setupCase(null, mk("foo"), null);
                writeTrashFile("foo/bar/baz", "");
                writeTrashFile("foo/blahblah", "");

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 da7ea2c9cc0ed91ee4d18fadae705f7ff77820b1..9b0bc7b01cb5a6c4115cdb26e6f9f9b5b5ea268b 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java
@@ -300,15 +300,21 @@ public class DirCacheCheckout {
         * @param m the tree to merge
         * @param i the index
         * @param f the working tree
+        * @throws IOException
         */
        void processEntry(CanonicalTreeParser m, DirCacheBuildIterator i,
-                       WorkingTreeIterator f) {
+                       WorkingTreeIterator f) throws IOException {
                if (m != null) {
                        // There is an entry in the merge commit. Means: we want to update
                        // what's currently in the index and working-tree to that one
                        if (i == null) {
                                // The index entry is missing
-                               update(m.getEntryPathString(), m.getEntryObjectId(),
+                               if (f != null && !FileMode.TREE.equals(f.getEntryFileMode())
+                                               && !f.isEntryIgnored()) {
+                                       // don't overwrite an untracked and not ignored file
+                                       conflicts.add(walk.getPathString());
+                               } else
+                                       update(m.getEntryPathString(), m.getEntryObjectId(),
                                                m.getEntryFileMode());
                        } else if (f == null || !m.idEqual(i)) {
                                // The working tree file is missing or the merge content differs
@@ -344,6 +350,13 @@ public class DirCacheCheckout {
                                                // conflicts set
                                                remove(i.getEntryPathString());
                                                conflicts.remove(i.getEntryPathString());
+                                       } else {
+                                               // We are about to remove an untracked file. Check that
+                                               // it is ignored - otherwise that's an conflict
+                                               if (!f.isEntryIgnored())
+                                                       conflicts.add(walk.getPathString());
+                                               else
+                                                       remove(f.getEntryPathString());
                                        }
                                }
                        } else {
@@ -626,6 +639,16 @@ public class DirCacheCheckout {
                }
 
                if (i == null) {
+                       // make sure not to overwrite untracked files
+                       if (f != null) {
+                               // a dirty worktree: the index is empty but we have a
+                               // workingtree-file
+                               if (mId == null || !mId.equals(f.getEntryObjectId())) {
+                                       conflict(name, null, h, m);
+                                       return;
+                               }
+                       }
+
                        /**
                         * <pre>
                         *                  I (index)                H        M        Result