Martin Ågren <martin.agren@xxxxxxxxx> writes:
> +test_expect_success 'merge commit gets exported with --import-marks' '
> + test_create_repo merging &&
> + (
> + cd merging &&
> + test_commit initial &&
> + git checkout -b topic &&
> + test_commit on-topic &&
> + git checkout master &&
> + test_commit on-master &&
> + test_tick &&
> + git merge --no-ff -m Yeah topic &&
> +
> + echo ":1 $(git rev-parse HEAD^^)" >marks &&
> + git fast-export --import-marks=marks master >out &&
> + grep Yeah out
> + )
> +'
This test looks much better than the one in the earlier iteration,
but I do not think the updated "fix" below is better. It might be
just aesthetics and I suspect I won't find it as disturbing if we
could push with
object_array_push(commits, (struct object *)commit);
or something that is more clearly symmetric to object_array_pop().
The "Queue again" comment is needed only because use of "add"
highlights the lack of symmetry.
With add_object_array(), it looks somewhat more odd than your
previous
peek it to check;
if (it should not be molested)
return;
pop to mark it consumed;
consume it;
sequence, in which peek() and pop() were more obviously related
operations on the same "array" object.
And I do not think it is a good idea to introduce _push() only for
symmetry (it would merely be a less capable version of add whose
name is spelled differently). Hence my preference for peek-check-pop
over pop-oops-push-again-but-push-spelled-as-add.
Not worth a reroll, though. I just wanted to spread better design
sense to contributors ;-)
> test_done
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index 27b2cc138e..7b8dfc5af1 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -651,8 +651,11 @@ static void handle_tail(struct object_array *commits, struct rev_info *revs,
> struct commit *commit;
> while (commits->nr) {
> commit = (struct commit *)object_array_pop(commits);
> - if (has_unshown_parent(commit))
> + if (has_unshown_parent(commit)) {
> + /* Queue again, to be handled later */
> + add_object_array(&commit->object, NULL, commits);
> return;
> + }
> handle_commit(commit, revs, paths_of_changed_objects);
> }
> }
