[PATCH 0/4] Loosening "two project merge" safety

[PATCH 0/4] Loosening "two project merge" safety

[Junio C Hamano]

Yaroslav Halchenko gave a vague "forcing 'git merge' users to always
give --allow-unrelated-histories option when they create crap/insane
merges are not nice", which I couldn't guess the validity due to
lack of concrete use case.  Just in case it is substantiated, here
is a series to selectively and safely loosen the safety for specific
use cases and users.

Junio C Hamano (4):
  t3033: avoid 'ambiguous refs' warning
  pull: pass --allow-unrelated-histories to "git merge"
  merge: GIT_MERGE_ALLOW_UNRELATED_HISTORIES environment
  merge: introduce merge.allowUnrelatedhistories configuration option

 Documentation/git-merge.txt     | 14 +-------------
 Documentation/git.txt           |  7 +++++++
 Documentation/merge-config.txt  |  7 +++++++
 Documentation/merge-options.txt |  8 ++++++++
 builtin/merge.c                 |  6 ++++++
 builtin/pull.c                  | 11 +++++++++++
 t/t3033-merge-toplevel.sh       | 31 ++++++++++++++++++++++++++++++-
 t/t5521-pull-options.sh         | 28 ++++++++++++++++++++++++++++
 8 files changed, 98 insertions(+), 14 deletions(-)

-- 
2.8.1-422-g6d9b748

[PATCH 1/4] t3033: avoid 'ambiguous refs' warning

[Junio C Hamano]

Because "test_commit five" creates a commit and point it with a tag
'five', doing so on a branch whose name is 'five' will later result
in an 'ambiguous refs' warning.  Even though it is harmless because
all the later references are for the tag, there is no reason for the
branch to be called 'five'.  Give it a name that describes its
purpose more clearly, i.e. "newroot".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t3033-merge-toplevel.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t3033-merge-toplevel.sh b/t/t3033-merge-toplevel.sh
index c1379b0..d314599 100755
--- a/t/t3033-merge-toplevel.sh
+++ b/t/t3033-merge-toplevel.sh
@@ -19,7 +19,7 @@ test_expect_success setup '
 	test_commit three &&
 	git checkout right &&
 	test_commit four &&
-	git checkout --orphan five &&
+	git checkout --orphan newroot &&
 	test_commit five &&
 	git checkout master
 '
-- 
2.8.1-422-g6d9b748

[PATCH 2/4] pull: pass --allow-unrelated-histories to "git merge"

[Junio C Hamano]

An earlier commit said:

    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".

Prepare a patch to make it a reality, just in case it is needed.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-merge.txt     | 14 +-------------
 Documentation/merge-options.txt |  8 ++++++++
 builtin/pull.c                  |  6 ++++++
 t/t5521-pull-options.sh         | 21 +++++++++++++++++++++
 4 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 689aa4c..b758d55 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -11,6 +11,7 @@ SYNOPSIS
 [verse]
 'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]
 	[-s <strategy>] [-X <strategy-option>] [-S[<keyid>]]
+	[--[no-]allow-unrelated-histories]
 	[--[no-]rerere-autoupdate] [-m <msg>] [<commit>...]
 'git merge' <msg> HEAD <commit>...
 'git merge' --abort
@@ -98,19 +99,6 @@ commit or stash your changes before running 'git merge'.
 'git merge --abort' is equivalent to 'git reset --merge' when
 `MERGE_HEAD` is present.
 
---allow-unrelated-histories::
-	By default, `git merge` command refuses to merge histories
-	that do not share a common ancestor.  This option can be
-	used to override this safety when merging histories of two
-	projects that started their lives independently.  As that is
-	a very rare occasion, no configuration variable to enable
-	this by default exists and will not be added, and the list
-	of options at the top of this documentation does not mention
-	this option.  Also `git pull` does not pass this option down
-	to `git merge` (instead, you `git fetch` first, examine what
-	you will be merging and then `git merge` locally with this
-	option).
-
 <commit>...::
 	Commits, usually other branch heads, to merge into our branch.
 	Specifying more than one commit will create a merge with
diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index f08e9b8..dfb43d0 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -114,3 +114,11 @@ ifndef::git-pull[]
 	reporting.
 
 endif::git-pull[]
+
+--allow-unrelated-histories::
+	By default, `git merge` command refuses to merge histories
+	that do not share a common ancestor.  This option can be
+	used to override this safety when merging histories of two
+	projects that started their lives independently. As that is
+	a very rare occasion, no configuration variable to enable
+	this by default exists and will not be added.
diff --git a/builtin/pull.c b/builtin/pull.c
index 5145fc6..797932d 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -86,6 +86,7 @@ static char *opt_verify_signatures;
 static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
 static char *opt_gpg_sign;
+static int opt_allow_unrelated_histories;
 
 /* Options passed to git-fetch */
 static char *opt_all;
@@ -155,6 +156,9 @@ static struct option pull_options[] = {
 	OPT_PASSTHRU('S', "gpg-sign", &opt_gpg_sign, N_("key-id"),
 		N_("GPG sign commit"),
 		PARSE_OPT_OPTARG),
+	OPT_SET_INT(0, "allow-unrelated-histories",
+		    &opt_allow_unrelated_histories,
+		    N_("allow merging unrelated histories"), 1),
 
 	/* Options passed to git-fetch */
 	OPT_GROUP(N_("Options related to fetching")),
@@ -603,6 +607,8 @@ static int run_merge(void)
 	argv_array_pushv(&args, opt_strategy_opts.argv);
 	if (opt_gpg_sign)
 		argv_array_push(&args, opt_gpg_sign);
+	if (opt_allow_unrelated_histories > 0)
+		argv_array_push(&args, "--allow-unrelated-histories");
 
 	argv_array_push(&args, "FETCH_HEAD");
 	ret = run_command_v_opt(args.argv, RUN_GIT_CMD);
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index 18372ca..ded8f98 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -144,4 +144,25 @@ test_expect_success 'git pull --all --dry-run' '
 	)
 '
 
+test_expect_success 'git pull --allow-unrelated-histories' '
+	test_when_finished "rm -fr src dst" &&
+	git init src &&
+	(
+		cd src &&
+		test_commit one &&
+		test_commit two
+	) &&
+	git clone src dst &&
+	(
+		cd src &&
+		git checkout --orphan side HEAD^ &&
+		test_commit three
+	) &&
+	(
+		cd dst &&
+		test_must_fail git pull ../src side &&
+		git pull --allow-unrelated-histories ../src side
+	)
+'
+
 test_done
-- 
2.8.1-422-g6d9b748

Re: [PATCH 2/4] pull: pass --allow-unrelated-histories to "git merge"

[Stefan Beller]

On Thu, Apr 21, 2016 at 12:24 PM, Junio C Hamano <gitster@pobox.com> wrote:
> An earlier commit said:

And by earlier you meant to say e379fdf34f (2016-03-18, merge: refuse
to create too cool a merge by default)?

Re: [PATCH 2/4] pull: pass --allow-unrelated-histories to "git merge"

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> On Thu, Apr 21, 2016 at 12:24 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> An earlier commit said:
>
> And by earlier you meant to say e379fdf34f (2016-03-18, merge: refuse
> to create too cool a merge by default)?

The text quoted does come from that commit, but because there is
nothing more that a reader would need to read from that commit to
follow and understand this change, including its log message with
the quoted parts, I did not give a reference to the commit, in order
to avoid wasting readers' time.  In other words, yes I meant that
commit, and no, I didn't mean to "say" e379fdf....  I meant to say
"An earlier commit" and taht is what I did.

[PATCH 3/4] merge: GIT_MERGE_ALLOW_UNRELATED_HISTORIES environment

[Junio C Hamano]

It is rumored that some scripts rely on being able to regularly
create two project merges.  Instead of forcing them to pass the
option --allow-unrelated-histories when they call "git merge", allow
them to set and export an environment at the beginning and forget
about it.

This will be less damaging than adding a configuration variable to
disable the safety, as contaminating the configuration file of users
of such a script will allow any invocation of "git merge", not
limited to such a script, to go without the safety.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git.txt | 3 +++
 builtin/merge.c       | 3 +++
 builtin/pull.c        | 7 ++++++-
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 754dc80..5c9380d 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -1138,6 +1138,9 @@ of clones and fetches.
 	  - any external helpers are named by their protocol (e.g., use
 	    `hg` to allow the `git-remote-hg` helper)
 
+'GIT_MERGE_ALLOW_UNRELATED_HISTORIES'::
+	Allow "git merge" to merge unrelated histories by default.
+
 
 Discussion[[Discussion]]
 ------------------------
diff --git a/builtin/merge.c b/builtin/merge.c
index e3db41b..4e8b1a1 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1191,6 +1191,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		head_commit = lookup_commit_or_die(head_sha1, "HEAD");
 
 	git_config(git_merge_config, NULL);
+	if (getenv("GIT_MERGE_ALLOW_UNRELATED_HISTORIES"))
+		allow_unrelated_histories =
+			git_env_bool("GIT_MERGE_ALLOW_UNRELATED_HISTORIES", 0);
 
 	if (branch_mergeoptions)
 		parse_branch_merge_options(branch_mergeoptions);
diff --git a/builtin/pull.c b/builtin/pull.c
index 797932d..4e886a5 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -86,7 +86,7 @@ static char *opt_verify_signatures;
 static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
 static char *opt_gpg_sign;
-static int opt_allow_unrelated_histories;
+static int opt_allow_unrelated_histories = -1; /* unspecified */
 
 /* Options passed to git-fetch */
 static char *opt_all;
@@ -159,6 +159,9 @@ static struct option pull_options[] = {
 	OPT_SET_INT(0, "allow-unrelated-histories",
 		    &opt_allow_unrelated_histories,
 		    N_("allow merging unrelated histories"), 1),
+	OPT_SET_INT(0, "no-allow-unrelated-histories",
+		    &opt_allow_unrelated_histories,
+		    N_("do not allow merging unrelated histories"), 0),
 
 	/* Options passed to git-fetch */
 	OPT_GROUP(N_("Options related to fetching")),
@@ -609,6 +612,8 @@ static int run_merge(void)
 		argv_array_push(&args, opt_gpg_sign);
 	if (opt_allow_unrelated_histories > 0)
 		argv_array_push(&args, "--allow-unrelated-histories");
+	else if (!opt_allow_unrelated_histories)
+		argv_array_push(&args, "--no-allow-unrelated-histories");
 
 	argv_array_push(&args, "FETCH_HEAD");
 	ret = run_command_v_opt(args.argv, RUN_GIT_CMD);
-- 
2.8.1-422-g6d9b748

[PATCH 4/4] merge: introduce merge.allowUnrelatedhistories configuration option

[Junio C Hamano]

It was rumored that in an unspecified workflow it is common to
create what Kernel folks call "crazy" and "insane" merges of two
unrelated histories, and having to give --allow-unrelated-histories
option every time is cumbersome.

Just in case the rumor will substanticated with a proper rationale
in the future, prepare a change to allow disabling the safety by
default.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git.txt          |  4 ++++
 Documentation/merge-config.txt |  7 +++++++
 builtin/merge.c                |  3 +++
 t/t3033-merge-toplevel.sh      | 29 +++++++++++++++++++++++++++++
 t/t5521-pull-options.sh        |  9 ++++++++-
 5 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 5c9380d..f2edac1 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -1140,6 +1140,10 @@ of clones and fetches.
 
 'GIT_MERGE_ALLOW_UNRELATED_HISTORIES'::
 	Allow "git merge" to merge unrelated histories by default.
+	It is recommended that a script that regularly wants to
+	create such a merge to set and export this environment
+	variable upfront, instead of forcing its users to set
+	merge.allowunrelatedhistories configuration variable.
 
 
 Discussion[[Discussion]]
diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt
index 002ca58..8b3d14b 100644
--- a/Documentation/merge-config.txt
+++ b/Documentation/merge-config.txt
@@ -47,6 +47,13 @@ merge.stat::
 	Whether to print the diffstat between ORIG_HEAD and the merge result
 	at the end of the merge.  True by default.
 
+merge.allowUnrelatedhistories::
+	Setting this option to true (false) makes `git merge` and `git
+	pull` to pretend as if the `--allow-unrelated-histories`
+	(`--no-allow-unrelated-histories`) option was given from the
+	command line.  The configuration is ignored when one of these
+	options is explicitly given from the command line.
+
 merge.tool::
 	Controls which merge tool is used by linkgit:git-mergetool[1].
 	The list below shows the valid built-in values.
diff --git a/builtin/merge.c b/builtin/merge.c
index 4e8b1a1..e979c68 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -583,6 +583,9 @@ static int git_merge_config(const char *k, const char *v, void *cb)
 	} else if (!strcmp(k, "commit.gpgsign")) {
 		sign_commit = git_config_bool(k, v) ? "" : NULL;
 		return 0;
+	} else if (!strcmp(k, "merge.allowunrelatedhistories")) {
+		allow_unrelated_histories = git_config_bool(k, v);
+		return 0;
 	}
 
 	status = fmt_merge_msg_config(k, v, cb);
diff --git a/t/t3033-merge-toplevel.sh b/t/t3033-merge-toplevel.sh
index d314599..583b837 100755
--- a/t/t3033-merge-toplevel.sh
+++ b/t/t3033-merge-toplevel.sh
@@ -149,4 +149,33 @@ test_expect_success 'two-project merge with --allow-unrelated-histories' '
 	git diff --exit-code five
 '
 
+test_expect_success 'two-project merge with merge.allowunrelatedhistories' '
+	t3033_reset &&
+
+	# make sure configuration parser works
+	git reset --hard four &&
+	test_config merge.allowunrelatedhistories notabool &&
+	test_must_fail git merge . HEAD &&
+
+	# disabled explicitly and redundantly by configuration
+	git reset --hard four &&
+	test_config merge.allowunrelatedhistories false &&
+	test_must_fail git merge five &&
+
+	# disabled explicitly by configuration, overridden by command line
+	git reset --hard four &&
+	test_config merge.allowunrelatedhistories false &&
+	git merge --allow-unrelated-histories five &&
+
+	# enabled by configuration but explicitly disabled
+	git reset --hard four &&
+	test_config merge.allowunrelatedhistories true &&
+	test_must_fail git merge --no-allow-unrelated-histories five &&
+
+	# enabled by configuration
+	git reset --hard four &&
+	test_config merge.allowunrelatedhistories true &&
+	git merge five
+'
+
 test_done
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index ded8f98..50f0887 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -161,7 +161,14 @@ test_expect_success 'git pull --allow-unrelated-histories' '
 	(
 		cd dst &&
 		test_must_fail git pull ../src side &&
-		git pull --allow-unrelated-histories ../src side
+		git pull --allow-unrelated-histories ../src side &&
+
+		git reset --hard one &&
+		git config merge.allowunrelatedhistories no &&
+		test_must_fail git pull ../src side &&
+		git config merge.allowunrelatedhistories yes &&
+		test_must_fail git pull --no-allow-unrelated-histories ../src side &&
+		git pull ../src side
 	)
 '
 
-- 
2.8.1-422-g6d9b748

Re: [PATCH 0/4] Loosening "two project merge" safety

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Yaroslav Halchenko gave a vague "forcing 'git merge' users to always
> give --allow-unrelated-histories option when they create crap/insane
> merges are not nice", which I couldn't guess the validity due to
> lack of concrete use case.  Just in case it is substantiated, here
> is a series to selectively and safely loosen the safety for specific
> use cases and users.
>
> Junio C Hamano (4):
>   t3033: avoid 'ambiguous refs' warning
>   pull: pass --allow-unrelated-histories to "git merge"
>   merge: GIT_MERGE_ALLOW_UNRELATED_HISTORIES environment
>   merge: introduce merge.allowUnrelatedhistories configuration option

I've queued the first two on 'pu'.

I do not think the Kernel folks do not mind the latter two too much,
but I am holding onto them for now.  Unless the original "Gaah" did
not come from Linus, I might even have said that this additional
safety should be opt-in for people who know what they are doing
(i.e. those who want the safety would set the new configuration),
but I am undecided right now.

>
>  Documentation/git-merge.txt     | 14 +-------------
>  Documentation/git.txt           |  7 +++++++
>  Documentation/merge-config.txt  |  7 +++++++
>  Documentation/merge-options.txt |  8 ++++++++
>  builtin/merge.c                 |  6 ++++++
>  builtin/pull.c                  | 11 +++++++++++
>  t/t3033-merge-toplevel.sh       | 31 ++++++++++++++++++++++++++++++-
>  t/t5521-pull-options.sh         | 28 ++++++++++++++++++++++++++++
>  8 files changed, 98 insertions(+), 14 deletions(-)