git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [BUG?] 'git rebase --update-refs --whitespace=fix' incompatible, but not enforced

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Jan 18, 2023, 16:16 UTC
Message-ID
<8a86074e-3f76-472c-15e2-9aa53762ef8f@dunelm.org.uk>
In-Reply-To
<9445830b-d172-c3b6-ef60-ae4931cab84b@github.com>
Hi Stolee
On 18/01/2023 15:51, Derrick Stolee wrote:
Show 8 quoted lines
> On 1/17/2023 5:02 PM, Philippe Blain wrote:
> Subject: [PATCH] rebase: die for both --apply and --update-refs
> 
> The --apply backend is not compatible with the --update-refs option,
> which requires the interactive backend. Without any warning, users
> running 'git rebase --whitespace=fix' with --update-refs (or
> rebase.updateRefs=true in their config) will realize that their non-HEAD
> branches did not come along for the ride.

I think that for other options such as "--exec" that require the "merge" backend we call imply_merge() and then have some code that will error out if there are also options that require the "apply" backend. Is it possible to do that here?

Show 13 quoted lines
> Fix this with a die() message in the presence of both options. Since we
> cannot distinguish between the --update-refs option and the config
> option at this point, do an extra check to see if --update-refs was
> implied by the config and present a helpful warning to use
> --no-update-refs.
> 
> It is possible that users might want to be able to use options such as
> --whitespace=fix with rebase.updateRefs=true in their config without
> explicitly adding --no-update-refs. However, it is probably safest to
> require them to explicitly opt-in to that behavior. Users with the
> config option likely expect that their refs will be automatically
> updated and this will help warn them that the action they are doing is
> likely destructive to their branch organization.
I agree that requiring an explicit --no-update-refs in that case is best.
Best Wishes
Phillip
Show 62 quoted lines
> Reported-by: Philippe Blain <levraiphilippeblain@gmail.com>
> Signed-off-by: Derrick Stolee <derrickstolee@github.com>
> ---
>   builtin/rebase.c              | 21 +++++++++++++++++++++
>   t/t3404-rebase-interactive.sh | 12 ++++++++++++
>   2 files changed, 33 insertions(+)
> 
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 1481c5b6a5b..5330258c865 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -1247,6 +1247,27 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>   		die(_("The --edit-todo action can only be used during "
>   		      "interactive rebase."));
>   
> +	/* Check for arguments that imply --apply before checking --apply itself. */
> +	if (options.update_refs) {
> +		const char *incompatible = NULL;
> +		if (options.git_am_opts.nr)
> +			incompatible = options.git_am_opts.v[0];
> +		else if (options.type == REBASE_APPLY)
> +			incompatible = "--apply";
> +
> +		if (incompatible) {
> +			int from_config = 0;
> +			if (!git_config_get_bool("rebase.updaterefs", &from_config) &&
> +			    from_config) {
> +				warning(_("you have 'rebase.updateRefs' enabled in your config, "
> +					  "but it is incompatible with one or more options;"));
> +				warning(_("run again with '--no-update-refs' to resolve the issue"));
> +			}
> +			die(_("options '%s' and '%s' cannot be used together"),
> +			    "--upate-refs", incompatible);
> +		}
> +	}
> +
>   	if (trace2_is_enabled()) {
>   		if (is_merge(&options))
>   			trace2_cmd_mode("interactive");
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 462cefd25df..8681c8a07f8 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -2123,6 +2123,18 @@ test_expect_success '--update-refs: check failed ref update' '
>   	test_cmp expect err.trimmed
>   '
>   
> +test_expect_success '--apply options are incompatible with --update-refs' '
> +	for opt in "--whitespace=fix" "-C1" "--apply"
> +	do
> +		test_must_fail git rebase "$opt" --update-refs HEAD~1 2>err &&
> +		grep "options '\''--upate-refs'\'' and '\''$opt'\'' cannot be used together" err &&
> +
> +		test_must_fail git -c rebase.updateRefs=true rebase "$opt" HEAD~1 2>err &&
> +		grep "options '\''--upate-refs'\'' and '\''$opt'\'' cannot be used together" err &&
> +		grep "you have '\''rebase.updateRefs'\'' enabled" err || return 1
> +	done
> +'
> +
>   # This must be the last test in this file
>   test_expect_success '$EDITOR and friends are unchanged' '
>   	test_editor_unchanged
Previous: Derrick StoleeNext: Elijah Newren
Message 3 of 6 in “[BUG?] 'git rebase --update-refs --whitespace=fix' incompatible, but not enforced”
  1. Philippe BlainJan 17, 2023
  2. Derrick StoleeJan 18, 2023
  3. Phillip WoodJan 18, 2023
  4. Elijah NewrenJan 18, 2023
  5. Derrick StoleeJan 18, 2023
  6. Philippe BlainJan 18, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.