In particular on Windows, where shell scripts are even more expensive
than on MacOSX or Linux, it makes sense to move a loop that forks
Git at least once for every line in the todo list into a builtin.

Note: The original code did not try to skip unnecessary picks of root
commits but punts instead (probably --root was not considered common
enough of a use case to bother optimizing). We do the same, for now.

Signed-off-by: Johannes Schindelin <johannes.schinde...@gmx.de>
---
 builtin/rebase--helper.c   |  6 +++-
 git-rebase--interactive.sh | 41 ++-------------------
 sequencer.c                | 90 ++++++++++++++++++++++++++++++++++++++++++++++
 sequencer.h                |  1 +
 4 files changed, 99 insertions(+), 39 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index e706eac710d..de3ccd9bfbc 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -14,7 +14,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
        int keep_empty = 0;
        enum {
                CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_SHA1S, EXPAND_SHA1S,
-               CHECK_TODO_LIST
+               CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS
        } command = 0;
        struct option options[] = {
                OPT_BOOL(0, "ff", &opts.allow_ff, N_("allow fast-forward")),
@@ -31,6 +31,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
                        N_("expand SHA-1s in the todo list"), EXPAND_SHA1S),
                OPT_CMDMODE(0, "check-todo-list", &command,
                        N_("check the todo list"), CHECK_TODO_LIST),
+               OPT_CMDMODE(0, "skip-unnecessary-picks", &command,
+                       N_("skip unnecessary picks"), SKIP_UNNECESSARY_PICKS),
                OPT_END()
        };
 
@@ -55,5 +57,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
                return !!transform_todo_ids(0);
        if (command == CHECK_TODO_LIST && argc == 1)
                return !!check_todo_list();
+       if (command == SKIP_UNNECESSARY_PICKS && argc == 1)
+               return !!skip_unnecessary_picks();
        usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 1649506e1e4..931bc09e0cf 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -713,43 +713,6 @@ do_rest () {
        done
 }
 
-# skip picking commits whose parents are unchanged
-skip_unnecessary_picks () {
-       fd=3
-       while read -r command rest
-       do
-               # fd=3 means we skip the command
-               case "$fd,$command" in
-               3,pick|3,p)
-                       # pick a commit whose parent is current $onto -> skip
-                       sha1=${rest%% *}
-                       case "$(git rev-parse --verify --quiet "$sha1"^)" in
-                       "$onto"*)
-                               onto=$sha1
-                               ;;
-                       *)
-                               fd=1
-                               ;;
-                       esac
-                       ;;
-               3,"$comment_char"*|3,)
-                       # copy comments
-                       ;;
-               *)
-                       fd=1
-                       ;;
-               esac
-               printf '%s\n' "$command${rest:+ }$rest" >&$fd
-       done <"$todo" >"$todo.new" 3>>"$done" &&
-       mv -f "$todo".new "$todo" &&
-       case "$(peek_next_command)" in
-       squash|s|fixup|f)
-               record_in_rewritten "$onto"
-               ;;
-       esac ||
-               die "$(gettext "Could not skip unnecessary pick commands")"
-}
-
 transform_todo_ids () {
        while read -r command rest
        do
@@ -1172,7 +1135,9 @@ git rebase--helper --check-todo-list || {
 
 expand_todo_ids
 
-test -d "$rewritten" || test -n "$force_rebase" || skip_unnecessary_picks
+test -d "$rewritten" || test -n "$force_rebase" ||
+onto="$(git rebase--helper --skip-unnecessary-picks)" ||
+die "Could not skip unnecessary pick commands"
 
 checkout_onto
 if test -z "$rebase_root" && test ! -d "$rewritten"
diff --git a/sequencer.c b/sequencer.c
index 3a935fa4cbc..bbbc98c9116 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2616,3 +2616,93 @@ int check_todo_list(void)
 
        return res;
 }
+
+/* skip picking commits whose parents are unchanged */
+int skip_unnecessary_picks(void)
+{
+       const char *todo_file = rebase_path_todo();
+       struct strbuf buf = STRBUF_INIT;
+       struct todo_list todo_list = TODO_LIST_INIT;
+       struct object_id onto_oid, *oid = &onto_oid, *parent_oid;
+       int fd, i;
+
+       if (!read_oneliner(&buf, rebase_path_onto(), 0))
+               return error(_("could not read 'onto'"));
+       if (get_sha1(buf.buf, onto_oid.hash)) {
+               strbuf_release(&buf);
+               return error(_("need a HEAD to fixup"));
+       }
+       strbuf_release(&buf);
+
+       fd = open(todo_file, O_RDONLY);
+       if (fd < 0) {
+               return error_errno(_("could not open '%s'"), todo_file);
+       }
+       if (strbuf_read(&todo_list.buf, fd, 0) < 0) {
+               close(fd);
+               return error(_("could not read '%s'."), todo_file);
+       }
+       close(fd);
+       if (parse_insn_buffer(todo_list.buf.buf, &todo_list) < 0) {
+               todo_list_release(&todo_list);
+               return -1;
+       }
+
+       for (i = 0; i < todo_list.nr; i++) {
+               struct todo_item *item = todo_list.items + i;
+
+               if (item->command >= TODO_NOOP)
+                       continue;
+               if (item->command != TODO_PICK)
+                       break;
+               if (parse_commit(item->commit)) {
+                       todo_list_release(&todo_list);
+                       return error(_("could not parse commit '%s'"),
+                               oid_to_hex(&item->commit->object.oid));
+               }
+               if (!item->commit->parents)
+                       break; /* root commit */
+               if (item->commit->parents->next)
+                       break; /* merge commit */
+               parent_oid = &item->commit->parents->item->object.oid;
+               if (hashcmp(parent_oid->hash, oid->hash))
+                       break;
+               oid = &item->commit->object.oid;
+       }
+       if (i > 0) {
+               int offset = i < todo_list.nr ?
+                       todo_list.items[i].offset_in_buf : todo_list.buf.len;
+               const char *done_path = rebase_path_done();
+
+               fd = open(done_path, O_CREAT | O_WRONLY | O_APPEND, 0666);
+               if (write_in_full(fd, todo_list.buf.buf, offset) < 0) {
+                       todo_list_release(&todo_list);
+                       return error_errno(_("could not write to '%s'"),
+                               done_path);
+               }
+               close(fd);
+
+               fd = open(rebase_path_todo(), O_WRONLY, 0666);
+               if (write_in_full(fd, todo_list.buf.buf + offset,
+                               todo_list.buf.len - offset) < 0) {
+                       todo_list_release(&todo_list);
+                       return error_errno(_("could not write to '%s'"),
+                               rebase_path_todo());
+               }
+               if (ftruncate(fd, todo_list.buf.len - offset) < 0) {
+                       todo_list_release(&todo_list);
+                       return error_errno(_("could not truncate '%s'"),
+                               rebase_path_todo());
+               }
+               close(fd);
+
+               todo_list.current = i;
+               if (is_fixup(peek_command(&todo_list, 0)))
+                       record_in_rewritten(oid, peek_command(&todo_list, 0));
+       }
+
+       todo_list_release(&todo_list);
+       printf("%s\n", oid_to_hex(oid));
+
+       return 0;
+}
diff --git a/sequencer.h b/sequencer.h
index 4978a61b83b..28e1fc1e9bb 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -50,6 +50,7 @@ int sequencer_make_script(int keep_empty, FILE *out,
 
 int transform_todo_ids(int shorten_sha1s);
 int check_todo_list(void);
+int skip_unnecessary_picks(void);
 
 extern const char sign_off_header[];
 
-- 
2.12.2.windows.2.406.gd14a8f8640f


Reply via email to