cherry-pick: detect bogus arguments to --mainline
The cherry-pick and revert commands use OPT_INTEGER() to parse --mainline. The stock parser is smart enough to reject non-numeric nonsense, but it doesn't know that parent counting starts at 1. Worse, the value "0" is indistinguishable from the unset case, so a user who assumes the counting is 0-based will get a confusing message: $ git cherry-pick -m 0 $merge error: commit ... is a merge but no -m option was given. Let's use a custom callback that enforces our range. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King committed
Mar 15, 2017 at 12:56 UTC
b16a991c1be5681b4b673d4343dfcc0c2f5ad498
2 files changed
+29
-1
builtin/revert.c
+20
-1
@@ -54,6 +54,24 @@ static int option_parse_x(const struct option *opt,
54
return 0;
55
}
56
57
+static int option_parse_m(const struct option *opt,
58
+ const char *arg, int unset)
59
+{
60
+ struct replay_opts *replay = opt->value;
61
+ char *end;
62
+
63
+ if (unset) {
64
+ replay->mainline = 0;
65
+ return 0;
66
+ }
67
+
68
+ replay->mainline = strtol(arg, &end, 10);
69
+ if (*end || replay->mainline <= 0)
70
+ return opterror(opt, "expects a number greater than zero", 0);
71
+
72
+ return 0;
73
+}
74
+
75
LAST_ARG_MUST_BE_NULL
76
static void verify_opt_compatible(const char *me, const char *base_opt, ...)
77
{
@@ -84,7 +102,8 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
102
OPT_BOOL('e', "edit", &opts->edit, N_("edit the commit message")),
103
OPT_NOOP_NOARG('r', NULL),
104
OPT_BOOL('s', "signoff", &opts->signoff, N_("add Signed-off-by:")),
87
- OPT_INTEGER('m', "mainline", &opts->mainline, N_("parent number")),
105
+ OPT_CALLBACK('m', "mainline", opts, N_("parent-number"),
106
+ N_("select mainline parent"), option_parse_m),
107
OPT_RERERE_AUTOUPDATE(&opts->allow_rerere_auto),
108
OPT_STRING(0, "strategy", &opts->strategy, N_("strategy"), N_("merge strategy")),
109
OPT_CALLBACK('X', "strategy-option", &opts, N_("option"),
t/t3502-cherry-pick-merge.sh
+9
@@ -31,6 +31,15 @@ test_expect_success setup '
31
32
'
33
34
+test_expect_success 'cherry-pick -m complains of bogus numbers' '
35
+ # expect 129 here to distinguish between cases where
36
+ # there was nothing to cherry-pick
37
+ test_expect_code 129 git cherry-pick -m &&
38
+ test_expect_code 129 git cherry-pick -m foo b &&
39
+ test_expect_code 129 git cherry-pick -m -1 b &&
40
+ test_expect_code 129 git cherry-pick -m 0 b
41
+'
42
+
43
test_expect_success 'cherry-pick a non-merge with -m should fail' '
44
45
git reset --hard &&