You converted "ce->ce_mode = 0" to "ce->ce_flags |= CE_REMOVE" in an
earlier commit 7a51ed6 (Make on-disk index representation
separate from in-core one).
I am having two issues with this conversion, related to read-tree.
If you say "git reset --hard" with an unmerged path in the
index, the entry does not get resurrected from the HEAD. It
instead just goes away (i.e. you lose a path in the index).
If you run "git reset --hard" twice, the second run will
resurrect it, of course, as the first one removed the unmerged
paths.
"git reset --hard" internally runs "read-tree -u --reset HEAD",
and the oneway_merge() misbehaves.
commit 7a51ed66f653c248993b3c4a61932e47933d835e
Author: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Date: Mon Jan 14 16:03:17 2008 -0800
Make on-disk index representation separate from in-core one
diff --git a/builtin-read-tree.c b/builtin-read-tree.c
index c0ea034..5785401 100644
--- a/builtin-read-tree.c
+++ b/builtin-read-tree.c
@@ -45,8 +45,7 @@ static int read_cache_unmerged(void)
continue;
cache_tree_invalidate_path(active_cache_tree, ce->name);
last = ce;
- ce->ce_mode = 0;
- ce->ce_flags &= ~htons(CE_STAGEMASK);
+ ce->ce_flags |= CE_REMOVE;
}
*dst++ = ce;
}
One issue is somewhat apparent. The conversion forgot to drop
the stage information (i.e. it does not tell "that stage#0 path
is to be removed" anymore).
Another thing is a bit trickier. Now because you do not smudge
ce->ce_mode, when oneway_merge in unpack-trees.c compares it
(which comes as *old) with what is read from HEAD, it triggers
this codepath:
if (old && same(old, a)) {
if (o->reset) {
struct stat st;
if (lstat(old->name, &st) ||
ce_match_stat(old, &st, CE_MATCH_IGNORE_VALID))
old->ce_flags |= CE_UPDATE;
}
return keep_entry(old, o);
}
Here, same(old, a) yields true, old->ce_flags gets CE_UPDATE,
and then keep_entry(old, o) keeps that old entry, which is at
stage #1 and has (CE_REMOVE|CE_UPDATE) flags set. This index is
written out, making the resulting index empty.
The reason we keep an index entry with ce_mode = 0 (and now
CE_REMOVE) when we want to remive it is because we would want to
be able to say "propagate this change to the work tree" when run
with CE_UPDATE. But I think the reason read_cache_unmerged()
says "this unmerged entry is gone" is _not_ because we would
want to remove the corresponding path from the work tree.
The old code happened to work because "ce_mode = 0" entries
would have never matched with what was read from the HEAD tree,
and we would never have triggered the keep_entry() codepath.
We could of course hack read_cache_unmerged() to:
if (ce_stage(ce)) {
if (last && !strcmp(ce->name, last->name))
continue;
cache_tree_invalidate_path(active_cache_tree, ce->name);
last = ce;
ce->ce_flags &= ~CE_STAGEMASK;
/* Do not match with entries from trees! */
ce->ce_mode = 0;
}
*dst++ = ce;
but I am wondering if we should instead really _remove_ entries
from the index instead, just like the attached patch.
Thoughts?
builtin-read-tree.c | 2 +-
t/t7104-reset.sh | 35 +++++++++++++++++++++++++++++++++++
2 files changed, 36 insertions(+), 1 deletions(-)
diff --git a/builtin-read-tree.c b/builtin-read-tree.c
index 5785401..726fb0b 100644
--- a/builtin-read-tree.c
+++ b/builtin-read-tree.c
@@ -45,7 +45,7 @@ static int read_cache_unmerged(void)
continue;
cache_tree_invalidate_path(active_cache_tree, ce->name);
last = ce;
- ce->ce_flags |= CE_REMOVE;
+ continue;
}
*dst++ = ce;
}
diff --git a/t/t7104-reset.sh b/t/t7104-reset.sh
new file mode 100755
index 0000000..831078c
--- /dev/null
+++ b/t/t7104-reset.sh
@@ -0,0 +1,35 @@
+#!/bin/sh
+
+test_description='reset --hard unmerged'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+
+ >hello &&
+ git add hello &&
+ git commit -m world &&
+
+ H=$(git rev-parse :hello) &&
+ git rm --cached hello &&
+ for i in 1 2 3
+ do
+ echo "100644 $H $i hello"
+ done | git update-index --index-info &&
+
+ rm -f hello &&
+ mkdir -p hello &&
+ >hello/world &&
+ test "$(git ls-files -o)" = hello/world
+
+'
+
+test_expect_failure 'reset --hard loses the index' '
+
+ git reset --hard &&
+ git ls-files --error-unmatch hello &&
+ test -f hello
+
+'
+
+test_done
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
