From: Phillip Wood <phillip.wood123@gmail.com>
To: git@vger.kernel.org
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
"Phillip Wood" <phillip.wood123@gmail.com>,
"Phillip Wood" <phillip.wood@dunelm.org.uk>
Subject: [PATCH 2/4] rebase -m: cleanup --strategy-option handling
Date: Wed, 15 Mar 2023 15:14:57 +0000 [thread overview]
Message-ID: <a01b182644f4ad3b4cf30cf1895dc9ea1e9fa197.1678893298.git.phillip.wood@dunelm.org.uk> (raw)
In-Reply-To: <cover.1678893298.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
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.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
builtin/rebase.c | 36 +++++++++++++++-------------------
sequencer.c | 2 +-
sequencer.h | 1 -
t/t3436-rebase-more-options.sh | 10 ++++++++--
4 files changed, 25 insertions(+), 24 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 516ad1b12a..5194d64c24 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -116,7 +116,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;
@@ -142,6 +143,7 @@ struct rebase_options {
.config_autosquash = -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)
@@ -175,8 +177,14 @@ 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);
+ if (opts->strategy_opts.nr) {
+ ALLOC_ARRAY(replay.xopts, opts->strategy_opts.nr);
+ replay.xopts_nr = opts->strategy_opts.nr;
+ replay.xopts_alloc = opts->strategy_opts.nr;
+ for (size_t i = 0; i < opts->strategy_opts.nr; i++)
+ replay.xopts[i] =
+ xstrdup(opts->strategy_opts.items[i].string);
+ }
if (opts->squash_onto) {
oidcpy(&replay.squash_onto, opts->squash_onto);
@@ -1012,7 +1020,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
int ignore_whitespace = 0;
const char *gpg_sign = NULL;
const char *rebase_merges = 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;
@@ -1121,7 +1128,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")),
@@ -1435,23 +1442,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);
@@ -1826,10 +1823,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
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/sequencer.c b/sequencer.c
index 55b3ba3a51..83ea1016ae 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2918,7 +2918,7 @@ 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;
diff --git a/sequencer.h b/sequencer.h
index 3bcdfa1b58..5de5cfa664 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -247,7 +247,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/t3436-rebase-more-options.sh b/t/t3436-rebase-more-options.sh
index c3184c9ade..3adf42f47d 100755
--- a/t/t3436-rebase-more-options.sh
+++ b/t/t3436-rebase-more-options.sh
@@ -41,19 +41,25 @@ test_expect_success 'setup' '
'
test_expect_success 'bad -X <strategy-option> arguments: unclosed quote' '
+ test_when_finished "test_might_fail git rebase --abort" &&
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 &&
+ GIT_SEQUENCE_EDITOR="echo break >" \
+ git rebase -i -X"bad argument\"" side main &&
+ test_expect_code 128 git rebase --continue >out 2>actual &&
test_must_be_empty out &&
test_cmp expect actual
'
test_expect_success 'bad -X <strategy-option> arguments: bad escape' '
+ test_when_finished "test_might_fail git rebase --abort" &&
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 &&
+ GIT_SEQUENCE_EDITOR="echo break >" \
+ git rebase -i -X"bad escape \\" side main &&
+ test_expect_code 128 git rebase --continue >out 2>actual &&
test_must_be_empty out &&
test_cmp expect actual
'
--
2.39.2
next prev parent reply other threads:[~2023-03-15 15:15 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-15 15:14 [PATCH 0/4] rebase: cleanup merge strategy option handling Phillip Wood
2023-03-15 15:14 ` [PATCH 1/4] rebase: stop reading and writing unnecessary strategy state Phillip Wood
2023-03-15 15:14 ` Phillip Wood [this message]
2023-03-15 15:14 ` [PATCH 3/4] rebase -m: fix serialization of strategy options Phillip Wood
2023-04-01 19:27 ` Elijah Newren
2023-04-04 13:02 ` Phillip Wood
2023-03-15 15:14 ` [PATCH 4/4] rebase: remove a couple of redundant strategy tests Phillip Wood
2023-04-01 19:31 ` Elijah Newren
2023-04-04 13:04 ` Phillip Wood
2023-03-15 16:03 ` [PATCH 0/4] rebase: cleanup merge strategy option handling Junio C Hamano
2023-03-15 16:50 ` Felipe Contreras
2023-04-01 19:32 ` Elijah Newren
2023-04-05 15:21 ` [PATCH v2 0/5] " Phillip Wood
2023-04-05 15:21 ` [PATCH v2 1/5] rebase: stop reading and writing unnecessary strategy state Phillip Wood
2023-04-05 15:21 ` [PATCH v2 2/5] sequencer: use struct strvec to store merge strategy options Phillip Wood
2023-04-07 5:44 ` Elijah Newren
2023-04-05 15:21 ` [PATCH v2 3/5] rebase -m: cleanup --strategy-option handling Phillip Wood
2023-04-05 15:21 ` [PATCH v2 4/5] rebase -m: fix serialization of strategy options Phillip Wood
2023-04-07 5:47 ` Elijah Newren
2023-04-05 15:21 ` [PATCH v2 5/5] rebase: remove a couple of redundant strategy tests Phillip Wood
2023-04-07 5:49 ` [PATCH v2 0/5] rebase: cleanup merge strategy option handling Elijah Newren
2023-04-07 13:57 ` Phillip Wood
2023-04-07 17:53 ` Junio C Hamano
2023-04-07 13:55 ` [PATCH v3 " Phillip Wood
2023-04-07 13:55 ` [PATCH v3 1/5] rebase: stop reading and writing unnecessary strategy state Phillip Wood
2023-04-07 13:55 ` [PATCH v3 2/5] sequencer: use struct strvec to store merge strategy options Phillip Wood
2023-04-07 13:55 ` [PATCH v3 3/5] rebase -m: cleanup --strategy-option handling Phillip Wood
2023-04-07 13:55 ` [PATCH v3 4/5] rebase -m: fix serialization of strategy options Phillip Wood
2023-04-07 13:55 ` [PATCH v3 5/5] rebase: remove a couple of redundant strategy tests Phillip Wood
2023-04-10 9:08 ` [PATCH v4 0/5] rebase: cleanup merge strategy option handling Phillip Wood
2023-04-10 9:08 ` [PATCH v4 1/5] rebase: stop reading and writing unnecessary strategy state Phillip Wood
2023-04-10 9:08 ` [PATCH v4 2/5] sequencer: use struct strvec to store merge strategy options Phillip Wood
2023-04-10 9:08 ` [PATCH v4 3/5] rebase -m: cleanup --strategy-option handling Phillip Wood
2023-04-10 9:08 ` [PATCH v4 4/5] rebase -m: fix serialization of strategy options Phillip Wood
2023-04-10 9:08 ` [PATCH v4 5/5] rebase: remove a couple of redundant strategy tests Phillip Wood
2023-04-10 16:46 ` [PATCH v4 0/5] rebase: cleanup merge strategy option handling Elijah Newren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a01b182644f4ad3b4cf30cf1895dc9ea1e9fa197.1678893298.git.phillip.wood@dunelm.org.uk \
--to=phillip.wood123@gmail.com \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=phillip.wood@dunelm.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).