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

Re: [PATCH 4/5] rebase --keep-base: imply --reapply-cherry-picks

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 15, 2022, 20:58 UTC
Message-ID
<xmqqlerpz0j8.fsf@gitster.g>
In-Reply-To
<9cd4c372ee4b3e5ba45c66a43ad0edaf52f0eed9.1660576283.git.gitgitgadget@gmail.com>
"Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 52 quoted lines
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> As --keep-base does not rebase the branch it is confusing if it
> removes commits that have been cherry-picked to the upstream branch.
> As --reapply-cherry-picks is not supported by the "apply" backend this
> commit ensures that cherry-picks are reapplied by forcing the upstream
> commit to match the onto commit unless --no-reapply-cherry-picks is
> given.
>
> Reported-by: Philippe Blain <levraiphilippeblain@gmail.com>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  Documentation/git-rebase.txt     |  2 +-
>  builtin/rebase.c                 | 15 ++++++++++++++-
>  t/t3416-rebase-onto-threedots.sh | 21 +++++++++++++++++++++
>  3 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 080658c8710..dc0c6c54e27 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -218,7 +218,7 @@ leave out at most one of A and B, in which case it defaults to HEAD.
>  	merge base of `<upstream>` and `<branch>`. Running
>  	`git rebase --keep-base <upstream> <branch>` is equivalent to
>  	running
> -	`git rebase --onto <upstream>...<branch> <upstream> <branch>`.
> +	`git rebase --reapply-cherry-picks --onto <upstream>...<branch> <upstream> <branch>`.
>  +
>  This option is useful in the case where one is developing a feature on
>  top of an upstream branch. While the feature is being worked on, the
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 86ea731ca3a..b6b3e00e3b1 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -1181,6 +1181,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>  	prepare_repo_settings(the_repository);
>  	the_repository->settings.command_requires_full_index = 0;
>  
> +	options.reapply_cherry_picks = -1;
>  	options.allow_empty_message = 1;
>  	git_config(rebase_config, &options);
>  	/* options.gpg_sign_opt will be either "-S" or NULL */
> @@ -1240,6 +1241,12 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>  		if (options.root)
>  			die(_("options '%s' and '%s' cannot be used together"), "--keep-base", "--root");
>  	}
> +	/*
> +	 * --keep-base defaults to --reapply-cherry-picks as it is confusing if
> +	 * commits disappear when using this option.
> +	 */
> +	if (options.reapply_cherry_picks < 0)
> +		options.reapply_cherry_picks = keep_base;

It makes me wonder if an explicit "--no-reapply-cherry-picks" makes sense in combination with "--keep-base". If that happens, we do not take this "By default, reapply is enabled with keep-base".

Show 11 quoted lines
> @@ -1416,7 +1423,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>  	if (options.empty != EMPTY_UNSPECIFIED)
>  		imply_merge(&options, "--empty");
>  
> -	if (options.reapply_cherry_picks)
> +	/*
> +	 * --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");

Interesting. The idea is that we shouldn't care how much progress (which may include cherry-picks) the upstream side made, and it is no use to compare the commits between the F (fork point) and O (our tip) against the commits between updated U (upstream) and F (fork point) to notice that X' is a cherry-pick from our X.

              o---X---o---O (our work)
             /
	----F----o----o----o----X'----U (upstream)

So almost ignoring U (except for obviously figure out F, possibly, for the purpose of keep-base) is an effective way to keep X on our history, and when it happens, we do not have to explicitly pass the "--reapply" option to underlying rebase machinery. Makes sense.

If an explicit "--no-reapply-cherry-picks" with "--keep-base" is given, we still skip this and do not call imply_merge() ...

Show 6 quoted lines
> @@ -1680,6 +1691,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>  	}
>  	if (keep_base) {
>  		oidcpy(&merge_base, &options.onto->object.oid);
> +		if (options.reapply_cherry_picks)
> +			options.upstream = options.onto;

... but this is also skipped in such a case. I do not offhand know if the combination makes practical sense, but this should allow the combination to "work". OK.

Thanks.
Previous: Phillip Wood via GitGitGadgetNext: Jonathan Tan
Message 21 of 82 in “rebase --keep-base: imply --reapply-cherry-picks and --no-fork-point”
  1. 0/5 rebase --keep-base: imply --reapply-cherry-picks and --no-fork-pointPhillip Wood via GitGitGadget, Aug 15, 2022
  2. 1/5 t3416: set $EDITOR in subshellPhillip Wood via GitGitGadget, Aug 15, 2022
  3. Junio C HamanoAug 15, 2022
  4. Phillip WoodAug 16, 2022
  5. Jonathan TanAug 24, 2022
  6. Phillip WoodAug 30, 2022
  7. 2/5 rebase: store orig_head as a commitPhillip Wood via GitGitGadget, Aug 15, 2022
  8. Junio C HamanoAug 15, 2022
  9. Johannes SchindelinAug 16, 2022
  10. Elijah NewrenAug 18, 2022
  11. 3/5 rebase: factor out merge_base calculationPhillip Wood via GitGitGadget, Aug 15, 2022
  12. Junio C HamanoAug 15, 2022
  13. Johannes SchindelinAug 16, 2022
  14. Junio C HamanoAug 16, 2022
  15. Phillip WoodAug 16, 2022
  16. Junio C HamanoAug 16, 2022
  17. Elijah NewrenAug 18, 2022
  18. Jonathan TanAug 24, 2022
  19. Phillip WoodAug 30, 2022
  20. 4/5 rebase --keep-base: imply --reapply-cherry-picksPhillip Wood via GitGitGadget, Aug 15, 2022
  21. Junio C HamanoAug 15, 2022
  22. Jonathan TanAug 24, 2022
  23. Phillip WoodAug 30, 2022
  24. Philippe BlainAug 25, 2022
  25. Phillip WoodSep 5, 2022
  26. 5/5 rebase --keep-base: imply --no-fork-pointPhillip Wood via GitGitGadget, Aug 15, 2022
  27. Junio C HamanoAug 15, 2022
  28. Jonathan TanAug 24, 2022
  29. Phillip WoodSep 5, 2022
  30. Johannes SchindelinAug 16, 2022
  31. Jonathan TanAug 24, 2022
  32. 0/7 rebase --keep-base: imply --reapply-cherry-picks and --no-fork-pointPhillip Wood via GitGitGadget, Sep 7, 2022
  33. 1/7 t3416: tighten two testsPhillip Wood via GitGitGadget, Sep 7, 2022
  34. Junio C HamanoSep 7, 2022
  35. 2/7 t3416: set $EDITOR in subshellPhillip Wood via GitGitGadget, Sep 7, 2022
  36. Junio C HamanoSep 7, 2022
  37. 3/7 rebase: store orig_head as a commitPhillip Wood via GitGitGadget, Sep 7, 2022
  38. Junio C HamanoSep 7, 2022
  39. Phillip WoodSep 8, 2022
  40. 5/7 rebase: factor out branch_base calculationPhillip Wood via GitGitGadget, Sep 7, 2022
  41. Junio C HamanoSep 7, 2022
  42. 4/7 rebase: rename merge_base to branch_basePhillip Wood via GitGitGadget, Sep 7, 2022
  43. Junio C HamanoSep 7, 2022
  44. 6/7 rebase --keep-base: imply --reapply-cherry-picksPhillip Wood via GitGitGadget, Sep 7, 2022
  45. 7/7 rebase --keep-base: imply --no-fork-pointPhillip Wood via GitGitGadget, Sep 7, 2022
  46. Denton LiuSep 8, 2022
  47. Phillip WoodSep 8, 2022
  48. 0/8 rebase --keep-base: imply --reapply-cherry-picks and --no-fork-pointPhillip Wood via GitGitGadget, Oct 13, 2022
  49. 1/8 t3416: tighten two testsPhillip Wood via GitGitGadget, Oct 13, 2022
  50. 2/8 t3416: set $EDITOR in subshellPhillip Wood via GitGitGadget, Oct 13, 2022
  51. 3/8 rebase: be stricter when reading state files containing oidsPhillip Wood via GitGitGadget, Oct 13, 2022
  52. Junio C HamanoOct 13, 2022
  53. Ævar Arnfjörð BjarmasonOct 13, 2022
  54. Junio C HamanoOct 13, 2022
  55. 4/8 rebase: store orig_head as a commitPhillip Wood via GitGitGadget, Oct 13, 2022
  56. Junio C HamanoOct 13, 2022
  57. Phillip WoodOct 13, 2022
  58. Junio C HamanoOct 13, 2022
  59. 6/8 rebase: factor out branch_base calculationPhillip Wood via GitGitGadget, Oct 13, 2022
  60. Ævar Arnfjörð BjarmasonOct 13, 2022
  61. Phillip WoodOct 17, 2022
  62. Ævar Arnfjörð BjarmasonOct 17, 2022
  63. 5/8 rebase: rename merge_base to branch_basePhillip Wood via GitGitGadget, Oct 13, 2022
  64. Ævar Arnfjörð BjarmasonOct 13, 2022
  65. Phillip WoodOct 17, 2022
  66. Ævar Arnfjörð BjarmasonOct 17, 2022
  67. Phillip WoodOct 17, 2022
  68. Ævar Arnfjörð BjarmasonOct 17, 2022
  69. Phillip WoodOct 19, 2022
  70. 7/8 rebase --keep-base: imply --reapply-cherry-picksPhillip Wood via GitGitGadget, Oct 13, 2022
  71. 8/8 rebase --keep-base: imply --no-fork-pointPhillip Wood via GitGitGadget, Oct 13, 2022
  72. 0/8 rebase --keep-base: imply --reapply-cherry-picks and --no-fork-pointPhillip Wood via GitGitGadget, Oct 17, 2022
  73. 1/8 t3416: tighten two testsPhillip Wood via GitGitGadget, Oct 17, 2022
  74. 2/8 t3416: set $EDITOR in subshellPhillip Wood via GitGitGadget, Oct 17, 2022
  75. 3/8 rebase: be stricter when reading state files containing oidsPhillip Wood via GitGitGadget, Oct 17, 2022
  76. Junio C HamanoOct 17, 2022
  77. Phillip WoodOct 19, 2022
  78. 4/8 rebase: store orig_head as a commitPhillip Wood via GitGitGadget, Oct 17, 2022
  79. 6/8 rebase: factor out branch_base calculationPhillip Wood via GitGitGadget, Oct 17, 2022
  80. 5/8 rebase: rename merge_base to branch_basePhillip Wood via GitGitGadget, Oct 17, 2022
  81. 7/8 rebase --keep-base: imply --reapply-cherry-picksPhillip Wood via GitGitGadget, Oct 17, 2022
  82. 8/8 rebase --keep-base: imply --no-fork-pointPhillip Wood via GitGitGadget, Oct 17, 2022

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.