threads / patch / 42100

patch, 4 partspull: pass --allow-unrelated-histories to "git merge"

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

## tl;dr

8 messages between Apr 21, 2016 and Apr 21, 2016. Diffs are folded; open one to read it.

replies: 7people: 2as markdown or json

Junio C Hamano· Apr 21, 2016, 19:24 UTC · lore

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

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
Junio C Hamano· Apr 21, 2016, 19:24 UTC · re: Junio C Hamano · lore

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

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(-)
Show changes to t/t3033-merge-toplevel.sh +1 −1
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
Junio C Hamano· Apr 21, 2016, 19:24 UTC · re: Junio C Hamano · lore
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(-)
Show changes to 4 files +36 −12

Documentation/git-merge.txt, Documentation/merge-options.txt, builtin/pull.c, t/t5521-pull-options.sh

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
Stefan Beller· Apr 21, 2016, 19:36 UTC · re: Junio C Hamano · lore

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

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)?

Junio C Hamano· Apr 21, 2016, 21:32 UTC · re: Stefan Beller · lore

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

Stefan Beller <sbeller@google.com> writes:
Show 5 quoted lines
> 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.

Junio C Hamano· Apr 21, 2016, 19:24 UTC · re: Junio C Hamano · lore

[PATCH 3/4] merge: GIT_MERGE_ALLOW_UNRELATED_HISTORIES environment

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(-)
Show changes to 3 files +12 −1

Documentation/git.txt, builtin/merge.c, builtin/pull.c

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
Junio C Hamano· Apr 21, 2016, 19:25 UTC · re: Junio C Hamano · lore

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

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(-)
Show changes to 5 files +51 −1

Documentation/git.txt, Documentation/merge-config.txt, builtin/merge.c, t/t3033-merge-toplevel.sh, t/t5521-pull-options.sh

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
Junio C Hamano· Apr 21, 2016, 22:29 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <gitster@pobox.com> writes:
Show 12 quoted lines
> 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.

Show 10 quoted lines
>
>  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(-)

← back to recent threads