| Message ID | 20230222051709.464275-2-alexhenrie24@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/3] rebase: add documentation and tests for --no-rebase-merges | expand |
Alex Henrie <alexhenrie24@gmail.com> writes: > diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh > index e0d910c229..b8ba323dbc 100755 > --- a/t/t3430-rebase-merges.sh > +++ b/t/t3430-rebase-merges.sh > @@ -293,6 +293,12 @@ test_expect_success 'do not rebase cousins unless asked for' ' > EOF > ' > > +test_expect_success '--rebase-merges="" is invalid syntax' ' > + echo "fatal: Unknown mode: " >expect && > + ! git rebase --rebase-merges="" HEAD^ 2>actual && "!" takes any non-zero exit, even a segfaulting "git". Let's use test_must_fail to make sure it dies in a controlled way, i.e. test_must_fail git rebase --rebase-merges="" HEAD^ 2>actual && > + test_cmp expect actual > +' > + > test_expect_success 'refs/rewritten/* is worktree-local' ' > git worktree add wt && > cat >wt/script-from-scratch <<-\EOF && Other than that, looking good. Thanks.
On Wed, Feb 22, 2023 at 4:56 PM Junio C Hamano <gitster@pobox.com> wrote: > > Alex Henrie <alexhenrie24@gmail.com> writes: > > > +test_expect_success '--rebase-merges="" is invalid syntax' ' > > + echo "fatal: Unknown mode: " >expect && > > + ! git rebase --rebase-merges="" HEAD^ 2>actual && > > "!" takes any non-zero exit, even a segfaulting "git". Let's use > test_must_fail to make sure it dies in a controlled way, i.e. > > test_must_fail git rebase --rebase-merges="" HEAD^ 2>actual && Done in v4. Thanks for the feedback. -Alex
diff --git a/builtin/rebase.c b/builtin/rebase.c index 6635f10d52..b68fc2fbb7 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1140,7 +1140,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) {OPTION_STRING, 'r', "rebase-merges", &rebase_merges, N_("mode"), N_("try to rebase merges instead of skipping them"), - PARSE_OPT_OPTARG, NULL, (intptr_t)""}, + PARSE_OPT_OPTARG, NULL, (intptr_t)"no-rebase-cousins"}, OPT_BOOL(0, "fork-point", &options.fork_point, N_("use 'merge-base --fork-point' to refine upstream")), OPT_STRING('s', "strategy", &options.strategy, @@ -1437,9 +1437,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) imply_merge(&options, "--exec"); if (rebase_merges) { - if (!*rebase_merges) - ; /* default mode; do nothing */ - else if (!strcmp("rebase-cousins", rebase_merges)) + if (!strcmp("rebase-cousins", rebase_merges)) options.rebase_cousins = 1; else if (strcmp("no-rebase-cousins", rebase_merges)) die(_("Unknown mode: %s"), rebase_merges); diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh index e0d910c229..b8ba323dbc 100755 --- a/t/t3430-rebase-merges.sh +++ b/t/t3430-rebase-merges.sh @@ -293,6 +293,12 @@ test_expect_success 'do not rebase cousins unless asked for' ' EOF ' +test_expect_success '--rebase-merges="" is invalid syntax' ' + echo "fatal: Unknown mode: " >expect && + ! git rebase --rebase-merges="" HEAD^ 2>actual && + test_cmp expect actual +' + test_expect_success 'refs/rewritten/* is worktree-local' ' git worktree add wt && cat >wt/script-from-scratch <<-\EOF &&
The unusual syntax --rebase-merges="" (that is, --rebase-merges with an empty string argument) has been an undocumented synonym of --rebase-merges=no-rebase-cousins. Stop accepting that syntax to avoid confusion when a rebase.merges config option is introduced, where rebase.merges="" will be equivalent to not passing --rebase-merges. Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> --- builtin/rebase.c | 6 ++---- t/t3430-rebase-merges.sh | 6 ++++++ 2 files changed, 8 insertions(+), 4 deletions(-)