From 6b9315d5a14b4d1389cff347fcfcb86624bbf645 Mon Sep 17 00:00:00 2001 From: Clemens Buchacher Date: Thu, 1 Jan 2009 21:54:31 +0100 Subject: [PATCH 1/3] unpack-trees: handle failure in verify_absent Commit 203a2fe1 (Allow callers of unpack_trees() to handle failure) changed the "die on error" behavior to "return failure code". verify_absent did not handle errors returned by verify_clean_subdirectory, however. Signed-off-by: Junio C Hamano --- t/t1001-read-tree-m-2way.sh | 24 ++++++++++++++++++++++++ unpack-trees.c | 8 +++++--- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/t/t1001-read-tree-m-2way.sh b/t/t1001-read-tree-m-2way.sh index 4b44e131b2..7f6ab31c36 100755 --- a/t/t1001-read-tree-m-2way.sh +++ b/t/t1001-read-tree-m-2way.sh @@ -341,4 +341,28 @@ test_expect_success \ check_cache_at DF/DF dirty && :' +test_expect_success \ + 'a/b (untracked) vs a case setup.' \ + 'rm -f .git/index && + : >a && + git update-index --add a && + treeM=`git write-tree` && + echo treeM $treeM && + git ls-tree $treeM && + git ls-files --stage >treeM.out && + + rm -f a && + git update-index --remove a && + mkdir a && + : >a/b && + treeH=`git write-tree` && + echo treeH $treeH && + git ls-tree $treeH' + +test_expect_success \ + 'a/b (untracked) vs a, plus c/d case test.' \ + '! git read-tree -u -m "$treeH" "$treeM" && + git ls-files --stage && + test -f a/b' + test_done diff --git a/unpack-trees.c b/unpack-trees.c index cba0aca062..3f42c2997f 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -587,7 +587,7 @@ static int verify_absent(struct cache_entry *ce, const char *action, return 0; if (!lstat(ce->name, &st)) { - int cnt; + int ret; int dtype = ce_to_dtype(ce); struct cache_entry *result; @@ -615,7 +615,9 @@ static int verify_absent(struct cache_entry *ce, const char *action, * files that are in "foo/" we would lose * it. */ - cnt = verify_clean_subdirectory(ce, action, o); + ret = verify_clean_subdirectory(ce, action, o); + if (ret < 0) + return ret; /* * If this removed entries from the index, @@ -634,7 +636,7 @@ static int verify_absent(struct cache_entry *ce, const char *action, * We need to increment it by the number of * deleted entries here. */ - o->pos += cnt; + o->pos += ret; return 0; } From 837e5fe95d9d6e597b748d338e45cca231f558fa Mon Sep 17 00:00:00 2001 From: Clemens Buchacher Date: Thu, 1 Jan 2009 21:54:32 +0100 Subject: [PATCH 2/3] unpack-trees: fix path search bug in verify_absent Commit 0cf73755 (unpack-trees.c: assume submodules are clean during check-out) changed an argument to verify_absent from 'path' to 'ce', which is however shadowed by a local variable of the same name. The bug triggers if verify_absent is used on a tree entry, for which the index contains one or more subsequent directories of the same length. The affected subdirectories are removed from the index. The testcase included in this commit bisects to 55218834 (checkout: do not lose staged removal), which reveals the bug in this case, but is otherwise unrelated. Signed-off-by: Junio C Hamano --- t/t1001-read-tree-m-2way.sh | 27 +++++++++++++++++++++++++++ unpack-trees.c | 20 ++++++++++---------- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/t/t1001-read-tree-m-2way.sh b/t/t1001-read-tree-m-2way.sh index 7f6ab31c36..271bc4e17f 100755 --- a/t/t1001-read-tree-m-2way.sh +++ b/t/t1001-read-tree-m-2way.sh @@ -365,4 +365,31 @@ test_expect_success \ git ls-files --stage && test -f a/b' +test_expect_success \ + 'a/b vs a, plus c/d case setup.' \ + 'rm -f .git/index && + rm -fr a && + : >a && + mkdir c && + : >c/d && + git update-index --add a c/d && + treeM=`git write-tree` && + echo treeM $treeM && + git ls-tree $treeM && + git ls-files --stage >treeM.out && + + rm -f a && + mkdir a + : >a/b && + git update-index --add --remove a a/b && + treeH=`git write-tree` && + echo treeH $treeH && + git ls-tree $treeH' + +test_expect_success \ + 'a/b vs a, plus c/d case test.' \ + 'git read-tree -u -m "$treeH" "$treeM" && + git ls-files --stage | tee >treeMcheck.out && + test_cmp treeM.out treeMcheck.out' + test_done diff --git a/unpack-trees.c b/unpack-trees.c index 3f42c2997f..48e538ab7e 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -516,22 +516,22 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action, namelen = strlen(ce->name); pos = index_name_pos(o->src_index, ce->name, namelen); if (0 <= pos) - return cnt; /* we have it as nondirectory */ + return 0; /* we have it as nondirectory */ pos = -pos - 1; for (i = pos; i < o->src_index->cache_nr; i++) { - struct cache_entry *ce = o->src_index->cache[i]; - int len = ce_namelen(ce); + struct cache_entry *ce2 = o->src_index->cache[i]; + int len = ce_namelen(ce2); if (len < namelen || - strncmp(ce->name, ce->name, namelen) || - ce->name[namelen] != '/') + strncmp(ce->name, ce2->name, namelen) || + ce2->name[namelen] != '/') break; /* - * ce->name is an entry in the subdirectory. + * ce2->name is an entry in the subdirectory. */ - if (!ce_stage(ce)) { - if (verify_uptodate(ce, o)) + if (!ce_stage(ce2)) { + if (verify_uptodate(ce2, o)) return -1; - add_entry(o, ce, CE_REMOVE, 0); + add_entry(o, ce2, CE_REMOVE, 0); } cnt++; } @@ -623,7 +623,7 @@ static int verify_absent(struct cache_entry *ce, const char *action, * If this removed entries from the index, * what that means is: * - * (1) the caller unpack_trees_rec() saw path/foo + * (1) the caller unpack_callback() saw path/foo * in the index, and it has not removed it because * it thinks it is handling 'path' as blob with * D/F conflict; From 7b9e3ce0255002075809917e0384cba3a0d18589 Mon Sep 17 00:00:00 2001 From: Clemens Buchacher Date: Thu, 1 Jan 2009 21:54:33 +0100 Subject: [PATCH 3/3] unpack-trees: remove redundant path search in verify_absent Since the only caller, verify_absent, relies on the fact that o->pos points to the next index entry anyways, there is no need to recompute its position. Furthermore, if a nondirectory entry were found, this would return too early, because there could still be an untracked directory in the way. This is currently not a problem, because verify_absent is only called if the index does not have this entry. Signed-off-by: Junio C Hamano --- unpack-trees.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/unpack-trees.c b/unpack-trees.c index 48e538ab7e..556813eca8 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -493,7 +493,7 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action, * anything in the existing directory there. */ int namelen; - int pos, i; + int i; struct dir_struct d; char *pathbuf; int cnt = 0; @@ -514,11 +514,7 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action, * in that directory. */ namelen = strlen(ce->name); - pos = index_name_pos(o->src_index, ce->name, namelen); - if (0 <= pos) - return 0; /* we have it as nondirectory */ - pos = -pos - 1; - for (i = pos; i < o->src_index->cache_nr; i++) { + for (i = o->pos; i < o->src_index->cache_nr; i++) { struct cache_entry *ce2 = o->src_index->cache[i]; int len = ce_namelen(ce2); if (len < namelen ||