sequencer: advise if skipping cherry-picked commit
Silently skipping commits when rebasing with --no-reapply-cherry-picks (currently the default behavior) can cause user confusion. Issue warnings when this happens, as well as advice on how to preserve the skipped commits. These warnings and advice are displayed only when using the (default) "merge" rebase backend. Update the git-rebase docs to mention the warnings and advice. Signed-off-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Josh Steadmon committed
Aug 30, 2021 at 14:46 UTC
767a4ca648f8791c1fb623bd9f79fd8d7f026499
7 files changed
+38
-6
Documentation/config/advice.txt
+3
@@ -44,6 +44,9 @@ advice.*::
44
Shown when linkgit:git-push[1] rejects a forced update of
45
a branch when its remote-tracking ref has updates that we
46
do not have locally.
47
+ skippedCherryPicks::
48
+ Shown when linkgit:git-rebase[1] skips a commit that has already
49
+ been cherry-picked onto the upstream branch.
50
statusAheadBehind::
51
Shown when linkgit:git-status[1] computes the ahead/behind
52
counts for a local ref compared to its remote tracking ref,
Documentation/git-rebase.txt
+8
-4
@@ -79,9 +79,10 @@ remain the checked-out branch.
79
80
If the upstream branch already contains a change you have made (e.g.,
81
because you mailed a patch which was applied upstream), then that commit
82
-will be skipped. For example, running `git rebase master` on the
83
-following history (in which `A'` and `A` introduce the same set of changes,
84
-but have different committer information):
82
+will be skipped and warnings will be issued (if the `merge` backend is
83
+used). For example, running `git rebase master` on the following
84
+history (in which `A'` and `A` introduce the same set of changes, but
85
+have different committer information):
86
87
------------
88
A---B---C topic
@@ -312,7 +313,10 @@ See also INCOMPATIBLE OPTIONS below.
313
By default (or if `--no-reapply-cherry-picks` is given), these commits
314
will be automatically dropped. Because this necessitates reading all
315
upstream commits, this can be expensive in repos with a large number
315
-of upstream commits that need to be read.
316
+of upstream commits that need to be read. When using the `merge`
317
+backend, warnings will be issued for each dropped commit (unless
318
+`--quiet` is given). Advice will also be issued unless
319
+`advice.skippedCherryPicks` is set to false (see linkgit:git-config[1]).
320
+
321
`--reapply-cherry-picks` allows rebase to forgo reading all upstream
322
commits, potentially improving performance.
advice.c
+3
@@ -34,6 +34,7 @@ int advice_checkout_ambiguous_remote_branch_name = 1;
34
int advice_submodule_alternate_error_strategy_die = 1;
35
int advice_add_ignored_file = 1;
36
int advice_add_empty_pathspec = 1;
37
+int advice_skipped_cherry_picks = 1;
38
39
static int advice_use_color = -1;
40
static char advice_colors[][COLOR_MAXLEN] = {
@@ -96,6 +97,7 @@ static struct {
97
{ "submoduleAlternateErrorStrategyDie", &advice_submodule_alternate_error_strategy_die },
98
{ "addIgnoredFile", &advice_add_ignored_file },
99
{ "addEmptyPathspec", &advice_add_empty_pathspec },
100
+ { "skippedCherryPicks", &advice_skipped_cherry_picks },
101
102
/* make this an alias for backward compatibility */
103
{ "pushNonFastForward", &advice_push_update_rejected }
@@ -133,6 +135,7 @@ static struct {
135
[ADVICE_RM_HINTS] = { "rmHints", 1 },
136
[ADVICE_SEQUENCER_IN_USE] = { "sequencerInUse", 1 },
137
[ADVICE_SET_UPSTREAM_FAILURE] = { "setUpstreamFailure", 1 },
138
+ [ADVICE_SKIPPED_CHERRY_PICKS] = { "skippedCherryPicks", 1 },
139
[ADVICE_STATUS_AHEAD_BEHIND_WARNING] = { "statusAheadBehindWarning", 1 },
140
[ADVICE_STATUS_HINTS] = { "statusHints", 1 },
141
[ADVICE_STATUS_U_OPTION] = { "statusUoption", 1 },
advice.h
+2
@@ -35,6 +35,7 @@ extern int advice_checkout_ambiguous_remote_branch_name;
35
extern int advice_submodule_alternate_error_strategy_die;
36
extern int advice_add_ignored_file;
37
extern int advice_add_empty_pathspec;
38
+extern int advice_skipped_cherry_picks;
39
40
/*
41
* To add a new advice, you need to:
@@ -75,6 +76,7 @@ extern int advice_add_empty_pathspec;
76
ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE,
77
ADVICE_UPDATE_SPARSE_PATH,
78
ADVICE_WAITING_FOR_EDITOR,
79
+ ADVICE_SKIPPED_CHERRY_PICKS,
80
};
81
82
int git_default_advice_config(const char *var, const char *value);
builtin/rebase.c
+1
@@ -405,6 +405,7 @@ static int run_sequencer_rebase(struct rebase_options *opts,
405
flags |= opts->root_with_onto ? TODO_LIST_ROOT_WITH_ONTO : 0;
406
flags |= command == ACTION_SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0;
407
flags |= opts->reapply_cherry_picks ? TODO_LIST_REAPPLY_CHERRY_PICKS : 0;
408
+ flags |= opts->flags & REBASE_NO_QUIET ? TODO_LIST_WARN_SKIPPED_CHERRY_PICKS : 0;
409
410
switch (command) {
411
case ACTION_NONE: {
sequencer.c
+20
-2
@@ -5099,6 +5099,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,
5099
int keep_empty = flags & TODO_LIST_KEEP_EMPTY;
5100
int rebase_cousins = flags & TODO_LIST_REBASE_COUSINS;
5101
int root_with_onto = flags & TODO_LIST_ROOT_WITH_ONTO;
5102
+ int skipped_commit = 0;
5103
struct strbuf buf = STRBUF_INIT, oneline = STRBUF_INIT;
5104
struct strbuf label = STRBUF_INIT;
5105
struct commit_list *commits = NULL, **tail = &commits, *iter;
@@ -5149,8 +5150,13 @@ static int make_script_with_merges(struct pretty_print_context *pp,
5150
oidset_insert(&interesting, &commit->object.oid);
5151
5152
is_empty = is_original_commit_empty(commit);
5152
- if (!is_empty && (commit->object.flags & PATCHSAME))
5153
+ if (!is_empty && (commit->object.flags & PATCHSAME)) {
5154
+ if (flags & TODO_LIST_WARN_SKIPPED_CHERRY_PICKS)
5155
+ warning(_("skipped previously applied commit %s"),
5156
+ short_commit_name(commit));
5157
+ skipped_commit = 1;
5158
continue;
5159
+ }
5160
if (is_empty && !keep_empty)
5161
continue;
5162
@@ -5214,6 +5220,9 @@ static int make_script_with_merges(struct pretty_print_context *pp,
5220
oidcpy(&entry->entry.oid, &commit->object.oid);
5221
oidmap_put(&commit2todo, entry);
5222
}
5223
+ if (skipped_commit)
5224
+ advise_if_enabled(ADVICE_SKIPPED_CHERRY_PICKS,
5225
+ _("use --reapply-cherry-picks to include skipped commits"));
5226
5227
/*
5228
* Second phase:
@@ -5334,6 +5343,7 @@ int sequencer_make_script(struct repository *r, struct strbuf *out, int argc,
5343
const char *insn = flags & TODO_LIST_ABBREVIATE_CMDS ? "p" : "pick";
5344
int rebase_merges = flags & TODO_LIST_REBASE_MERGES;
5345
int reapply_cherry_picks = flags & TODO_LIST_REAPPLY_CHERRY_PICKS;
5346
+ int skipped_commit = 0;
5347
5348
repo_init_revisions(r, &revs, NULL);
5349
revs.verbose_header = 1;
@@ -5369,8 +5379,13 @@ int sequencer_make_script(struct repository *r, struct strbuf *out, int argc,
5379
while ((commit = get_revision(&revs))) {
5380
int is_empty = is_original_commit_empty(commit);
5381
5372
- if (!is_empty && (commit->object.flags & PATCHSAME))
5382
+ if (!is_empty && (commit->object.flags & PATCHSAME)) {
5383
+ if (flags & TODO_LIST_WARN_SKIPPED_CHERRY_PICKS)
5384
+ warning(_("skipped previously applied commit %s"),
5385
+ short_commit_name(commit));
5386
+ skipped_commit = 1;
5387
continue;
5388
+ }
5389
if (is_empty && !keep_empty)
5390
continue;
5391
strbuf_addf(out, "%s %s ", insn,
@@ -5380,6 +5395,9 @@ int sequencer_make_script(struct repository *r, struct strbuf *out, int argc,
5395
strbuf_addf(out, " %c empty", comment_line_char);
5396
strbuf_addch(out, '\n');
5397
}
5398
+ if (skipped_commit)
5399
+ advise_if_enabled(ADVICE_SKIPPED_CHERRY_PICKS,
5400
+ _("use --reapply-cherry-picks to include skipped commits"));
5401
return 0;
5402
}
5403
sequencer.h
+1
@@ -156,6 +156,7 @@ int sequencer_remove_state(struct replay_opts *opts);
156
*/
157
#define TODO_LIST_ROOT_WITH_ONTO (1U << 6)
158
#define TODO_LIST_REAPPLY_CHERRY_PICKS (1U << 7)
159
+#define TODO_LIST_WARN_SKIPPED_CHERRY_PICKS (1U << 8)
160
161
int sequencer_make_script(struct repository *r, struct strbuf *out, int argc,
162
const char **argv, unsigned flags);