summaryrefslogtreecommitdiffstatsabout
diff options
context:
space:
mode:
authorMarkus Duft2012-07-17 12:32:38 (EDT)
committer Christian Halstrick2012-07-19 04:30:59 (EDT)
commit3ea694c2523d909190b5350e13254a62e94ec5d5 (patch)
tree2300dc1a49a79d291c9392f844f402d2387385a3
parentaeb28b1f3e8763c006dd26e690fbcad0920bc1a4 (diff)
downloadjgit-3ea694c2523d909190b5350e13254a62e94ec5d5.zip
jgit-3ea694c2523d909190b5350e13254a62e94ec5d5.tar.gz
jgit-3ea694c2523d909190b5350e13254a62e94ec5d5.tar.bz2
Teach ResolveMerger to create more correct DirCacheEntry'srefs/changes/46/6646/8
Currently, after a merge/cherry-pick/rebase, all index entries are smudged as the ResolveMerger never sets entry lengths and/or modification times. This change teaches it to re-set them at least for things it did not touch. The other entries are then repaired when the index is persisted, or entries are checked out. Change-Id: I0944f2017483d32043d0d09409b13055b5609a4b Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>
-rw-r--r--org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryTestCase.java29
-rw-r--r--org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/ResolveMergerTest.java178
-rw-r--r--org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java100
3 files changed, 280 insertions, 27 deletions
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryTestCase.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryTestCase.java
index 0c573eb..c06322e 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryTestCase.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryTestCase.java
@@ -397,4 +397,33 @@ public abstract class RepositoryTestCase extends LocalDiskRepositoryTestCase {
RefUpdate refUpdate = db.updateRef(Constants.HEAD);
refUpdate.link(branchName);
}
+
+ /**
+ * Writes a number of files in the working tree. The first content specified
+ * will be written into a file named '0', the second into a file named "1"
+ * and so on. If <code>null</code> is specified as content then this file is
+ * skipped.
+ *
+ * @param ensureDistinctTimestamps
+ * if set to <code>true</code> then between two write operations
+ * this method will wait to ensure that the second file will get
+ * a different lastmodification timestamp than the first file.
+ * @param contents
+ * the contents which should be written into the files
+ * @return the File object associated to the last written file.
+ * @throws IOException
+ * @throws InterruptedException
+ */
+ protected File writeTrashFiles(boolean ensureDistinctTimestamps,
+ String... contents)
+ throws IOException, InterruptedException {
+ File f = null;
+ for (int i = 0; i < contents.length; i++)
+ if (contents[i] != null) {
+ if (ensureDistinctTimestamps && (f != null))
+ fsTick(f);
+ f = writeTrashFile(Integer.toString(i), contents[i]);
+ }
+ return f;
+ }
}
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/ResolveMergerTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/ResolveMergerTest.java
index 4cb0896..95497d8 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/ResolveMergerTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/ResolveMergerTest.java
@@ -42,12 +42,18 @@
*/
package org.eclipse.jgit.merge;
-import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.*;
import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.MergeResult;
+import org.eclipse.jgit.api.MergeResult.MergeStatus;
+import org.eclipse.jgit.dircache.DirCache;
import org.eclipse.jgit.lib.RepositoryTestCase;
+import org.eclipse.jgit.merge.ResolveMerger.MergeFailureReason;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.treewalk.FileTreeIterator;
import org.eclipse.jgit.util.FileUtils;
@@ -95,4 +101,174 @@ public class ResolveMergerTest extends RepositoryTestCase {
assertFalse(ok);
}
+ @Test
+ public void checkLockedFilesToBeDeleted() throws Exception {
+ Git git = Git.wrap(db);
+
+ writeTrashFile("a.txt", "orig");
+ writeTrashFile("b.txt", "orig");
+ git.add().addFilepattern("a.txt").addFilepattern("b.txt").call();
+ RevCommit first = git.commit().setMessage("added a.txt, b.txt").call();
+
+ // modify and delete files on the master branch
+ writeTrashFile("a.txt", "master");
+ git.rm().addFilepattern("b.txt").call();
+ RevCommit masterCommit = git.commit()
+ .setMessage("modified a.txt, deleted b.txt").setAll(true)
+ .call();
+
+ // switch back to a side branch
+ git.checkout().setCreateBranch(true).setStartPoint(first)
+ .setName("side").call();
+ writeTrashFile("c.txt", "side");
+ git.add().addFilepattern("c.txt").call();
+ git.commit().setMessage("added c.txt").call();
+
+ // Get a handle to the the file so on windows it can't be deleted.
+ FileInputStream fis = new FileInputStream(new File(db.getWorkTree(),
+ "b.txt"));
+ MergeResult mergeRes = git.merge().include(masterCommit).call();
+ if (mergeRes.getMergeStatus().equals(MergeStatus.FAILED)) {
+ // probably windows
+ assertEquals(1, mergeRes.getFailingPaths().size());
+ assertEquals(MergeFailureReason.COULD_NOT_DELETE, mergeRes
+ .getFailingPaths().get("b.txt"));
+ }
+ assertEquals("[a.txt, mode:100644, content:master]"
+ + "[c.txt, mode:100644, content:side]", indexState(CONTENT));
+ fis.close();
+ }
+
+ @Test
+ public void checkForCorrectIndex() throws Exception {
+ File f;
+ long lastTs4, lastTsIndex;
+ Git git = Git.wrap(db);
+ File indexFile = db.getIndexFile();
+
+ // Create initial content and remember when the last file was written.
+ f = writeTrashFiles(false, "orig", "orig", "1\n2\n3", "orig", "orig");
+ lastTs4 = f.lastModified();
+
+ // add all files, commit and check this doesn't update any working tree
+ // files and that the index is in a new file system timer tick. Make
+ // sure to wait long enough before adding so the index doesn't contain
+ // racily clean entries
+ fsTick(f);
+ git.add().addFilepattern(".").call();
+ RevCommit firstCommit = git.commit().setMessage("initial commit")
+ .call();
+ checkConsistentLastModified("0", "1", "2", "3", "4");
+ checkModificationTimeStampOrder("1", "2", "3", "4", "<.git/index");
+ assertEquals("Commit should not touch working tree file 4", lastTs4,
+ new File(db.getWorkTree(), "4").lastModified());
+ lastTsIndex = indexFile.lastModified();
+
+ // Do modifications on the master branch. Then add and commit. This
+ // should touch only "0", "2 and "3"
+ fsTick(indexFile);
+ f = writeTrashFiles(false, "master", null, "1master\n2\n3", "master",
+ null);
+ fsTick(f);
+ git.add().addFilepattern(".").call();
+ RevCommit masterCommit = git.commit().setMessage("master commit")
+ .call();
+ checkConsistentLastModified("0", "1", "2", "3", "4");
+ checkModificationTimeStampOrder("1", "4", "*" + lastTs4, "<*"
+ + lastTsIndex, "<0",
+ "2", "3", "<.git/index");
+ lastTsIndex = indexFile.lastModified();
+
+ // Checkout a side branch. This should touch only "0", "2 and "3"
+ fsTick(indexFile);
+ git.checkout().setCreateBranch(true).setStartPoint(firstCommit)
+ .setName("side").call();
+ checkConsistentLastModified("0", "1", "2", "3", "4");
+ checkModificationTimeStampOrder("1", "4", "*" + lastTs4, "<*"
+ + lastTsIndex, "<0",
+ "2", "3", ".git/index");
+ lastTsIndex = indexFile.lastModified();
+
+ // This checkout may have populated worktree and index so fast that we
+ // may have smudged entries now. Check that we have the right content
+ // and then rewrite the index to get rid of smudged state
+ assertEquals("[0, mode:100644, content:orig]" //
+ + "[1, mode:100644, content:orig]" //
+ + "[2, mode:100644, content:1\n2\n3]" //
+ + "[3, mode:100644, content:orig]" //
+ + "[4, mode:100644, content:orig]", //
+ indexState(CONTENT));
+ fsTick(indexFile);
+ f = writeTrashFiles(false, "orig", "orig", "1\n2\n3", "orig", "orig");
+ lastTs4 = f.lastModified();
+ fsTick(f);
+ git.add().addFilepattern(".").call();
+ checkConsistentLastModified("0", "1", "2", "3", "4");
+ checkModificationTimeStampOrder("*" + lastTsIndex, "<0", "1", "2", "3",
+ "4", "<.git/index");
+ lastTsIndex = indexFile.lastModified();
+
+ // Do modifications on the side branch. Touch only "1", "2 and "3"
+ fsTick(indexFile);
+ f = writeTrashFiles(false, null, "side", "1\n2\n3side", "side", null);
+ fsTick(f);
+ git.add().addFilepattern(".").call();
+ git.commit().setMessage("side commit").call();
+ checkConsistentLastModified("0", "1", "2", "3", "4");
+ checkModificationTimeStampOrder("0", "4", "*" + lastTs4, "<*"
+ + lastTsIndex, "<1", "2", "3", "<.git/index");
+ lastTsIndex = indexFile.lastModified();
+
+ // merge master and side. Should only touch "0," "2" and "3"
+ fsTick(indexFile);
+ git.merge().include(masterCommit).call();
+ checkConsistentLastModified("0", "1", "2", "4");
+ checkModificationTimeStampOrder("4", "*" + lastTs4, "<1", "<*"
+ + lastTsIndex, "<0", "2", "3", ".git/index");
+ assertEquals(
+ "[0, mode:100644, content:master]" //
+ + "[1, mode:100644, content:side]" //
+ + "[2, mode:100644, content:1master\n2\n3side\n]" //
+ + "[3, mode:100644, stage:1, content:orig][3, mode:100644, stage:2, content:side][3, mode:100644, stage:3, content:master]" //
+ + "[4, mode:100644, content:orig]", //
+ indexState(CONTENT));
+ }
+
+ // Assert that every specified index entry has the same last modification
+ // timestamp as the associated file
+ private void checkConsistentLastModified(String... pathes)
+ throws IOException {
+ DirCache dc = db.readDirCache();
+ File workTree = db.getWorkTree();
+ for (String path : pathes)
+ assertEquals(
+ "IndexEntry with path "
+ + path
+ + " has lastmodified with is different from the worktree file",
+ new File(workTree, path).lastModified(), dc.getEntry(path)
+ .getLastModified());
+ }
+
+ // Assert that modification timestamps of working tree files are as
+ // expected. You may specify n files. It is asserted that every file
+ // i+1 is not older than file i. If a path of file i+1 is prefixed with "<"
+ // then this file must be younger then file i. A path "*<modtime>"
+ // represents a file with a modification time of <modtime>
+ // E.g. ("a", "b", "<c", "f/a.txt") means: a<=b<c<=f/a.txt
+ private void checkModificationTimeStampOrder(String... pathes) {
+ long lastMod = Long.MIN_VALUE;
+ for (String p : pathes) {
+ boolean strong = p.startsWith("<");
+ boolean fixed = p.charAt(strong ? 1 : 0) == '*';
+ p = p.substring((strong ? 1 : 0) + (fixed ? 1 : 0));
+ long curMod = fixed ? Long.valueOf(p).longValue() : new File(
+ db.getWorkTree(), p).lastModified();
+ if (strong)
+ assertTrue("path " + p + " is not younger than predecesssor",
+ curMod > lastMod);
+ else
+ assertTrue("path " + p + " is older than predecesssor",
+ curMod >= lastMod);
+ }
+ }
}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java
index 2410d6f..3034bfb 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java
@@ -204,6 +204,13 @@ public class ResolveMerger extends ThreeWayMerger {
}
if (!inCore) {
+ // No problem found. The only thing left to be done is to
+ // checkout
+ // all files from "theirs" which have been selected to go into
+ // the
+ // new index.
+ checkout();
+
// All content-merges are successfully done. If we can now write the
// new index we are on quite safe ground. Even if the checkout of
// files coming from "theirs" fails the user can work around such
@@ -214,10 +221,6 @@ public class ResolveMerger extends ThreeWayMerger {
}
builder = null;
- // No problem found. The only thing left to be done is to checkout
- // all files from "theirs" which have been selected to go into the
- // new index.
- checkout();
} else {
builder.finish();
builder = null;
@@ -313,13 +316,18 @@ public class ResolveMerger extends ThreeWayMerger {
* @param path
* @param p
* @param stage
+ * @param lastMod
+ * @param len
* @return the entry which was added to the index
*/
- private DirCacheEntry add(byte[] path, CanonicalTreeParser p, int stage) {
+ private DirCacheEntry add(byte[] path, CanonicalTreeParser p, int stage,
+ long lastMod, long len) {
if (p != null && !p.getEntryFileMode().equals(FileMode.TREE)) {
DirCacheEntry e = new DirCacheEntry(path, stage);
e.setFileMode(p.getEntryFileMode());
e.setObjectId(p.getEntryObjectId());
+ e.setLastModified(lastMod);
+ e.setLength(len);
builder.add(e);
return e;
}
@@ -327,6 +335,25 @@ public class ResolveMerger extends ThreeWayMerger {
}
/**
+ * adds a entry to the index builder which is a copy of the specified
+ * DirCacheEntry
+ *
+ * @param e
+ * the entry which should be copied
+ *
+ * @return the entry which was added to the index
+ */
+ private DirCacheEntry keep(DirCacheEntry e) {
+ DirCacheEntry newEntry = new DirCacheEntry(e.getPathString(), e.getStage());
+ newEntry.setFileMode(e.getFileMode());
+ newEntry.setObjectId(e.getObjectId());
+ newEntry.setLastModified(e.getLastModified());
+ newEntry.setLength(e.getLength());
+ builder.add(newEntry);
+ return newEntry;
+ }
+
+ /**
* Processes one path and tries to merge. This method will do all do all
* trivial (not content) merges and will also detect if a merge will fail.
* The merge will fail when one of the following is true
@@ -382,12 +409,27 @@ public class ResolveMerger extends ThreeWayMerger {
if (isIndexDirty())
return false;
+ DirCacheEntry ourDce = null;
+
+ if (index == null) {
+ // create a fake DCE, but only if ours is valid. ours is kept only
+ // in case it is valid, so a null ourDce is ok in all other cases.
+ if (modeO != 0) {
+ ourDce = new DirCacheEntry(tw.getRawPath());
+ ourDce.setObjectId(tw.getObjectId(T_OURS));
+ ourDce.setFileMode(tw.getFileMode(T_OURS));
+ }
+ } else {
+ ourDce = index.getDirCacheEntry();
+ }
+
if (nonTree(modeO) && nonTree(modeT) && tw.idEqual(T_OURS, T_THEIRS)) {
// OURS and THEIRS have equal content. Check the file mode
if (modeO == modeT) {
// content and mode of OURS and THEIRS are equal: it doesn't
- // matter which one we choose. OURS is chosen.
- add(tw.getRawPath(), ours, DirCacheEntry.STAGE_0);
+ // matter which one we choose. OURS is chosen. Since the index
+ // is clean (the index matches already OURS) we can keep the existing one
+ keep(ourDce);
// no checkout needed!
return true;
} else {
@@ -398,22 +440,25 @@ public class ResolveMerger extends ThreeWayMerger {
if (newMode != FileMode.MISSING.getBits()) {
if (newMode == modeO)
// ours version is preferred
- add(tw.getRawPath(), ours, DirCacheEntry.STAGE_0);
+ keep(ourDce);
else {
// the preferred version THEIRS has a different mode
// than ours. Check it out!
if (isWorktreeDirty(work))
return false;
+ // we know about length and lastMod only after we have written the new content.
+ // This will happen later. Set these values to 0 for know.
DirCacheEntry e = add(tw.getRawPath(), theirs,
- DirCacheEntry.STAGE_0);
+ DirCacheEntry.STAGE_0, 0, 0);
toBeCheckedOut.put(tw.getPathString(), e);
}
return true;
} else {
- // FileModes are not mergeable. We found a conflict on modes
- add(tw.getRawPath(), base, DirCacheEntry.STAGE_1);
- add(tw.getRawPath(), ours, DirCacheEntry.STAGE_2);
- add(tw.getRawPath(), theirs, DirCacheEntry.STAGE_3);
+ // FileModes are not mergeable. We found a conflict on modes.
+ // For conflicting entries we don't know lastModified and length.
+ add(tw.getRawPath(), base, DirCacheEntry.STAGE_1, 0, 0);
+ add(tw.getRawPath(), ours, DirCacheEntry.STAGE_2, 0, 0);
+ add(tw.getRawPath(), theirs, DirCacheEntry.STAGE_3, 0, 0);
unmergedPaths.add(tw.getPathString());
mergeResults.put(
tw.getPathString(),
@@ -426,8 +471,8 @@ public class ResolveMerger extends ThreeWayMerger {
if (nonTree(modeO) && modeB == modeT && tw.idEqual(T_BASE, T_THEIRS)) {
// THEIRS was not changed compared to BASE. All changes must be in
- // OURS. OURS is chosen.
- add(tw.getRawPath(), ours, DirCacheEntry.STAGE_0);
+ // OURS. OURS is chosen. We can keep the existing entry.
+ keep(ourDce);
// no checkout needed!
return true;
}
@@ -440,8 +485,11 @@ public class ResolveMerger extends ThreeWayMerger {
if (isWorktreeDirty(work))
return false;
if (nonTree(modeT)) {
+ // we know about length and lastMod only after we have written
+ // the new content.
+ // This will happen later. Set these values to 0 for know.
DirCacheEntry e = add(tw.getRawPath(), theirs,
- DirCacheEntry.STAGE_0);
+ DirCacheEntry.STAGE_0, 0, 0);
if (e != null)
toBeCheckedOut.put(tw.getPathString(), e);
return true;
@@ -460,16 +508,16 @@ public class ResolveMerger extends ThreeWayMerger {
// detected later
if (nonTree(modeO) && !nonTree(modeT)) {
if (nonTree(modeB))
- add(tw.getRawPath(), base, DirCacheEntry.STAGE_1);
- add(tw.getRawPath(), ours, DirCacheEntry.STAGE_2);
+ add(tw.getRawPath(), base, DirCacheEntry.STAGE_1, 0, 0);
+ add(tw.getRawPath(), ours, DirCacheEntry.STAGE_2, 0, 0);
unmergedPaths.add(tw.getPathString());
enterSubtree = false;
return true;
}
if (nonTree(modeT) && !nonTree(modeO)) {
if (nonTree(modeB))
- add(tw.getRawPath(), base, DirCacheEntry.STAGE_1);
- add(tw.getRawPath(), theirs, DirCacheEntry.STAGE_3);
+ add(tw.getRawPath(), base, DirCacheEntry.STAGE_1, 0, 0);
+ add(tw.getRawPath(), theirs, DirCacheEntry.STAGE_3, 0, 0);
unmergedPaths.add(tw.getPathString());
enterSubtree = false;
return true;
@@ -502,10 +550,10 @@ public class ResolveMerger extends ThreeWayMerger {
if (((modeO != 0 && !tw.idEqual(T_BASE, T_OURS)) || (modeT != 0 && !tw
.idEqual(T_BASE, T_THEIRS)))) {
- add(tw.getRawPath(), base, DirCacheEntry.STAGE_1);
- add(tw.getRawPath(), ours, DirCacheEntry.STAGE_2);
+ add(tw.getRawPath(), base, DirCacheEntry.STAGE_1, 0, 0);
+ add(tw.getRawPath(), ours, DirCacheEntry.STAGE_2, 0, 0);
DirCacheEntry e = add(tw.getRawPath(), theirs,
- DirCacheEntry.STAGE_3);
+ DirCacheEntry.STAGE_3, 0, 0);
// OURS was deleted checkout THEIRS
if (modeO == 0) {
@@ -609,9 +657,9 @@ public class ResolveMerger extends ThreeWayMerger {
// a conflict occurred, the file will contain conflict markers
// the index will be populated with the three stages and only the
// workdir (if used) contains the halfways merged content
- add(tw.getRawPath(), base, DirCacheEntry.STAGE_1);
- add(tw.getRawPath(), ours, DirCacheEntry.STAGE_2);
- add(tw.getRawPath(), theirs, DirCacheEntry.STAGE_3);
+ add(tw.getRawPath(), base, DirCacheEntry.STAGE_1, 0, 0);
+ add(tw.getRawPath(), ours, DirCacheEntry.STAGE_2, 0, 0);
+ add(tw.getRawPath(), theirs, DirCacheEntry.STAGE_3, 0, 0);
mergeResults.put(tw.getPathString(), result);
} else {
// no conflict occurred, the file will contain fully merged content.