Git Mailing List Archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] rebase: add a --rebase-merges=drop option
@ 2023-02-20  3:32 Alex Henrie
  2023-02-20  3:32 ` [PATCH 2/2] rebase: add a config option for --rebase-merges Alex Henrie
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Alex Henrie @ 2023-02-20  3:32 UTC (permalink / raw)
  To: git, tao, gitster, newren, phillip.wood123, Johannes.Schindelin
  Cc: Alex Henrie

Name the new option "drop" intead of "no" or "false" to avoid confusion
in the future if --rebase-merges grows the ability to truly "rebase"
merge commits by reusing the conflict resolution information from the
original merge commit, and we want to add an option to ignore the
conflict resolution information.

This option can be used to countermand a previous --rebase-merges
option.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 Documentation/git-rebase.txt |  2 +-
 builtin/rebase.c             |  2 +-
 t/t3430-rebase-merges.sh     | 30 ++++++++++++++++++++++++++++++
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 9a295bcee4..92e90f96aa 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -528,7 +528,7 @@ have the long commit hash prepended to the format.
 See also INCOMPATIBLE OPTIONS below.
 
 -r::
---rebase-merges[=(rebase-cousins|no-rebase-cousins)]::
+--rebase-merges[=(rebase-cousins|no-rebase-cousins|drop)]::
 	By default, a rebase will simply drop merge commits from the todo
 	list, and put the rebased commits into a single, linear branch.
 	With `--rebase-merges`, the rebase will instead try to preserve
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 6635f10d52..96c0474379 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1436,7 +1436,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 	if (options.exec.nr)
 		imply_merge(&options, "--exec");
 
-	if (rebase_merges) {
+	if (rebase_merges && strcmp("drop", rebase_merges)) {
 		if (!*rebase_merges)
 			; /* default mode; do nothing */
 		else if (!strcmp("rebase-cousins", rebase_merges))
diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index fa2a06c19f..861c8405f2 100755
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -250,6 +250,36 @@ test_expect_success 'with a branch tip that was cherry-picked already' '
 	EOF
 '
 
+test_expect_success 'do not rebase merges unless asked' '
+	git checkout -b rebase-merges-default E &&
+	before="$(git rev-parse --verify HEAD)" &&
+	test_tick &&
+	git rebase --rebase-merges C &&
+	test_cmp_rev HEAD $before &&
+	test_tick &&
+	git rebase C &&
+	test_cmp_graph C.. <<-\EOF
+	* B
+	* D
+	o C
+	EOF
+'
+
+test_expect_success 'do not rebase merges when asked to drop them' '
+	git checkout -b rebase-merges-drop E &&
+	before="$(git rev-parse --verify HEAD)" &&
+	test_tick &&
+	git rebase --rebase-merges C &&
+	test_cmp_rev HEAD $before &&
+	test_tick &&
+	git rebase --rebase-merges=drop C &&
+	test_cmp_graph C.. <<-\EOF
+	* B
+	* D
+	o C
+	EOF
+'
+
 test_expect_success 'do not rebase cousins unless asked for' '
 	git checkout -b cousins main &&
 	before="$(git rev-parse --verify HEAD)" &&
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/2] rebase: add a config option for --rebase-merges
  2023-02-20  3:32 [PATCH 1/2] rebase: add a --rebase-merges=drop option Alex Henrie
@ 2023-02-20  3:32 ` Alex Henrie
  2023-02-20  9:38   ` Phillip Wood
  2023-02-20 16:41   ` Elijah Newren
  2023-02-20  9:31 ` [PATCH 1/2] rebase: add a --rebase-merges=drop option Phillip Wood
  2023-02-20 21:42 ` Junio C Hamano
  2 siblings, 2 replies; 16+ messages in thread
From: Alex Henrie @ 2023-02-20  3:32 UTC (permalink / raw)
  To: git, tao, gitster, newren, phillip.wood123, Johannes.Schindelin
  Cc: Alex Henrie

At the same time, stop accepting --rebase-merges="" as a synonym of
--rebase-merges=no-rebase-cousins.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 Documentation/config/rebase.txt |  3 ++
 builtin/rebase.c                | 50 ++++++++++++++-----
 t/t3430-rebase-merges.sh        | 87 +++++++++++++++++++++++++++++++++
 3 files changed, 127 insertions(+), 13 deletions(-)

diff --git a/Documentation/config/rebase.txt b/Documentation/config/rebase.txt
index f19bd0e040..d956ec4441 100644
--- a/Documentation/config/rebase.txt
+++ b/Documentation/config/rebase.txt
@@ -67,3 +67,6 @@ rebase.rescheduleFailedExec::
 
 rebase.forkPoint::
 	If set to false set `--no-fork-point` option by default.
+
+rebase.merges::
+	Default value of `--rebase-merges` option.
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 96c0474379..ab4c3b2870 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -771,6 +771,25 @@ static int run_specific_rebase(struct rebase_options *opts)
 	return status ? -1 : 0;
 }
 
+static void parse_merges_value(struct rebase_options *options, const char *value)
+{
+	if (value) {
+		if (!strcmp("drop", value)) {
+			options->rebase_merges = 0;
+			return;
+		}
+
+		if (!strcmp("no-rebase-cousins", value))
+			options->rebase_cousins = 0;
+		else if (!strcmp("rebase-cousins", value))
+			options->rebase_cousins = 1;
+		else
+			die(_("Unknown mode: %s"), value);
+	}
+
+	options->rebase_merges = 1;
+}
+
 static int rebase_config(const char *var, const char *value, void *data)
 {
 	struct rebase_options *opts = data;
@@ -815,6 +834,14 @@ static int rebase_config(const char *var, const char *value, void *data)
 		return 0;
 	}
 
+	if (!strcmp(var, "rebase.merges")) {
+		const char *rebase_merges;
+		if (!git_config_string(&rebase_merges, var, value) &&
+		    rebase_merges && *rebase_merges)
+			parse_merges_value(opts, rebase_merges);
+		return 0;
+	}
+
 	if (!strcmp(var, "rebase.backend")) {
 		return git_config_string(&opts->default_backend, var, value);
 	}
@@ -980,6 +1007,13 @@ static int parse_opt_empty(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
+static int parse_opt_merges(const struct option *opt, const char *arg, int unset)
+{
+	BUG_ON_OPT_NEG(unset);
+	parse_merges_value(opt->value, arg);
+	return 0;
+}
+
 static void NORETURN error_on_missing_default_upstream(void)
 {
 	struct branch *current_branch = branch_get(NULL);
@@ -1035,7 +1069,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 	struct object_id branch_base;
 	int ignore_whitespace = 0;
 	const char *gpg_sign = NULL;
-	const char *rebase_merges = NULL;
 	struct string_list strategy_options = STRING_LIST_INIT_NODUP;
 	struct object_id squash_onto;
 	char *squash_onto_name = NULL;
@@ -1137,10 +1170,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 			   &options.allow_empty_message,
 			   N_("allow rebasing commits with empty messages"),
 			   PARSE_OPT_HIDDEN),
-		{OPTION_STRING, 'r', "rebase-merges", &rebase_merges,
-			N_("mode"),
+		OPT_CALLBACK_F('r', "rebase-merges", &options, N_("mode"),
 			N_("try to rebase merges instead of skipping them"),
-			PARSE_OPT_OPTARG, NULL, (intptr_t)""},
+			PARSE_OPT_OPTARG, parse_opt_merges),
 		OPT_BOOL(0, "fork-point", &options.fork_point,
 			 N_("use 'merge-base --fork-point' to refine upstream")),
 		OPT_STRING('s', "strategy", &options.strategy,
@@ -1436,16 +1468,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 	if (options.exec.nr)
 		imply_merge(&options, "--exec");
 
-	if (rebase_merges && strcmp("drop", rebase_merges)) {
-		if (!*rebase_merges)
-			; /* default mode; do nothing */
-		else if (!strcmp("rebase-cousins", rebase_merges))
-			options.rebase_cousins = 1;
-		else if (strcmp("no-rebase-cousins", rebase_merges))
-			die(_("Unknown mode: %s"), rebase_merges);
-		options.rebase_merges = 1;
+	if (options.rebase_merges)
 		imply_merge(&options, "--rebase-merges");
-	}
 
 	if (options.type == REBASE_APPLY) {
 		if (ignore_whitespace)
diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index 861c8405f2..9be07249cc 100755
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -298,6 +298,92 @@ test_expect_success 'do not rebase cousins unless asked for' '
 	EOF
 '
 
+test_expect_success '--rebase-merges="" is invalid syntax' '
+	echo "fatal: Unknown mode: " >expect &&
+	! git rebase --rebase-merges="" HEAD^ 2>actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rebase.merges="" is equivalent to not passing --rebase-merges' '
+	git config rebase.merges "" &&
+	git checkout -b config-merges-blank E &&
+	git rebase C &&
+	test_cmp_graph C.. <<-\EOF
+	* B
+	* D
+	o C
+	EOF
+'
+
+test_expect_success 'rebase.merges=rebase-cousins is equivalent to --rebase-merges=rebase-cousins' '
+	git config rebase.merges rebase-cousins &&
+	git checkout -b config-rebase-cousins main &&
+	git rebase HEAD^ &&
+	test_cmp_graph HEAD^.. <<-\EOF
+	*   Merge the topic branch '\''onebranch'\''
+	|\
+	| * D
+	| * G
+	|/
+	o H
+	EOF
+'
+
+test_expect_success '--rebase-merges=drop overrides rebase.merges=no-rebase-cousins' '
+	git config rebase.merges no-rebase-cousins &&
+	git checkout -b override-config-no-rebase-cousins E &&
+	git rebase --rebase-merges=drop C &&
+	test_cmp_graph C.. <<-\EOF
+	* B
+	* D
+	o C
+	EOF
+'
+
+test_expect_success '--rebase-merges=no-rebase-cousins overrides rebase.merges=rebase-cousins' '
+	git config rebase.merges rebase-cousins &&
+	git checkout -b override-config-rebase-cousins main &&
+	git rebase --rebase-merges=no-rebase-cousins HEAD^ &&
+	test_cmp_graph HEAD^.. <<-\EOF
+	*   Merge the topic branch '\''onebranch'\''
+	|\
+	| * D
+	| * G
+	o | H
+	|/
+	o A
+	EOF
+'
+
+test_expect_success '--rebase-merges overrides rebase.merges=drop' '
+	git config rebase.merges drop &&
+	git checkout -b override-config-merges-drop main &&
+	git rebase --rebase-merges HEAD^ &&
+	test_cmp_graph HEAD^.. <<-\EOF
+	*   Merge the topic branch '\''onebranch'\''
+	|\
+	| * D
+	| * G
+	o | H
+	|/
+	o A
+	EOF
+'
+
+test_expect_success '--rebase-merges does not override rebase.merges=rebase-cousins' '
+	git config rebase.merges rebase-cousins &&
+	git checkout -b no-override-config-rebase-cousins main &&
+	git rebase --rebase-merges HEAD^ &&
+	test_cmp_graph HEAD^.. <<-\EOF
+	*   Merge the topic branch '\''onebranch'\''
+	|\
+	| * D
+	| * G
+	|/
+	o H
+	EOF
+'
+
 test_expect_success 'refs/rewritten/* is worktree-local' '
 	git worktree add wt &&
 	cat >wt/script-from-scratch <<-\EOF &&
@@ -408,6 +494,7 @@ test_expect_success 'a "merge" into a root commit is a fast-forward' '
 '
 
 test_expect_success 'A root commit can be a cousin, treat it that way' '
+	git config --unset rebase.merges &&
 	git checkout --orphan khnum &&
 	test_commit yama &&
 	git checkout -b asherah main &&
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] rebase: add a --rebase-merges=drop option
  2023-02-20  3:32 [PATCH 1/2] rebase: add a --rebase-merges=drop option Alex Henrie
  2023-02-20  3:32 ` [PATCH 2/2] rebase: add a config option for --rebase-merges Alex Henrie
@ 2023-02-20  9:31 ` Phillip Wood
  2023-02-20 17:03   ` Alex Henrie
  2023-02-20 21:42 ` Junio C Hamano
  2 siblings, 1 reply; 16+ messages in thread
From: Phillip Wood @ 2023-02-20  9:31 UTC (permalink / raw)
  To: Alex Henrie, git, tao, gitster, newren, phillip.wood123,
	Johannes.Schindelin

Hi Alex

On 20/02/2023 03:32, Alex Henrie wrote:
> Name the new option "drop" intead of "no" or "false" to avoid confusion > in the future if --rebase-merges grows the ability to truly "rebase"
> merge commits by reusing the conflict resolution information from the
> original merge commit, and we want to add an option to ignore the
> conflict resolution information.
> 
> This option can be used to countermand a previous --rebase-merges
> option.

I'm a bit confused as to the reason for this change - what's the 
advantage over just saying --no-rebase-merges which already exists?

Best Wishes

Phillip

> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---
>   Documentation/git-rebase.txt |  2 +-
>   builtin/rebase.c             |  2 +-
>   t/t3430-rebase-merges.sh     | 30 ++++++++++++++++++++++++++++++
>   3 files changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 9a295bcee4..92e90f96aa 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -528,7 +528,7 @@ have the long commit hash prepended to the format.
>   See also INCOMPATIBLE OPTIONS below.
>   
>   -r::
> ---rebase-merges[=(rebase-cousins|no-rebase-cousins)]::
> +--rebase-merges[=(rebase-cousins|no-rebase-cousins|drop)]::
>   	By default, a rebase will simply drop merge commits from the todo
>   	list, and put the rebased commits into a single, linear branch.
>   	With `--rebase-merges`, the rebase will instead try to preserve
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 6635f10d52..96c0474379 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -1436,7 +1436,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>   	if (options.exec.nr)
>   		imply_merge(&options, "--exec");
>   
> -	if (rebase_merges) {
> +	if (rebase_merges && strcmp("drop", rebase_merges)) {
>   		if (!*rebase_merges)
>   			; /* default mode; do nothing */
>   		else if (!strcmp("rebase-cousins", rebase_merges))
> diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
> index fa2a06c19f..861c8405f2 100755
> --- a/t/t3430-rebase-merges.sh
> +++ b/t/t3430-rebase-merges.sh
> @@ -250,6 +250,36 @@ test_expect_success 'with a branch tip that was cherry-picked already' '
>   	EOF
>   '
>   
> +test_expect_success 'do not rebase merges unless asked' '
> +	git checkout -b rebase-merges-default E &&
> +	before="$(git rev-parse --verify HEAD)" &&
> +	test_tick &&
> +	git rebase --rebase-merges C &&
> +	test_cmp_rev HEAD $before &&
> +	test_tick &&
> +	git rebase C &&
> +	test_cmp_graph C.. <<-\EOF
> +	* B
> +	* D
> +	o C
> +	EOF
> +'
> +
> +test_expect_success 'do not rebase merges when asked to drop them' '
> +	git checkout -b rebase-merges-drop E &&
> +	before="$(git rev-parse --verify HEAD)" &&
> +	test_tick &&
> +	git rebase --rebase-merges C &&
> +	test_cmp_rev HEAD $before &&
> +	test_tick &&
> +	git rebase --rebase-merges=drop C &&
> +	test_cmp_graph C.. <<-\EOF
> +	* B
> +	* D
> +	o C
> +	EOF
> +'
> +
>   test_expect_success 'do not rebase cousins unless asked for' '
>   	git checkout -b cousins main &&
>   	before="$(git rev-parse --verify HEAD)" &&

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] rebase: add a config option for --rebase-merges
  2023-02-20  3:32 ` [PATCH 2/2] rebase: add a config option for --rebase-merges Alex Henrie
@ 2023-02-20  9:38   ` Phillip Wood
  2023-02-20 17:06     ` Alex Henrie
  2023-02-20 16:41   ` Elijah Newren
  1 sibling, 1 reply; 16+ messages in thread
From: Phillip Wood @ 2023-02-20  9:38 UTC (permalink / raw)
  To: Alex Henrie, git, tao, gitster, newren, phillip.wood123,
	Johannes.Schindelin

Hi Alex

On 20/02/2023 03:32, Alex Henrie wrote:

I think the commit message could benefit from some justification for why 
this config option is useful. I don't object to it being added but you 
need to make the case for why it is a good idea.

> At the same time, stop accepting --rebase-merges="" as a synonym of
> --rebase-merges=no-rebase-cousins.

Please try to avoid combining unrelated changes in the same patch. I 
agree that accepting an empty argument to mean "no-rebase-cousins" is 
slightly odd but as that is the default I'm not sure it is doing any harm.

Best Wishes

Phillip

> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---
>   Documentation/config/rebase.txt |  3 ++
>   builtin/rebase.c                | 50 ++++++++++++++-----
>   t/t3430-rebase-merges.sh        | 87 +++++++++++++++++++++++++++++++++
>   3 files changed, 127 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/config/rebase.txt b/Documentation/config/rebase.txt
> index f19bd0e040..d956ec4441 100644
> --- a/Documentation/config/rebase.txt
> +++ b/Documentation/config/rebase.txt
> @@ -67,3 +67,6 @@ rebase.rescheduleFailedExec::
>   
>   rebase.forkPoint::
>   	If set to false set `--no-fork-point` option by default.
> +
> +rebase.merges::
> +	Default value of `--rebase-merges` option.
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 96c0474379..ab4c3b2870 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -771,6 +771,25 @@ static int run_specific_rebase(struct rebase_options *opts)
>   	return status ? -1 : 0;
>   }
>   
> +static void parse_merges_value(struct rebase_options *options, const char *value)
> +{
> +	if (value) {
> +		if (!strcmp("drop", value)) {
> +			options->rebase_merges = 0;
> +			return;
> +		}
> +
> +		if (!strcmp("no-rebase-cousins", value))
> +			options->rebase_cousins = 0;
> +		else if (!strcmp("rebase-cousins", value))
> +			options->rebase_cousins = 1;
> +		else
> +			die(_("Unknown mode: %s"), value);
> +	}
> +
> +	options->rebase_merges = 1;
> +}
> +
>   static int rebase_config(const char *var, const char *value, void *data)
>   {
>   	struct rebase_options *opts = data;
> @@ -815,6 +834,14 @@ static int rebase_config(const char *var, const char *value, void *data)
>   		return 0;
>   	}
>   
> +	if (!strcmp(var, "rebase.merges")) {
> +		const char *rebase_merges;
> +		if (!git_config_string(&rebase_merges, var, value) &&
> +		    rebase_merges && *rebase_merges)
> +			parse_merges_value(opts, rebase_merges);
> +		return 0;
> +	}
> +
>   	if (!strcmp(var, "rebase.backend")) {
>   		return git_config_string(&opts->default_backend, var, value);
>   	}
> @@ -980,6 +1007,13 @@ static int parse_opt_empty(const struct option *opt, const char *arg, int unset)
>   	return 0;
>   }
>   
> +static int parse_opt_merges(const struct option *opt, const char *arg, int unset)
> +{
> +	BUG_ON_OPT_NEG(unset);
> +	parse_merges_value(opt->value, arg);
> +	return 0;
> +}
> +
>   static void NORETURN error_on_missing_default_upstream(void)
>   {
>   	struct branch *current_branch = branch_get(NULL);
> @@ -1035,7 +1069,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>   	struct object_id branch_base;
>   	int ignore_whitespace = 0;
>   	const char *gpg_sign = NULL;
> -	const char *rebase_merges = NULL;
>   	struct string_list strategy_options = STRING_LIST_INIT_NODUP;
>   	struct object_id squash_onto;
>   	char *squash_onto_name = NULL;
> @@ -1137,10 +1170,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>   			   &options.allow_empty_message,
>   			   N_("allow rebasing commits with empty messages"),
>   			   PARSE_OPT_HIDDEN),
> -		{OPTION_STRING, 'r', "rebase-merges", &rebase_merges,
> -			N_("mode"),
> +		OPT_CALLBACK_F('r', "rebase-merges", &options, N_("mode"),
>   			N_("try to rebase merges instead of skipping them"),
> -			PARSE_OPT_OPTARG, NULL, (intptr_t)""},
> +			PARSE_OPT_OPTARG, parse_opt_merges),
>   		OPT_BOOL(0, "fork-point", &options.fork_point,
>   			 N_("use 'merge-base --fork-point' to refine upstream")),
>   		OPT_STRING('s', "strategy", &options.strategy,
> @@ -1436,16 +1468,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>   	if (options.exec.nr)
>   		imply_merge(&options, "--exec");
>   
> -	if (rebase_merges && strcmp("drop", rebase_merges)) {
> -		if (!*rebase_merges)
> -			; /* default mode; do nothing */
> -		else if (!strcmp("rebase-cousins", rebase_merges))
> -			options.rebase_cousins = 1;
> -		else if (strcmp("no-rebase-cousins", rebase_merges))
> -			die(_("Unknown mode: %s"), rebase_merges);
> -		options.rebase_merges = 1;
> +	if (options.rebase_merges)
>   		imply_merge(&options, "--rebase-merges");
> -	}
>   
>   	if (options.type == REBASE_APPLY) {
>   		if (ignore_whitespace)
> diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
> index 861c8405f2..9be07249cc 100755
> --- a/t/t3430-rebase-merges.sh
> +++ b/t/t3430-rebase-merges.sh
> @@ -298,6 +298,92 @@ test_expect_success 'do not rebase cousins unless asked for' '
>   	EOF
>   '
>   
> +test_expect_success '--rebase-merges="" is invalid syntax' '
> +	echo "fatal: Unknown mode: " >expect &&
> +	! git rebase --rebase-merges="" HEAD^ 2>actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'rebase.merges="" is equivalent to not passing --rebase-merges' '
> +	git config rebase.merges "" &&
> +	git checkout -b config-merges-blank E &&
> +	git rebase C &&
> +	test_cmp_graph C.. <<-\EOF
> +	* B
> +	* D
> +	o C
> +	EOF
> +'
> +
> +test_expect_success 'rebase.merges=rebase-cousins is equivalent to --rebase-merges=rebase-cousins' '
> +	git config rebase.merges rebase-cousins &&
> +	git checkout -b config-rebase-cousins main &&
> +	git rebase HEAD^ &&
> +	test_cmp_graph HEAD^.. <<-\EOF
> +	*   Merge the topic branch '\''onebranch'\''
> +	|\
> +	| * D
> +	| * G
> +	|/
> +	o H
> +	EOF
> +'
> +
> +test_expect_success '--rebase-merges=drop overrides rebase.merges=no-rebase-cousins' '
> +	git config rebase.merges no-rebase-cousins &&
> +	git checkout -b override-config-no-rebase-cousins E &&
> +	git rebase --rebase-merges=drop C &&
> +	test_cmp_graph C.. <<-\EOF
> +	* B
> +	* D
> +	o C
> +	EOF
> +'
> +
> +test_expect_success '--rebase-merges=no-rebase-cousins overrides rebase.merges=rebase-cousins' '
> +	git config rebase.merges rebase-cousins &&
> +	git checkout -b override-config-rebase-cousins main &&
> +	git rebase --rebase-merges=no-rebase-cousins HEAD^ &&
> +	test_cmp_graph HEAD^.. <<-\EOF
> +	*   Merge the topic branch '\''onebranch'\''
> +	|\
> +	| * D
> +	| * G
> +	o | H
> +	|/
> +	o A
> +	EOF
> +'
> +
> +test_expect_success '--rebase-merges overrides rebase.merges=drop' '
> +	git config rebase.merges drop &&
> +	git checkout -b override-config-merges-drop main &&
> +	git rebase --rebase-merges HEAD^ &&
> +	test_cmp_graph HEAD^.. <<-\EOF
> +	*   Merge the topic branch '\''onebranch'\''
> +	|\
> +	| * D
> +	| * G
> +	o | H
> +	|/
> +	o A
> +	EOF
> +'
> +
> +test_expect_success '--rebase-merges does not override rebase.merges=rebase-cousins' '
> +	git config rebase.merges rebase-cousins &&
> +	git checkout -b no-override-config-rebase-cousins main &&
> +	git rebase --rebase-merges HEAD^ &&
> +	test_cmp_graph HEAD^.. <<-\EOF
> +	*   Merge the topic branch '\''onebranch'\''
> +	|\
> +	| * D
> +	| * G
> +	|/
> +	o H
> +	EOF
> +'
> +
>   test_expect_success 'refs/rewritten/* is worktree-local' '
>   	git worktree add wt &&
>   	cat >wt/script-from-scratch <<-\EOF &&
> @@ -408,6 +494,7 @@ test_expect_success 'a "merge" into a root commit is a fast-forward' '
>   '
>   
>   test_expect_success 'A root commit can be a cousin, treat it that way' '
> +	git config --unset rebase.merges &&
>   	git checkout --orphan khnum &&
>   	test_commit yama &&
>   	git checkout -b asherah main &&

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] rebase: add a config option for --rebase-merges
  2023-02-20  3:32 ` [PATCH 2/2] rebase: add a config option for --rebase-merges Alex Henrie
  2023-02-20  9:38   ` Phillip Wood
@ 2023-02-20 16:41   ` Elijah Newren
  1 sibling, 0 replies; 16+ messages in thread
From: Elijah Newren @ 2023-02-20 16:41 UTC (permalink / raw)
  To: Alex Henrie; +Cc: git, tao, gitster, phillip.wood123, Johannes.Schindelin

On Sun, Feb 19, 2023 at 7:33 PM Alex Henrie <alexhenrie24@gmail.com> wrote:
>
> At the same time, stop accepting --rebase-merges="" as a synonym of
> --rebase-merges=no-rebase-cousins.

I thought this meant you were turning "git rebase --rebase-merges ..."
into an error unless the '=' was provided.  I checked out the code to
verify and realized my understanding was wrong.  Maybe I'm not as
bright as other folks, but just in case, it might be worth clarifying
that you're not doing that in the commit message.

Also, I agree with Phillip that this change should probably be split
out into its own commit, if it's kept.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] rebase: add a --rebase-merges=drop option
  2023-02-20  9:31 ` [PATCH 1/2] rebase: add a --rebase-merges=drop option Phillip Wood
@ 2023-02-20 17:03   ` Alex Henrie
  0 siblings, 0 replies; 16+ messages in thread
From: Alex Henrie @ 2023-02-20 17:03 UTC (permalink / raw)
  To: phillip.wood
  Cc: git, tao, gitster, newren, phillip.wood123, Johannes.Schindelin

On Mon, Feb 20, 2023 at 2:31 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> On 20/02/2023 03:32, Alex Henrie wrote:
> > Name the new option "drop" intead of "no" or "false" to avoid confusion > in the future if --rebase-merges grows the ability to truly "rebase"
> > merge commits by reusing the conflict resolution information from the
> > original merge commit, and we want to add an option to ignore the
> > conflict resolution information.
> >
> > This option can be used to countermand a previous --rebase-merges
> > option.
>
> I'm a bit confused as to the reason for this change - what's the
> advantage over just saying --no-rebase-merges which already exists?

I didn't know that there was a --no-rebase-merges option because there
is no documentation about it and no tests for it. I will replace this
patch with patches that add the missing documentation and tests.

-Alex

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] rebase: add a config option for --rebase-merges
  2023-02-20  9:38   ` Phillip Wood
@ 2023-02-20 17:06     ` Alex Henrie
  0 siblings, 0 replies; 16+ messages in thread
From: Alex Henrie @ 2023-02-20 17:06 UTC (permalink / raw)
  To: phillip.wood
  Cc: git, tao, gitster, newren, phillip.wood123, Johannes.Schindelin

On Mon, Feb 20, 2023 at 2:38 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> On 20/02/2023 03:32, Alex Henrie wrote:
>
> I think the commit message could benefit from some justification for why
> this config option is useful. I don't object to it being added but you
> need to make the case for why it is a good idea.

The purpose of the new option is to accommodate users who would like
--rebase-merges to be on by default and to facilitate possibly turning
on --rebase-merges by default without configuration in a future
version of Git. I'll add a note about that to the config message.

> > At the same time, stop accepting --rebase-merges="" as a synonym of
> > --rebase-merges=no-rebase-cousins.
>
> Please try to avoid combining unrelated changes in the same patch. I
> agree that accepting an empty argument to mean "no-rebase-cousins" is
> slightly odd but as that is the default I'm not sure it is doing any harm.

I wrote the code so that `git config rebase.merges ""` has the same
effect on `git rebase` as `git config --unset rebase.merges`, because
I think that's what most people are going to expect. I'd like to get
rid of the odd syntax --rebase-merges="" because a user might
reasonably expect it to do the same thing as `git config rebase.merges
""`, but it doesn't. On top of that, the config option uses the same
helper function as the command-line option. So I consider removing
--rebase-merges="" to be intertwined with adding the config option,
but I'll split them into separate patches anyway.

Thanks for the feedback,

-Alex

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] rebase: add a --rebase-merges=drop option
  2023-02-20  3:32 [PATCH 1/2] rebase: add a --rebase-merges=drop option Alex Henrie
  2023-02-20  3:32 ` [PATCH 2/2] rebase: add a config option for --rebase-merges Alex Henrie
  2023-02-20  9:31 ` [PATCH 1/2] rebase: add a --rebase-merges=drop option Phillip Wood
@ 2023-02-20 21:42 ` Junio C Hamano
  2023-02-21 16:08   ` Philip Oakley
  2 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2023-02-20 21:42 UTC (permalink / raw)
  To: Alex Henrie; +Cc: git, tao, newren, phillip.wood123, Johannes.Schindelin

Alex Henrie <alexhenrie24@gmail.com> writes:

> Name the new option "drop" intead of "no" or "false" to avoid confusion

This is traditionally called "flattening the history".  Don't we
confuse uesrs by introducing a new phrase?

rebase-merges is about transplanting the history without flattening,
i.e. keeping the mergy commit graph topology.  If there are only two
kinds of rebase (i.e. keeping the topology which is rebase-merges
and the other "flattening" kind) operation, shouldn't the option be
called "--no-rebase-merges" instead?  --rebase-merges=no is also
understandable.

> in the future if --rebase-merges grows the ability to truly "rebase"
> merge commits by reusing the conflict resolution information from the
> original merge commit, and we want to add an option to ignore the
> conflict resolution information.

I am not sure why such a change "in the future" is not merely a
bugfix of the current "--rebase-merges", though.  Once it is fixed,
is there a reason to make the fixed behaviour only available behind
an option?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] rebase: add a --rebase-merges=drop option
  2023-02-20 21:42 ` Junio C Hamano
@ 2023-02-21 16:08   ` Philip Oakley
  2023-02-21 18:42     ` Junio C Hamano
  0 siblings, 1 reply; 16+ messages in thread
From: Philip Oakley @ 2023-02-21 16:08 UTC (permalink / raw)
  To: Junio C Hamano, Alex Henrie
  Cc: git, tao, newren, phillip.wood123, Johannes.Schindelin

Hi Junio,

On 20/02/2023 21:42, Junio C Hamano wrote:
> Alex Henrie <alexhenrie24@gmail.com> writes:
>
>> Name the new option "drop" intead of "no" or "false" to avoid confusion
> This is traditionally called "flattening the history".  Don't we
> confuse uesrs by introducing a new phrase?

While "flatten.." is used on list, we rarely mention it in our man
pages, and usually only in a cautionary manner via the
rev-list-options.txt under "--show-linear-break".

It's not always clear what is meant by 'flattening' and which aspects
are included/excluded from the flattened display. I suspect that a
recent question on the git-users list [1] originates from the same
confusions.

Maybe it's something that could be included in the Glossary to
supplement the not well known how-to discussion in
keep-canonical-history-correct.txt

>
> rebase-merges is about transplanting the history without flattening,
> i.e. keeping the mergy commit graph topology.  If there are only two
> kinds of rebase (i.e. keeping the topology which is rebase-merges
> and the other "flattening" kind) operation, shouldn't the option be
> called "--no-rebase-merges" instead?  --rebase-merges=no is also
> understandable.
>
>> in the future if --rebase-merges grows the ability to truly "rebase"
>> merge commits by reusing the conflict resolution information from the
>> original merge commit, and we want to add an option to ignore the
>> conflict resolution information.
> I am not sure why such a change "in the future" is not merely a
> bugfix of the current "--rebase-merges", though.  Once it is fixed,
> is there a reason to make the fixed behaviour only available behind
> an option?
[1]
https://groups.google.com/d/msgid/git-users/057bd9e2-b20b-4794-b8a0-bc16ede374c1n%40googlegroups.com

--

Philip


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] rebase: add a --rebase-merges=drop option
  2023-02-21 16:08   ` Philip Oakley
@ 2023-02-21 18:42     ` Junio C Hamano
  2023-05-13 16:51       ` [PATCH 0/1] cover-letter: flatten Philip Oakley
  2023-05-13 16:56       ` [PATCH 1/1] doc: Glossary, describe Flattening Philip Oakley
  0 siblings, 2 replies; 16+ messages in thread
From: Junio C Hamano @ 2023-02-21 18:42 UTC (permalink / raw)
  To: Philip Oakley
  Cc: Alex Henrie, git, tao, newren, phillip.wood123,
	Johannes.Schindelin

Philip Oakley <philipoakley@iee.email> writes:

> Maybe it's something that could be included in the Glossary to
> supplement the not well known how-to discussion in
> keep-canonical-history-correct.txt

Yeah, "linearizing the history" may be much easier to understand
than "flattening".  In any case, a canonical reference would be a
good first step.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 0/1] cover-letter: flatten
  2023-02-21 18:42     ` Junio C Hamano
@ 2023-05-13 16:51       ` Philip Oakley
  2023-05-13 16:56       ` [PATCH 1/1] doc: Glossary, describe Flattening Philip Oakley
  1 sibling, 0 replies; 16+ messages in thread
From: Philip Oakley @ 2023-05-13 16:51 UTC (permalink / raw)
  To: gitster
  Cc: Johannes.Schindelin, alexhenrie24, git, newren, philipoakley,
	phillip.wood123, tao

The discussion on `--rebase-merges` documentation [1] noted the
variety of terminologies that could be used when looking at commit
history.

This change consolidates the current colloquial use of 'flatten' as
a synonym for the linearizing of commit sequences, and notes some
of the potential misunderstandings.

Philip

Philip Oakley (2):
  doc: Glossary, describe Flattening
  cover-letter: flatten


-- 
2.40.0.windows.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/1] doc: Glossary, describe Flattening
  2023-02-21 18:42     ` Junio C Hamano
  2023-05-13 16:51       ` [PATCH 0/1] cover-letter: flatten Philip Oakley
@ 2023-05-13 16:56       ` Philip Oakley
  2023-05-15  6:59         ` Junio C Hamano
  2023-05-19 21:35         ` Kristoffer Haugsbakk
  1 sibling, 2 replies; 16+ messages in thread
From: Philip Oakley @ 2023-05-13 16:56 UTC (permalink / raw)
  To: gitster
  Cc: Johannes.Schindelin, alexhenrie24, git, newren, philipoakley,
	phillip.wood123, tao

Clarify the term 'flatten' and the unexpected effects that the user
may come across, such as discussed in [1] and [2].

[1] https://lore.kernel.org/git/xmqqr0ukggk5.fsf@gitster.g/

[2] https://lore.kernel.org/git/xmqq5ybug8s8.fsf@gitster.g/

Signed-off-by: Philip Oakley <philipoakley@iee.email>
---
 Documentation/glossary-content.txt | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
index 5a537268e2..36125e503c 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -173,6 +173,19 @@ current branch integrates with) obviously do not work, as there is no
 	missing from the local <<def_object_database,object database>>,
 	and to get them, too.  See also linkgit:git-fetch[1].
 
+[[def_flatten]]flatten::
+	Flattening is a common term for the 'linearizing' of a
+	selected portion of the <<def_commit_graph_general,commit graph>>.
+	Flattening may include excluding commits, or rearranging commits,
+	for the linearized sequence.
+	In particular, linkgit:git-log[1] and linkgit:git-show[1] have a
+	range of "History Simplification" techniques that affect which
+	commits are included, and how they are linearized.
+	The default linkgit:git-rebase[1] will drop merge commits when it
+	flattens history, which also may be unexpected.
+	The two common linearization types are chronological (date-time), and
+	topological (shape) based orderings. Generation numbering is topological.
+
 [[def_file_system]]file system::
 	Linus Torvalds originally designed Git to be a user space file system,
 	i.e. the infrastructure to hold files and directories. That ensured the
-- 
2.40.0.windows.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/1] doc: Glossary, describe Flattening
  2023-05-13 16:56       ` [PATCH 1/1] doc: Glossary, describe Flattening Philip Oakley
@ 2023-05-15  6:59         ` Junio C Hamano
  2023-05-27 16:28           ` Philip Oakley
  2023-05-19 21:35         ` Kristoffer Haugsbakk
  1 sibling, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2023-05-15  6:59 UTC (permalink / raw)
  To: Philip Oakley
  Cc: Johannes.Schindelin, alexhenrie24, git, newren, phillip.wood123,
	tao

Philip Oakley <philipoakley@iee.email> writes:

> +[[def_flatten]]flatten::
> +	Flattening is a common term for the 'linearizing' of a
> +	selected portion of the <<def_commit_graph_general,commit graph>>.
> +	Flattening may include excluding commits, or rearranging commits,
> +	for the linearized sequence.

Thanks for writing.  I agree that it is a good idea to define the
verb "flatten".  The above I agree with 100%.

I think I was one of the first ones who used the verb in the context
of Git; what I wanted to convey with the verb was what it happens
when you use "am" to rebuild some history made into a series of
patches using the "format-patch" command on a part of the history.

When you have materials from two or more topic branches merged to
your primary integration branch, you would omit the merge commits on
the integration branch and send patches for commits on these topics
in a linearized way.  Applying these patches one by one will result
in a linearlized history, containing patches from all of these
topics (hopefully this is done in a topological order).

> +	In particular, linkgit:git-log[1] and linkgit:git-show[1] have a
> +	range of "History Simplification" techniques that affect which
> +	commits are included, and how they are linearized.

I didn't think (and I do not yet agree, but I may change my mind
after thinking about it further) that the history simplification had
much to do with flattening.

Even after a history is simplified (in the sense how rev-list family
of commands do so), there will still be merge commits left if both
branches contribute something to the end result.  So unless a merge
is to cauterize the side branch (i.e. in order to record the fact
that we already have everything we may want possibly merge to the
integration branch from the side branch, we create a merge commit
that merges the branch but does not change the tree from the parent
commit on the integration branch), history simplification may not
contribute to "excluding" commits.

> +	The default linkgit:git-rebase[1] will drop merge commits when it
> +	flattens history, which also may be unexpected.

I am tempted to suggest dropping ", which also may be unexpected"
here.  When learning a new system, there may be things a learner may
not expect (that is why we have documents), so it is not all that
useful to say "this may not be expected", expecially if we do not
mention why it behaves that way to clear the "unexpected"-ness.

And in this case, the reason may be obvious and it is OK to be left
unsaid---"git rebase" (without an option to keep merge commits) was
designed to be a way to flatten history, and a flattened history by
definition cannot have any merge commits in it.

> +	The two common linearization types are chronological (date-time), and
> +	topological (shape) based orderings. Generation numbering is topological.

Good.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/1] doc: Glossary, describe Flattening
  2023-05-13 16:56       ` [PATCH 1/1] doc: Glossary, describe Flattening Philip Oakley
  2023-05-15  6:59         ` Junio C Hamano
@ 2023-05-19 21:35         ` Kristoffer Haugsbakk
  2023-05-27 16:46           ` Philip Oakley
  1 sibling, 1 reply; 16+ messages in thread
From: Kristoffer Haugsbakk @ 2023-05-19 21:35 UTC (permalink / raw)
  To: Philip Oakley
  Cc: Johannes Schindelin, alexhenrie24, git, Elijah Newren,
	Phillip Wood, tao, Junio C Hamano

Hi

On Sat, May 13, 2023, at 18:56, Philip Oakley wrote:
> Clarify the term 'flatten' and the unexpected effects that the user
> may come across, such as discussed in [1] and [2].

Nice to see this effort. I would like more “labels” such as this one to
conceptualize things because sometimes it feels that Git concepts are
just handled bottom-up. Specifically in the case of rebase it seems that
(judging by things like StackOverflow) the pedagogy amounts to
explaining how rebase *works* (without factoring in `--rebase-merges`)
and then explaining how that in turn means that a linearization kind of
“falls out” of that process. And then it seems that you are expected to
remember that bottom-up explanation without putting any kind of label on
it; it’s just what it is.

> +[[def_flatten]]flatten::
> +	Flattening is a common term for the 'linearizing' of a
> +	selected portion of the <<def_commit_graph_general,commit graph>>.
> +	Flattening may include excluding commits, or rearranging commits,
> +	for the linearized sequence.
> +	In particular, linkgit:git-log[1] and linkgit:git-show[1] have a
> +	range of "History Simplification" techniques that affect which
> +	commits are included, and how they are linearized.
> +	The default linkgit:git-rebase[1] will drop merge commits when it
> +	flattens history, which also may be unexpected.
> +	The two common linearization types are chronological (date-time), and
> +	topological (shape) based orderings. Generation numbering is topological.

When I first read this I thought, ah, so this is an explanation of how
linearized rebases are born. But this part also mentions history
viewing. Then I thought: does my history viewing (git-log(1)) work the
same as shuffling around changes into new (and linearized) commits? And
can git-rebase-(1) move between chronological and topological and
ordering? But these two things feel different to me (just a feeling,
UX-wise). So after reading this I am left wondering if different parts
of this paragraph apply *only* to history viewing and to rebase
(“rewriting”).

Again, this is just how I immediately read this paragraph as a user.

-- 
Kristoffer Haugsbakk

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/1] doc: Glossary, describe Flattening
  2023-05-15  6:59         ` Junio C Hamano
@ 2023-05-27 16:28           ` Philip Oakley
  0 siblings, 0 replies; 16+ messages in thread
From: Philip Oakley @ 2023-05-27 16:28 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes.Schindelin, alexhenrie24, git, newren, phillip.wood123,
	tao

My somewhat delayed response..
On 15/05/2023 07:59, Junio C Hamano wrote:
> Philip Oakley <philipoakley@iee.email> writes:
>
>> +[[def_flatten]]flatten::
>> +	Flattening is a common term for the 'linearizing' of a
>> +	selected portion of the <<def_commit_graph_general,commit graph>>.
>> +	Flattening may include excluding commits, or rearranging commits,
>> +	for the linearized sequence.
> Thanks for writing.  I agree that it is a good idea to define the
> verb "flatten".  The above I agree with 100%.
>
> I think I was one of the first ones who used the verb in the context
> of Git; what I wanted to convey with the verb was what it happens
> when you use "am" to rebuild some history made into a series of
> patches using the "format-patch" command on a part of the history.

That is useful context.
>
> When you have materials from two or more topic branches merged to
> your primary integration branch, you would omit the merge commits on
> the integration branch

I think that implicitly such patches can't include any (the
non-existent) 'merge-patches'..

>  and send patches for commits on these topics
> in a linearized way.  Applying these patches one by one will result
> in a linearlized history, containing patches from all of these
> topics (hopefully this is done in a topological order).

and hence chronological order as a secondary effect
>> +	In particular, linkgit:git-log[1] and linkgit:git-show[1] have a
>> +	range of "History Simplification" techniques that affect which
>> +	commits are included, and how they are linearized.
> I didn't think (and I do not yet agree, but I may change my mind
> after thinking about it further) that the history simplification had
> much to do with flattening.

I was looking at the 'linearised' (i.e. an ordered list) viewpoint, as I
didn't have that historic context you mentioned.

>
> Even after a history is simplified (in the sense how rev-list family
> of commands do so), there will still be merge commits left if both
> branches contribute something to the end result.  So unless a merge
> is to cauterize the side branch (i.e. in order to record the fact
> that we already have everything we may want possibly merge to the
> integration branch from the side branch, we create a merge commit
> that merges the branch but does not change the tree from the parent
> commit on the integration branch), history simplification may not
> contribute to "excluding" commits.

In the linear list perspective, the dropping of commits as
'simplification' would be "excluding" them, in exactly the same way that
the patch list had dropped merge commits..

>
>> +	The default linkgit:git-rebase[1] will drop merge commits when it
>> +	flattens history, which also may be unexpected.
> I am tempted to suggest dropping ", which also may be unexpected"
> here.  When learning a new system, there may be things a learner may
> not expect (that is why we have documents),

Cautions and warnings, in my view, should be part of the manual,
especially if they keep coming back to bite folks. "Try it, fail and
lose work" isn't ideal when trying to learn unfamiliar  techniques.

>  so it is not all that
> useful to say "this may not be expected", expecially if we do not
> mention why it behaves that way to clear the "unexpected"-ness.

true.
>
> And in this case, the reason may be obvious and it is OK to be left
> unsaid---"git rebase" (without an option to keep merge commits) was
> designed to be a way to flatten history, and a flattened history by
> definition cannot have any merge commits in it.

as long as we don't conflate it with rev-list family linear listings..
>
>> +	The two common linearization types are chronological (date-time), and
>> +	topological (shape) based orderings. Generation numbering is topological.
> Good.

Thanks.
--
Philip

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/1] doc: Glossary, describe Flattening
  2023-05-19 21:35         ` Kristoffer Haugsbakk
@ 2023-05-27 16:46           ` Philip Oakley
  0 siblings, 0 replies; 16+ messages in thread
From: Philip Oakley @ 2023-05-27 16:46 UTC (permalink / raw)
  To: Kristoffer Haugsbakk
  Cc: Johannes Schindelin, alexhenrie24, git, Elijah Newren,
	Phillip Wood, tao, Junio C Hamano

On 19/05/2023 22:35, Kristoffer Haugsbakk wrote:
> Hi
> 
> On Sat, May 13, 2023, at 18:56, Philip Oakley wrote:
>> Clarify the term 'flatten' and the unexpected effects that the user
>> may come across, such as discussed in [1] and [2].
> 
> Nice to see this effort. I would like more “labels” such as this one to
> conceptualize things because sometimes it feels that Git concepts are
> just handled bottom-up.

It would be good to get a list of those concepts that could benefit from
better explanations. These conceptual views (and misunderstandings)
often only emerge after a period of usage.

>  Specifically in the case of rebase it seems that
> (judging by things like StackOverflow) the pedagogy amounts to
> explaining how rebase *works* (without factoring in `--rebase-merges`)
> and then explaining how that in turn means that a linearization kind of
> “falls out” of that process. And then it seems that you are expected to
> remember that bottom-up explanation without putting any kind of label on
> it; it’s just what it is.

It's not helped by the default settings for some commands being
opinionated as to the workflow of the creators (see also 'pull';-).

Other cases are simply 'new' so falling into the 'naming is hard'
phlogiston camp (staging area concept comes to mind).

> 
>> +[[def_flatten]]flatten::
>> +	Flattening is a common term for the 'linearizing' of a
>> +	selected portion of the <<def_commit_graph_general,commit graph>>.
>> +	Flattening may include excluding commits, or rearranging commits,
>> +	for the linearized sequence.
>> +	In particular, linkgit:git-log[1] and linkgit:git-show[1] have a
>> +	range of "History Simplification" techniques that affect which
>> +	commits are included, and how they are linearized.
>> +	The default linkgit:git-rebase[1] will drop merge commits when it
>> +	flattens history, which also may be unexpected.
>> +	The two common linearization types are chronological (date-time), and
>> +	topological (shape) based orderings. Generation numbering is topological.
> 
> When I first read this I thought, ah, so this is an explanation of how
> linearized rebases are born. But this part also mentions history
> viewing. Then I thought: does my history viewing (git-log(1)) work the
> same as shuffling around changes into new (and linearized) commits? And
> can git-rebase-(1) move between chronological and topological and
> ordering? 

I had latched on to the 'linearise' (ordered list of commits)
perspective which fits both rebase (merge commits dropped) and the
history simplification (uninteresting commits dropped) viewpoint, both
of which can give people trouble of the missing commits.

(see also reply to Junio)

>  But these two things feel different to me (just a feeling,
> UX-wise). So after reading this I am left wondering if different parts
> of this paragraph apply *only* to history viewing and to rebase
> (“rewriting”).
> 
> Again, this is just how I immediately read this paragraph as a user.

Thanks for the feedback. Especially the first reading perspective. It's
all too easy for writers to feel that explaining away a confusion has
solved the problem.

I'll probably tighten 'flatten' to be for rebase only on the basis of a
patch list, without merges. Not sure what to do about the two types of
re-list linearisations just yet.
--
Philip


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2023-05-27 16:46 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-20  3:32 [PATCH 1/2] rebase: add a --rebase-merges=drop option Alex Henrie
2023-02-20  3:32 ` [PATCH 2/2] rebase: add a config option for --rebase-merges Alex Henrie
2023-02-20  9:38   ` Phillip Wood
2023-02-20 17:06     ` Alex Henrie
2023-02-20 16:41   ` Elijah Newren
2023-02-20  9:31 ` [PATCH 1/2] rebase: add a --rebase-merges=drop option Phillip Wood
2023-02-20 17:03   ` Alex Henrie
2023-02-20 21:42 ` Junio C Hamano
2023-02-21 16:08   ` Philip Oakley
2023-02-21 18:42     ` Junio C Hamano
2023-05-13 16:51       ` [PATCH 0/1] cover-letter: flatten Philip Oakley
2023-05-13 16:56       ` [PATCH 1/1] doc: Glossary, describe Flattening Philip Oakley
2023-05-15  6:59         ` Junio C Hamano
2023-05-27 16:28           ` Philip Oakley
2023-05-19 21:35         ` Kristoffer Haugsbakk
2023-05-27 16:46           ` Philip Oakley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).