Skip to content

Commit 3c957e6

Browse files
gitstergitster
committed
Merge branch 'pw/rebase-cleanup-merge-strategy-option-handling'
Clean-up of the code path that deals with merge strategy option handling in "git rebase". * pw/rebase-cleanup-merge-strategy-option-handling: rebase: remove a couple of redundant strategy tests rebase -m: fix serialization of strategy options rebase -m: cleanup --strategy-option handling sequencer: use struct strvec to store merge strategy options rebase: stop reading and writing unnecessary strategy state
2 parents 66bf8f1 + 05106aa commit 3c957e6

File tree

11 files changed

+114
-178
lines changed

11 files changed

+114
-178
lines changed

‎alias.c‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include "alloc.h"
44
#include "config.h"
55
#include "gettext.h"
6+
#include "strbuf.h"
67
#include "string-list.h"
78

89
struct config_alias_data {
@@ -46,6 +47,23 @@ void list_aliases(struct string_list *list)
4647
read_early_config(config_alias_cb, &data);
4748
}
4849

50+
void quote_cmdline(struct strbuf *buf, const char **argv)
51+
{
52+
for (const char **argp = argv; *argp; argp++) {
53+
if (argp != argv)
54+
strbuf_addch(buf, ' ');
55+
strbuf_addch(buf, '"');
56+
for (const char *p = *argp; *p; p++) {
57+
const char c = *p;
58+
59+
if (c == '"' || c =='\\')
60+
strbuf_addch(buf, '\\');
61+
strbuf_addch(buf, c);
62+
}
63+
strbuf_addch(buf, '"');
64+
}
65+
}
66+
4967
#define SPLIT_CMDLINE_BAD_ENDING 1
5068
#define SPLIT_CMDLINE_UNCLOSED_QUOTE 2
5169
#define SPLIT_CMDLINE_ARGC_OVERFLOW 3

‎alias.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
#ifndef ALIAS_H
22
#define ALIAS_H
33

4+
struct strbuf;
45
struct string_list;
56

67
char *alias_lookup(const char *alias);
8+
/* Quote argv so buf can be parsed by split_cmdline() */
9+
void quote_cmdline(struct strbuf *buf, const char **argv);
710
int split_cmdline(char *cmdline, const char ***argv);
811
/* Takes a negative value returned by split_cmdline */
912
const char *split_cmdline_strerror(int cmdline_errno);

‎builtin/rebase.c‎

Lines changed: 10 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,8 @@ struct rebase_options {
121121
struct string_list exec;
122122
int allow_empty_message;
123123
int rebase_merges, rebase_cousins;
124-
char *strategy, *strategy_opts;
124+
char *strategy;
125+
struct string_list strategy_opts;
125126
struct strbuf git_format_patch_opt;
126127
int reschedule_failed_exec;
127128
int reapply_cherry_picks;
@@ -150,6 +151,7 @@ struct rebase_options {
150151
.config_rebase_merges = -1, \
151152
.update_refs = -1, \
152153
.config_update_refs = -1, \
154+
.strategy_opts = STRING_LIST_INIT_NODUP,\
153155
}
154156

155157
static struct replay_opts get_replay_opts(const struct rebase_options *opts)
@@ -183,8 +185,8 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts)
183185
replay.default_strategy = NULL;
184186
}
185187

186-
if (opts->strategy_opts)
187-
parse_strategy_opts(&replay, opts->strategy_opts);
188+
for (size_t i = 0; i < opts->strategy_opts.nr; i++)
189+
strvec_push(&replay.xopts, opts->strategy_opts.items[i].string);
188190

189191
if (opts->squash_onto) {
190192
oidcpy(&replay.squash_onto, opts->squash_onto);
@@ -492,24 +494,6 @@ static int read_basic_state(struct rebase_options *opts)
492494
opts->gpg_sign_opt = xstrdup(buf.buf);
493495
}
494496

495-
if (file_exists(state_dir_path("strategy", opts))) {
496-
strbuf_reset(&buf);
497-
if (!read_oneliner(&buf, state_dir_path("strategy", opts),
498-
READ_ONELINER_WARN_MISSING))
499-
return -1;
500-
free(opts->strategy);
501-
opts->strategy = xstrdup(buf.buf);
502-
}
503-
504-
if (file_exists(state_dir_path("strategy_opts", opts))) {
505-
strbuf_reset(&buf);
506-
if (!read_oneliner(&buf, state_dir_path("strategy_opts", opts),
507-
READ_ONELINER_WARN_MISSING))
508-
return -1;
509-
free(opts->strategy_opts);
510-
opts->strategy_opts = xstrdup(buf.buf);
511-
}
512-
513497
strbuf_release(&buf);
514498

515499
return 0;
@@ -527,12 +511,6 @@ static int rebase_write_basic_state(struct rebase_options *opts)
527511
write_file(state_dir_path("quiet", opts), "%s", "");
528512
if (opts->flags & REBASE_VERBOSE)
529513
write_file(state_dir_path("verbose", opts), "%s", "");
530-
if (opts->strategy)
531-
write_file(state_dir_path("strategy", opts), "%s",
532-
opts->strategy);
533-
if (opts->strategy_opts)
534-
write_file(state_dir_path("strategy_opts", opts), "%s",
535-
opts->strategy_opts);
536514
if (opts->allow_rerere_autoupdate > 0)
537515
write_file(state_dir_path("allow_rerere_autoupdate", opts),
538516
"-%s-rerere-autoupdate",
@@ -1089,7 +1067,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
10891067
struct object_id branch_base;
10901068
int ignore_whitespace = 0;
10911069
const char *gpg_sign = NULL;
1092-
struct string_list strategy_options = STRING_LIST_INIT_NODUP;
10931070
struct object_id squash_onto;
10941071
char *squash_onto_name = NULL;
10951072
char *keep_base_onto_name = NULL;
@@ -1197,7 +1174,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
11971174
N_("use 'merge-base --fork-point' to refine upstream")),
11981175
OPT_STRING('s', "strategy", &options.strategy,
11991176
N_("strategy"), N_("use the given merge strategy")),
1200-
OPT_STRING_LIST('X', "strategy-option", &strategy_options,
1177+
OPT_STRING_LIST('X', "strategy-option", &options.strategy_opts,
12011178
N_("option"),
12021179
N_("pass the argument through to the merge "
12031180
"strategy")),
@@ -1500,23 +1477,13 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
15001477
} else {
15011478
/* REBASE_MERGE */
15021479
if (ignore_whitespace) {
1503-
string_list_append(&strategy_options,
1480+
string_list_append(&options.strategy_opts,
15041481
"ignore-space-change");
15051482
}
15061483
}
15071484

1508-
if (strategy_options.nr) {
1509-
int i;
1510-
1511-
if (!options.strategy)
1512-
options.strategy = "ort";
1513-
1514-
strbuf_reset(&buf);
1515-
for (i = 0; i < strategy_options.nr; i++)
1516-
strbuf_addf(&buf, " --%s",
1517-
strategy_options.items[i].string);
1518-
options.strategy_opts = xstrdup(buf.buf);
1519-
}
1485+
if (options.strategy_opts.nr && !options.strategy)
1486+
options.strategy = "ort";
15201487

15211488
if (options.strategy) {
15221489
options.strategy = xstrdup(options.strategy);
@@ -1898,10 +1865,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
18981865
free(options.gpg_sign_opt);
18991866
string_list_clear(&options.exec, 0);
19001867
free(options.strategy);
1901-
free(options.strategy_opts);
1868+
string_list_clear(&options.strategy_opts, 0);
19021869
strbuf_release(&options.git_format_patch_opt);
19031870
free(squash_onto_name);
19041871
free(keep_base_onto_name);
1905-
string_list_clear(&strategy_options, 0);
19061872
return !!ret;
19071873
}

‎builtin/revert.c‎

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -45,20 +45,6 @@ static const char * const *revert_or_cherry_pick_usage(struct replay_opts *opts)
4545
return opts->action == REPLAY_REVERT ? revert_usage : cherry_pick_usage;
4646
}
4747

48-
static int option_parse_x(const struct option *opt,
49-
const char *arg, int unset)
50-
{
51-
struct replay_opts **opts_ptr = opt->value;
52-
struct replay_opts *opts = *opts_ptr;
53-
54-
if (unset)
55-
return 0;
56-
57-
ALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);
58-
opts->xopts[opts->xopts_nr++] = xstrdup(arg);
59-
return 0;
60-
}
61-
6248
static int option_parse_m(const struct option *opt,
6349
const char *arg, int unset)
6450
{
@@ -116,8 +102,8 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
116102
N_("select mainline parent"), option_parse_m),
117103
OPT_RERERE_AUTOUPDATE(&opts->allow_rerere_auto),
118104
OPT_STRING(0, "strategy", &opts->strategy, N_("strategy"), N_("merge strategy")),
119-
OPT_CALLBACK('X', "strategy-option", &opts, N_("option"),
120-
N_("option for merge strategy"), option_parse_x),
105+
OPT_STRVEC('X', "strategy-option", &opts->xopts, N_("option"),
106+
N_("option for merge strategy")),
121107
{ OPTION_STRING, 'S', "gpg-sign", &opts->gpg_sign, N_("key-id"),
122108
N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
123109
OPT_END()
@@ -178,7 +164,7 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
178164
"--signoff", opts->signoff,
179165
"--mainline", opts->mainline,
180166
"--strategy", opts->strategy ? 1 : 0,
181-
"--strategy-option", opts->xopts ? 1 : 0,
167+
"--strategy-option", opts->xopts.nr ? 1 : 0,
182168
"-x", opts->record_origin,
183169
"--ff", opts->allow_ff,
184170
"--rerere-autoupdate", opts->allow_rerere_auto == RERERE_AUTOUPDATE,

‎parse-options-cb.c‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,22 @@ int parse_opt_string_list(const struct option *opt, const char *arg, int unset)
210210
return 0;
211211
}
212212

213+
int parse_opt_strvec(const struct option *opt, const char *arg, int unset)
214+
{
215+
struct strvec *v = opt->value;
216+
217+
if (unset) {
218+
strvec_clear(v);
219+
return 0;
220+
}
221+
222+
if (!arg)
223+
return -1;
224+
225+
strvec_push(v, arg);
226+
return 0;
227+
}
228+
213229
int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset)
214230
{
215231
return 0;

‎parse-options.h‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,15 @@ struct option {
287287
.help = (h), \
288288
.callback = &parse_opt_string_list, \
289289
}
290+
#define OPT_STRVEC(s, l, v, a, h) { \
291+
.type = OPTION_CALLBACK, \
292+
.short_name = (s), \
293+
.long_name = (l), \
294+
.value = (v), \
295+
.argh = (a), \
296+
.help = (h), \
297+
.callback = &parse_opt_strvec, \
298+
}
290299
#define OPT_UYN(s, l, v, h) { \
291300
.type = OPTION_CALLBACK, \
292301
.short_name = (s), \
@@ -480,6 +489,7 @@ int parse_opt_commits(const struct option *, const char *, int);
480489
int parse_opt_commit(const struct option *, const char *, int);
481490
int parse_opt_tertiary(const struct option *, const char *, int);
482491
int parse_opt_string_list(const struct option *, const char *, int);
492+
int parse_opt_strvec(const struct option *, const char *, int);
483493
int parse_opt_noop_cb(const struct option *, const char *, int);
484494
int parse_opt_passthru(const struct option *, const char *, int);
485495
int parse_opt_passthru_argv(const struct option *, const char *, int);

0 commit comments

Comments
 (0)