diff options
| -rw-r--r-- | alias.c | 18 | ||||
| -rw-r--r-- | alias.h | 3 | ||||
| -rw-r--r-- | builtin/rebase.c | 54 | ||||
| -rw-r--r-- | builtin/revert.c | 20 | ||||
| -rw-r--r-- | parse-options-cb.c | 16 | ||||
| -rw-r--r-- | parse-options.h | 10 | ||||
| -rw-r--r-- | sequencer.c | 58 | ||||
| -rw-r--r-- | sequencer.h | 12 | ||||
| -rwxr-xr-x | t/t3402-rebase-merge.sh | 21 | ||||
| -rwxr-xr-x | t/t3418-rebase-continue.sh | 62 | ||||
| -rwxr-xr-x | t/t3436-rebase-more-options.sh | 18 |
11 files changed, 114 insertions, 178 deletions
@@ -3,6 +3,7 @@ #include "alloc.h" #include "config.h" #include "gettext.h" +#include "strbuf.h" #include "string-list.h" struct config_alias_data { @@ -46,6 +47,23 @@ void list_aliases(struct string_list *list) read_early_config(config_alias_cb, &data); } +void quote_cmdline(struct strbuf *buf, const char **argv) +{ + for (const char **argp = argv; *argp; argp++) { + if (argp != argv) + strbuf_addch(buf, ' '); + strbuf_addch(buf, '"'); + for (const char *p = *argp; *p; p++) { + const char c = *p; + + if (c == '"' || c =='\\') + strbuf_addch(buf, '\\'); + strbuf_addch(buf, c); + } + strbuf_addch(buf, '"'); + } +} + #define SPLIT_CMDLINE_BAD_ENDING 1 #define SPLIT_CMDLINE_UNCLOSED_QUOTE 2 #define SPLIT_CMDLINE_ARGC_OVERFLOW 3 @@ -1,9 +1,12 @@ #ifndef ALIAS_H #define ALIAS_H +struct strbuf; struct string_list; char *alias_lookup(const char *alias); +/* Quote argv so buf can be parsed by split_cmdline() */ +void quote_cmdline(struct strbuf *buf, const char **argv); int split_cmdline(char *cmdline, const char ***argv); /* Takes a negative value returned by split_cmdline */ const char *split_cmdline_strerror(int cmdline_errno); diff --git a/builtin/rebase.c b/builtin/rebase.c index 680fe3c145..1ceac603c7 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -121,7 +121,8 @@ struct rebase_options { struct string_list exec; int allow_empty_message; int rebase_merges, rebase_cousins; - char *strategy, *strategy_opts; + char *strategy; + struct string_list strategy_opts; struct strbuf git_format_patch_opt; int reschedule_failed_exec; int reapply_cherry_picks; @@ -150,6 +151,7 @@ struct rebase_options { .config_rebase_merges = -1, \ .update_refs = -1, \ .config_update_refs = -1, \ + .strategy_opts = STRING_LIST_INIT_NODUP,\ } 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) replay.default_strategy = NULL; } - if (opts->strategy_opts) - parse_strategy_opts(&replay, opts->strategy_opts); + for (size_t i = 0; i < opts->strategy_opts.nr; i++) + strvec_push(&replay.xopts, opts->strategy_opts.items[i].string); if (opts->squash_onto) { oidcpy(&replay.squash_onto, opts->squash_onto); @@ -492,24 +494,6 @@ static int read_basic_state(struct rebase_options *opts) opts->gpg_sign_opt = xstrdup(buf.buf); } - if (file_exists(state_dir_path("strategy", opts))) { - strbuf_reset(&buf); - if (!read_oneliner(&buf, state_dir_path("strategy", opts), - READ_ONELINER_WARN_MISSING)) - return -1; - free(opts->strategy); - opts->strategy = xstrdup(buf.buf); - } - - if (file_exists(state_dir_path("strategy_opts", opts))) { - strbuf_reset(&buf); - if (!read_oneliner(&buf, state_dir_path("strategy_opts", opts), - READ_ONELINER_WARN_MISSING)) - return -1; - free(opts->strategy_opts); - opts->strategy_opts = xstrdup(buf.buf); - } - strbuf_release(&buf); return 0; @@ -527,12 +511,6 @@ static int rebase_write_basic_state(struct rebase_options *opts) write_file(state_dir_path("quiet", opts), "%s", ""); if (opts->flags & REBASE_VERBOSE) write_file(state_dir_path("verbose", opts), "%s", ""); - if (opts->strategy) - write_file(state_dir_path("strategy", opts), "%s", - opts->strategy); - if (opts->strategy_opts) - write_file(state_dir_path("strategy_opts", opts), "%s", - opts->strategy_opts); if (opts->allow_rerere_autoupdate > 0) write_file(state_dir_path("allow_rerere_autoupdate", opts), "-%s-rerere-autoupdate", @@ -1089,7 +1067,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) struct object_id branch_base; int ignore_whitespace = 0; const char *gpg_sign = NULL; - struct string_list strategy_options = STRING_LIST_INIT_NODUP; struct object_id squash_onto; char *squash_onto_name = NULL; char *keep_base_onto_name = NULL; @@ -1197,7 +1174,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) N_("use 'merge-base --fork-point' to refine upstream")), OPT_STRING('s', "strategy", &options.strategy, N_("strategy"), N_("use the given merge strategy")), - OPT_STRING_LIST('X', "strategy-option", &strategy_options, + OPT_STRING_LIST('X', "strategy-option", &options.strategy_opts, N_("option"), N_("pass the argument through to the merge " "strategy")), @@ -1500,23 +1477,13 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) } else { /* REBASE_MERGE */ if (ignore_whitespace) { - string_list_append(&strategy_options, + string_list_append(&options.strategy_opts, "ignore-space-change"); } } - if (strategy_options.nr) { - int i; - - if (!options.strategy) - options.strategy = "ort"; - - strbuf_reset(&buf); - for (i = 0; i < strategy_options.nr; i++) - strbuf_addf(&buf, " --%s", - strategy_options.items[i].string); - options.strategy_opts = xstrdup(buf.buf); - } + if (options.strategy_opts.nr && !options.strategy) + options.strategy = "ort"; if (options.strategy) { options.strategy = xstrdup(options.strategy); @@ -1898,10 +1865,9 @@ cleanup: free(options.gpg_sign_opt); string_list_clear(&options.exec, 0); free(options.strategy); - free(options.strategy_opts); + string_list_clear(&options.strategy_opts, 0); strbuf_release(&options.git_format_patch_opt); free(squash_onto_name); free(keep_base_onto_name); - string_list_clear(&strategy_options, 0); return !!ret; } diff --git a/builtin/revert.c b/builtin/revert.c index f72761bf88..0240ec8593 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -45,20 +45,6 @@ static const char * const *revert_or_cherry_pick_usage(struct replay_opts *opts) return opts->action == REPLAY_REVERT ? revert_usage : cherry_pick_usage; } -static int option_parse_x(const struct option *opt, - const char *arg, int unset) -{ - struct replay_opts **opts_ptr = opt->value; - struct replay_opts *opts = *opts_ptr; - - if (unset) - return 0; - - ALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc); - opts->xopts[opts->xopts_nr++] = xstrdup(arg); - return 0; -} - static int option_parse_m(const struct option *opt, const char *arg, int unset) { @@ -116,8 +102,8 @@ static int run_sequencer(int argc, const char **argv, const char *prefix, N_("select mainline parent"), option_parse_m), OPT_RERERE_AUTOUPDATE(&opts->allow_rerere_auto), OPT_STRING(0, "strategy", &opts->strategy, N_("strategy"), N_("merge strategy")), - OPT_CALLBACK('X', "strategy-option", &opts, N_("option"), - N_("option for merge strategy"), option_parse_x), + OPT_STRVEC('X', "strategy-option", &opts->xopts, N_("option"), + N_("option for merge strategy")), { OPTION_STRING, 'S', "gpg-sign", &opts->gpg_sign, N_("key-id"), N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, OPT_END() @@ -178,7 +164,7 @@ static int run_sequencer(int argc, const char **argv, const char *prefix, "--signoff", opts->signoff, "--mainline", opts->mainline, "--strategy", opts->strategy ? 1 : 0, - "--strategy-option", opts->xopts ? 1 : 0, + "--strategy-option", opts->xopts.nr ? 1 : 0, "-x", opts->record_origin, "--ff", opts->allow_ff, "--rerere-autoupdate", opts->allow_rerere_auto == RERERE_AUTOUPDATE, diff --git a/parse-options-cb.c b/parse-options-cb.c index 2488e6c030..10f2f75e83 100644 --- a/parse-options-cb.c +++ b/parse-options-cb.c @@ -210,6 +210,22 @@ int parse_opt_string_list(const struct option *opt, const char *arg, int unset) return 0; } +int parse_opt_strvec(const struct option *opt, const char *arg, int unset) +{ + struct strvec *v = opt->value; + + if (unset) { + strvec_clear(v); + return 0; + } + + if (!arg) + return -1; + + strvec_push(v, arg); + return 0; +} + int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset) { return 0; diff --git a/parse-options.h b/parse-options.h index 6f6462fdf9..8e48efe524 100644 --- a/parse-options.h +++ b/parse-options.h @@ -287,6 +287,15 @@ struct option { .help = (h), \ .callback = &parse_opt_string_list, \ } +#define OPT_STRVEC(s, l, v, a, h) { \ + .type = OPTION_CALLBACK, \ + .short_name = (s), \ + .long_name = (l), \ + .value = (v), \ + .argh = (a), \ + .help = (h), \ + .callback = &parse_opt_strvec, \ +} #define OPT_UYN(s, l, v, h) { \ .type = OPTION_CALLBACK, \ .short_name = (s), \ @@ -480,6 +489,7 @@ int parse_opt_commits(const struct option *, const char *, int); int parse_opt_commit(const struct option *, const char *, int); int parse_opt_tertiary(const struct option *, const char *, int); int parse_opt_string_list(const struct option *, const char *, int); +int parse_opt_strvec(const struct option *, const char *, int); int parse_opt_noop_cb(const struct option *, const char *, int); int parse_opt_passthru(const struct option *, const char *, int); int parse_opt_passthru_argv(const struct option *, const char *, int); diff --git a/sequencer.c b/sequencer.c index 6985ca526a..d2c7698c48 100644 --- a/sequencer.c +++ b/sequencer.c @@ -359,9 +359,7 @@ void replay_opts_release(struct replay_opts *opts) free(opts->reflog_action); free(opts->default_strategy); free(opts->strategy); - for (size_t i = 0; i < opts->xopts_nr; i++) - free(opts->xopts[i]); - free(opts->xopts); + strvec_clear (&opts->xopts); strbuf_release(&opts->current_fixups); if (opts->revs) release_revisions(opts->revs); @@ -699,8 +697,8 @@ static int do_recursive_merge(struct repository *r, next_tree = next ? repo_get_commit_tree(r, next) : empty_tree(r); base_tree = base ? repo_get_commit_tree(r, base) : empty_tree(r); - for (i = 0; i < opts->xopts_nr; i++) - parse_merge_opt(&o, opts->xopts[i]); + for (i = 0; i < opts->xopts.nr; i++) + parse_merge_opt(&o, opts->xopts.v[i]); if (!opts->strategy || !strcmp(opts->strategy, "ort")) { memset(&result, 0, sizeof(result)); @@ -2336,7 +2334,7 @@ static int do_pick_commit(struct repository *r, commit_list_insert(base, &common); commit_list_insert(next, &remotes); res |= try_merge_command(r, opts->strategy, - opts->xopts_nr, (const char **)opts->xopts, + opts->xopts.nr, opts->xopts.v, common, oid_to_hex(&head), remotes); free_commit_list(common); free_commit_list(remotes); @@ -2909,8 +2907,7 @@ static int populate_opts_cb(const char *key, const char *value, void *data) else if (!strcmp(key, "options.gpg-sign")) git_config_string_dup(&opts->gpg_sign, key, value); else if (!strcmp(key, "options.strategy-option")) { - ALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc); - opts->xopts[opts->xopts_nr++] = xstrdup(value); + strvec_push(&opts->xopts, value); } else if (!strcmp(key, "options.allow-rerere-auto")) opts->allow_rerere_auto = git_config_bool_or_int(key, value, &error_flag) ? @@ -2927,27 +2924,27 @@ static int populate_opts_cb(const char *key, const char *value, void *data) return 0; } -void parse_strategy_opts(struct replay_opts *opts, char *raw_opts) +static void parse_strategy_opts(struct replay_opts *opts, char *raw_opts) { int i; int count; + const char **argv; char *strategy_opts_string = raw_opts; if (*strategy_opts_string == ' ') strategy_opts_string++; - count = split_cmdline(strategy_opts_string, - (const char ***)&opts->xopts); + count = split_cmdline(strategy_opts_string, &argv); if (count < 0) - die(_("could not split '%s': %s"), strategy_opts_string, + BUG("could not split '%s': %s", strategy_opts_string, split_cmdline_strerror(count)); - opts->xopts_nr = count; - for (i = 0; i < opts->xopts_nr; i++) { - const char *arg = opts->xopts[i]; + for (i = 0; i < count; i++) { + const char *arg = argv[i]; skip_prefix(arg, "--", &arg); - opts->xopts[i] = xstrdup(arg); + strvec_push(&opts->xopts, arg); } + free(argv); } static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf) @@ -2955,7 +2952,6 @@ static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf) strbuf_reset(buf); if (!read_oneliner(buf, rebase_path_strategy(), 0)) return; - free(opts->strategy); opts->strategy = strbuf_detach(buf, NULL); if (!read_oneliner(buf, rebase_path_strategy_opts(), 0)) return; @@ -3064,12 +3060,13 @@ done_rebase_i: static void write_strategy_opts(struct replay_opts *opts) { - int i; struct strbuf buf = STRBUF_INIT; - for (i = 0; i < opts->xopts_nr; ++i) - strbuf_addf(&buf, " --%s", opts->xopts[i]); - + /* + * Quote strategy options so that they can be read correctly + * by split_cmdline(). + */ + quote_cmdline(&buf, opts->xopts.v); write_file(rebase_path_strategy_opts(), "%s\n", buf.buf); strbuf_release(&buf); } @@ -3092,7 +3089,7 @@ int write_basic_state(struct replay_opts *opts, const char *head_name, write_file(rebase_path_verbose(), "%s", ""); if (opts->strategy) write_file(rebase_path_strategy(), "%s\n", opts->strategy); - if (opts->xopts_nr > 0) + if (opts->xopts.nr > 0) write_strategy_opts(opts); if (opts->allow_rerere_auto == RERERE_AUTOUPDATE) @@ -3461,13 +3458,10 @@ static int save_opts(struct replay_opts *opts) if (opts->gpg_sign) res |= git_config_set_in_file_gently(opts_file, "options.gpg-sign", opts->gpg_sign); - if (opts->xopts) { - int i; - for (i = 0; i < opts->xopts_nr; i++) - res |= git_config_set_multivar_in_file_gently(opts_file, - "options.strategy-option", - opts->xopts[i], "^$", 0); - } + for (size_t i = 0; i < opts->xopts.nr; i++) + res |= git_config_set_multivar_in_file_gently(opts_file, + "options.strategy-option", + opts->xopts.v[i], "^$", 0); if (opts->allow_rerere_auto) res |= git_config_set_in_file_gently(opts_file, "options.allow-rerere-auto", @@ -3879,7 +3873,7 @@ static int do_merge(struct repository *r, struct commit *head_commit, *merge_commit, *i; struct commit_list *bases, *j; struct commit_list *to_merge = NULL, **tail = &to_merge; - const char *strategy = !opts->xopts_nr && + const char *strategy = !opts->xopts.nr && (!opts->strategy || !strcmp(opts->strategy, "recursive") || !strcmp(opts->strategy, "ort")) ? @@ -4063,9 +4057,9 @@ static int do_merge(struct repository *r, strvec_push(&cmd.args, "octopus"); else { strvec_push(&cmd.args, strategy); - for (k = 0; k < opts->xopts_nr; k++) + for (k = 0; k < opts->xopts.nr; k++) strvec_pushf(&cmd.args, - "-X%s", opts->xopts[k]); + "-X%s", opts->xopts.v[k]); } if (!(flags & TODO_EDIT_MERGE_MSG)) strvec_push(&cmd.args, "--no-edit"); diff --git a/sequencer.h b/sequencer.h index 33dbaf5b66..913a0f652d 100644 --- a/sequencer.h +++ b/sequencer.h @@ -2,6 +2,7 @@ #define SEQUENCER_H #include "strbuf.h" +#include "strvec.h" #include "wt-status.h" struct commit; @@ -60,8 +61,7 @@ struct replay_opts { /* Merge strategy */ char *default_strategy; /* from config options */ char *strategy; - char **xopts; - size_t xopts_nr, xopts_alloc; + struct strvec xopts; /* Reflog */ char *reflog_action; @@ -80,7 +80,12 @@ struct replay_opts { /* Private use */ const char *reflog_message; }; -#define REPLAY_OPTS_INIT { .edit = -1, .action = -1, .current_fixups = STRBUF_INIT } +#define REPLAY_OPTS_INIT { \ + .edit = -1, \ + .action = -1, \ + .current_fixups = STRBUF_INIT, \ + .xopts = STRVEC_INIT, \ +} /* * Note that ordering matters in this enum. Not only must it match the mapping @@ -247,7 +252,6 @@ int read_oneliner(struct strbuf *buf, const char *path, unsigned flags); int read_author_script(const char *path, char **name, char **email, char **date, int allow_missing); -void parse_strategy_opts(struct replay_opts *opts, char *raw_opts); int write_basic_state(struct replay_opts *opts, const char *head_name, struct commit *onto, const struct object_id *orig_head); void sequencer_post_commit_cleanup(struct repository *r, int verbose); diff --git a/t/t3402-rebase-merge.sh b/t/t3402-rebase-merge.sh index 7e46f4ca85..79b0640c00 100755 --- a/t/t3402-rebase-merge.sh +++ b/t/t3402-rebase-merge.sh @@ -131,27 +131,6 @@ test_expect_success 'picking rebase' ' esac ' -test_expect_success 'rebase -s funny -Xopt' ' - test_when_finished "rm -fr test-bin funny.was.run" && - mkdir test-bin && - cat >test-bin/git-merge-funny <<-EOF && - #!$SHELL_PATH - case "\$1" in --opt) ;; *) exit 2 ;; esac - shift && - >funny.was.run && - exec git merge-recursive "\$@" - EOF - chmod +x test-bin/git-merge-funny && - git reset --hard && - git checkout -b test-funny main^ && - test_commit funny && - ( - PATH=./test-bin:$PATH && - git rebase -s funny -Xopt main - ) && - test -f funny.was.run -' - test_expect_success 'rebase --skip works with two conflicts in a row' ' git checkout second-side && tr "[A-Z]" "[a-z]" <newfile >tmp && diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh index 130e2f9b55..2d0789e554 100755 --- a/t/t3418-rebase-continue.sh +++ b/t/t3418-rebase-continue.sh @@ -62,61 +62,39 @@ test_expect_success 'rebase --continue remembers merge strategy and options' ' rm -fr .git/rebase-* && git reset --hard commit-new-file-F2-on-topic-branch && test_commit "commit-new-file-F3-on-topic-branch" F3 32 && - test_when_finished "rm -fr test-bin funny.was.run" && + test_when_finished "rm -fr test-bin" && mkdir test-bin && - cat >test-bin/git-merge-funny <<-EOF && - #!$SHELL_PATH - case "\$1" in --opt) ;; *) exit 2 ;; esac - shift && - >funny.was.run && - exec git merge-recursive "\$@" + + write_script test-bin/git-merge-funny <<-\EOF && + printf "[%s]\n" $# "$1" "$2" "$3" "$5" >actual + shift 3 && + exec git merge-recursive "$@" EOF - chmod +x test-bin/git-merge-funny && - ( - PATH=./test-bin:$PATH && - test_must_fail git rebase -s funny -Xopt main topic - ) && - test -f funny.was.run && - rm funny.was.run && - echo "Resolved" >F2 && - git add F2 && - ( - PATH=./test-bin:$PATH && - git rebase --continue - ) && - test -f funny.was.run -' -test_expect_success 'rebase -i --continue handles merge strategy and options' ' - rm -fr .git/rebase-* && - git reset --hard commit-new-file-F2-on-topic-branch && - test_commit "commit-new-file-F3-on-topic-branch-for-dash-i" F3 32 && - test_when_finished "rm -fr test-bin funny.was.run funny.args" && - mkdir test-bin && - cat >test-bin/git-merge-funny <<-EOF && - #!$SHELL_PATH - echo "\$@" >>funny.args - case "\$1" in --opt) ;; *) exit 2 ;; esac - case "\$2" in --foo) ;; *) exit 2 ;; esac - case "\$4" in --) ;; *) exit 2 ;; esac - shift 2 && - >funny.was.run && - exec git merge-recursive "\$@" + cat >expect <<-\EOF && + [7] + [--option=arg with space] + [--op"tion\] + [--new + line ] + [--] EOF - chmod +x test-bin/git-merge-funny && + + rm -f actual && ( PATH=./test-bin:$PATH && - test_must_fail git rebase -i -s funny -Xopt -Xfoo main topic + test_must_fail git rebase -s funny -X"option=arg with space" \ + -Xop\"tion\\ -X"new${LF}line " main topic ) && - test -f funny.was.run && - rm funny.was.run && + test_cmp expect actual && + rm actual && echo "Resolved" >F2 && git add F2 && ( PATH=./test-bin:$PATH && git rebase --continue ) && - test -f funny.was.run + test_cmp expect actual ' test_expect_success 'rebase -r passes merge strategy options correctly' ' diff --git a/t/t3436-rebase-more-options.sh b/t/t3436-rebase-more-options.sh index c3184c9ade..94671d3c46 100755 --- a/t/t3436-rebase-more-options.sh +++ b/t/t3436-rebase-more-options.sh @@ -40,24 +40,6 @@ test_expect_success 'setup' ' EOF ' -test_expect_success 'bad -X <strategy-option> arguments: unclosed quote' ' - cat >expect <<-\EOF && - fatal: could not split '\''--bad'\'': unclosed quote - EOF - test_expect_code 128 git rebase -X"bad argument\"" side main >out 2>actual && - test_must_be_empty out && - test_cmp expect actual -' - -test_expect_success 'bad -X <strategy-option> arguments: bad escape' ' - cat >expect <<-\EOF && - fatal: could not split '\''--bad'\'': cmdline ends with \ - EOF - test_expect_code 128 git rebase -X"bad escape \\" side main >out 2>actual && - test_must_be_empty out && - test_cmp expect actual -' - test_expect_success '--ignore-whitespace works with apply backend' ' test_must_fail git rebase --apply main side && git rebase --abort && |