Skip to content

Commit 837e5fe

Browse files
Clemens Buchachergitster
authored andcommitted
unpack-trees: fix path search bug in verify_absent
Commit 0cf7375 (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 5521883 (checkout: do not lose staged removal), which reveals the bug in this case, but is otherwise unrelated. Signed-off-by: Junio C Hamano <[email protected]>
1 parent 6b9315d commit 837e5fe

File tree

2 files changed

+37
-10
lines changed

2 files changed

+37
-10
lines changed

t/t1001-read-tree-m-2way.sh

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,4 +365,31 @@ test_expect_success \
365365
git ls-files --stage &&
366366
test -f a/b'
367367

368+
test_expect_success \
369+
'a/b vs a, plus c/d case setup.' \
370+
'rm -f .git/index &&
371+
rm -fr a &&
372+
: >a &&
373+
mkdir c &&
374+
: >c/d &&
375+
git update-index --add a c/d &&
376+
treeM=`git write-tree` &&
377+
echo treeM $treeM &&
378+
git ls-tree $treeM &&
379+
git ls-files --stage >treeM.out &&
380+
381+
rm -f a &&
382+
mkdir a
383+
: >a/b &&
384+
git update-index --add --remove a a/b &&
385+
treeH=`git write-tree` &&
386+
echo treeH $treeH &&
387+
git ls-tree $treeH'
388+
389+
test_expect_success \
390+
'a/b vs a, plus c/d case test.' \
391+
'git read-tree -u -m "$treeH" "$treeM" &&
392+
git ls-files --stage | tee >treeMcheck.out &&
393+
test_cmp treeM.out treeMcheck.out'
394+
368395
test_done

unpack-trees.c

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -516,22 +516,22 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action,
516516
namelen = strlen(ce->name);
517517
pos = index_name_pos(o->src_index, ce->name, namelen);
518518
if (0 <= pos)
519-
return cnt; /* we have it as nondirectory */
519+
return 0; /* we have it as nondirectory */
520520
pos = -pos - 1;
521521
for (i = pos; i < o->src_index->cache_nr; i++) {
522-
struct cache_entry *ce = o->src_index->cache[i];
523-
int len = ce_namelen(ce);
522+
struct cache_entry *ce2 = o->src_index->cache[i];
523+
int len = ce_namelen(ce2);
524524
if (len < namelen ||
525-
strncmp(ce->name, ce->name, namelen) ||
526-
ce->name[namelen] != '/')
525+
strncmp(ce->name, ce2->name, namelen) ||
526+
ce2->name[namelen] != '/')
527527
break;
528528
/*
529-
* ce->name is an entry in the subdirectory.
529+
* ce2->name is an entry in the subdirectory.
530530
*/
531-
if (!ce_stage(ce)) {
532-
if (verify_uptodate(ce, o))
531+
if (!ce_stage(ce2)) {
532+
if (verify_uptodate(ce2, o))
533533
return -1;
534-
add_entry(o, ce, CE_REMOVE, 0);
534+
add_entry(o, ce2, CE_REMOVE, 0);
535535
}
536536
cnt++;
537537
}
@@ -623,7 +623,7 @@ static int verify_absent(struct cache_entry *ce, const char *action,
623623
* If this removed entries from the index,
624624
* what that means is:
625625
*
626-
* (1) the caller unpack_trees_rec() saw path/foo
626+
* (1) the caller unpack_callback() saw path/foo
627627
* in the index, and it has not removed it because
628628
* it thinks it is handling 'path' as blob with
629629
* D/F conflict;

0 commit comments

Comments
 (0)