Skip to content

Commit d107541

Browse files
jherlandgitster
authored andcommitted
t9301: Fix testcase covering up a bug in fast-import's notes fanout handling
There is a bug in fast-import where the fanout levels of an existing notes tree being loaded into the fast-import machinery is disregarded. Instead, any tree loaded is assumed to have a fanout level of 0. If the true fanout level is deeper, any attempt to remove a note from that tree will silently fail (as the note will not be found at fanout level 0). However, this bug was covered up by the way in which the t9301 testcase was written: When generating the fast-import commands to test mass removal of notes, we appended these commands to an already existing 'input' file which happened to already contain the fast-import commands used in the previous subtest to generate the very same notes tree. This would normally be harmless (but suboptimal) as the notes created were identical to the notes already present in the notes tree. But the act of repeating all the notes additions caused the internal fast-import data structures to recalculate the fanout, instead of hanging on to the initial (incorrect) fanout (that causes the bug described above). Thus, the subsequent removal of notes in the same 'input' file would succeed, thereby covering up the bug described above. This patch creates a new 'input' file instead of appending to the file from the previous subtest. Thus, we end up properly testing removal of notes that were added by a previous fast-import command. As a side effect, the notes removal can no longer refer to commits using the marks set by the previous fast-import run, instead the commits names must be referenced directly. The underlying fast-import bug is still present after this patch, but now we have at least uncovered it. Therefore, the affected subtests are labeled as expected failures until the underlying bug is fixed. Signed-off-by: Johan Herland <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent fc14b89 commit d107541

File tree

1 file changed

+6
-7
lines changed

1 file changed

+6
-7
lines changed

t/t9301-fast-import-notes.sh

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,7 @@ test_expect_success 'verify that non-notes are untouched by a fanout change' '
507507
'
508508
remaining_notes=10
509509
test_tick
510-
cat >>input <<INPUT_END
510+
cat >input <<INPUT_END
511511
commit refs/notes/many_notes
512512
committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
513513
data <<COMMIT
@@ -516,12 +516,11 @@ COMMIT
516516
from refs/notes/many_notes^0
517517
INPUT_END
518518

519-
i=$remaining_notes
520-
while test $i -lt $num_commits
519+
i=$(($num_commits - $remaining_notes))
520+
for sha1 in $(git rev-list -n $i refs/heads/many_commits)
521521
do
522-
i=$(($i + 1))
523522
cat >>input <<INPUT_END
524-
N 0000000000000000000000000000000000000000 :$i
523+
N 0000000000000000000000000000000000000000 $sha1
525524
INPUT_END
526525
done
527526

@@ -541,7 +540,7 @@ EXPECT_END
541540
i=$(($i - 1))
542541
done
543542

544-
test_expect_success 'remove lots of notes' '
543+
test_expect_failure 'remove lots of notes' '
545544
546545
git fast-import <input &&
547546
GIT_NOTES_REF=refs/notes/many_notes git log refs/heads/many_commits |
@@ -550,7 +549,7 @@ test_expect_success 'remove lots of notes' '
550549
551550
'
552551

553-
test_expect_success 'verify that removing notes trigger fanout consolidation' '
552+
test_expect_failure 'verify that removing notes trigger fanout consolidation' '
554553
555554
# All entries in the top-level notes tree should be a full SHA1
556555
git ls-tree --name-only -r refs/notes/many_notes |

0 commit comments

Comments
 (0)