Re: [PATCH] rebase: add a config option for --no-fork-point

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 20, 2021, 23:00 UTC
Message-ID: <xmqqturbdxi2.fsf@gitster.c.googlers.com>
In-Reply-To: <20210120044435.53509-1-alexhenrie24@gmail.com>

Alex Henrie <alexhenrie24@gmail.com> writes:

Show 9 quoted lines

> @@ -1095,6 +1097,12 @@ static int rebase_config(const char *var, const char *value, void *data)
>  		return 0;
>  	}
>  
> +	if (!strcmp(var, "rebase.forkpoint")) {
> +		if (!git_config_bool(var, value))
> +			opts->fork_point = 0;
> +		return 0;
> +	}

It is curious that this code seems to deliberatly ignore

	[rebase] forkpoint = true

but honors

	[rebase] forkpoint = false

Intended?  IOW, why isn't this just like this?

	if (!strcmp(var, "rebase.forkpoint")) {
		opts->fork_point = git_config_bool(var, value);
		return 0;
	}

Show 24 quoted lines

> @@ -1306,7 +1314,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>  	const char *gpg_sign = NULL;
>  	struct string_list exec = STRING_LIST_INIT_NODUP;
>  	const char *rebase_merges = NULL;
> -	int fork_point = -1;
>  	struct string_list strategy_options = STRING_LIST_INIT_NODUP;
>  	struct object_id squash_onto;
>  	char *squash_onto_name = NULL;
> @@ -1406,7 +1413,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>  			N_("mode"),
>  			N_("try to rebase merges instead of skipping them"),
>  			PARSE_OPT_OPTARG, NULL, (intptr_t)""},
> -		OPT_BOOL(0, "fork-point", &fork_point,
> +		OPT_BOOL(0, "fork-point", &options.fork_point,
>  			 N_("use 'merge-base --fork-point' to refine upstream")),
>  		OPT_STRING('s', "strategy", &options.strategy,
>  			   N_("strategy"), N_("use the given merge strategy")),
> @@ -1494,7 +1501,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>  			die(_("cannot combine '--keep-base' with '--root'"));
>  	}
>  
> -	if (options.root && fork_point > 0)
> +	if (options.root && options.fork_point > 0)
>  		die(_("cannot combine '--root' with '--fork-point'"));

Is that because of this code?

If so, perhaps the configuration parser should set the .fork_point to (-1), so that "[rebase] forkpoint = false" that appears in your ~/.gitconfig file can be countermanded with "[rebase] forkpoint" that is placed in .git/config for one particular project that you do not mind using the feature?

Show 9 quoted lines

> +test_expect_success '--fork-point and --root both given' '
> +	test_must_fail git rebase --fork-point --root 2>err &&
> +	test_i18ngrep "cannot combine" err
> +'
> +
> +test_expect_success 'rebase.forkPoint true and --root given' '
> +	test_config rebase.forkPoint true &&
> +	git rebase --root
> +'

test_expect_success 'rebase.forkPoint true overrides earlier false' '
	test_config rebase.forkPoint false &&
	test_config rebase.forkPoint true &&
	... check "git rebase" does use the fork-point feature here ...
'

Thanks.