rebase -m: fix serialization of strategy options

To store the strategy options rebase prepends " --" to each one and
writes them to a file. To load them it reads the file and passes the
contents to split_cmdline(). This roughly mimics the behavior of the
scripted rebase but has a couple of limitations, (1) options containing
whitespace are not properly preserved (this is true of the scripted
rebase as well) and (2) options containing '"' or '\' are incorrectly
parsed and may cause the parser to return an error.

Fix these limitations by quoting each option when they are stored so
that they can be parsed correctly. Now that "--preserve-merges" no
longer exist this change also stops prepending "--" to the options when
they are stored as that was an artifact of the scripted rebase.

These changes are backwards compatible so the files written by an older
version of git can still be read. They are also forwards compatible,
the file can still be parsed by recent versions of git as they treat the
"--" prefix as optional.

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>
This commit is contained in:
Phillip Wood 2023-04-10 10:08:30 +01:00 committed by Junio C Hamano
parent 4a8bc9860a
commit 4960e5c7bd
5 changed files with 49 additions and 41 deletions

18
alias.c
View File

@ -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

View File

@ -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);

View File

@ -2925,7 +2925,7 @@ static void parse_strategy_opts(struct replay_opts *opts, char *raw_opts)
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));
for (i = 0; i < count; i++) {
const char *arg = argv[i];
@ -3049,12 +3049,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.v[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);
}

View File

@ -62,29 +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 &&
cat >expect <<-\EOF &&
[7]
[--option=arg with space]
[--op"tion\]
[--new
line ]
[--]
EOF
rm -f actual &&
(
PATH=./test-bin:$PATH &&
test_must_fail git rebase -s funny -Xopt 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 -i --continue handles merge strategy and options' '

View File

@ -40,30 +40,6 @@ test_expect_success 'setup' '
EOF
'
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
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
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
'
test_expect_success '--ignore-whitespace works with apply backend' '
test_must_fail git rebase --apply main side &&
git rebase --abort &&