From 4557f1add261b70887136498084e7491a3f9535f Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 9 Feb 2017 23:23:06 +0100 Subject: [PATCH 1/2] rebase--helper: add a builtin helper for interactive rebases Git's interactive rebase is still implemented as a shell script, despite its complexity. This implies that it suffers from the portability point of view, from lack of expressibility, and of course also from performance. The latter issue is particularly serious on Windows, where we pay a hefty price for relying so much on POSIX. Unfortunately, being such a huge shell script also means that we missed the train when it would have been relatively easy to port it to C, and instead piled feature upon feature onto that poor script that originally never intended to be more than a slightly pimped cherry-pick in a loop. To open the road toward better performance (in addition to all the other benefits of C over shell scripts), let's just start *somewhere*. The approach taken here is to add a builtin helper that at first intends to take care of the parts of the interactive rebase that are most affected by the performance penalties mentioned above. In particular, after we spent all those efforts on preparing the sequencer to process rebase -i's git-rebase-todo scripts, we implement the `git rebase -i --continue` functionality as a new builtin, git-rebase--helper. Once that is in place, we can work gradually on tackling the rest of the technical debt. Note that the rebase--helper needs to learn about the transient --ff/--no-ff options of git-rebase, as the corresponding flag is not persisted to, and re-read from, the state directory. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- .gitignore | 1 + Makefile | 1 + builtin.h | 1 + builtin/rebase--helper.c | 40 ++++++++++++++++++++++++++++++++++++++++ git.c | 1 + 5 files changed, 44 insertions(+) create mode 100644 builtin/rebase--helper.c diff --git a/.gitignore b/.gitignore index b1020b875f..833ef3b0b7 100644 --- a/.gitignore +++ b/.gitignore @@ -114,6 +114,7 @@ /git-read-tree /git-rebase /git-rebase--am +/git-rebase--helper /git-rebase--interactive /git-rebase--merge /git-receive-pack diff --git a/Makefile b/Makefile index 8e4081e061..29f8663509 100644 --- a/Makefile +++ b/Makefile @@ -932,6 +932,7 @@ BUILTIN_OBJS += builtin/prune.o BUILTIN_OBJS += builtin/pull.o BUILTIN_OBJS += builtin/push.o BUILTIN_OBJS += builtin/read-tree.o +BUILTIN_OBJS += builtin/rebase--helper.o BUILTIN_OBJS += builtin/receive-pack.o BUILTIN_OBJS += builtin/reflog.o BUILTIN_OBJS += builtin/remote.o diff --git a/builtin.h b/builtin.h index 67f80519da..9e4a89816d 100644 --- a/builtin.h +++ b/builtin.h @@ -103,6 +103,7 @@ extern int cmd_prune_packed(int argc, const char **argv, const char *prefix); extern int cmd_pull(int argc, const char **argv, const char *prefix); extern int cmd_push(int argc, const char **argv, const char *prefix); extern int cmd_read_tree(int argc, const char **argv, const char *prefix); +extern int cmd_rebase__helper(int argc, const char **argv, const char *prefix); extern int cmd_receive_pack(int argc, const char **argv, const char *prefix); extern int cmd_reflog(int argc, const char **argv, const char *prefix); extern int cmd_remote(int argc, const char **argv, const char *prefix); diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c new file mode 100644 index 0000000000..ca1ebb2fa1 --- /dev/null +++ b/builtin/rebase--helper.c @@ -0,0 +1,40 @@ +#include "builtin.h" +#include "cache.h" +#include "parse-options.h" +#include "sequencer.h" + +static const char * const builtin_rebase_helper_usage[] = { + N_("git rebase--helper []"), + NULL +}; + +int cmd_rebase__helper(int argc, const char **argv, const char *prefix) +{ + struct replay_opts opts = REPLAY_OPTS_INIT; + enum { + CONTINUE = 1, ABORT + } command = 0; + struct option options[] = { + OPT_BOOL(0, "ff", &opts.allow_ff, N_("allow fast-forward")), + OPT_CMDMODE(0, "continue", &command, N_("continue rebase"), + CONTINUE), + OPT_CMDMODE(0, "abort", &command, N_("abort rebase"), + ABORT), + OPT_END() + }; + + git_config(git_default_config, NULL); + + opts.action = REPLAY_INTERACTIVE_REBASE; + opts.allow_ff = 1; + opts.allow_empty = 1; + + argc = parse_options(argc, argv, NULL, options, + builtin_rebase_helper_usage, PARSE_OPT_KEEP_ARGV0); + + if (command == CONTINUE && argc == 1) + return !!sequencer_continue(&opts); + if (command == ABORT && argc == 1) + return !!sequencer_remove_state(&opts); + usage_with_options(builtin_rebase_helper_usage, options); +} diff --git a/git.c b/git.c index c887272b12..33f52acbcc 100644 --- a/git.c +++ b/git.c @@ -473,6 +473,7 @@ static struct cmd_struct commands[] = { { "pull", cmd_pull, RUN_SETUP | NEED_WORK_TREE }, { "push", cmd_push, RUN_SETUP }, { "read-tree", cmd_read_tree, RUN_SETUP | SUPPORT_SUPER_PREFIX}, + { "rebase--helper", cmd_rebase__helper, RUN_SETUP | NEED_WORK_TREE }, { "receive-pack", cmd_receive_pack }, { "reflog", cmd_reflog, RUN_SETUP }, { "remote", cmd_remote, RUN_SETUP }, From 18633e1a22a68bbe8e6311a1039d13ebbf6fd041 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 9 Feb 2017 23:23:11 +0100 Subject: [PATCH 2/2] rebase -i: use the rebase--helper builtin Now that the sequencer learned to process a "normal" interactive rebase, we use it. The original shell script is still used for "non-normal" interactive rebases, i.e. when --root or --preserve-merges was passed. Please note that the --root option (via the $squash_onto variable) needs special handling only for the very first command, hence it is still okay to use the helper upon continue/skip. Also please note that the --no-ff setting is volatile, i.e. when the interactive rebase is interrupted at any stage, there is no record of it. Therefore, we have to pass it from the shell script to the rebase--helper. Note: the test t3404 had to be adjusted because the the error messages produced by the sequencer comply with our current convention to start with a lower-case letter. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- git-rebase--interactive.sh | 13 +++++++++++++ t/t3404-rebase-interactive.sh | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 4734094a3f..2c9c0165b5 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -1069,6 +1069,10 @@ git_rebase__interactive () { case "$action" in continue) + if test ! -d "$rewritten" + then + exec git rebase--helper ${force_rebase:+--no-ff} --continue + fi # do we have anything to commit? if git diff-index --cached --quiet HEAD -- then @@ -1128,6 +1132,10 @@ first and then run 'git rebase --continue' again.")" skip) git rerere clear + if test ! -d "$rewritten" + then + exec git rebase--helper ${force_rebase:+--no-ff} --continue + fi do_rest return 0 ;; @@ -1314,6 +1322,11 @@ expand_todo_ids test -d "$rewritten" || test -n "$force_rebase" || skip_unnecessary_picks checkout_onto +if test -z "$rebase_root" && test ! -d "$rewritten" +then + require_clean_work_tree "rebase" + exec git rebase--helper ${force_rebase:+--no-ff} --continue +fi do_rest } diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index e2f18d11f6..33d392ba11 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -556,7 +556,7 @@ test_expect_success 'clean error after failed "exec"' ' echo "edited again" > file7 && git add file7 && test_must_fail git rebase --continue 2>error && - test_i18ngrep "You have staged changes in your working tree." error + test_i18ngrep "you have staged changes in your working tree" error ' test_expect_success 'rebase a detached HEAD' '