Skip to content

Commit e9bba53

Browse files
gitstergitster
committed
Merge branch 'jh/fast-import-notes'
* jh/fast-import-notes: fast-import: Fix incorrect fanout level when modifying existing notes refs t9301: Add 2nd testcase exposing bugs in fast-import's notes fanout handling t9301: Fix testcase covering up a bug in fast-import's notes fanout handling
2 parents 0bbaa5c + 1838685 commit e9bba53

File tree

2 files changed

+83
-8
lines changed

2 files changed

+83
-8
lines changed

‎fast-import.c‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2173,6 +2173,11 @@ static uintmax_t do_change_note_fanout(
21732173

21742174
if (tmp_hex_sha1_len == 40 && !get_sha1_hex(hex_sha1, sha1)) {
21752175
/* This is a note entry */
2176+
if (fanout == 0xff) {
2177+
/* Counting mode, no rename */
2178+
num_notes++;
2179+
continue;
2180+
}
21762181
construct_path_with_fanout(hex_sha1, fanout, realpath);
21772182
if (!strcmp(fullpath, realpath)) {
21782183
/* Note entry is in correct location */
@@ -2379,7 +2384,7 @@ static void file_change_cr(struct branch *b, int rename)
23792384
leaf.tree);
23802385
}
23812386

2382-
static void note_change_n(struct branch *b, unsigned char old_fanout)
2387+
static void note_change_n(struct branch *b, unsigned char *old_fanout)
23832388
{
23842389
const char *p = command_buf.buf + 2;
23852390
static struct strbuf uq = STRBUF_INIT;
@@ -2390,6 +2395,23 @@ static void note_change_n(struct branch *b, unsigned char old_fanout)
23902395
uint16_t inline_data = 0;
23912396
unsigned char new_fanout;
23922397

2398+
/*
2399+
* When loading a branch, we don't traverse its tree to count the real
2400+
* number of notes (too expensive to do this for all non-note refs).
2401+
* This means that recently loaded notes refs might incorrectly have
2402+
* b->num_notes == 0, and consequently, old_fanout might be wrong.
2403+
*
2404+
* Fix this by traversing the tree and counting the number of notes
2405+
* when b->num_notes == 0. If the notes tree is truly empty, the
2406+
* calculation should not take long.
2407+
*/
2408+
if (b->num_notes == 0 && *old_fanout == 0) {
2409+
/* Invoke change_note_fanout() in "counting mode". */
2410+
b->num_notes = change_note_fanout(&b->branch_tree, 0xff);
2411+
*old_fanout = convert_num_notes_to_fanout(b->num_notes);
2412+
}
2413+
2414+
/* Now parse the notemodify command. */
23932415
/* <dataref> or 'inline' */
23942416
if (*p == ':') {
23952417
char *x;
@@ -2450,7 +2472,7 @@ static void note_change_n(struct branch *b, unsigned char old_fanout)
24502472
typename(type), command_buf.buf);
24512473
}
24522474

2453-
construct_path_with_fanout(sha1_to_hex(commit_sha1), old_fanout, path);
2475+
construct_path_with_fanout(sha1_to_hex(commit_sha1), *old_fanout, path);
24542476
if (tree_content_remove(&b->branch_tree, path, NULL))
24552477
b->num_notes--;
24562478

@@ -2637,7 +2659,7 @@ static void parse_new_commit(void)
26372659
else if (!prefixcmp(command_buf.buf, "C "))
26382660
file_change_cr(b, 0);
26392661
else if (!prefixcmp(command_buf.buf, "N "))
2640-
note_change_n(b, prev_fanout);
2662+
note_change_n(b, &prev_fanout);
26412663
else if (!strcmp("deleteall", command_buf.buf))
26422664
file_change_deleteall(b);
26432665
else if (!prefixcmp(command_buf.buf, "ls "))

‎t/t9301-fast-import-notes.sh‎

Lines changed: 58 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -505,9 +505,63 @@ test_expect_success 'verify that non-notes are untouched by a fanout change' '
505505
test_cmp expect_non-note3 actual
506506
507507
'
508+
509+
# Change the notes for the three top commits
510+
test_tick
511+
cat >input <<INPUT_END
512+
commit refs/notes/many_notes
513+
committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
514+
data <<COMMIT
515+
changing notes for the top three commits
516+
COMMIT
517+
from refs/notes/many_notes^0
518+
INPUT_END
519+
520+
rm expect
521+
i=$num_commits
522+
j=0
523+
while test $j -lt 3
524+
do
525+
cat >>input <<INPUT_END
526+
N inline refs/heads/many_commits~$j
527+
data <<EOF
528+
changed note for commit #$i
529+
EOF
530+
INPUT_END
531+
cat >>expect <<EXPECT_END
532+
commit #$i
533+
changed note for commit #$i
534+
EXPECT_END
535+
i=$(($i - 1))
536+
j=$(($j + 1))
537+
done
538+
539+
test_expect_success 'change a few existing notes' '
540+
541+
git fast-import <input &&
542+
GIT_NOTES_REF=refs/notes/many_notes git log -n3 refs/heads/many_commits |
543+
grep "^ " > actual &&
544+
test_cmp expect actual
545+
546+
'
547+
548+
test_expect_success 'verify that changing notes respect existing fanout' '
549+
550+
# None of the entries in the top-level notes tree should be a full SHA1
551+
git ls-tree --name-only refs/notes/many_notes |
552+
while read path
553+
do
554+
if test $(expr length "$path") -ge 40
555+
then
556+
return 1
557+
fi
558+
done
559+
560+
'
561+
508562
remaining_notes=10
509563
test_tick
510-
cat >>input <<INPUT_END
564+
cat >input <<INPUT_END
511565
commit refs/notes/many_notes
512566
committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
513567
data <<COMMIT
@@ -516,12 +570,11 @@ COMMIT
516570
from refs/notes/many_notes^0
517571
INPUT_END
518572

519-
i=$remaining_notes
520-
while test $i -lt $num_commits
573+
i=$(($num_commits - $remaining_notes))
574+
for sha1 in $(git rev-list -n $i refs/heads/many_commits)
521575
do
522-
i=$(($i + 1))
523576
cat >>input <<INPUT_END
524-
N 0000000000000000000000000000000000000000 :$i
577+
N 0000000000000000000000000000000000000000 $sha1
525578
INPUT_END
526579
done
527580

0 commit comments

Comments
 (0)