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