rebase --keep-base: imply --reapply-cherry-picks

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Phillip Wood committed Oct 17, 2022 at 13:17 UTC ce5238a690821d1de230091dd6c9c13a99ed6752
3 files changed +52 -11
Documentation/git-rebase.txt
+16 -10
@@ -218,12 +218,14 @@ leave out at most one of A and B, in which case it defaults to HEAD.
218 merge base of `<upstream>` and `<branch>`. Running
219 `git rebase --keep-base <upstream> <branch>` is equivalent to
220 running
221 - `git rebase --onto <upstream>...<branch> <upstream> <branch>`.
221 + `git rebase --reapply-cherry-picks --onto <upstream>...<branch> <upstream> <branch>`.
222 +
223 This option is useful in the case where one is developing a feature on
224 top of an upstream branch. While the feature is being worked on, the
225 upstream branch may advance and it may not be the best idea to keep
226 -rebasing on top of the upstream but to keep the base commit as-is.
226 +rebasing on top of the upstream but to keep the base commit as-is. As
227 +the base commit is unchanged this option implies `--reapply-cherry-picks`
228 +to avoid losing commits.
229 +
230 Although both this option and `--fork-point` find the merge base between
231 `<upstream>` and `<branch>`, this option uses the merge base as the _starting
@@ -278,7 +280,8 @@ See also INCOMPATIBLE OPTIONS below.
280 Note that commits which start empty are kept (unless `--no-keep-empty`
281 is specified), and commits which are clean cherry-picks (as determined
282 by `git log --cherry-mark ...`) are detected and dropped as a
281 -preliminary step (unless `--reapply-cherry-picks` is passed).
283 +preliminary step (unless `--reapply-cherry-picks` or `--keep-base` is
284 +passed).
285 +
286 See also INCOMPATIBLE OPTIONS below.
287
@@ -311,13 +314,16 @@ See also INCOMPATIBLE OPTIONS below.
314 upstream changes, the behavior towards them is controlled by
315 the `--empty` flag.)
316 +
314 -By default (or if `--no-reapply-cherry-picks` is given), these commits
315 -will be automatically dropped. Because this necessitates reading all
316 -upstream commits, this can be expensive in repos with a large number
317 -of upstream commits that need to be read. When using the 'merge'
318 -backend, warnings will be issued for each dropped commit (unless
319 -`--quiet` is given). Advice will also be issued unless
320 -`advice.skippedCherryPicks` is set to false (see linkgit:git-config[1]).
317 +
318 +In the absence of `--keep-base` (or if `--no-reapply-cherry-picks` is
319 +given), these commits will be automatically dropped. Because this
320 +necessitates reading all upstream commits, this can be expensive in
321 +repositories with a large number of upstream commits that need to be
322 +read. When using the 'merge' backend, warnings will be issued for each
323 +dropped commit (unless `--quiet` is given). Advice will also be issued
324 +unless `advice.skippedCherryPicks` is set to false (see
325 +linkgit:git-config[1]).
326 +
327 +
328 `--reapply-cherry-picks` allows rebase to forgo reading all upstream
329 commits, potentially improving performance.
builtin/rebase.c
+15 -1
@@ -1171,6 +1171,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
1171 prepare_repo_settings(the_repository);
1172 the_repository->settings.command_requires_full_index = 0;
1173
1174 + options.reapply_cherry_picks = -1;
1175 options.allow_empty_message = 1;
1176 git_config(rebase_config, &options);
1177 /* options.gpg_sign_opt will be either "-S" or NULL */
@@ -1230,6 +1231,12 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
1231 if (options.root)
1232 die(_("options '%s' and '%s' cannot be used together"), "--keep-base", "--root");
1233 }
1234 + /*
1235 + * --keep-base defaults to --reapply-cherry-picks to avoid losing
1236 + * commits when using this option.
1237 + */
1238 + if (options.reapply_cherry_picks < 0)
1239 + options.reapply_cherry_picks = keep_base;
1240
1241 if (options.root && options.fork_point > 0)
1242 die(_("options '%s' and '%s' cannot be used together"), "--root", "--fork-point");
@@ -1406,7 +1413,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
1413 if (options.empty != EMPTY_UNSPECIFIED)
1414 imply_merge(&options, "--empty");
1415
1409 - if (options.reapply_cherry_picks)
1416 + /*
1417 + * --keep-base implements --reapply-cherry-picks by altering upstream so
1418 + * it works with both backends.
1419 + */
1420 + if (options.reapply_cherry_picks && !keep_base)
1421 imply_merge(&options, "--reapply-cherry-picks");
1422
1423 if (gpg_sign)
@@ -1672,6 +1683,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
1683 fill_branch_base(&options, &branch_base);
1684 }
1685
1686 + if (keep_base && options.reapply_cherry_picks)
1687 + options.upstream = options.onto;
1688 +
1689 if (options.fork_point > 0)
1690 options.restrict_revision =
1691 get_fork_point(options.upstream_name, options.orig_head);
t/t3416-rebase-onto-threedots.sh
+21
@@ -199,6 +199,27 @@ test_expect_success 'rebase --keep-base requires a single merge base' '
199 grep "need exactly one merge base with branch" err
200 '
201
202 +test_expect_success 'rebase --keep-base keeps cherry picks' '
203 + git checkout -f -B main E &&
204 + git cherry-pick F &&
205 + (
206 + set_fake_editor &&
207 + EXPECT_COUNT=2 git rebase -i --keep-base HEAD G
208 + ) &&
209 + test_cmp_rev HEAD G
210 +'
211 +
212 +test_expect_success 'rebase --keep-base --no-reapply-cherry-picks' '
213 + git checkout -f -B main E &&
214 + git cherry-pick F &&
215 + (
216 + set_fake_editor &&
217 + EXPECT_COUNT=1 git rebase -i --keep-base \
218 + --no-reapply-cherry-picks HEAD G
219 + ) &&
220 + test_cmp_rev HEAD^ C
221 +'
222 +
223 # This must be the last test in this file
224 test_expect_success '$EDITOR and friends are unchanged' '
225 test_editor_unchanged