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

Re: [PATCH v4 5/9] rebase: add coverage of other incompatible options

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Jan 23, 2023, 20:08 UTC
Message-ID
<759fb313-ce88-4eb7-96c0-4adeb75ca9f9@dunelm.org.uk>
In-Reply-To
<5e4851e611ee18112bd71939ee900e02a8d590c5.1674367961.git.gitgitgadget@gmail.com>
Hi Elijah
On 22/01/2023 06:12, Elijah Newren via GitGitGadget wrote:
Show 11 quoted lines
> From: Elijah Newren <newren@gmail.com>
> 
> The git-rebase manual noted several sets of incompatible options, but
> we were missing tests for a few of these.  Further, we were missing
> code checks for some of these, which could result in command line
> options being silently ignored.
> 
> Also, note that adding a check for autosquash means that using
> --whitespace=fix together with the config setting rebase.autosquash=true
> will trigger an error.  A subsequent commit will improve the error
> message.

Thanks for updating the commit message and for the new commits at the end of the series.

Show 23 quoted lines
> Signed-off-by: Elijah Newren <newren@gmail.com>
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -1224,6 +1224,26 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>   		if (options.fork_point < 0)
>   			options.fork_point = 0;
>   	}
> +	/*
> +	 * The apply backend does not support --[no-]reapply-cherry-picks.
> +	 * The behavior it implements by default is equivalent to
> +	 * --no-reapply-cherry-picks (due to passing --cherry-picks to
> +	 * format-patch), but --keep-base alters the upstream such that no
> +	 * cherry-picks can be found (effectively making it act like
> +	 * --reapply-cherry-picks).
> +	 *
> +	 * Now, if the user does specify --[no-]reapply-cherry-picks, but
> +	 * does so in such a way that options.reapply_cherry_picks ==
> +	 * keep_base, then the behavior they get will match what they
> +	 * expect despite options.reapply_cherry_picks being ignored.  We
> +	 * could just allow the flag in that case, but it seems better to
> +	 * just alert the user that they've specified a flag that the
> +	 * backend ignores.
> +	 */

I'm a bit confused by this. --keep-base works with either --reapply-cherry-picks (which is the default if --keep-base is given) or --no-reapply-cherry-picks. Just below this hunk we have

	if (options.reapply_cherry_picks < 0)
		options.reapply_cherry_picks = keep_base;

So we only set options.reapply_cherry_picks to match keep_base if the user did not specify -[-no]-reapply-cherry-picks on the commandline.

Best Wishes
Phillip
Show 74 quoted lines
> +	if (options.reapply_cherry_picks >= 0)
> +		imply_merge(&options, options.reapply_cherry_picks ? "--reapply-cherry-picks" :
> +								     "--no-reapply-cherry-picks");
> +
>   	/*
>   	 * --keep-base defaults to --reapply-cherry-picks to avoid losing
>   	 * commits when using this option.
> @@ -1406,13 +1426,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>   	if (options.empty != EMPTY_UNSPECIFIED)
>   		imply_merge(&options, "--empty");
>   
> -	/*
> -	 * --keep-base implements --reapply-cherry-picks by altering upstream so
> -	 * it works with both backends.
> -	 */
> -	if (options.reapply_cherry_picks && !keep_base)
> -		imply_merge(&options, "--reapply-cherry-picks");
> -
>   	if (gpg_sign)
>   		options.gpg_sign_opt = xstrfmt("-S%s", gpg_sign);
>   
> @@ -1503,6 +1516,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>   	if (options.update_refs)
>   		imply_merge(&options, "--update-refs");
>   
> +	if (options.autosquash)
> +		imply_merge(&options, "--autosquash");
> +
>   	if (options.type == REBASE_UNSPECIFIED) {
>   		if (!strcmp(options.default_backend, "merge"))
>   			imply_merge(&options, "--merge");
> diff --git a/t/t3422-rebase-incompatible-options.sh b/t/t3422-rebase-incompatible-options.sh
> index f86274990b0..6a17b571ec7 100755
> --- a/t/t3422-rebase-incompatible-options.sh
> +++ b/t/t3422-rebase-incompatible-options.sh
> @@ -50,6 +50,11 @@ test_rebase_am_only () {
>   		test_must_fail git rebase $opt --strategy-option=ours A
>   	"
>   
> +	test_expect_success "$opt incompatible with --autosquash" "
> +		git checkout B^0 &&
> +		test_must_fail git rebase $opt --autosquash A
> +	"
> +
>   	test_expect_success "$opt incompatible with --interactive" "
>   		git checkout B^0 &&
>   		test_must_fail git rebase $opt --interactive A
> @@ -60,6 +65,26 @@ test_rebase_am_only () {
>   		test_must_fail git rebase $opt --exec 'true' A
>   	"
>   
> +	test_expect_success "$opt incompatible with --keep-empty" "
> +		git checkout B^0 &&
> +		test_must_fail git rebase $opt --keep-empty A
> +	"
> +
> +	test_expect_success "$opt incompatible with --empty=..." "
> +		git checkout B^0 &&
> +		test_must_fail git rebase $opt --empty=ask A
> +	"
> +
> +	test_expect_success "$opt incompatible with --no-reapply-cherry-picks" "
> +		git checkout B^0 &&
> +		test_must_fail git rebase $opt --no-reapply-cherry-picks A
> +	"
> +
> +	test_expect_success "$opt incompatible with --reapply-cherry-picks" "
> +		git checkout B^0 &&
> +		test_must_fail git rebase $opt --reapply-cherry-picks A
> +	"
> +
>   	test_expect_success "$opt incompatible with --update-refs" "
>   		git checkout B^0 &&
>   		test_must_fail git rebase $opt --update-refs A
Previous: Elijah Newren via GitGitGadgetNext: Elijah Newren
Message 39 of 70 in “rebase: mark --update-refs as requiring the merge backend”
  1. rebase: mark --update-refs as requiring the merge backendElijah Newren via GitGitGadget, Jan 19, 2023
  2. Derrick StoleeJan 19, 2023
  3. Elijah NewrenJan 20, 2023
  4. Junio C HamanoJan 20, 2023
  5. Elijah NewrenJan 20, 2023
  6. 0/2 rebase: mark --update-refs as requiring the merge backendElijah Newren via GitGitGadget, Jan 20, 2023
  7. 1/2 rebase: remove completely useless -C optionElijah Newren via GitGitGadget, Jan 20, 2023
  8. Eric SunshineJan 20, 2023
  9. Elijah NewrenJan 20, 2023
  10. Martin ÅgrenJan 20, 2023
  11. Elijah NewrenJan 20, 2023
  12. Junio C HamanoJan 20, 2023
  13. Elijah NewrenJan 20, 2023
  14. Junio C HamanoJan 20, 2023
  15. Elijah NewrenJan 21, 2023
  16. Junio C HamanoJan 22, 2023
  17. 2/2 rebase: mark --update-refs as requiring the merge backendElijah Newren via GitGitGadget, Jan 20, 2023
  18. Phillip WoodJan 20, 2023
  19. Elijah NewrenJan 21, 2023
  20. 0/7 rebase: fix several code/testing/documentation issues around flag incompatibilitiesElijah Newren via GitGitGadget, Jan 21, 2023
  21. 1/7 rebase: mark --update-refs as requiring the merge backendElijah Newren via GitGitGadget, Jan 21, 2023
  22. 2/7 rebase: flag --apply and --merge as incompatibleElijah Newren via GitGitGadget, Jan 21, 2023
  23. 3/7 rebase: remove --allow-empty-message from incompatible optsElijah Newren via GitGitGadget, Jan 21, 2023
  24. Phillip WoodJan 21, 2023
  25. 4/7 rebase: fix docs about incompatibilities with --rootElijah Newren via GitGitGadget, Jan 21, 2023
  26. 5/7 rebase: add coverage of other incompatible optionsElijah Newren via GitGitGadget, Jan 21, 2023
  27. Phillip WoodJan 21, 2023
  28. Phillip WoodJan 21, 2023
  29. Elijah NewrenJan 22, 2023
  30. 6/7 rebase: clarify the OPT_CMDMODE incompatibilitiesElijah Newren via GitGitGadget, Jan 21, 2023
  31. 7/7 rebase: fix formatting of rebase --reapply-cherry-picks option in docsElijah Newren via GitGitGadget, Jan 21, 2023
  32. Phillip WoodJan 21, 2023
  33. 0/9 rebase: fix several code/testing/documentation issues around flag incompatibilitiesElijah Newren via GitGitGadget, Jan 22, 2023
  34. 2/9 rebase: flag --apply and --merge as incompatibleElijah Newren via GitGitGadget, Jan 22, 2023
  35. 1/9 rebase: mark --update-refs as requiring the merge backendElijah Newren via GitGitGadget, Jan 22, 2023
  36. 3/9 rebase: remove --allow-empty-message from incompatible optsElijah Newren via GitGitGadget, Jan 22, 2023
  37. 8/9 rebase: put rebase_options initialization in single placeElijah Newren via GitGitGadget, Jan 22, 2023
  38. 5/9 rebase: add coverage of other incompatible optionsElijah Newren via GitGitGadget, Jan 22, 2023
  39. Phillip WoodJan 23, 2023
  40. Elijah NewrenJan 24, 2023
  41. Phillip WoodJan 24, 2023
  42. Phillip WoodJan 24, 2023
  43. Junio C HamanoJan 24, 2023
  44. Elijah NewrenJan 24, 2023
  45. Phillip WoodJan 24, 2023
  46. Elijah NewrenJan 24, 2023
  47. Phillip WoodJan 24, 2023
  48. 9/9 rebase: provide better error message for apply options vs. merge configElijah Newren via GitGitGadget, Jan 22, 2023
  49. 4/9 rebase: fix docs about incompatibilities with --rootElijah Newren via GitGitGadget, Jan 22, 2023
  50. 7/9 rebase: fix formatting of rebase --reapply-cherry-picks option in docsElijah Newren via GitGitGadget, Jan 22, 2023
  51. 6/9 rebase: clarify the OPT_CMDMODE incompatibilitiesElijah Newren via GitGitGadget, Jan 22, 2023
  52. Derrick StoleeJan 23, 2023
  53. Elijah NewrenJan 24, 2023
  54. 00/10 rebase: fix several code/testing/documentation issues around flag incompatibilitiesElijah Newren via GitGitGadget, Jan 25, 2023
  55. 01/10 rebase: mark --update-refs as requiring the merge backendElijah Newren via GitGitGadget, Jan 25, 2023
  56. 02/10 rebase: flag --apply and --merge as incompatibleElijah Newren via GitGitGadget, Jan 25, 2023
  57. 03/10 rebase: remove --allow-empty-message from incompatible optsElijah Newren via GitGitGadget, Jan 25, 2023
  58. 04/10 rebase: fix docs about incompatibilities with --rootElijah Newren via GitGitGadget, Jan 25, 2023
  59. 05/10 rebase: fix incompatiblity checks for --[no-]reapply-cherry-picksElijah Newren via GitGitGadget, Jan 25, 2023
  60. Phillip WoodJan 25, 2023
  61. 08/10 rebase: fix formatting of rebase --reapply-cherry-picks option in docsElijah Newren via GitGitGadget, Jan 25, 2023
  62. 06/10 rebase: add coverage of other incompatible optionsElijah Newren via GitGitGadget, Jan 25, 2023
  63. 07/10 rebase: clarify the OPT_CMDMODE incompatibilitiesElijah Newren via GitGitGadget, Jan 25, 2023
  64. 10/10 rebase: provide better error message for apply options vs. merge configElijah Newren via GitGitGadget, Jan 25, 2023
  65. 09/10 rebase: put rebase_options initialization in single placeElijah Newren via GitGitGadget, Jan 25, 2023
  66. Phillip WoodJan 25, 2023
  67. Junio C HamanoJan 25, 2023
  68. Elijah NewrenJan 25, 2023
  69. rebase --merge vs --whitespace=fix, was Re: [PATCH v5 00/10] rebase: fix several code/testing/documentation issues around flag incompatibilitiesJohannes Schindelin, Feb 2, 2023
  70. Elijah NewrenFeb 2, 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.