Re: [BUG] Git fast-export with import marks file omits merge commits

[Junio C Hamano]

Isaac Chou <isaac.c...@microfocus.com> writes:

> I inspected the source code (builtin/fast-export.c) for the
> fast-export issue I encountered, and it looks like the merge
> commit is discarded too early by the call to object_array_pop()
> after only one of the two UNSHOWN parents is processed in the
> method handle_tail().  The poped merge commit still has one
> UNSHOWN parent, therefore it is not processed and is lost in the
> output.  Can someone advise me on how to submit a code change or
> bug report in order to get the fix into the code base?
>
> Thanks,
>
> Isaac
>
> -----Original Message-----
> From: git-ow...@vger.kernel.org [mailto:git-ow...@vger.kernel.org] On Behalf 
> Of Isaac Chou
> Sent: Monday, April 16, 2018 3:58 PM
> To: git@vger.kernel.org
> Subject: Git fast-export with import marks file omits merge commits
>
> Hello,
>
> I came across a change of behavior with Git version 2.15 and later
> where the fast-export command would omit the merge commits.  The
> same use case works correctly with Git version 2.14 and older.
> ...

There indeed are some differences between v2.14 and v2.15 around the
code that returns early when has_unshown_parent() says "yes" [*1*],
but the decision to return early when the function says "yes" hasn't
changed between that timeperiod---it dates back to f2dc849e ("Add
'git fast-export', the sister of 'git fast-import'", 2007-12-02),
i.e. the very beginning of the program's life.

I'll CC those who wrote the original and b3e8ca89 ("fast-export: do
not copy from modified file", 2017-09-20) and 71992039
("object_array: add and use `object_array_pop()`", 2017-09-23),
which are the only two commits that touch the surrounding area
during that timeperiod, to ask if something jumps at them.

Thanks.


[Footnotes]

*1* An excerpt from 'git diff v2.14.0 v2.15.0 builtin/fast-export.c'
    reads like so:

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index d412c0a8f3..2fb60d6d48 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
...
@@ -630,15 +645,15 @@ static void *anonymize_tag(const void *old, size_t *len)
        return strbuf_detach(&out, len);
 }
 
-static void handle_tail(struct object_array *commits, struct rev_info *revs)
+static void handle_tail(struct object_array *commits, struct rev_info *revs,
+                       struct string_list *paths_of_changed_objects)
 {
        struct commit *commit;
        while (commits->nr) {
-               commit = (struct commit *)commits->objects[commits->nr - 
1].item;
+               commit = (struct commit *)object_array_pop(commits);
                if (has_unshown_parent(commit))
                        return;
-               handle_commit(commit, revs);
-               commits->nr--;
+               handle_commit(commit, revs, paths_of_changed_objects);
        }
 }