rebase -i: skip unnecessary picks using the rebase--helper

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.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Johannes Schindelin committed Jul 14, 2017 at 16:45 UTC cdac2b01ff77d32305610aeb26396e25bffa9dba
4 files changed +116 -39
builtin/rebase--helper.c
+5 -1
@@ -15,7 +15,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
15 int keep_empty = 0;
16 enum {
17 CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_SHA1S, EXPAND_SHA1S,
18 - CHECK_TODO_LIST
18 + CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS
19 } command = 0;
20 struct option options[] = {
21 OPT_BOOL(0, "ff", &opts.allow_ff, N_("allow fast-forward")),
@@ -32,6 +32,8 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
32 N_("expand SHA-1s in the todo list"), EXPAND_SHA1S),
33 OPT_CMDMODE(0, "check-todo-list", &command,
34 N_("check the todo list"), CHECK_TODO_LIST),
35 + OPT_CMDMODE(0, "skip-unnecessary-picks", &command,
36 + N_("skip unnecessary picks"), SKIP_UNNECESSARY_PICKS),
37 OPT_END()
38 };
39
@@ -56,5 +58,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
58 return !!transform_todo_ids(0);
59 if (command == CHECK_TODO_LIST && argc == 1)
60 return !!check_todo_list();
61 + if (command == SKIP_UNNECESSARY_PICKS && argc == 1)
62 + return !!skip_unnecessary_picks();
63 usage_with_options(builtin_rebase_helper_usage, options);
64 }
git-rebase--interactive.sh
+3 -38
@@ -713,43 +713,6 @@ do_rest () {
713 done
714 }
715
716 -# skip picking commits whose parents are unchanged
717 -skip_unnecessary_picks () {
718 - fd=3
719 - while read -r command rest
720 - do
721 - # fd=3 means we skip the command
722 - case "$fd,$command" in
723 - 3,pick|3,p)
724 - # pick a commit whose parent is current $onto -> skip
725 - sha1=${rest%% *}
726 - case "$(git rev-parse --verify --quiet "$sha1"^)" in
727 - "$onto"*)
728 - onto=$sha1
729 - ;;
730 - *)
731 - fd=1
732 - ;;
733 - esac
734 - ;;
735 - 3,"$comment_char"*|3,)
736 - # copy comments
737 - ;;
738 - *)
739 - fd=1
740 - ;;
741 - esac
742 - printf '%s\n' "$command${rest:+ }$rest" >&$fd
743 - done <"$todo" >"$todo.new" 3>>"$done" &&
744 - mv -f "$todo".new "$todo" &&
745 - case "$(peek_next_command)" in
746 - squash|s|fixup|f)
747 - record_in_rewritten "$onto"
748 - ;;
749 - esac ||
750 - die "$(gettext "Could not skip unnecessary pick commands")"
751 -}
752 -
716 expand_todo_ids() {
717 git rebase--helper --expand-ids
718 }
@@ -1149,7 +1112,9 @@ git rebase--helper --check-todo-list || {
1112
1113 expand_todo_ids
1114
1152 -test -d "$rewritten" || test -n "$force_rebase" || skip_unnecessary_picks
1115 +test -d "$rewritten" || test -n "$force_rebase" ||
1116 +onto="$(git rebase--helper --skip-unnecessary-picks)" ||
1117 +die "Could not skip unnecessary pick commands"
1118
1119 checkout_onto
1120 if test -z "$rebase_root" && test ! -d "$rewritten"
sequencer.c
+107
@@ -2641,3 +2641,110 @@ leave_check:
2641
2642 return res;
2643 }
2644 +
2645 +/* skip picking commits whose parents are unchanged */
2646 +int skip_unnecessary_picks(void)
2647 +{
2648 + const char *todo_file = rebase_path_todo();
2649 + struct strbuf buf = STRBUF_INIT;
2650 + struct todo_list todo_list = TODO_LIST_INIT;
2651 + struct object_id onto_oid, *oid = &onto_oid, *parent_oid;
2652 + int fd, i;
2653 +
2654 + if (!read_oneliner(&buf, rebase_path_onto(), 0))
2655 + return error(_("could not read 'onto'"));
2656 + if (get_oid(buf.buf, &onto_oid)) {
2657 + strbuf_release(&buf);
2658 + return error(_("need a HEAD to fixup"));
2659 + }
2660 + strbuf_release(&buf);
2661 +
2662 + fd = open(todo_file, O_RDONLY);
2663 + if (fd < 0) {
2664 + return error_errno(_("could not open '%s'"), todo_file);
2665 + }
2666 + if (strbuf_read(&todo_list.buf, fd, 0) < 0) {
2667 + close(fd);
2668 + return error(_("could not read '%s'."), todo_file);
2669 + }
2670 + close(fd);
2671 + if (parse_insn_buffer(todo_list.buf.buf, &todo_list) < 0) {
2672 + todo_list_release(&todo_list);
2673 + return -1;
2674 + }
2675 +
2676 + for (i = 0; i < todo_list.nr; i++) {
2677 + struct todo_item *item = todo_list.items + i;
2678 +
2679 + if (item->command >= TODO_NOOP)
2680 + continue;
2681 + if (item->command != TODO_PICK)
2682 + break;
2683 + if (parse_commit(item->commit)) {
2684 + todo_list_release(&todo_list);
2685 + return error(_("could not parse commit '%s'"),
2686 + oid_to_hex(&item->commit->object.oid));
2687 + }
2688 + if (!item->commit->parents)
2689 + break; /* root commit */
2690 + if (item->commit->parents->next)
2691 + break; /* merge commit */
2692 + parent_oid = &item->commit->parents->item->object.oid;
2693 + if (hashcmp(parent_oid->hash, oid->hash))
2694 + break;
2695 + oid = &item->commit->object.oid;
2696 + }
2697 + if (i > 0) {
2698 + int offset = i < todo_list.nr ?
2699 + todo_list.items[i].offset_in_buf : todo_list.buf.len;
2700 + const char *done_path = rebase_path_done();
2701 +
2702 + fd = open(done_path, O_CREAT | O_WRONLY | O_APPEND, 0666);
2703 + if (fd < 0) {
2704 + error_errno(_("could not open '%s' for writing"),
2705 + done_path);
2706 + todo_list_release(&todo_list);
2707 + return -1;
2708 + }
2709 + if (write_in_full(fd, todo_list.buf.buf, offset) < 0) {
2710 + error_errno(_("could not write to '%s'"), done_path);
2711 + todo_list_release(&todo_list);
2712 + close(fd);
2713 + return -1;
2714 + }
2715 + close(fd);
2716 +
2717 + fd = open(rebase_path_todo(), O_WRONLY, 0666);
2718 + if (fd < 0) {
2719 + error_errno(_("could not open '%s' for writing"),
2720 + rebase_path_todo());
2721 + todo_list_release(&todo_list);
2722 + return -1;
2723 + }
2724 + if (write_in_full(fd, todo_list.buf.buf + offset,
2725 + todo_list.buf.len - offset) < 0) {
2726 + error_errno(_("could not write to '%s'"),
2727 + rebase_path_todo());
2728 + close(fd);
2729 + todo_list_release(&todo_list);
2730 + return -1;
2731 + }
2732 + if (ftruncate(fd, todo_list.buf.len - offset) < 0) {
2733 + error_errno(_("could not truncate '%s'"),
2734 + rebase_path_todo());
2735 + todo_list_release(&todo_list);
2736 + close(fd);
2737 + return -1;
2738 + }
2739 + close(fd);
2740 +
2741 + todo_list.current = i;
2742 + if (is_fixup(peek_command(&todo_list, 0)))
2743 + record_in_rewritten(oid, peek_command(&todo_list, 0));
2744 + }
2745 +
2746 + todo_list_release(&todo_list);
2747 + printf("%s\n", oid_to_hex(oid));
2748 +
2749 + return 0;
2750 +}
sequencer.h
+1
@@ -50,6 +50,7 @@ int sequencer_make_script(int keep_empty, FILE *out,
50
51 int transform_todo_ids(int shorten_ids);
52 int check_todo_list(void);
53 +int skip_unnecessary_picks(void);
54
55 extern const char sign_off_header[];
56