diff mbox series

[v8,3/3] rebase: add a config option for --rebase-merges

Message ID 20230320055955.461138-4-alexhenrie24@gmail.com (mailing list archive)
State Superseded
Headers show
Series rebase: document, clean up, and introduce a config option for --rebase-merges | expand

Commit Message

Alex Henrie March 20, 2023, 5:59 a.m. UTC
The purpose of the new option is to accommodate users who would like
--rebase-merges to be on by default and to facilitate turning on
--rebase-merges by default without configuration in a future version of
Git.

Name the new option rebase.rebaseMerges, even though it is a little
redundant, for consistency with the name of the command line option and
to be clear when scrolling through values in the [rebase] section of
.gitconfig.

Support setting rebase.rebaseMerges to the nonspecific value "true" for
users who don't need to or don't want to learn about the difference
between rebase-cousins and no-rebase-cousins.

Make --rebase-merges without an argument on the command line override
any value of rebase.rebaseMerges in the configuration, for consistency
with other command line flags with optional arguments that have an
associated config option.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 Documentation/config/rebase.txt        | 10 ++++
 Documentation/git-rebase.txt           |  3 +-
 builtin/rebase.c                       | 81 ++++++++++++++++++--------
 t/t3422-rebase-incompatible-options.sh | 17 ++++++
 t/t3430-rebase-merges.sh               | 34 +++++++++++
 5 files changed, 121 insertions(+), 24 deletions(-)

Comments

Phillip Wood March 22, 2023, 4:54 p.m. UTC | #1
Hi Alex

On 20/03/2023 05:59, Alex Henrie wrote:
> The purpose of the new option is to accommodate users who would like
> --rebase-merges to be on by default and to facilitate turning on
> --rebase-merges by default without configuration in a future version of
> Git.
> 
> Name the new option rebase.rebaseMerges, even though it is a little
> redundant, for consistency with the name of the command line option and
> to be clear when scrolling through values in the [rebase] section of
> .gitconfig.
> 
> Support setting rebase.rebaseMerges to the nonspecific value "true" for
> users who don't need to or don't want to learn about the difference
> between rebase-cousins and no-rebase-cousins.
> 
> Make --rebase-merges without an argument on the command line override
> any value of rebase.rebaseMerges in the configuration, for consistency
> with other command line flags with optional arguments that have an
> associated config option.

Hurray! Thanks for re-rolling. I've left a couple of comments below, I 
had hoped this would be the final version but I've realized that the way 
you've rearranged the error handling for apply and merge options is not 
a good idea (see below). Apart from that I'm happy.

> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---
>   Documentation/config/rebase.txt        | 10 ++++
>   Documentation/git-rebase.txt           |  3 +-
>   builtin/rebase.c                       | 81 ++++++++++++++++++--------
>   t/t3422-rebase-incompatible-options.sh | 17 ++++++
>   t/t3430-rebase-merges.sh               | 34 +++++++++++
>   5 files changed, 121 insertions(+), 24 deletions(-)
> 
> diff --git a/Documentation/config/rebase.txt b/Documentation/config/rebase.txt
> index f19bd0e040..afaf6dad99 100644
> --- a/Documentation/config/rebase.txt
> +++ b/Documentation/config/rebase.txt
> @@ -67,3 +67,13 @@ rebase.rescheduleFailedExec::
>   
>   rebase.forkPoint::
>   	If set to false set `--no-fork-point` option by default.
> +
> +rebase.rebaseMerges::
> +	Whether and how to set the `--rebase-merges` option by default. Can
> +	be `rebase-cousins`, `no-rebase-cousins`, or a boolean. Setting to
> +	true or to `no-rebase-cousins` is equivalent to
> +	`--rebase-merges=no-rebase-cousins`, setting to `rebase-cousins` is
> +	equivalent to `--rebase-merges=rebase-cousins`, and setting to false is
> +	equivalent to `--no-rebase-merges`. Passing `--rebase-merges` on the
> +	command line, with or without an argument, overrides any
> +	`rebase.rebaseMerges` configuration.

Sounds good

> [...]   
> +	if (!strcmp(var, "rebase.rebasemerges")) {
> +		opts->config_rebase_merges = git_parse_maybe_bool(value);
> +		if (opts->config_rebase_merges < 0) {
> +			opts->config_rebase_merges = 1;
> +			parse_rebase_merges_value(opts, value);
> +		} else
> +			opts->rebase_cousins = 0;

The "else" clause should have braces because the "if" cause requires 
them (see Documentation/CodingGuidelines). I don't think it is worth 
re-rolling just for this though.

> [...]   
> +static int parse_opt_rebase_merges(const struct option *opt, const char *arg, int unset)
> +{
> +	struct rebase_options *options = opt->value;
> +
> +	options->rebase_merges = !unset;
> +	options->rebase_cousins = 0;

This makes --rebase-merges without an option override 
rebase.rebaseMerges - good.

 > [...]
> @@ -1514,13 +1542,15 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>   				break;
>   
>   		if (i >= 0 || options.type == REBASE_APPLY) {
> -			if (is_merge(&options))
> -				die(_("apply options and merge options "
> -					  "cannot be used together"));

This isn't a new change but having thought about it I'm not sure moving 
this code is a good idea. If the user runs

	git -c rebase.updateRefs=true rebase --whitespace=fix --exec "make test"

then instead of getting an message saying that they cannot use apply and 
merge options together they'll get a message suggesting they pass 
--no-update-refs which wont fix the problem for them.

> -			else if (options.autosquash == -1 && options.config_autosquash == 1)
> +			if (options.autosquash == -1 && options.config_autosquash == 1)
>   				die(_("apply options are incompatible with rebase.autoSquash.  Consider adding --no-autosquash"));
> +			else if (options.rebase_merges == -1 && options.config_rebase_merges == 1)
> +				die(_("apply options are incompatible with rebase.rebaseMerges.  Consider adding --no-rebase-merges"));
>   			else if (options.update_refs == -1 && options.config_update_refs == 1)
>   				die(_("apply options are incompatible with rebase.updateRefs.  Consider adding --no-update-refs"));
> +			else if (is_merge(&options))
> +				die(_("apply options and merge options "
> +					  "cannot be used together"));
>   			else
>   				options.type = REBASE_APPLY;
>   		}

Best Wishes

Phillip
Junio C Hamano March 23, 2023, 6:45 p.m. UTC | #2
Phillip Wood <phillip.wood123@gmail.com> writes:

>> @@ -1514,13 +1542,15 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>>   				break;
>>     		if (i >= 0 || options.type == REBASE_APPLY) {
>> -			if (is_merge(&options))
>> -				die(_("apply options and merge options "
>> -					  "cannot be used together"));
>
> This isn't a new change but having thought about it I'm not sure
> moving this code is a good idea. If the user runs
>
> 	git -c rebase.updateRefs=true rebase --whitespace=fix --exec "make test"
>
> then instead of getting an message saying that they cannot use apply
> and merge options together they'll get a message suggesting they pass
> --no-update-refs which wont fix the problem for them.

Hmph.  Instead of dying here, ...

>> +			if (options.autosquash == -1 && options.config_autosquash == 1)
>>   				die(_("apply options are incompatible with rebase.autoSquash.  Consider adding --no-autosquash"));
>> +			else if (options.rebase_merges == -1 && options.config_rebase_merges == 1)
>> +				die(_("apply options are incompatible with rebase.rebaseMerges.  Consider adding --no-rebase-merges"));
>>   			else if (options.update_refs == -1 && options.config_update_refs == 1)
>>   				die(_("apply options are incompatible with rebase.updateRefs.  Consider adding --no-update-refs"));

... we get caught here, and the next one is not triggered.

>> +			else if (is_merge(&options))
>> +				die(_("apply options and merge options "
>> +					  "cannot be used together"));
>>   			else
>>   				options.type = REBASE_APPLY;

What's the reason why "cannot be used together" is moved to the last
in the chain?

The first two new conditions in this chain try to catch an
invocation with some apply-specific command line option
(e.g. "--whitespace=fix") when used with configuration variables
specific to the merge-backend (e.g. "rebase.merges") and suggest
overriding the configuration from the command line, and I suspect
that the motivation behind this change is that their error messages
are more specific than the generic "apply and merge do not mix".

But are these two new errors and hints universally a good suggestion
to make?  I actually think a command line option forcing us to use
the apply backend should simply silently ignore (aka "override")
configuration variables that take effect only when we are using the
merge-backend.  "git rebase --whitespace=fix --rebase-merges",
giving both from the command line, is making conflicting and
unsatisfyable request, and it is worth giving an error message.  

But if you configure rebase.autosquash only because you want to it
to be in effect for your invocations of "git rebase -i", you
shouldn't have to override it from the command line when you say
"git rebase" or "git rebase --whitespace=fix", should you?

Thanks.
Phillip Wood March 24, 2023, 2:52 p.m. UTC | #3
Hi Junio

On 23/03/2023 18:45, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>>> @@ -1514,13 +1542,15 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>>>    				break;
>>>      		if (i >= 0 || options.type == REBASE_APPLY) {
>>> -			if (is_merge(&options))
>>> -				die(_("apply options and merge options "
>>> -					  "cannot be used together"));
>>
>> This isn't a new change but having thought about it I'm not sure
>> moving this code is a good idea. If the user runs
>>
>> 	git -c rebase.updateRefs=true rebase --whitespace=fix --exec "make test"
>>
>> then instead of getting an message saying that they cannot use apply
>> and merge options together they'll get a message suggesting they pass
>> --no-update-refs which wont fix the problem for them.
> 
> Hmph.  Instead of dying here, ...
> 
>>> +			if (options.autosquash == -1 && options.config_autosquash == 1)
>>>    				die(_("apply options are incompatible with rebase.autoSquash.  Consider adding --no-autosquash"));
>>> +			else if (options.rebase_merges == -1 && options.config_rebase_merges == 1)
>>> +				die(_("apply options are incompatible with rebase.rebaseMerges.  Consider adding --no-rebase-merges"));
>>>    			else if (options.update_refs == -1 && options.config_update_refs == 1)
>>>    				die(_("apply options are incompatible with rebase.updateRefs.  Consider adding --no-update-refs"));
> 
> ... we get caught here, and the next one is not triggered.
> 
>>> +			else if (is_merge(&options))
>>> +				die(_("apply options and merge options "
>>> +					  "cannot be used together"));
>>>    			else
>>>    				options.type = REBASE_APPLY;
> 
> What's the reason why "cannot be used together" is moved to the last
> in the chain?
> 
> The first two new conditions in this chain try to catch an
> invocation with some apply-specific command line option
> (e.g. "--whitespace=fix") when used with configuration variables
> specific to the merge-backend (e.g. "rebase.merges") and suggest
> overriding the configuration from the command line, and I suspect
> that the motivation behind this change is that their error messages
> are more specific than the generic "apply and merge do not mix".
> 
> But are these two new errors and hints universally a good suggestion
> to make?  I actually think a command line option forcing us to use
> the apply backend should simply silently ignore (aka "override")
> configuration variables that take effect only when we are using the
> merge-backend.  "git rebase --whitespace=fix --rebase-merges",
> giving both from the command line, is making conflicting and
> unsatisfyable request, and it is worth giving an error message.
> 
> But if you configure rebase.autosquash only because you want to it
> to be in effect for your invocations of "git rebase -i", you
> shouldn't have to override it from the command line when you say
> "git rebase" or "git rebase --whitespace=fix", should you?

I think there are two conflicting viewpoints here which depend on what 
one thinks the user wants when they set these configuration variables. 
As I understand it Elijah's thinking was that if the user set 
rebase.autosquash they'd be surprised when their fixup commits were not 
squashed by

	git rebase --whitespace=fix

and that's why his patch series changed things to error out.

The other point of view is that the user expects that these 
configuration variables to apply only when they are using the "merge" 
backend and so we should not error out.

Personally I lean a little more towards the latter but I don't feel that 
strongly about it and can see an argument for having either behavior. I 
do think we should leave the ordering alone though so the user gets a 
useful error message in the case of "git rebase --exec 'make test' 
--whitespace=fix"

Best Wishes

Phillip

> Thanks.
Alex Henrie March 25, 2023, 5:21 a.m. UTC | #4
On Wed, Mar 22, 2023 at 10:54 AM Phillip Wood <phillip.wood123@gmail.com> wrote:

> Hurray! Thanks for re-rolling.

Thanks for making sure that we got the UI right!

> On 20/03/2023 05:59, Alex Henrie wrote:

> > +     if (!strcmp(var, "rebase.rebasemerges")) {
> > +             opts->config_rebase_merges = git_parse_maybe_bool(value);
> > +             if (opts->config_rebase_merges < 0) {
> > +                     opts->config_rebase_merges = 1;
> > +                     parse_rebase_merges_value(opts, value);
> > +             } else
> > +                     opts->rebase_cousins = 0;
>
> The "else" clause should have braces because the "if" cause requires
> them (see Documentation/CodingGuidelines). I don't think it is worth
> re-rolling just for this though.

Thanks for pointing out that documentation. I'm going to make a v9
anyway, and I'll add the braces then. By the way, actions speak louder
than words: While writing the patch, I found several examples of the
braces being omitted in cases like this in other places in rebase.c,
so I assumed that that was the preferred style here. If you want to
encourage people to follow the CodingGuidelines document, the best way
would be to clean up the existing code to conform to it.

-Alex
Alex Henrie March 25, 2023, 5:23 a.m. UTC | #5
On Thu, Mar 23, 2023 at 12:45 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Phillip Wood <phillip.wood123@gmail.com> writes:
>
> >> @@ -1514,13 +1542,15 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
> >>                              break;
> >>              if (i >= 0 || options.type == REBASE_APPLY) {
> >> -                    if (is_merge(&options))
> >> -                            die(_("apply options and merge options "
> >> -                                      "cannot be used together"));
> >
> > This isn't a new change but having thought about it I'm not sure
> > moving this code is a good idea. If the user runs
> >
> >       git -c rebase.updateRefs=true rebase --whitespace=fix --exec "make test"
> >
> > then instead of getting an message saying that they cannot use apply
> > and merge options together they'll get a message suggesting they pass
> > --no-update-refs which wont fix the problem for them.
>
> Hmph.  Instead of dying here, ...
>
> >> +                    if (options.autosquash == -1 && options.config_autosquash == 1)
> >>                              die(_("apply options are incompatible with rebase.autoSquash.  Consider adding --no-autosquash"));
> >> +                    else if (options.rebase_merges == -1 && options.config_rebase_merges == 1)
> >> +                            die(_("apply options are incompatible with rebase.rebaseMerges.  Consider adding --no-rebase-merges"));
> >>                      else if (options.update_refs == -1 && options.config_update_refs == 1)
> >>                              die(_("apply options are incompatible with rebase.updateRefs.  Consider adding --no-update-refs"));
>
> ... we get caught here, and the next one is not triggered.
>
> >> +                    else if (is_merge(&options))
> >> +                            die(_("apply options and merge options "
> >> +                                      "cannot be used together"));
> >>                      else
> >>                              options.type = REBASE_APPLY;
>
> What's the reason why "cannot be used together" is moved to the last
> in the chain?
>
> The first two new conditions in this chain try to catch an
> invocation with some apply-specific command line option
> (e.g. "--whitespace=fix") when used with configuration variables
> specific to the merge-backend (e.g. "rebase.merges") and suggest
> overriding the configuration from the command line, and I suspect
> that the motivation behind this change is that their error messages
> are more specific than the generic "apply and merge do not mix".

Phillip specifically asked for `git -c rebase.rebaseMerges=true rebase
--whitespace=fix` to print "error: apply options are incompatible with
rebase.rebaseMerges. Consider adding --no-rebase-merges".[1] I could
have sworn that "if (is_merge(&options))" had to be after "if
(options.rebase_merges == -1 && options.config_rebase_merges == 1)" to
make that happen, but now it works without that change. I must have
been debugging with some intermediate version that still had
'imply_merge(&options, "--rebase-merges")' before this if chain. I'll
send a v9 that puts "if (is_merge(&options))" back at the top.

Thanks for your attention to detail!

-Alex

[1] https://lore.kernel.org/git/7ba8a92d-8c94-5226-5416-6ed3a8e2e389@dunelm.org.uk/
diff mbox series

Patch

diff --git a/Documentation/config/rebase.txt b/Documentation/config/rebase.txt
index f19bd0e040..afaf6dad99 100644
--- a/Documentation/config/rebase.txt
+++ b/Documentation/config/rebase.txt
@@ -67,3 +67,13 @@  rebase.rescheduleFailedExec::
 
 rebase.forkPoint::
 	If set to false set `--no-fork-point` option by default.
+
+rebase.rebaseMerges::
+	Whether and how to set the `--rebase-merges` option by default. Can
+	be `rebase-cousins`, `no-rebase-cousins`, or a boolean. Setting to
+	true or to `no-rebase-cousins` is equivalent to
+	`--rebase-merges=no-rebase-cousins`, setting to `rebase-cousins` is
+	equivalent to `--rebase-merges=rebase-cousins`, and setting to false is
+	equivalent to `--no-rebase-merges`. Passing `--rebase-merges` on the
+	command line, with or without an argument, overrides any
+	`rebase.rebaseMerges` configuration.
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 4e57a87624..e7b39ad244 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -537,7 +537,8 @@  See also INCOMPATIBLE OPTIONS below.
 	by recreating the merge commits. Any resolved merge conflicts or
 	manual amendments in these merge commits will have to be
 	resolved/re-applied manually. `--no-rebase-merges` can be used to
-	countermand a previous `--rebase-merges`.
+	countermand both the `rebase.rebaseMerges` config option and a previous
+	`--rebase-merges`.
 +
 When rebasing merges, there are two modes: `rebase-cousins` and
 `no-rebase-cousins`. If the mode is not specified, it defaults to
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 4b3f29a449..fd284e24ab 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -124,6 +124,7 @@  struct rebase_options {
 	int fork_point;
 	int update_refs;
 	int config_autosquash;
+	int config_rebase_merges;
 	int config_update_refs;
 };
 
@@ -141,6 +142,8 @@  struct rebase_options {
 		.allow_empty_message = 1,               \
 		.autosquash = -1,                       \
 		.config_autosquash = -1,                \
+		.rebase_merges = -1,                    \
+		.config_rebase_merges = -1,             \
 		.update_refs = -1,                      \
 		.config_update_refs = -1,               \
 	}
@@ -772,6 +775,16 @@  static int run_specific_rebase(struct rebase_options *opts)
 	return status ? -1 : 0;
 }
 
+static void parse_rebase_merges_value(struct rebase_options *options, const char *value)
+{
+	if (!strcmp("no-rebase-cousins", value))
+		options->rebase_cousins = 0;
+	else if (!strcmp("rebase-cousins", value))
+		options->rebase_cousins = 1;
+	else
+		die(_("Unknown rebase-merges mode: %s"), value);
+}
+
 static int rebase_config(const char *var, const char *value, void *data)
 {
 	struct rebase_options *opts = data;
@@ -801,6 +814,16 @@  static int rebase_config(const char *var, const char *value, void *data)
 		return 0;
 	}
 
+	if (!strcmp(var, "rebase.rebasemerges")) {
+		opts->config_rebase_merges = git_parse_maybe_bool(value);
+		if (opts->config_rebase_merges < 0) {
+			opts->config_rebase_merges = 1;
+			parse_rebase_merges_value(opts, value);
+		} else
+			opts->rebase_cousins = 0;
+		return 0;
+	}
+
 	if (!strcmp(var, "rebase.updaterefs")) {
 		opts->config_update_refs = git_config_bool(var, value);
 		return 0;
@@ -981,6 +1004,28 @@  static int parse_opt_empty(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
+static int parse_opt_rebase_merges(const struct option *opt, const char *arg, int unset)
+{
+	struct rebase_options *options = opt->value;
+
+	options->rebase_merges = !unset;
+	options->rebase_cousins = 0;
+
+	if (arg) {
+		if (!*arg) {
+			warning(_("--rebase-merges with an empty string "
+				  "argument is deprecated and will stop "
+				  "working in a future version of Git. Use "
+				  "--rebase-merges without an argument "
+				  "instead, which does the same thing."));
+			return 0;
+		}
+		parse_rebase_merges_value(options, arg);
+	}
+
+	return 0;
+}
+
 static void NORETURN error_on_missing_default_upstream(void)
 {
 	struct branch *current_branch = branch_get(NULL);
@@ -1036,7 +1081,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;
-	const char *rebase_merges = NULL;
 	struct string_list strategy_options = STRING_LIST_INIT_NODUP;
 	struct object_id squash_onto;
 	char *squash_onto_name = NULL;
@@ -1138,10 +1182,9 @@  int cmd_rebase(int argc, const char **argv, const char *prefix)
 			   &options.allow_empty_message,
 			   N_("allow rebasing commits with empty messages"),
 			   PARSE_OPT_HIDDEN),
-		{OPTION_STRING, 'r', "rebase-merges", &rebase_merges,
-			N_("mode"),
+		OPT_CALLBACK_F('r', "rebase-merges", &options, N_("mode"),
 			N_("try to rebase merges instead of skipping them"),
-			PARSE_OPT_OPTARG, NULL, (intptr_t)"no-rebase-cousins"},
+			PARSE_OPT_OPTARG, parse_opt_rebase_merges),
 		OPT_BOOL(0, "fork-point", &options.fork_point,
 			 N_("use 'merge-base --fork-point' to refine upstream")),
 		OPT_STRING('s', "strategy", &options.strategy,
@@ -1437,21 +1480,6 @@  int cmd_rebase(int argc, const char **argv, const char *prefix)
 	if (options.exec.nr)
 		imply_merge(&options, "--exec");
 
-	if (rebase_merges) {
-		if (!*rebase_merges)
-			warning(_("--rebase-merges with an empty string "
-				  "argument is deprecated and will stop "
-				  "working in a future version of Git. Use "
-				  "--rebase-merges without an argument "
-				  "instead, which does the same thing."));
-		else if (!strcmp("rebase-cousins", rebase_merges))
-			options.rebase_cousins = 1;
-		else if (strcmp("no-rebase-cousins", rebase_merges))
-			die(_("Unknown mode: %s"), rebase_merges);
-		options.rebase_merges = 1;
-		imply_merge(&options, "--rebase-merges");
-	}
-
 	if (options.type == REBASE_APPLY) {
 		if (ignore_whitespace)
 			strvec_push(&options.git_am_opts,
@@ -1514,13 +1542,15 @@  int cmd_rebase(int argc, const char **argv, const char *prefix)
 				break;
 
 		if (i >= 0 || options.type == REBASE_APPLY) {
-			if (is_merge(&options))
-				die(_("apply options and merge options "
-					  "cannot be used together"));
-			else if (options.autosquash == -1 && options.config_autosquash == 1)
+			if (options.autosquash == -1 && options.config_autosquash == 1)
 				die(_("apply options are incompatible with rebase.autoSquash.  Consider adding --no-autosquash"));
+			else if (options.rebase_merges == -1 && options.config_rebase_merges == 1)
+				die(_("apply options are incompatible with rebase.rebaseMerges.  Consider adding --no-rebase-merges"));
 			else if (options.update_refs == -1 && options.config_update_refs == 1)
 				die(_("apply options are incompatible with rebase.updateRefs.  Consider adding --no-update-refs"));
+			else if (is_merge(&options))
+				die(_("apply options and merge options "
+					  "cannot be used together"));
 			else
 				options.type = REBASE_APPLY;
 		}
@@ -1531,6 +1561,11 @@  int cmd_rebase(int argc, const char **argv, const char *prefix)
 	options.update_refs = (options.update_refs >= 0) ? options.update_refs :
 			     ((options.config_update_refs >= 0) ? options.config_update_refs : 0);
 
+	if (options.rebase_merges == 1)
+		imply_merge(&options, "--rebase-merges");
+	options.rebase_merges = (options.rebase_merges >= 0) ? options.rebase_merges :
+				((options.config_rebase_merges >= 0) ? options.config_rebase_merges : 0);
+
 	if (options.autosquash == 1)
 		imply_merge(&options, "--autosquash");
 	options.autosquash = (options.autosquash >= 0) ? options.autosquash :
diff --git a/t/t3422-rebase-incompatible-options.sh b/t/t3422-rebase-incompatible-options.sh
index 4711b37a28..2eba00bdf5 100755
--- a/t/t3422-rebase-incompatible-options.sh
+++ b/t/t3422-rebase-incompatible-options.sh
@@ -85,6 +85,11 @@  test_rebase_am_only () {
 		test_must_fail git rebase $opt --reapply-cherry-picks A
 	"
 
+	test_expect_success "$opt incompatible with --rebase-merges" "
+		git checkout B^0 &&
+		test_must_fail git rebase $opt --rebase-merges A
+	"
+
 	test_expect_success "$opt incompatible with --update-refs" "
 		git checkout B^0 &&
 		test_must_fail git rebase $opt --update-refs A
@@ -101,6 +106,12 @@  test_rebase_am_only () {
 		grep -e --no-autosquash err
 	"
 
+	test_expect_success "$opt incompatible with rebase.rebaseMerges" "
+		git checkout B^0 &&
+		test_must_fail git -c rebase.rebaseMerges=true rebase $opt A 2>err &&
+		grep -e --no-rebase-merges err
+	"
+
 	test_expect_success "$opt incompatible with rebase.updateRefs" "
 		git checkout B^0 &&
 		test_must_fail git -c rebase.updateRefs=true rebase $opt A 2>err &&
@@ -113,6 +124,12 @@  test_rebase_am_only () {
 		git -c rebase.autosquash=true rebase --no-autosquash $opt A
 	"
 
+	test_expect_success "$opt okay with overridden rebase.rebaseMerges" "
+		test_when_finished \"git reset --hard B^0\" &&
+		git checkout B^0 &&
+		git -c rebase.rebaseMerges=true rebase --no-rebase-merges $opt A
+	"
+
 	test_expect_success "$opt okay with overridden rebase.updateRefs" "
 		test_when_finished \"git reset --hard B^0\" &&
 		git checkout B^0 &&
diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index d46d9545f2..f03599c63b 100755
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -278,6 +278,40 @@  test_expect_success 'do not rebase cousins unless asked for' '
 	EOF
 '
 
+test_expect_success 'rebase.rebaseMerges=rebase-cousins is equivalent to --rebase-merges=rebase-cousins' '
+	test_config rebase.rebaseMerges rebase-cousins &&
+	git checkout -b config-rebase-cousins main &&
+	git rebase HEAD^ &&
+	test_cmp_graph HEAD^.. <<-\EOF
+	*   Merge the topic branch '\''onebranch'\''
+	|\
+	| * D
+	| * G
+	|/
+	o H
+	EOF
+'
+
+test_expect_success '--no-rebase-merges overrides rebase.rebaseMerges=no-rebase-cousins' '
+	test_config rebase.rebaseMerges no-rebase-cousins &&
+	git checkout -b override-config-no-rebase-cousins E &&
+	git rebase --no-rebase-merges C &&
+	test_cmp_graph C.. <<-\EOF
+	* B
+	* D
+	o C
+	EOF
+'
+
+test_expect_success '--rebase-merges overrides rebase.rebaseMerges=rebase-cousins' '
+	test_config rebase.rebaseMerges rebase-cousins &&
+	git checkout -b override-config-rebase-cousins E &&
+	before="$(git rev-parse --verify HEAD)" &&
+	test_tick &&
+	git rebase --rebase-merges C &&
+	test_cmp_rev HEAD $before
+'
+
 test_expect_success 'refs/rewritten/* is worktree-local' '
 	git worktree add wt &&
 	cat >wt/script-from-scratch <<-\EOF &&