rebase -m: cleanup --strategy-option handling
When handling "--strategy-option" rebase collects the commands into a struct string_list, then concatenates them into a string, prepending "--" to each one before splitting the string and removing the "--" prefix. This is an artifact of the scripted rebase and the need to support "rebase --preserve-merges". Now that "--preserve-merges" no-longer exists we can cleanup the way the argument is handled. The tests for a bad strategy option are adjusted now that parse_strategy_opts() is no-longer called when starting a rebase. The fact that it only errors out when running "git rebase --continue" is a mixed blessing but the next commit will fix the root cause of the parsing problem so lets not worry about that here. Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Phillip Wood committed
Apr 10, 2023 at 10:08 UTC
4a8bc9860aa09942dd0e6540d53102219a7e920d
4 files changed
+19
-24
builtin/rebase.c
+10
-20
@@ -117,7 +117,8 @@ struct rebase_options {
117
struct string_list exec;
118
int allow_empty_message;
119
int rebase_merges, rebase_cousins;
120
- char *strategy, *strategy_opts;
120
+ char *strategy;
121
+ struct string_list strategy_opts;
122
struct strbuf git_format_patch_opt;
123
int reschedule_failed_exec;
124
int reapply_cherry_picks;
@@ -143,6 +144,7 @@ struct rebase_options {
144
.config_autosquash = -1, \
145
.update_refs = -1, \
146
.config_update_refs = -1, \
147
+ .strategy_opts = STRING_LIST_INIT_NODUP,\
148
}
149
150
static struct replay_opts get_replay_opts(const struct rebase_options *opts)
@@ -176,8 +178,8 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts)
178
replay.default_strategy = NULL;
179
}
180
179
- if (opts->strategy_opts)
180
- parse_strategy_opts(&replay, opts->strategy_opts);
181
+ for (size_t i = 0; i < opts->strategy_opts.nr; i++)
182
+ strvec_push(&replay.xopts, opts->strategy_opts.items[i].string);
183
184
if (opts->squash_onto) {
185
oidcpy(&replay.squash_onto, opts->squash_onto);
@@ -1013,7 +1015,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
1015
int ignore_whitespace = 0;
1016
const char *gpg_sign = NULL;
1017
const char *rebase_merges = NULL;
1016
- struct string_list strategy_options = STRING_LIST_INIT_NODUP;
1018
struct object_id squash_onto;
1019
char *squash_onto_name = NULL;
1020
char *keep_base_onto_name = NULL;
@@ -1122,7 +1123,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
1123
N_("use 'merge-base --fork-point' to refine upstream")),
1124
OPT_STRING('s', "strategy", &options.strategy,
1125
N_("strategy"), N_("use the given merge strategy")),
1125
- OPT_STRING_LIST('X', "strategy-option", &strategy_options,
1126
+ OPT_STRING_LIST('X', "strategy-option", &options.strategy_opts,
1127
N_("option"),
1128
N_("pass the argument through to the merge "
1129
"strategy")),
@@ -1436,23 +1437,13 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
1437
} else {
1438
/* REBASE_MERGE */
1439
if (ignore_whitespace) {
1439
- string_list_append(&strategy_options,
1440
+ string_list_append(&options.strategy_opts,
1441
"ignore-space-change");
1442
}
1443
}
1444
1444
- if (strategy_options.nr) {
1445
- int i;
1446
-
1447
- if (!options.strategy)
1448
- options.strategy = "ort";
1449
-
1450
- strbuf_reset(&buf);
1451
- for (i = 0; i < strategy_options.nr; i++)
1452
- strbuf_addf(&buf, " --%s",
1453
- strategy_options.items[i].string);
1454
- options.strategy_opts = xstrdup(buf.buf);
1455
- }
1445
+ if (options.strategy_opts.nr && !options.strategy)
1446
+ options.strategy = "ort";
1447
1448
if (options.strategy) {
1449
options.strategy = xstrdup(options.strategy);
@@ -1827,10 +1818,9 @@ cleanup:
1818
free(options.gpg_sign_opt);
1819
string_list_clear(&options.exec, 0);
1820
free(options.strategy);
1830
- free(options.strategy_opts);
1821
+ string_list_clear(&options.strategy_opts, 0);
1822
strbuf_release(&options.git_format_patch_opt);
1823
free(squash_onto_name);
1824
free(keep_base_onto_name);
1834
- string_list_clear(&strategy_options, 0);
1825
return !!ret;
1826
}
sequencer.c
+1
-1
@@ -2913,7 +2913,7 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
2913
return 0;
2914
}
2915
2916
-void parse_strategy_opts(struct replay_opts *opts, char *raw_opts)
2916
+static void parse_strategy_opts(struct replay_opts *opts, char *raw_opts)
2917
{
2918
int i;
2919
int count;
sequencer.h
-1
@@ -252,7 +252,6 @@ int read_oneliner(struct strbuf *buf,
252
const char *path, unsigned flags);
253
int read_author_script(const char *path, char **name, char **email, char **date,
254
int allow_missing);
255
-void parse_strategy_opts(struct replay_opts *opts, char *raw_opts);
255
int write_basic_state(struct replay_opts *opts, const char *head_name,
256
struct commit *onto, const struct object_id *orig_head);
257
void sequencer_post_commit_cleanup(struct repository *r, int verbose);
t/t3436-rebase-more-options.sh
+8
-2
@@ -41,19 +41,25 @@ test_expect_success 'setup' '
41
'
42
43
test_expect_success 'bad -X <strategy-option> arguments: unclosed quote' '
44
+ test_when_finished "test_might_fail git rebase --abort" &&
45
cat >expect <<-\EOF &&
46
fatal: could not split '\''--bad'\'': unclosed quote
47
EOF
47
- test_expect_code 128 git rebase -X"bad argument\"" side main >out 2>actual &&
48
+ GIT_SEQUENCE_EDITOR="echo break >" \
49
+ git rebase -i -X"bad argument\"" side main &&
50
+ test_expect_code 128 git rebase --continue >out 2>actual &&
51
test_must_be_empty out &&
52
test_cmp expect actual
53
'
54
55
test_expect_success 'bad -X <strategy-option> arguments: bad escape' '
56
+ test_when_finished "test_might_fail git rebase --abort" &&
57
cat >expect <<-\EOF &&
58
fatal: could not split '\''--bad'\'': cmdline ends with \
59
EOF
56
- test_expect_code 128 git rebase -X"bad escape \\" side main >out 2>actual &&
60
+ GIT_SEQUENCE_EDITOR="echo break >" \
61
+ git rebase -i -X"bad escape \\" side main &&
62
+ test_expect_code 128 git rebase --continue >out 2>actual &&
63
test_must_be_empty out &&
64
test_cmp expect actual
65
'