fast-export: fix regression skipping some merge-commits

7199203937 (object_array: add and use `object_array_pop()`, 2017-09-23) noted that the pattern `object = array.objects[--array.nr].item` could be abstracted as `object = object_array_pop(&array)`. Unfortunately, one of the conversions was horribly wrong. Between grabbing the last object (i.e., peeking at it) and decreasing the object count, the original code would sometimes return early. The updated code on the other hand, will always pop the last element, then maybe do the early return without doing anything with the object. The end result is that merge commits where all the parents have still not been exported will simply be dropped, meaning that they will be completely missing from the exported data. Re-add a commit when it is not yet time to handle it. An alternative that was considered was to peek-then-pop. That carries some risk with it since the peeking and popping need to act on the same object, in a concerted fashion. Add a test that would have caught this. Reported-by: Isaac Chou <Isaac.Chou@microfocus.com> Analyzed-by: Isaac Chou <Isaac.Chou@microfocus.com> Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Martin Ågren committed Apr 21, 2018 at 00:12 UTC be011bbe001facf71bd636494eb253aa5151d26a
2 files changed +22 -1
builtin/fast-export.c
+4 -1
@@ -651,8 +651,11 @@ static void handle_tail(struct object_array *commits, struct rev_info *revs,
651 struct commit *commit;
652 while (commits->nr) {
653 commit = (struct commit *)object_array_pop(commits);
654 - if (has_unshown_parent(commit))
654 + if (has_unshown_parent(commit)) {
655 + /* Queue again, to be handled later */
656 + add_object_array(&commit->object, NULL, commits);
657 return;
658 + }
659 handle_commit(commit, revs, paths_of_changed_objects);
660 }
661 }
t/t9350-fast-export.sh
+18
@@ -540,4 +540,22 @@ test_expect_success 'when using -C, do not declare copy when source of copy is a
540 test_cmp expected actual
541 '
542
543 +test_expect_success 'merge commit gets exported with --import-marks' '
544 + test_create_repo merging &&
545 + (
546 + cd merging &&
547 + test_commit initial &&
548 + git checkout -b topic &&
549 + test_commit on-topic &&
550 + git checkout master &&
551 + test_commit on-master &&
552 + test_tick &&
553 + git merge --no-ff -m Yeah topic &&
554 +
555 + echo ":1 $(git rev-parse HEAD^^)" >marks &&
556 + git fast-export --import-marks=marks master >out &&
557 + grep Yeah out
558 + )
559 +'
560 +
561 test_done