[PATCH 2/3] merge-base: return fork-point outside reflog
- From
Michael J Gruber <git@grubix.eu>
- Date
- Sep 14, 2017, 13:15 UTC
- Message-ID
- <5513a1415d11517c28158d9b4212d383a233182f.1505394278.git.git@grubix.eu>
- In-Reply-To
- <68633b20-9e2b-ae23-2ede-8728283250f0@grubix.eu>
4f21454b55 ("merge-base: handle --fork-point without reflog", 2016-10-12) fixed the case without reflog, but only partially: The original code checks whether the merge base candidates are from the list that we started with, which was the list of reflog entries before 4f21454b55 and the list of reflog entries - or the ref itself if empty - after. The test from 4f21454b55 tested in a situation where the merge base candidate equalled the commit at refname, so it was on this (1 item) list by accident.
In fact, per documentation "--fork-point" looks at the reflog in addition to doing the usual walk from the tip. The original design description in d96855ff51 ("merge-base: teach "--fork-point" mode", 2013-10-23) describes this as computing from a virtual merge-base of all the historical tips of refname. They may or may not all be present in the reflog (think pruning, non-ff fetching, fast forwarding etc.), so filtering by the current contents of the reflog is potentially harmful, and it does not seem to fulfill any purpose in the original design.
Remove the filtering and add a test for an out-of-reflog merge base.
Reported-by: Ekelhart Jakob <jakob.ekelhart@fsw.at> Signed-off-by: Michael J Gruber <git@grubix.eu> --- builtin/merge-base.c | 18 +++--------------- t/t6010-merge-base.sh | 8 ++++++++ 2 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/builtin/merge-base.c b/builtin/merge-base.c index 6dbd167d3b..926a7615ea 100644 --- a/builtin/merge-base.c +++ b/builtin/merge-base.c @@ -186,23 +186,11 @@ static int handle_fork_point(int argc, const char **argv) * There should be one and only one merge base, when we found * a common ancestor among reflog entries. */ - if (!bases || bases->next) { + if (!bases || bases->next) ret = 1; - goto cleanup_return; - } - - /* And the found one must be one of the reflog entries */ - for (i = 0; i < revs.nr; i++) - if (&bases->item->object == &revs.commit[i]->object) - break; /* found */ - if (revs.nr <= i) { - ret = 1; /* not found */ - goto cleanup_return; - } - - printf("%s\n", oid_to_hex(&bases->item->object.oid)); + else + printf("%s\n", oid_to_hex(&bases->item->object.oid)); -cleanup_return: free_commit_list(bases); return ret; } diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh index 17fffd7998..850463d4f2 100755 --- a/t/t6010-merge-base.sh +++ b/t/t6010-merge-base.sh @@ -267,6 +267,14 @@ test_expect_success '--fork-point works with empty reflog' ' test_cmp expect actual ' +test_expect_success '--fork-point works with merge-base outside reflog' ' + git -c core.logallrefupdates=false checkout no-reflog && + git -c core.logallrefupdates=false commit --allow-empty -m "Commit outside reflogs" && + git rev-parse base >expect && + git merge-base --fork-point no-reflog derived >actual && + test_cmp expect actual +' + test_expect_success 'merge-base --octopus --all for complex tree' ' # Best common ancestor for JE, JAA and JDD is JC # JE
-- 2.14.1.712.gda4591c8a2