{"thread":{"id":"59272","subject":"[PATCH 1/2] rebase: add a --rebase-merges=drop option","startedAt":"2023-02-20T03:33:05Z","lastAt":"2023-05-27T16:46:41Z","messageCount":16,"participants":["Alex Henrie","Phillip Wood","Elijah Newren","Junio C Hamano","Philip Oakley","Kristoffer Haugsbakk"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"472318","messageId":"20230220033224.10400-1-alexhenrie24@gmail.com","threadId":"59272","inReplyTo":null,"subject":"[PATCH 1/2] rebase: add a --rebase-merges=drop option","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2023-02-20T03:32:23Z","receivedAt":"2023-02-20T03:33:05Z","isPatch":true,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"Name the new option \"drop\" intead of \"no\" or \"false\" to avoid confusion\nin the future if --rebase-merges grows the ability to truly \"rebase\"\nmerge commits by reusing the conflict resolution information from the\noriginal merge commit, and we want to add an option to ignore the\nconflict resolution information.\n\nThis option can be used to countermand a previous --rebase-merges\noption.\n\nSigned-off-by: Alex Henrie \n---\n Documentation/git-rebase.txt | 2 +-\n builtin/rebase.c | 2 +-\n t/t3430-rebase-merges.sh | 30 ++++++++++++++++++++++++++++++\n 3 files changed, 32 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex 9a295bcee4..92e90f96aa 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -528,7 +528,7 @@ have the long commit hash prepended to the format.\n See also INCOMPATIBLE OPTIONS below.\n \n -r::\n---rebase-merges[=(rebase-cousins|no-rebase-cousins)]::\n+--rebase-merges[=(rebase-cousins|no-rebase-cousins|drop)]::\n \tBy default, a rebase will simply drop merge commits from the todo\n \tlist, and put the rebased commits into a single, linear branch.\n \tWith `--rebase-merges`, the rebase will instead try to preserve\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 6635f10d52..96c0474379 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -1436,7 +1436,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \tif (options.exec.nr)\n \t\timply_merge(&options, \"--exec\");\n \n-\tif (rebase_merges) {\n+\tif (rebase_merges && strcmp(\"drop\", rebase_merges)) {\n \t\tif (!*rebase_merges)\n \t\t\t; /* default mode; do nothing */\n \t\telse if (!strcmp(\"rebase-cousins\", rebase_merges))\ndiff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\nindex fa2a06c19f..861c8405f2 100755\n--- a/t/t3430-rebase-merges.sh\n+++ b/t/t3430-rebase-merges.sh\n@@ -250,6 +250,36 @@ test_expect_success 'with a branch tip that was cherry-picked already' '\n \tEOF\n '\n \n+test_expect_success 'do not rebase merges unless asked' '\n+\tgit checkout -b rebase-merges-default E &&\n+\tbefore=\"$(git rev-parse --verify HEAD)\" &&\n+\ttest_tick &&\n+\tgit rebase --rebase-merges C &&\n+\ttest_cmp_rev HEAD $before &&\n+\ttest_tick &&\n+\tgit rebase C &&\n+\ttest_cmp_graph C.. <<-\\EOF\n+\t* B\n+\t* D\n+\to C\n+\tEOF\n+'\n+\n+test_expect_success 'do not rebase merges when asked to drop them' '\n+\tgit checkout -b rebase-merges-drop E &&\n+\tbefore=\"$(git rev-parse --verify HEAD)\" &&\n+\ttest_tick &&\n+\tgit rebase --rebase-merges C &&\n+\ttest_cmp_rev HEAD $before &&\n+\ttest_tick &&\n+\tgit rebase --rebase-merges=drop C &&\n+\ttest_cmp_graph C.. <<-\\EOF\n+\t* B\n+\t* D\n+\to C\n+\tEOF\n+'\n+\n test_expect_success 'do not rebase cousins unless asked for' '\n \tgit checkout -b cousins main &&\n \tbefore=\"$(git rev-parse --verify HEAD)\" &&\n-- \n2.39.2\n\n"},{"id":"472319","messageId":"20230220033224.10400-2-alexhenrie24@gmail.com","threadId":"59272","inReplyTo":"20230220033224.10400-1-alexhenrie24@gmail.com","subject":"[PATCH 2/2] rebase: add a config option for --rebase-merges","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2023-02-20T03:32:24Z","receivedAt":"2023-02-20T03:33:07Z","isPatch":true,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"At the same time, stop accepting --rebase-merges=\"\" as a synonym of\n--rebase-merges=no-rebase-cousins.\n\nSigned-off-by: Alex Henrie \n---\n Documentation/config/rebase.txt | 3 ++\n builtin/rebase.c | 50 ++++++++++++++-----\n t/t3430-rebase-merges.sh | 87 +++++++++++++++++++++++++++++++++\n 3 files changed, 127 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/config/rebase.txt b/Documentation/config/rebase.txt\nindex f19bd0e040..d956ec4441 100644\n--- a/Documentation/config/rebase.txt\n+++ b/Documentation/config/rebase.txt\n@@ -67,3 +67,6 @@ rebase.rescheduleFailedExec::\n \n rebase.forkPoint::\n \tIf set to false set `--no-fork-point` option by default.\n+\n+rebase.merges::\n+\tDefault value of `--rebase-merges` option.\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 96c0474379..ab4c3b2870 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -771,6 +771,25 @@ static int run_specific_rebase(struct rebase_options *opts)\n \treturn status ? -1 : 0;\n }\n \n+static void parse_merges_value(struct rebase_options *options, const char *value)\n+{\n+\tif (value) {\n+\t\tif (!strcmp(\"drop\", value)) {\n+\t\t\toptions->rebase_merges = 0;\n+\t\t\treturn;\n+\t\t}\n+\n+\t\tif (!strcmp(\"no-rebase-cousins\", value))\n+\t\t\toptions->rebase_cousins = 0;\n+\t\telse if (!strcmp(\"rebase-cousins\", value))\n+\t\t\toptions->rebase_cousins = 1;\n+\t\telse\n+\t\t\tdie(_(\"Unknown mode: %s\"), value);\n+\t}\n+\n+\toptions->rebase_merges = 1;\n+}\n+\n static int rebase_config(const char *var, const char *value, void *data)\n {\n \tstruct rebase_options *opts = data;\n@@ -815,6 +834,14 @@ static int rebase_config(const char *var, const char *value, void *data)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"rebase.merges\")) {\n+\t\tconst char *rebase_merges;\n+\t\tif (!git_config_string(&rebase_merges, var, value) &&\n+\t\t rebase_merges && *rebase_merges)\n+\t\t\tparse_merges_value(opts, rebase_merges);\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(var, \"rebase.backend\")) {\n \t\treturn git_config_string(&opts->default_backend, var, value);\n \t}\n@@ -980,6 +1007,13 @@ static int parse_opt_empty(const struct option *opt, const char *arg, int unset)\n \treturn 0;\n }\n \n+static int parse_opt_merges(const struct option *opt, const char *arg, int unset)\n+{\n+\tBUG_ON_OPT_NEG(unset);\n+\tparse_merges_value(opt->value, arg);\n+\treturn 0;\n+}\n+\n static void NORETURN error_on_missing_default_upstream(void)\n {\n \tstruct branch *current_branch = branch_get(NULL);\n@@ -1035,7 +1069,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \tstruct object_id branch_base;\n \tint ignore_whitespace = 0;\n \tconst char *gpg_sign = NULL;\n-\tconst char *rebase_merges = NULL;\n \tstruct string_list strategy_options = STRING_LIST_INIT_NODUP;\n \tstruct object_id squash_onto;\n \tchar *squash_onto_name = NULL;\n@@ -1137,10 +1170,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\t\t &options.allow_empty_message,\n \t\t\t N_(\"allow rebasing commits with empty messages\"),\n \t\t\t PARSE_OPT_HIDDEN),\n-\t\t{OPTION_STRING, 'r', \"rebase-merges\", &rebase_merges,\n-\t\t\tN_(\"mode\"),\n+\t\tOPT_CALLBACK_F('r', \"rebase-merges\", &options, N_(\"mode\"),\n \t\t\tN_(\"try to rebase merges instead of skipping them\"),\n-\t\t\tPARSE_OPT_OPTARG, NULL, (intptr_t)\"\"},\n+\t\t\tPARSE_OPT_OPTARG, parse_opt_merges),\n \t\tOPT_BOOL(0, \"fork-point\", &options.fork_point,\n \t\t\t N_(\"use 'merge-base --fork-point' to refine upstream\")),\n \t\tOPT_STRING('s', \"strategy\", &options.strategy,\n@@ -1436,16 +1468,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \tif (options.exec.nr)\n \t\timply_merge(&options, \"--exec\");\n \n-\tif (rebase_merges && strcmp(\"drop\", rebase_merges)) {\n-\t\tif (!*rebase_merges)\n-\t\t\t; /* default mode; do nothing */\n-\t\telse if (!strcmp(\"rebase-cousins\", rebase_merges))\n-\t\t\toptions.rebase_cousins = 1;\n-\t\telse if (strcmp(\"no-rebase-cousins\", rebase_merges))\n-\t\t\tdie(_(\"Unknown mode: %s\"), rebase_merges);\n-\t\toptions.rebase_merges = 1;\n+\tif (options.rebase_merges)\n \t\timply_merge(&options, \"--rebase-merges\");\n-\t}\n \n \tif (options.type == REBASE_APPLY) {\n \t\tif (ignore_whitespace)\ndiff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\nindex 861c8405f2..9be07249cc 100755\n--- a/t/t3430-rebase-merges.sh\n+++ b/t/t3430-rebase-merges.sh\n@@ -298,6 +298,92 @@ test_expect_success 'do not rebase cousins unless asked for' '\n \tEOF\n '\n \n+test_expect_success '--rebase-merges=\"\" is invalid syntax' '\n+\techo \"fatal: Unknown mode: \" >expect &&\n+\t! git rebase --rebase-merges=\"\" HEAD^ 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'rebase.merges=\"\" is equivalent to not passing --rebase-merges' '\n+\tgit config rebase.merges \"\" &&\n+\tgit checkout -b config-merges-blank E &&\n+\tgit rebase C &&\n+\ttest_cmp_graph C.. <<-\\EOF\n+\t* B\n+\t* D\n+\to C\n+\tEOF\n+'\n+\n+test_expect_success 'rebase.merges=rebase-cousins is equivalent to --rebase-merges=rebase-cousins' '\n+\tgit config rebase.merges rebase-cousins &&\n+\tgit checkout -b config-rebase-cousins main &&\n+\tgit rebase HEAD^ &&\n+\ttest_cmp_graph HEAD^.. <<-\\EOF\n+\t* Merge the topic branch '\\''onebranch'\\''\n+\t|\\\n+\t| * D\n+\t| * G\n+\t|/\n+\to H\n+\tEOF\n+'\n+\n+test_expect_success '--rebase-merges=drop overrides rebase.merges=no-rebase-cousins' '\n+\tgit config rebase.merges no-rebase-cousins &&\n+\tgit checkout -b override-config-no-rebase-cousins E &&\n+\tgit rebase --rebase-merges=drop C &&\n+\ttest_cmp_graph C.. <<-\\EOF\n+\t* B\n+\t* D\n+\to C\n+\tEOF\n+'\n+\n+test_expect_success '--rebase-merges=no-rebase-cousins overrides rebase.merges=rebase-cousins' '\n+\tgit config rebase.merges rebase-cousins &&\n+\tgit checkout -b override-config-rebase-cousins main &&\n+\tgit rebase --rebase-merges=no-rebase-cousins HEAD^ &&\n+\ttest_cmp_graph HEAD^.. <<-\\EOF\n+\t* Merge the topic branch '\\''onebranch'\\''\n+\t|\\\n+\t| * D\n+\t| * G\n+\to | H\n+\t|/\n+\to A\n+\tEOF\n+'\n+\n+test_expect_success '--rebase-merges overrides rebase.merges=drop' '\n+\tgit config rebase.merges drop &&\n+\tgit checkout -b override-config-merges-drop main &&\n+\tgit rebase --rebase-merges HEAD^ &&\n+\ttest_cmp_graph HEAD^.. <<-\\EOF\n+\t* Merge the topic branch '\\''onebranch'\\''\n+\t|\\\n+\t| * D\n+\t| * G\n+\to | H\n+\t|/\n+\to A\n+\tEOF\n+'\n+\n+test_expect_success '--rebase-merges does not override rebase.merges=rebase-cousins' '\n+\tgit config rebase.merges rebase-cousins &&\n+\tgit checkout -b no-override-config-rebase-cousins main &&\n+\tgit rebase --rebase-merges HEAD^ &&\n+\ttest_cmp_graph HEAD^.. <<-\\EOF\n+\t* Merge the topic branch '\\''onebranch'\\''\n+\t|\\\n+\t| * D\n+\t| * G\n+\t|/\n+\to H\n+\tEOF\n+'\n+\n test_expect_success 'refs/rewritten/* is worktree-local' '\n \tgit worktree add wt &&\n \tcat >wt/script-from-scratch <<-\\EOF &&\n@@ -408,6 +494,7 @@ test_expect_success 'a \"merge\" into a root commit is a fast-forward' '\n '\n \n test_expect_success 'A root commit can be a cousin, treat it that way' '\n+\tgit config --unset rebase.merges &&\n \tgit checkout --orphan khnum &&\n \ttest_commit yama &&\n \tgit checkout -b asherah main &&\n-- \n2.39.2\n\n"},{"id":"472326","messageId":"8857003a-f33d-00c5-5e1e-bfbcd60924ae@dunelm.org.uk","threadId":"59272","inReplyTo":"20230220033224.10400-1-alexhenrie24@gmail.com","subject":"Re: [PATCH 1/2] rebase: add a --rebase-merges=drop option","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-02-20T09:31:30Z","receivedAt":"2023-02-20T09:31:39Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Alex\n\nOn 20/02/2023 03:32, Alex Henrie wrote:\n> 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\"\n> merge commits by reusing the conflict resolution information from the\n> original merge commit, and we want to add an option to ignore the\n> conflict resolution information.\n> \n> This option can be used to countermand a previous --rebase-merges\n> option.\n\nI'm a bit confused as to the reason for this change - what's the \nadvantage over just saying --no-rebase-merges which already exists?\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: Alex Henrie \n> ---\n> Documentation/git-rebase.txt | 2 +-\n> builtin/rebase.c | 2 +-\n> t/t3430-rebase-merges.sh | 30 ++++++++++++++++++++++++++++++\n> 3 files changed, 32 insertions(+), 2 deletions(-)\n> \n> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\n> index 9a295bcee4..92e90f96aa 100644\n> --- a/Documentation/git-rebase.txt\n> +++ b/Documentation/git-rebase.txt\n> @@ -528,7 +528,7 @@ have the long commit hash prepended to the format.\n> See also INCOMPATIBLE OPTIONS below.\n> \n> -r::\n> ---rebase-merges[=(rebase-cousins|no-rebase-cousins)]::\n> +--rebase-merges[=(rebase-cousins|no-rebase-cousins|drop)]::\n> \tBy default, a rebase will simply drop merge commits from the todo\n> \tlist, and put the rebased commits into a single, linear branch.\n> \tWith `--rebase-merges`, the rebase will instead try to preserve\n> diff --git a/builtin/rebase.c b/builtin/rebase.c\n> index 6635f10d52..96c0474379 100644\n> --- a/builtin/rebase.c\n> +++ b/builtin/rebase.c\n> @@ -1436,7 +1436,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n> \tif (options.exec.nr)\n> \t\timply_merge(&options, \"--exec\");\n> \n> -\tif (rebase_merges) {\n> +\tif (rebase_merges && strcmp(\"drop\", rebase_merges)) {\n> \t\tif (!*rebase_merges)\n> \t\t\t; /* default mode; do nothing */\n> \t\telse if (!strcmp(\"rebase-cousins\", rebase_merges))\n> diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\n> index fa2a06c19f..861c8405f2 100755\n> --- a/t/t3430-rebase-merges.sh\n> +++ b/t/t3430-rebase-merges.sh\n> @@ -250,6 +250,36 @@ test_expect_success 'with a branch tip that was cherry-picked already' '\n> \tEOF\n> '\n> \n> +test_expect_success 'do not rebase merges unless asked' '\n> +\tgit checkout -b rebase-merges-default E &&\n> +\tbefore=\"$(git rev-parse --verify HEAD)\" &&\n> +\ttest_tick &&\n> +\tgit rebase --rebase-merges C &&\n> +\ttest_cmp_rev HEAD $before &&\n> +\ttest_tick &&\n> +\tgit rebase C &&\n> +\ttest_cmp_graph C.. <<-\\EOF\n> +\t* B\n> +\t* D\n> +\to C\n> +\tEOF\n> +'\n> +\n> +test_expect_success 'do not rebase merges when asked to drop them' '\n> +\tgit checkout -b rebase-merges-drop E &&\n> +\tbefore=\"$(git rev-parse --verify HEAD)\" &&\n> +\ttest_tick &&\n> +\tgit rebase --rebase-merges C &&\n> +\ttest_cmp_rev HEAD $before &&\n> +\ttest_tick &&\n> +\tgit rebase --rebase-merges=drop C &&\n> +\ttest_cmp_graph C.. <<-\\EOF\n> +\t* B\n> +\t* D\n> +\to C\n> +\tEOF\n> +'\n> +\n> test_expect_success 'do not rebase cousins unless asked for' '\n> \tgit checkout -b cousins main &&\n> \tbefore=\"$(git rev-parse --verify HEAD)\" &&\n"},{"id":"472327","messageId":"fe9a3c86-0169-588f-2b12-e124d9d138d9@dunelm.org.uk","threadId":"59272","inReplyTo":"20230220033224.10400-2-alexhenrie24@gmail.com","subject":"Re: [PATCH 2/2] rebase: add a config option for --rebase-merges","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-02-20T09:38:23Z","receivedAt":"2023-02-20T09:38:42Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Alex\n\nOn 20/02/2023 03:32, Alex Henrie wrote:\n\nI think the commit message could benefit from some justification for why \nthis config option is useful. I don't object to it being added but you \nneed to make the case for why it is a good idea.\n\n> At the same time, stop accepting --rebase-merges=\"\" as a synonym of\n> --rebase-merges=no-rebase-cousins.\n\nPlease try to avoid combining unrelated changes in the same patch. I \nagree that accepting an empty argument to mean \"no-rebase-cousins\" is \nslightly odd but as that is the default I'm not sure it is doing any harm.\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: Alex Henrie \n> ---\n> Documentation/config/rebase.txt | 3 ++\n> builtin/rebase.c | 50 ++++++++++++++-----\n> t/t3430-rebase-merges.sh | 87 +++++++++++++++++++++++++++++++++\n> 3 files changed, 127 insertions(+), 13 deletions(-)\n> \n> diff --git a/Documentation/config/rebase.txt b/Documentation/config/rebase.txt\n> index f19bd0e040..d956ec4441 100644\n> --- a/Documentation/config/rebase.txt\n> +++ b/Documentation/config/rebase.txt\n> @@ -67,3 +67,6 @@ rebase.rescheduleFailedExec::\n> \n> rebase.forkPoint::\n> \tIf set to false set `--no-fork-point` option by default.\n> +\n> +rebase.merges::\n> +\tDefault value of `--rebase-merges` option.\n> diff --git a/builtin/rebase.c b/builtin/rebase.c\n> index 96c0474379..ab4c3b2870 100644\n> --- a/builtin/rebase.c\n> +++ b/builtin/rebase.c\n> @@ -771,6 +771,25 @@ static int run_specific_rebase(struct rebase_options *opts)\n> \treturn status ? -1 : 0;\n> }\n> \n> +static void parse_merges_value(struct rebase_options *options, const char *value)\n> +{\n> +\tif (value) {\n> +\t\tif (!strcmp(\"drop\", value)) {\n> +\t\t\toptions->rebase_merges = 0;\n> +\t\t\treturn;\n> +\t\t}\n> +\n> +\t\tif (!strcmp(\"no-rebase-cousins\", value))\n> +\t\t\toptions->rebase_cousins = 0;\n> +\t\telse if (!strcmp(\"rebase-cousins\", value))\n> +\t\t\toptions->rebase_cousins = 1;\n> +\t\telse\n> +\t\t\tdie(_(\"Unknown mode: %s\"), value);\n> +\t}\n> +\n> +\toptions->rebase_merges = 1;\n> +}\n> +\n> static int rebase_config(const char *var, const char *value, void *data)\n> {\n> \tstruct rebase_options *opts = data;\n> @@ -815,6 +834,14 @@ static int rebase_config(const char *var, const char *value, void *data)\n> \t\treturn 0;\n> \t}\n> \n> +\tif (!strcmp(var, \"rebase.merges\")) {\n> +\t\tconst char *rebase_merges;\n> +\t\tif (!git_config_string(&rebase_merges, var, value) &&\n> +\t\t rebase_merges && *rebase_merges)\n> +\t\t\tparse_merges_value(opts, rebase_merges);\n> +\t\treturn 0;\n> +\t}\n> +\n> \tif (!strcmp(var, \"rebase.backend\")) {\n> \t\treturn git_config_string(&opts->default_backend, var, value);\n> \t}\n> @@ -980,6 +1007,13 @@ static int parse_opt_empty(const struct option *opt, const char *arg, int unset)\n> \treturn 0;\n> }\n> \n> +static int parse_opt_merges(const struct option *opt, const char *arg, int unset)\n> +{\n> +\tBUG_ON_OPT_NEG(unset);\n> +\tparse_merges_value(opt->value, arg);\n> +\treturn 0;\n> +}\n> +\n> static void NORETURN error_on_missing_default_upstream(void)\n> {\n> \tstruct branch *current_branch = branch_get(NULL);\n> @@ -1035,7 +1069,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n> \tstruct object_id branch_base;\n> \tint ignore_whitespace = 0;\n> \tconst char *gpg_sign = NULL;\n> -\tconst char *rebase_merges = NULL;\n> \tstruct string_list strategy_options = STRING_LIST_INIT_NODUP;\n> \tstruct object_id squash_onto;\n> \tchar *squash_onto_name = NULL;\n> @@ -1137,10 +1170,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n> \t\t\t &options.allow_empty_message,\n> \t\t\t N_(\"allow rebasing commits with empty messages\"),\n> \t\t\t PARSE_OPT_HIDDEN),\n> -\t\t{OPTION_STRING, 'r', \"rebase-merges\", &rebase_merges,\n> -\t\t\tN_(\"mode\"),\n> +\t\tOPT_CALLBACK_F('r', \"rebase-merges\", &options, N_(\"mode\"),\n> \t\t\tN_(\"try to rebase merges instead of skipping them\"),\n> -\t\t\tPARSE_OPT_OPTARG, NULL, (intptr_t)\"\"},\n> +\t\t\tPARSE_OPT_OPTARG, parse_opt_merges),\n> \t\tOPT_BOOL(0, \"fork-point\", &options.fork_point,\n> \t\t\t N_(\"use 'merge-base --fork-point' to refine upstream\")),\n> \t\tOPT_STRING('s', \"strategy\", &options.strategy,\n> @@ -1436,16 +1468,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n> \tif (options.exec.nr)\n> \t\timply_merge(&options, \"--exec\");\n> \n> -\tif (rebase_merges && strcmp(\"drop\", rebase_merges)) {\n> -\t\tif (!*rebase_merges)\n> -\t\t\t; /* default mode; do nothing */\n> -\t\telse if (!strcmp(\"rebase-cousins\", rebase_merges))\n> -\t\t\toptions.rebase_cousins = 1;\n> -\t\telse if (strcmp(\"no-rebase-cousins\", rebase_merges))\n> -\t\t\tdie(_(\"Unknown mode: %s\"), rebase_merges);\n> -\t\toptions.rebase_merges = 1;\n> +\tif (options.rebase_merges)\n> \t\timply_merge(&options, \"--rebase-merges\");\n> -\t}\n> \n> \tif (options.type == REBASE_APPLY) {\n> \t\tif (ignore_whitespace)\n> diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\n> index 861c8405f2..9be07249cc 100755\n> --- a/t/t3430-rebase-merges.sh\n> +++ b/t/t3430-rebase-merges.sh\n> @@ -298,6 +298,92 @@ test_expect_success 'do not rebase cousins unless asked for' '\n> \tEOF\n> '\n> \n> +test_expect_success '--rebase-merges=\"\" is invalid syntax' '\n> +\techo \"fatal: Unknown mode: \" >expect &&\n> +\t! git rebase --rebase-merges=\"\" HEAD^ 2>actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'rebase.merges=\"\" is equivalent to not passing --rebase-merges' '\n> +\tgit config rebase.merges \"\" &&\n> +\tgit checkout -b config-merges-blank E &&\n> +\tgit rebase C &&\n> +\ttest_cmp_graph C.. <<-\\EOF\n> +\t* B\n> +\t* D\n> +\to C\n> +\tEOF\n> +'\n> +\n> +test_expect_success 'rebase.merges=rebase-cousins is equivalent to --rebase-merges=rebase-cousins' '\n> +\tgit config rebase.merges rebase-cousins &&\n> +\tgit checkout -b config-rebase-cousins main &&\n> +\tgit rebase HEAD^ &&\n> +\ttest_cmp_graph HEAD^.. <<-\\EOF\n> +\t* Merge the topic branch '\\''onebranch'\\''\n> +\t|\\\n> +\t| * D\n> +\t| * G\n> +\t|/\n> +\to H\n> +\tEOF\n> +'\n> +\n> +test_expect_success '--rebase-merges=drop overrides rebase.merges=no-rebase-cousins' '\n> +\tgit config rebase.merges no-rebase-cousins &&\n> +\tgit checkout -b override-config-no-rebase-cousins E &&\n> +\tgit rebase --rebase-merges=drop C &&\n> +\ttest_cmp_graph C.. <<-\\EOF\n> +\t* B\n> +\t* D\n> +\to C\n> +\tEOF\n> +'\n> +\n> +test_expect_success '--rebase-merges=no-rebase-cousins overrides rebase.merges=rebase-cousins' '\n> +\tgit config rebase.merges rebase-cousins &&\n> +\tgit checkout -b override-config-rebase-cousins main &&\n> +\tgit rebase --rebase-merges=no-rebase-cousins HEAD^ &&\n> +\ttest_cmp_graph HEAD^.. <<-\\EOF\n> +\t* Merge the topic branch '\\''onebranch'\\''\n> +\t|\\\n> +\t| * D\n> +\t| * G\n> +\to | H\n> +\t|/\n> +\to A\n> +\tEOF\n> +'\n> +\n> +test_expect_success '--rebase-merges overrides rebase.merges=drop' '\n> +\tgit config rebase.merges drop &&\n> +\tgit checkout -b override-config-merges-drop main &&\n> +\tgit rebase --rebase-merges HEAD^ &&\n> +\ttest_cmp_graph HEAD^.. <<-\\EOF\n> +\t* Merge the topic branch '\\''onebranch'\\''\n> +\t|\\\n> +\t| * D\n> +\t| * G\n> +\to | H\n> +\t|/\n> +\to A\n> +\tEOF\n> +'\n> +\n> +test_expect_success '--rebase-merges does not override rebase.merges=rebase-cousins' '\n> +\tgit config rebase.merges rebase-cousins &&\n> +\tgit checkout -b no-override-config-rebase-cousins main &&\n> +\tgit rebase --rebase-merges HEAD^ &&\n> +\ttest_cmp_graph HEAD^.. <<-\\EOF\n> +\t* Merge the topic branch '\\''onebranch'\\''\n> +\t|\\\n> +\t| * D\n> +\t| * G\n> +\t|/\n> +\to H\n> +\tEOF\n> +'\n> +\n> test_expect_success 'refs/rewritten/* is worktree-local' '\n> \tgit worktree add wt &&\n> \tcat >wt/script-from-scratch <<-\\EOF &&\n> @@ -408,6 +494,7 @@ test_expect_success 'a \"merge\" into a root commit is a fast-forward' '\n> '\n> \n> test_expect_success 'A root commit can be a cousin, treat it that way' '\n> +\tgit config --unset rebase.merges &&\n> \tgit checkout --orphan khnum &&\n> \ttest_commit yama &&\n> \tgit checkout -b asherah main &&\n"},{"id":"472340","messageId":"CABPp-BE8yAqFi6Pjyfn=Sk_q3udP8ffgbf=ZAbCkZBYTKcPN9g@mail.gmail.com","threadId":"59272","inReplyTo":"20230220033224.10400-2-alexhenrie24@gmail.com","subject":"Re: [PATCH 2/2] rebase: add a config option for --rebase-merges","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2023-02-20T16:41:22Z","receivedAt":"2023-02-20T16:42:22Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Sun, Feb 19, 2023 at 7:33 PM Alex Henrie wrote:\n>\n> At the same time, stop accepting --rebase-merges=\"\" as a synonym of\n> --rebase-merges=no-rebase-cousins.\n\nI thought this meant you were turning \"git rebase --rebase-merges ...\"\ninto an error unless the '=' was provided. I checked out the code to\nverify and realized my understanding was wrong. Maybe I'm not as\nbright as other folks, but just in case, it might be worth clarifying\nthat you're not doing that in the commit message.\n\nAlso, I agree with Phillip that this change should probably be split\nout into its own commit, if it's kept.\n"},{"id":"472345","messageId":"CAMMLpeSAspkSf3KDpoH_WFwGV4z8+4ag=kzqgHPypvXp9yiGyA@mail.gmail.com","threadId":"59272","inReplyTo":"8857003a-f33d-00c5-5e1e-bfbcd60924ae@dunelm.org.uk","subject":"Re: [PATCH 1/2] rebase: add a --rebase-merges=drop option","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2023-02-20T17:03:21Z","receivedAt":"2023-02-20T17:03:36Z","isPatch":true,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"On Mon, Feb 20, 2023 at 2:31 AM Phillip Wood wrote:\n>\n> On 20/02/2023 03:32, Alex Henrie wrote:\n> > 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\"\n> > merge commits by reusing the conflict resolution information from the\n> > original merge commit, and we want to add an option to ignore the\n> > conflict resolution information.\n> >\n> > This option can be used to countermand a previous --rebase-merges\n> > option.\n>\n> I'm a bit confused as to the reason for this change - what's the\n> advantage over just saying --no-rebase-merges which already exists?\n\nI didn't know that there was a --no-rebase-merges option because there\nis no documentation about it and no tests for it. I will replace this\npatch with patches that add the missing documentation and tests.\n\n-Alex\n"},{"id":"472346","messageId":"CAMMLpeQ8_Wz7sEE9M1t6oLF_BA7T_BT9TNfkKwgGOvf9fiio2Q@mail.gmail.com","threadId":"59272","inReplyTo":"fe9a3c86-0169-588f-2b12-e124d9d138d9@dunelm.org.uk","subject":"Re: [PATCH 2/2] rebase: add a config option for --rebase-merges","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2023-02-20T17:06:00Z","receivedAt":"2023-02-20T17:06:21Z","isPatch":true,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"On Mon, Feb 20, 2023 at 2:38 AM Phillip Wood wrote:\n>\n> On 20/02/2023 03:32, Alex Henrie wrote:\n>\n> I think the commit message could benefit from some justification for why\n> this config option is useful. I don't object to it being added but you\n> need to make the case for why it is a good idea.\n\nThe purpose of the new option is to accommodate users who would like\n--rebase-merges to be on by default and to facilitate possibly turning\non --rebase-merges by default without configuration in a future\nversion of Git. I'll add a note about that to the config message.\n\n> > At the same time, stop accepting --rebase-merges=\"\" as a synonym of\n> > --rebase-merges=no-rebase-cousins.\n>\n> Please try to avoid combining unrelated changes in the same patch. I\n> agree that accepting an empty argument to mean \"no-rebase-cousins\" is\n> slightly odd but as that is the default I'm not sure it is doing any harm.\n\nI wrote the code so that `git config rebase.merges \"\"` has the same\neffect on `git rebase` as `git config --unset rebase.merges`, because\nI think that's what most people are going to expect. I'd like to get\nrid of the odd syntax --rebase-merges=\"\" because a user might\nreasonably expect it to do the same thing as `git config rebase.merges\n\"\"`, but it doesn't. On top of that, the config option uses the same\nhelper function as the command-line option. So I consider removing\n--rebase-merges=\"\" to be intertwined with adding the config option,\nbut I'll split them into separate patches anyway.\n\nThanks for the feedback,\n\n-Alex\n"},{"id":"472358","messageId":"xmqqr0ukggk5.fsf@gitster.g","threadId":"59272","inReplyTo":"20230220033224.10400-1-alexhenrie24@gmail.com","subject":"Re: [PATCH 1/2] rebase: add a --rebase-merges=drop option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-02-20T21:42:02Z","receivedAt":"2023-02-20T21:42:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Henrie writes:\n\n> Name the new option \"drop\" intead of \"no\" or \"false\" to avoid confusion\n\nThis is traditionally called \"flattening the history\". Don't we\nconfuse uesrs by introducing a new phrase?\n\nrebase-merges is about transplanting the history without flattening,\ni.e. keeping the mergy commit graph topology. If there are only two\nkinds of rebase (i.e. keeping the topology which is rebase-merges\nand the other \"flattening\" kind) operation, shouldn't the option be\ncalled \"--no-rebase-merges\" instead? --rebase-merges=no is also\nunderstandable.\n\n> in the future if --rebase-merges grows the ability to truly \"rebase\"\n> merge commits by reusing the conflict resolution information from the\n> original merge commit, and we want to add an option to ignore the\n> conflict resolution information.\n\nI am not sure why such a change \"in the future\" is not merely a\nbugfix of the current \"--rebase-merges\", though. Once it is fixed,\nis there a reason to make the fixed behaviour only available behind\nan option?\n"},{"id":"472378","messageId":"a856dd16-9876-509b-6a99-11ea0020633c@iee.email","threadId":"59272","inReplyTo":"xmqqr0ukggk5.fsf@gitster.g","subject":"Re: [PATCH 1/2] rebase: add a --rebase-merges=drop option","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2023-02-21T16:08:19Z","receivedAt":"2023-02-21T16:08:29Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Hi Junio,\n\nOn 20/02/2023 21:42, Junio C Hamano wrote:\n> Alex Henrie writes:\n>\n>> Name the new option \"drop\" intead of \"no\" or \"false\" to avoid confusion\n> This is traditionally called \"flattening the history\". Don't we\n> confuse uesrs by introducing a new phrase?\n\nWhile \"flatten..\" is used on list, we rarely mention it in our man\npages, and usually only in a cautionary manner via the\nrev-list-options.txt under \"--show-linear-break\".\n\nIt's not always clear what is meant by 'flattening' and which aspects\nare included/excluded from the flattened display. I suspect that a\nrecent question on the git-users list [1] originates from the same\nconfusions.\n\nMaybe it's something that could be included in the Glossary to\nsupplement the not well known how-to discussion in\nkeep-canonical-history-correct.txt\n\n>\n> rebase-merges is about transplanting the history without flattening,\n> i.e. keeping the mergy commit graph topology. If there are only two\n> kinds of rebase (i.e. keeping the topology which is rebase-merges\n> and the other \"flattening\" kind) operation, shouldn't the option be\n> called \"--no-rebase-merges\" instead? --rebase-merges=no is also\n> understandable.\n>\n>> in the future if --rebase-merges grows the ability to truly \"rebase\"\n>> merge commits by reusing the conflict resolution information from the\n>> original merge commit, and we want to add an option to ignore the\n>> conflict resolution information.\n> I am not sure why such a change \"in the future\" is not merely a\n> bugfix of the current \"--rebase-merges\", though. Once it is fixed,\n> is there a reason to make the fixed behaviour only available behind\n> an option?\n[1]\nhttps://groups.google.com/d/msgid/git-users/057bd9e2-b20b-4794-b8a0-bc16ede374c1n%40googlegroups.com\n\n--\n\nPhilip\n\n"},{"id":"472390","messageId":"xmqq5ybug8s8.fsf@gitster.g","threadId":"59272","inReplyTo":"a856dd16-9876-509b-6a99-11ea0020633c@iee.email","subject":"Re: [PATCH 1/2] rebase: add a --rebase-merges=drop option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-02-21T18:42:15Z","receivedAt":"2023-02-21T18:42:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philip Oakley writes:\n\n> Maybe it's something that could be included in the Glossary to\n> supplement the not well known how-to discussion in\n> keep-canonical-history-correct.txt\n\nYeah, \"linearizing the history\" may be much easier to understand\nthan \"flattening\". In any case, a canonical reference would be a\ngood first step.\n\n"},{"id":"477257","messageId":"20230513165154.772-1-philipoakley@iee.email","threadId":"59272","inReplyTo":"xmqq5ybug8s8.fsf@gitster.g","subject":"[PATCH 0/1] cover-letter: flatten","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2023-05-13T16:51:53Z","receivedAt":"2023-05-13T16:52:12Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"The discussion on `--rebase-merges` documentation [1] noted the\nvariety of terminologies that could be used when looking at commit\nhistory.\n\nThis change consolidates the current colloquial use of 'flatten' as\na synonym for the linearizing of commit sequences, and notes some\nof the potential misunderstandings.\n\nPhilip\n\nPhilip Oakley (2):\n doc: Glossary, describe Flattening\n cover-letter: flatten\n\n\n-- \n2.40.0.windows.1\n\n"},{"id":"477258","messageId":"20230513165657.812-1-philipoakley@iee.email","threadId":"59272","inReplyTo":"xmqq5ybug8s8.fsf@gitster.g","subject":"[PATCH 1/1] doc: Glossary, describe Flattening","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2023-05-13T16:56:56Z","receivedAt":"2023-05-13T16:57:12Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Clarify the term 'flatten' and the unexpected effects that the user\nmay come across, such as discussed in [1] and [2].\n\n[1] https://lore.kernel.org/git/xmqqr0ukggk5.fsf@gitster.g/\n\n[2] https://lore.kernel.org/git/xmqq5ybug8s8.fsf@gitster.g/\n\nSigned-off-by: Philip Oakley \n---\n Documentation/glossary-content.txt | 13 +++++++++++++\n 1 file changed, 13 insertions(+)\n\ndiff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt\nindex 5a537268e2..36125e503c 100644\n--- a/Documentation/glossary-content.txt\n+++ b/Documentation/glossary-content.txt\n@@ -173,6 +173,19 @@ current branch integrates with) obviously do not work, as there is no\n \tmissing from the local <>,\n \tand to get them, too. See also linkgit:git-fetch[1].\n \n+[[def_flatten]]flatten::\n+\tFlattening is a common term for the 'linearizing' of a\n+\tselected portion of the <>.\n+\tFlattening may include excluding commits, or rearranging commits,\n+\tfor the linearized sequence.\n+\tIn particular, linkgit:git-log[1] and linkgit:git-show[1] have a\n+\trange of \"History Simplification\" techniques that affect which\n+\tcommits are included, and how they are linearized.\n+\tThe default linkgit:git-rebase[1] will drop merge commits when it\n+\tflattens history, which also may be unexpected.\n+\tThe two common linearization types are chronological (date-time), and\n+\ttopological (shape) based orderings. Generation numbering is topological.\n+\n [[def_file_system]]file system::\n \tLinus Torvalds originally designed Git to be a user space file system,\n \ti.e. the infrastructure to hold files and directories. That ensured the\n-- \n2.40.0.windows.1\n\n"},{"id":"477274","messageId":"xmqqh6seaxlk.fsf@gitster.g","threadId":"59272","inReplyTo":"20230513165657.812-1-philipoakley@iee.email","subject":"Re: [PATCH 1/1] doc: Glossary, describe Flattening","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-05-15T06:59:51Z","receivedAt":"2023-05-15T07:00:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philip Oakley writes:\n\n> +[[def_flatten]]flatten::\n> +\tFlattening is a common term for the 'linearizing' of a\n> +\tselected portion of the <>.\n> +\tFlattening may include excluding commits, or rearranging commits,\n> +\tfor the linearized sequence.\n\nThanks for writing. I agree that it is a good idea to define the\nverb \"flatten\". The above I agree with 100%.\n\nI think I was one of the first ones who used the verb in the context\nof Git; what I wanted to convey with the verb was what it happens\nwhen you use \"am\" to rebuild some history made into a series of\npatches using the \"format-patch\" command on a part of the history.\n\nWhen you have materials from two or more topic branches merged to\nyour primary integration branch, you would omit the merge commits on\nthe integration branch and send patches for commits on these topics\nin a linearized way. Applying these patches one by one will result\nin a linearlized history, containing patches from all of these\ntopics (hopefully this is done in a topological order).\n\n> +\tIn particular, linkgit:git-log[1] and linkgit:git-show[1] have a\n> +\trange of \"History Simplification\" techniques that affect which\n> +\tcommits are included, and how they are linearized.\n\nI didn't think (and I do not yet agree, but I may change my mind\nafter thinking about it further) that the history simplification had\nmuch to do with flattening.\n\nEven after a history is simplified (in the sense how rev-list family\nof commands do so), there will still be merge commits left if both\nbranches contribute something to the end result. So unless a merge\nis to cauterize the side branch (i.e. in order to record the fact\nthat we already have everything we may want possibly merge to the\nintegration branch from the side branch, we create a merge commit\nthat merges the branch but does not change the tree from the parent\ncommit on the integration branch), history simplification may not\ncontribute to \"excluding\" commits.\n\n> +\tThe default linkgit:git-rebase[1] will drop merge commits when it\n> +\tflattens history, which also may be unexpected.\n\nI am tempted to suggest dropping \", which also may be unexpected\"\nhere. When learning a new system, there may be things a learner may\nnot expect (that is why we have documents), so it is not all that\nuseful to say \"this may not be expected\", expecially if we do not\nmention why it behaves that way to clear the \"unexpected\"-ness.\n\nAnd in this case, the reason may be obvious and it is OK to be left\nunsaid---\"git rebase\" (without an option to keep merge commits) was\ndesigned to be a way to flatten history, and a flattened history by\ndefinition cannot have any merge commits in it.\n\n> +\tThe two common linearization types are chronological (date-time), and\n> +\ttopological (shape) based orderings. Generation numbering is topological.\n\nGood.\n"},{"id":"477608","messageId":"90871d5e-2838-4026-bd83-ab259f7b18dc@app.fastmail.com","threadId":"59272","inReplyTo":"20230513165657.812-1-philipoakley@iee.email","subject":"Re: [PATCH 1/1] doc: Glossary, describe Flattening","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2023-05-19T21:35:49Z","receivedAt":"2023-05-19T21:36:52Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Hi\n\nOn Sat, May 13, 2023, at 18:56, Philip Oakley wrote:\n> Clarify the term 'flatten' and the unexpected effects that the user\n> may come across, such as discussed in [1] and [2].\n\nNice to see this effort. I would like more “labels” such as this one to\nconceptualize things because sometimes it feels that Git concepts are\njust handled bottom-up. Specifically in the case of rebase it seems that\n(judging by things like StackOverflow) the pedagogy amounts to\nexplaining how rebase *works* (without factoring in `--rebase-merges`)\nand then explaining how that in turn means that a linearization kind of\n“falls out” of that process. And then it seems that you are expected to\nremember that bottom-up explanation without putting any kind of label on\nit; it’s just what it is.\n\n> +[[def_flatten]]flatten::\n> +\tFlattening is a common term for the 'linearizing' of a\n> +\tselected portion of the <>.\n> +\tFlattening may include excluding commits, or rearranging commits,\n> +\tfor the linearized sequence.\n> +\tIn particular, linkgit:git-log[1] and linkgit:git-show[1] have a\n> +\trange of \"History Simplification\" techniques that affect which\n> +\tcommits are included, and how they are linearized.\n> +\tThe default linkgit:git-rebase[1] will drop merge commits when it\n> +\tflattens history, which also may be unexpected.\n> +\tThe two common linearization types are chronological (date-time), and\n> +\ttopological (shape) based orderings. Generation numbering is topological.\n\nWhen I first read this I thought, ah, so this is an explanation of how\nlinearized rebases are born. But this part also mentions history\nviewing. Then I thought: does my history viewing (git-log(1)) work the\nsame as shuffling around changes into new (and linearized) commits? And\ncan git-rebase-(1) move between chronological and topological and\nordering? But these two things feel different to me (just a feeling,\nUX-wise). So after reading this I am left wondering if different parts\nof this paragraph apply *only* to history viewing and to rebase\n(“rewriting”).\n\nAgain, this is just how I immediately read this paragraph as a user.\n\n-- \nKristoffer Haugsbakk\n"},{"id":"477757","messageId":"43ff85b1-9f1d-480c-10fa-a856f625e35e@iee.email","threadId":"59272","inReplyTo":"xmqqh6seaxlk.fsf@gitster.g","subject":"Re: [PATCH 1/1] doc: Glossary, describe Flattening","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2023-05-27T16:28:57Z","receivedAt":"2023-05-27T16:29:06Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"My somewhat delayed response..\nOn 15/05/2023 07:59, Junio C Hamano wrote:\n> Philip Oakley writes:\n>\n>> +[[def_flatten]]flatten::\n>> +\tFlattening is a common term for the 'linearizing' of a\n>> +\tselected portion of the <>.\n>> +\tFlattening may include excluding commits, or rearranging commits,\n>> +\tfor the linearized sequence.\n> Thanks for writing. I agree that it is a good idea to define the\n> verb \"flatten\". The above I agree with 100%.\n>\n> I think I was one of the first ones who used the verb in the context\n> of Git; what I wanted to convey with the verb was what it happens\n> when you use \"am\" to rebuild some history made into a series of\n> patches using the \"format-patch\" command on a part of the history.\n\nThat is useful context.\n>\n> When you have materials from two or more topic branches merged to\n> your primary integration branch, you would omit the merge commits on\n> the integration branch\n\nI think that implicitly such patches can't include any (the\nnon-existent) 'merge-patches'..\n\n> and send patches for commits on these topics\n> in a linearized way. Applying these patches one by one will result\n> in a linearlized history, containing patches from all of these\n> topics (hopefully this is done in a topological order).\n\nand hence chronological order as a secondary effect\n>> +\tIn particular, linkgit:git-log[1] and linkgit:git-show[1] have a\n>> +\trange of \"History Simplification\" techniques that affect which\n>> +\tcommits are included, and how they are linearized.\n> I didn't think (and I do not yet agree, but I may change my mind\n> after thinking about it further) that the history simplification had\n> much to do with flattening.\n\nI was looking at the 'linearised' (i.e. an ordered list) viewpoint, as I\ndidn't have that historic context you mentioned.\n\n>\n> Even after a history is simplified (in the sense how rev-list family\n> of commands do so), there will still be merge commits left if both\n> branches contribute something to the end result. So unless a merge\n> is to cauterize the side branch (i.e. in order to record the fact\n> that we already have everything we may want possibly merge to the\n> integration branch from the side branch, we create a merge commit\n> that merges the branch but does not change the tree from the parent\n> commit on the integration branch), history simplification may not\n> contribute to \"excluding\" commits.\n\nIn the linear list perspective, the dropping of commits as\n'simplification' would be \"excluding\" them, in exactly the same way that\nthe patch list had dropped merge commits..\n\n>\n>> +\tThe default linkgit:git-rebase[1] will drop merge commits when it\n>> +\tflattens history, which also may be unexpected.\n> I am tempted to suggest dropping \", which also may be unexpected\"\n> here. When learning a new system, there may be things a learner may\n> not expect (that is why we have documents),\n\nCautions and warnings, in my view, should be part of the manual,\nespecially if they keep coming back to bite folks. \"Try it, fail and\nlose work\" isn't ideal when trying to learn unfamiliar  techniques.\n\n> so it is not all that\n> useful to say \"this may not be expected\", expecially if we do not\n> mention why it behaves that way to clear the \"unexpected\"-ness.\n\ntrue.\n>\n> And in this case, the reason may be obvious and it is OK to be left\n> unsaid---\"git rebase\" (without an option to keep merge commits) was\n> designed to be a way to flatten history, and a flattened history by\n> definition cannot have any merge commits in it.\n\nas long as we don't conflate it with rev-list family linear listings..\n>\n>> +\tThe two common linearization types are chronological (date-time), and\n>> +\ttopological (shape) based orderings. Generation numbering is topological.\n> Good.\n\nThanks.\n--\nPhilip\n"},{"id":"477758","messageId":"5bca2615-21e6-34fa-bb22-519acd74cbbd@iee.email","threadId":"59272","inReplyTo":"90871d5e-2838-4026-bd83-ab259f7b18dc@app.fastmail.com","subject":"Re: [PATCH 1/1] doc: Glossary, describe Flattening","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2023-05-27T16:46:29Z","receivedAt":"2023-05-27T16:46:41Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 19/05/2023 22:35, Kristoffer Haugsbakk wrote:\n> Hi\n> \n> On Sat, May 13, 2023, at 18:56, Philip Oakley wrote:\n>> Clarify the term 'flatten' and the unexpected effects that the user\n>> may come across, such as discussed in [1] and [2].\n> \n> Nice to see this effort. I would like more “labels” such as this one to\n> conceptualize things because sometimes it feels that Git concepts are\n> just handled bottom-up.\n\nIt would be good to get a list of those concepts that could benefit from\nbetter explanations. These conceptual views (and misunderstandings)\noften only emerge after a period of usage.\n\n> Specifically in the case of rebase it seems that\n> (judging by things like StackOverflow) the pedagogy amounts to\n> explaining how rebase *works* (without factoring in `--rebase-merges`)\n> and then explaining how that in turn means that a linearization kind of\n> “falls out” of that process. And then it seems that you are expected to\n> remember that bottom-up explanation without putting any kind of label on\n> it; it’s just what it is.\n\nIt's not helped by the default settings for some commands being\nopinionated as to the workflow of the creators (see also 'pull';-).\n\nOther cases are simply 'new' so falling into the 'naming is hard'\nphlogiston camp (staging area concept comes to mind).\n\n> \n>> +[[def_flatten]]flatten::\n>> +\tFlattening is a common term for the 'linearizing' of a\n>> +\tselected portion of the <>.\n>> +\tFlattening may include excluding commits, or rearranging commits,\n>> +\tfor the linearized sequence.\n>> +\tIn particular, linkgit:git-log[1] and linkgit:git-show[1] have a\n>> +\trange of \"History Simplification\" techniques that affect which\n>> +\tcommits are included, and how they are linearized.\n>> +\tThe default linkgit:git-rebase[1] will drop merge commits when it\n>> +\tflattens history, which also may be unexpected.\n>> +\tThe two common linearization types are chronological (date-time), and\n>> +\ttopological (shape) based orderings. Generation numbering is topological.\n> \n> When I first read this I thought, ah, so this is an explanation of how\n> linearized rebases are born. But this part also mentions history\n> viewing. Then I thought: does my history viewing (git-log(1)) work the\n> same as shuffling around changes into new (and linearized) commits? And\n> can git-rebase-(1) move between chronological and topological and\n> ordering? \n\nI had latched on to the 'linearise' (ordered list of commits)\nperspective which fits both rebase (merge commits dropped) and the\nhistory simplification (uninteresting commits dropped) viewpoint, both\nof which can give people trouble of the missing commits.\n\n(see also reply to Junio)\n\n> But these two things feel different to me (just a feeling,\n> UX-wise). So after reading this I am left wondering if different parts\n> of this paragraph apply *only* to history viewing and to rebase\n> (“rewriting”).\n> \n> Again, this is just how I immediately read this paragraph as a user.\n\nThanks for the feedback. Especially the first reading perspective. It's\nall too easy for writers to feel that explaining away a confusion has\nsolved the problem.\n\nI'll probably tighten 'flatten' to be for rebase only on the basis of a\npatch list, without merges. Not sure what to do about the two types of\nre-list linearisations just yet.\n--\nPhilip\n\n"}]}