[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
Re: [jgit-dev] A bug in WorkDirCheckout?
|
fredagen den 28 maj 2010 17.19.05 skrev Christian Halstrick:
> Hi,
>
> I am currently "porting" WorkDirCheckout to a new DirCacheCheckout to
> implement real merges. While doing so I detected something which
> looks like a bug to me:
>
> WorkDirCheckout.processEntry() should implement what is defined in a
> big table in the man page of "git read-tree" (see
> http://www.kernel.org/pub/software/scm/git/docs/git-read-tree.html ).
> I think that our implementation is not doing what is writen in the
> latest version of the man page in "case 3". E.g. the checkout should
> fail if the head-commit and merge-commit contain different content for
> a path but the index contains nothing for this path (means a deletion
> for this path was added to index). But our code doesn't respect that
> situation.
>
> I tried to fix this and wrote a test which tests a certain aspect of
> "case 3". I'll attach this fix. With my fix my new test goes through
> (without my fix the test fails). But my fix breaks 3 tests in
> ReadTreeTest.java. Any ideas somebody what's going wrong?
I had to hand apply your patch, but some comments
testRules1thru3_NoIndexEntry, last part is the same as your 3b. So the
existing test is wrong.
The other ones involved directories and workdireckout seem so do the wrong
thing.
e.g, in. testDirectoryFileConflicts_4, the input is
DF/DF(somecontent)
DF/DF(othercontent
DF(othercontent)
DF is selected during read without regard for the fact that the other
two entries conflict due to directory/filename reasons. Then your change
kicks in and sees the two differnt DF/DF's without and index entry (already
consumed) and decides that this is a conflict, which is right, so the test
is wrong here too,
In testDirectoryFileConflicts_9 we have
doit(mk("DF"), mkmap("DF", "QP"), mk("DF/DF"));
DF(content)
DF(othercontent)
DF/DF
which is the case above, Two different files and no (matching) index, i.e. a
conflict, which is to say that the current test is wrong.
-- robin
> >From d384452623e6447780fcc1df596665c8ab1aa272 Fri, 28 May 2010 17:18:38
> >+0200
>
> From: Christian Halstrick <christian.halstrick@xxxxxxx>
> Date: Fri, 28 May 2010 17:14:40 +0200
> Subject: [PATCH] fix WorkDirCheckout
>
> Signed-off-by: Christian Halstrick <christian.halstrick@xxxxxxx>
>
> 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
> 481691a..118ad54 100644
> --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ReadTreeTest.java
> +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ReadTreeTest.java
> @@ -90,6 +90,22 @@
> assertEquals(anotherId, readTree.updated.get("foo"));
> }
>
> + public void testRule3b_NoIndexEntry() throws IOException {
> + HashMap<String, String> headMap, mergeMap, idxMap;
> +
> + headMap = new HashMap<String, String>();
> + headMap.put("foo", "foo");
> + mergeMap = new HashMap<String, String>();
> + mergeMap.put("foo", "fox");
> + idxMap = new HashMap<String, String>();
> +
> + setupCase(headMap, mergeMap, idxMap);
> + theReadTree = go();
> +
> + assertTrue(theReadTree.updated.isEmpty());
> + assertTrue(theReadTree.conflicts.contains("foo"));
> + }
> +
> void setupCase(HashMap<String, String> headEntries,
> HashMap<String, String> mergeEntries,
> HashMap<String, String> indexEntries) throws IOException {
> diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/WorkDirCheckout.java
> b/org.eclipse.jgit/src/org/eclipse/jgit/lib/WorkDirCheckout.java
> index ee78202..d2f75f9 100644
> --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/WorkDirCheckout.java
> +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/WorkDirCheckout.java
> @@ -318,13 +318,19 @@
> 0 nothing nothing nothing (does not happen)
> 1 nothing nothing exists use M
> 2 nothing exists nothing remove path from
index
> - 3 nothing exists exists use M */
> + 3 nothing exists exists, use M if "initial
> checkout", + H == M
> keep
> index otherwise
> + exists, fail
> + H != M */
>
> if (h == null) {
> updated.put(name,mId);
> } else if (m == null) {
> removed.add(name);
> + } else if (!hId.equals(mId)) { // case 3b: H!=M
> + conflicts.add(name);
> } else {
> + // TODO detect "initial checkout" and act properly
> updated.put(name, mId);
> }
> } else if (h == null) {
>
> --
> Git Team Provider UI (Incubation) 0.8.0.201005251513
> _______________________________________________
> jgit-dev mailing list
> jgit-dev@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/jgit-dev