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

Alex Henrie [off-list ref] writes:

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;
	}

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?

+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.

On Wed, Jan 20, 2021 at 4:00 PM Junio C Hamano [off-list ref] wrote:

Alex Henrie [off-list ref] writes:

quoted

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

quoted

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

Yes. I was trying to avoid annoying the user with errors when
rebase.forkPoint is set to "true" and --root is given on the command
line.

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?

That's a great point. I will make that change and resubmit. Thanks!

-Alex