Re: [PATCH] merge: refuse to create too cool a merge by default

[Ramsay Jones <ramsay@ramsayjones.plus.com>]

On 18/03/16 20:21, Junio C Hamano wrote:
> While it makes sense to allow merging unrelated histories of two
> projects that started independently into one, in the way "gitk" was
> merged to "git" itself aka "the coolest merge ever", such a merge is
> still an unusual event.  Worse, if somebody creates an independent
> history by starting from a tarball of an established project and
> sends a pull request to the original project, "git merge" however
> happily creates such a merge without any sign of something unusual
> is happening.
> 
> Teach "git merge" to refuse to create such a merge by default,
> unless the user passes a new "--allow-unrelated-histories" option to
> tell it that the user is aware that two unrelated projects are
> merged.
> 
> Because such a "two project merge" is a rare event, a configuration
> option to always allow such a merge is not added.
> 
> We could add the same option to "git pull" and have it passed
> through to underlying "git merge".  I do not have a fundamental
> opposition against such a feature, but this commit does not do so
> and instead leaves it as low-hanging fruit for others, because such
> a "two project merge" would be done after fetching the other project
> into some location in the working tree of an existing project and
> making sure how well they fit together, it is sufficient to allow a
> local merge without such an option pass-through from "git pull" to
> "git merge".  Many tests that are updated by this patch does the
> pass-through manually by turning:
> 
> 	git pull something
> 
> into its equivalent:
> 
> 	git fetch something &&
>         git merge --allow-unrelated-histories FETCH_HEAD
> 
> If somebody is inclined to add such an option, updated tests in this
> change need to be adjusted back to:
> 
> 	git pull --allow-unrelated-histories something
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> 
>  builtin/merge.c                 | 12 +++++++++---
>  t/t3412-rebase-root.sh          |  2 +-
>  t/t5500-fetch-pack.sh           |  6 ++++--
>  t/t6009-rev-list-parent.sh      |  4 +++-
>  t/t6010-merge-base.sh           |  6 ++++--
>  t/t6012-rev-list-simplify.sh    |  2 +-
>  t/t6026-merge-attr.sh           |  3 ++-
>  t/t6029-merge-subtree.sh        |  2 +-
>  t/t6101-rev-parse-parents.sh    |  2 +-
>  t/t9400-git-cvsserver-server.sh |  3 ++-
>  10 files changed, 28 insertions(+), 14 deletions(-)
> 

[snip]

> diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh
> index 39b3238..e0c5f44 100755
> --- a/t/t6010-merge-base.sh
> +++ b/t/t6010-merge-base.sh
> @@ -215,11 +215,13 @@ test_expect_success 'criss-cross merge-base for octopus-step' '
>  	git reset --hard E &&
>  	test_commit CC2 &&
>  	test_tick &&
> -	git merge -s ours CC1 &&
> +	# E is a root commit unrelated to MMR root on which CC1 is based
> +	git merge -s ours --allow-unrelated-histories CC1 &&
>  	test_commit CC-o &&
>  	test_commit CCB &&
>  	git reset --hard CC1 &&
> -	git merge -s ours CC2 &&
> +	# E is a root commit unrelated to MMR root on which CC1 is based
> +	git merge -s ours --allow-unrelated-histories CC2 &&

I was only skimming this patch, but the above caught my eye - I assume
that the comment should reference CC2 not CC1. yes?

ATB,
Ramsay Jones