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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 19, 2018, 22:48 UTC
Message-ID: <xmqq604mertl.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <MW2PR18MB2284E3091991A3DB228CEBBEE5B50@MW2PR18MB2284.namprd18.prod.outlook.com>

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

Show 25 quoted lines

> 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-owner@vger.kernel.org [mailto:git-owner@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);
 	}
 }