Skip to content

Commit

Permalink
rebase: remove the rebase.useBuiltin setting
Browse files Browse the repository at this point in the history
Remove the rebase.useBuiltin setting, which was added as an escape
hatch to disable the builtin version of rebase first released with Git
2.20.

See [1] for the initial implementation of rebase.useBuiltin, and [2]
and [3] for the documentation and corresponding
GIT_TEST_REBASE_USE_BUILTIN option.

Carrying the legacy version is a maintenance burden as seen in
7e097e2 ("legacy-rebase: backport -C<n> and --whitespace=<option>
checks", 2018-11-20) and 9aea5e9 ("rebase: fix regression in
rebase.useBuiltin=false test mode", 2019-02-13). Since the built-in
version has been shown to be stable enough let's remove the legacy
version.

As noted in [3] having use_builtin_rebase() shell out to get its
config doesn't make any sense anymore, that was done for the purposes
of spawning the legacy rebase without having modified any global
state. Let's instead handle this case in rebase_config().

There's still a bunch of references to git-legacy-rebase in po/*.po,
but those will be dealt with in time by the i18n effort.

Even though this configuration variable only existed two releases
let's not entirely delete the entry from the docs, but note its
absence. Individual versions of git tend to be around for a while due
to distro packaging timelines, so e.g. if we're "lucky" a given
version like 2.21 might be installed on say OSX for half a decade.

That'll mean some people probably setting this in config, and then
when they later wonder if it's needed they can Google search the
config option name or check it in git-config. It also allows us to
refer to the docs from the warning for details.

1. 55071ea ("rebase: start implementing it as a builtin",
   2018-08-07)
2. d8d0a54 ("rebase doc: document rebase.useBuiltin", 2018-11-14)
3. 62c2393 ("tests: add a special setup where rebase.useBuiltin is
   off", 2018-11-14)
3. https://public-inbox.org/git/[email protected]/

Acked-by: Johannes Schindelin <[email protected]>
Signed-off-by: Ævar Arnfjörð Bjarmason <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
  • Loading branch information
avar authored and gitster committed Mar 20, 2019
1 parent 0e94f7a commit d03ebd4
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 833 deletions.
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@
/git-init-db
/git-interpret-trailers
/git-instaweb
/git-legacy-rebase
/git-log
/git-ls-files
/git-ls-remote
Expand Down
17 changes: 5 additions & 12 deletions Documentation/config/rebase.txt
Original file line number Diff line number Diff line change
@@ -1,16 +1,9 @@
rebase.useBuiltin::
Set to `false` to use the legacy shellscript implementation of
linkgit:git-rebase[1]. Is `true` by default, which means use
the built-in rewrite of it in C.
+
The C rewrite is first included with Git version 2.20. This option
serves an an escape hatch to re-enable the legacy version in case any
bugs are found in the rewrite. This option and the shellscript version
of linkgit:git-rebase[1] will be removed in some future release.
+
If you find some reason to set this option to `false` other than
one-off testing you should report the behavior difference as a bug in
git.
Unused configuration variable. Used in Git versions 2.20 and
2.21 as an escape hatch to enable the legacy shellscript
implementation of rebase. Now the built-in rewrite of it in C
is always used. Setting this will emit a warning, to alert any
remaining users that setting this now does nothing.

rebase.stat::
Whether to show a diffstat of what changed upstream since the last
Expand Down
1 change: 0 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,6 @@ SCRIPT_SH += git-merge-one-file.sh
SCRIPT_SH += git-merge-resolve.sh
SCRIPT_SH += git-mergetool.sh
SCRIPT_SH += git-quiltimport.sh
SCRIPT_SH += git-legacy-rebase.sh
SCRIPT_SH += git-remote-testgit.sh
SCRIPT_SH += git-request-pull.sh
SCRIPT_SH += git-stash.sh
Expand Down
50 changes: 11 additions & 39 deletions builtin/rebase.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,29 +46,6 @@ enum rebase_type {
REBASE_PRESERVE_MERGES
};

static int use_builtin_rebase(void)
{
struct child_process cp = CHILD_PROCESS_INIT;
struct strbuf out = STRBUF_INIT;
int ret, env = git_env_bool("GIT_TEST_REBASE_USE_BUILTIN", -1);

if (env != -1)
return env;

argv_array_pushl(&cp.args,
"config", "--bool", "rebase.usebuiltin", NULL);
cp.git_cmd = 1;
if (capture_command(&cp, &out, 6)) {
strbuf_release(&out);
return 1;
}

strbuf_trim(&out);
ret = !strcmp("true", out.buf);
strbuf_release(&out);
return ret;
}

struct rebase_options {
enum rebase_type type;
const char *state_dir;
Expand Down Expand Up @@ -106,6 +83,7 @@ struct rebase_options {
char *strategy, *strategy_opts;
struct strbuf git_format_patch_opt;
int reschedule_failed_exec;
int use_legacy_rebase;
};

static int is_interactive(struct rebase_options *opts)
Expand Down Expand Up @@ -869,6 +847,11 @@ static int rebase_config(const char *var, const char *value, void *data)
return 0;
}

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

return git_default_config(var, value, data);
}

Expand Down Expand Up @@ -1143,22 +1126,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
};
int i;

/*
* NEEDSWORK: Once the builtin rebase has been tested enough
* and git-legacy-rebase.sh is retired to contrib/, this preamble
* can be removed.
*/

if (!use_builtin_rebase()) {
const char *path = mkpath("%s/git-legacy-rebase",
git_exec_path());

if (sane_execvp(path, (char **)argv) < 0)
die_errno(_("could not exec %s"), path);
else
BUG("sane_execvp() returned???");
}

if (argc == 2 && !strcmp(argv[1], "-h"))
usage_with_options(builtin_rebase_usage,
builtin_rebase_options);
Expand All @@ -1169,6 +1136,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)

git_config(rebase_config, &options);

if (options.use_legacy_rebase ||
!git_env_bool("GIT_TEST_REBASE_USE_BUILTIN", -1))
warning(_("the rebase.useBuiltin support has been removed!\n"
"See its entry in 'git help config' for details."));

strbuf_reset(&buf);
strbuf_addf(&buf, "%s/applying", apply_dir());
if(file_exists(buf.buf))
Expand Down
Loading

0 comments on commit d03ebd4

Please sign in to comment.