git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v4 1/4] rebase: fully ignore rebase.autoSquash without -i

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Nov 13, 2023, 17:01 UTC
Message-ID
<7b2e35ee-716b-45c1-9570-643d2cfeeafd@gmail.com>
In-Reply-To
<20231111132720.78877-2-andy.koppe@gmail.com>
Hi Andy
On 11/11/2023 13:27, Andy Koppe wrote:
Show 33 quoted lines
> Setting the rebase.autoSquash config variable to true implies a couple
> of restrictions: it prevents preemptive fast-forwarding and it triggers
> conflicts with amend backend options. However, it only actually results
> in auto-squashing when combined with the --interactive (or -i) option,
> due to code in run_specific_rebase() that disables auto-squashing unless
> the REBASE_INTERACTIVE_EXPLICIT flag is set.
> 
> Doing autosquashing for rebase.autoSquash without --interactive would be
> problematic in terms of backward compatibility, but conversely, there is
> no need for the aforementioned restrictions without --interactive.
> 
> So drop the options.config_autosquash check from the conditions for
> clearing allow_preemptive_ff, as the case where it is combined with
> --interactive is already covered by the REBASE_INTERACTIVE_EXPLICIT
> flag check above it.
> 
> Also drop the "apply options are incompatible with rebase.autoSquash"
> error, because it is unreachable if it is restricted to --interactive,
> as apply options already cause an error when used with --interactive.
> Drop the tests for the error from t3422-rebase-incompatible-options.sh,
> which has separate tests for the conflicts of --interactive with apply
> options.
> 
> When neither --autosquash nor --no-autosquash are given, only set
> options.autosquash to true if rebase.autosquash is combined with
> --interactive.
> 
> Don't initialize options.config_autosquash to -1, as there is no need to
> distinguish between rebase.autoSquash being unset or explicitly set to
> false.
> 
> Finally, amend the rebase.autoSquash documentation to say it only
> affects interactive mode.

Thanks for the well reasoned explanation of the changes. I think documenting rebase.autosquash as only applying to interactive rebases is a good way forward. The code changes all look sensible to me.

Best Wishes
Phillip
Show 98 quoted lines
> Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
> ---
>   Documentation/config/rebase.txt        |  4 +++-
>   builtin/rebase.c                       | 13 ++++++-------
>   t/t3422-rebase-incompatible-options.sh | 12 ------------
>   3 files changed, 9 insertions(+), 20 deletions(-)
> 
> diff --git a/Documentation/config/rebase.txt b/Documentation/config/rebase.txt
> index 9c248accec..d59576dbb2 100644
> --- a/Documentation/config/rebase.txt
> +++ b/Documentation/config/rebase.txt
> @@ -9,7 +9,9 @@ rebase.stat::
>   	rebase. False by default.
>   
>   rebase.autoSquash::
> -	If set to true enable `--autosquash` option by default.
> +	If set to true, enable the `--autosquash` option of
> +	linkgit:git-rebase[1] by default for interactive mode.
> +	This can be overridden with the `--no-autosquash` option.
>   
>   rebase.autoStash::
>   	When set to true, automatically create a temporary stash entry
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 043c65dccd..a73de7892b 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -149,7 +149,6 @@ struct rebase_options {
>   		.reapply_cherry_picks = -1,             \
>   		.allow_empty_message = 1,               \
>   		.autosquash = -1,                       \
> -		.config_autosquash = -1,                \
>   		.rebase_merges = -1,                    \
>   		.config_rebase_merges = -1,             \
>   		.update_refs = -1,                      \
> @@ -1405,7 +1404,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>   	if ((options.flags & REBASE_INTERACTIVE_EXPLICIT) ||
>   	    (options.action != ACTION_NONE) ||
>   	    (options.exec.nr > 0) ||
> -	    (options.autosquash == -1 && options.config_autosquash == 1) ||
>   	    options.autosquash == 1) {
>   		allow_preemptive_ff = 0;
>   	}
> @@ -1508,8 +1506,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>   			if (is_merge(&options))
>   				die(_("apply options and merge options "
>   					  "cannot be used together"));
> -			else if (options.autosquash == -1 && options.config_autosquash == 1)
> -				die(_("apply options are incompatible with rebase.autoSquash.  Consider adding --no-autosquash"));
>   			else if (options.rebase_merges == -1 && options.config_rebase_merges == 1)
>   				die(_("apply options are incompatible with rebase.rebaseMerges.  Consider adding --no-rebase-merges"));
>   			else if (options.update_refs == -1 && options.config_update_refs == 1)
> @@ -1529,10 +1525,13 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>   	options.rebase_merges = (options.rebase_merges >= 0) ? options.rebase_merges :
>   				((options.config_rebase_merges >= 0) ? options.config_rebase_merges : 0);
>   
> -	if (options.autosquash == 1)
> +	if (options.autosquash == 1) {
>   		imply_merge(&options, "--autosquash");
> -	options.autosquash = (options.autosquash >= 0) ? options.autosquash :
> -			     ((options.config_autosquash >= 0) ? options.config_autosquash : 0);
> +	} else if (options.autosquash == -1) {
> +		options.autosquash =
> +			options.config_autosquash &&
> +			(options.flags & REBASE_INTERACTIVE_EXPLICIT);
> +	}
>   
>   	if (options.type == REBASE_UNSPECIFIED) {
>   		if (!strcmp(options.default_backend, "merge"))
> diff --git a/t/t3422-rebase-incompatible-options.sh b/t/t3422-rebase-incompatible-options.sh
> index 2eba00bdf5..b40f26250b 100755
> --- a/t/t3422-rebase-incompatible-options.sh
> +++ b/t/t3422-rebase-incompatible-options.sh
> @@ -100,12 +100,6 @@ test_rebase_am_only () {
>   		test_must_fail git rebase $opt --root A
>   	"
>   
> -	test_expect_success "$opt incompatible with rebase.autosquash" "
> -		git checkout B^0 &&
> -		test_must_fail git -c rebase.autosquash=true rebase $opt A 2>err &&
> -		grep -e --no-autosquash err
> -	"
> -
>   	test_expect_success "$opt incompatible with rebase.rebaseMerges" "
>   		git checkout B^0 &&
>   		test_must_fail git -c rebase.rebaseMerges=true rebase $opt A 2>err &&
> @@ -118,12 +112,6 @@ test_rebase_am_only () {
>   		grep -e --no-update-refs err
>   	"
>   
> -	test_expect_success "$opt okay with overridden rebase.autosquash" "
> -		test_when_finished \"git reset --hard B^0\" &&
> -		git checkout B^0 &&
> -		git -c rebase.autosquash=true rebase --no-autosquash $opt A
> -	"
> -
>   	test_expect_success "$opt okay with overridden rebase.rebaseMerges" "
>   		test_when_finished \"git reset --hard B^0\" &&
>   		git checkout B^0 &&
Previous: Andy KoppeNext: Andy Koppe
Message 16 of 32 in “rebase: support non-interactive autosquash”
  1. 1/2 rebase: support non-interactive autosquashAndy Koppe, Nov 3, 2023
  2. 2/2 docs: rewrite rebase --(no-)autosquash descriptionAndy Koppe, Nov 3, 2023
  3. Junio C HamanoNov 4, 2023
  4. Andy KoppeNov 4, 2023
  5. 1/2 rebase: support non-interactive autosquashAndy Koppe, Nov 4, 2023
  6. 2/2 docs: rewrite rebase --(no-)autosquash descriptionAndy Koppe, Nov 4, 2023
  7. 1/2 rebase: support non-interactive autosquashAndy Koppe, Nov 5, 2023
  8. 2/2 docs: rewrite rebase --(no-)autosquash descriptionAndy Koppe, Nov 5, 2023
  9. Phillip WoodNov 6, 2023
  10. Phillip WoodNov 6, 2023
  11. Andy KoppeNov 11, 2023
  12. 0/4 rebase: support --autosquash without -iAndy Koppe, Nov 11, 2023
  13. 2/4 rebase: support --autosquash without -iAndy Koppe, Nov 11, 2023
  14. Phillip WoodNov 13, 2023
  15. 1/4 rebase: fully ignore rebase.autoSquash without -iAndy Koppe, Nov 11, 2023
  16. Phillip WoodNov 13, 2023
  17. 4/4 docs: rewrite rebase --(no-)autosquash descriptionAndy Koppe, Nov 11, 2023
  18. Andy KoppeNov 11, 2023
  19. 3/4 rebase: test autosquash with and without -iAndy Koppe, Nov 11, 2023
  20. Junio C HamanoNov 13, 2023
  21. Phillip WoodNov 13, 2023
  22. 4/4 rebase: rewrite --(no-)autosquash documentationAndy Koppe, Nov 11, 2023
  23. Junio C HamanoNov 13, 2023
  24. Phillip WoodNov 13, 2023
  25. 0/3 rebase: support --autosquash without -iAndy Koppe, Nov 14, 2023
  26. 1/3 rebase: fully ignore rebase.autoSquash without -iAndy Koppe, Nov 14, 2023
  27. 2/3 rebase: support --autosquash without -iAndy Koppe, Nov 14, 2023
  28. 3/3 rebase: rewrite --(no-)autosquash documentationAndy Koppe, Nov 14, 2023
  29. Phillip WoodNov 15, 2023
  30. Junio C HamanoNov 16, 2023
  31. Junio C HamanoNov 6, 2023
  32. Andy KoppeNov 11, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.