rebase -i: rearrange fixup/squash lines using the rebase--helper

This operation has quadratic complexity, which is especially painful on Windows, where shell scripts are *already* slow (mainly due to the overhead of the POSIX emulation layer). Let's reimplement this with linear complexity (using a hash map to match the commits' subject lines) for the common case; Sadly, the fixup/squash feature's design neglected performance considerations, allowing arbitrary prefixes (read: `fixup! hell` will match the commit subject `hello world`), which means that we are stuck with quadratic performance in the worst case. The reimplemented logic also happens to fix a bug where commented-out lines (representing empty patches) were dropped by the previous code. While at it, clarify how the fixup/squash feature works in `git rebase -i`'s man page. 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 c44a4c650c66eb7b8d50c57fd4e1bff1add7bf77
6 files changed +213 -98
Documentation/git-rebase.txt
+9 -7
@@ -430,13 +430,15 @@ without an explicit `--interactive`.
430 --autosquash::
431 --no-autosquash::
432 When the commit log message begins with "squash! ..." (or
433 - "fixup! ..."), and there is a commit whose title begins with
434 - the same ..., automatically modify the todo list of rebase -i
435 - so that the commit marked for squashing comes right after the
436 - commit to be modified, and change the action of the moved
437 - commit from `pick` to `squash` (or `fixup`). Ignores subsequent
438 - "fixup! " or "squash! " after the first, in case you referred to an
439 - earlier fixup/squash with `git commit --fixup/--squash`.
433 + "fixup! ..."), and there is already a commit in the todo list that
434 + matches the same `...`, automatically modify the todo list of rebase
435 + -i so that the commit marked for squashing comes right after the
436 + commit to be modified, and change the action of the moved commit
437 + from `pick` to `squash` (or `fixup`). A commit matches the `...` if
438 + the commit subject matches, or if the `...` refers to the commit's
439 + hash. As a fall-back, partial matches of the commit subject work,
440 + too. The recommended way to create fixup/squash commits is by using
441 + the `--fixup`/`--squash` options of linkgit:git-commit[1].
442 +
443 This option is only valid when the `--interactive` option is used.
444 +
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, SKIP_UNNECESSARY_PICKS
18 + CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH
19 } command = 0;
20 struct option options[] = {
21 OPT_BOOL(0, "ff", &opts.allow_ff, N_("allow fast-forward")),
@@ -34,6 +34,8 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
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_CMDMODE(0, "rearrange-squash", &command,
38 + N_("rearrange fixup/squash lines"), REARRANGE_SQUASH),
39 OPT_END()
40 };
41
@@ -60,5 +62,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
62 return !!check_todo_list();
63 if (command == SKIP_UNNECESSARY_PICKS && argc == 1)
64 return !!skip_unnecessary_picks();
65 + if (command == REARRANGE_SQUASH && argc == 1)
66 + return !!rearrange_squash();
67 usage_with_options(builtin_rebase_helper_usage, options);
68 }
git-rebase--interactive.sh
+1 -89
@@ -721,94 +721,6 @@ collapse_todo_ids() {
721 git rebase--helper --shorten-ids
722 }
723
724 -# Rearrange the todo list that has both "pick sha1 msg" and
725 -# "pick sha1 fixup!/squash! msg" appears in it so that the latter
726 -# comes immediately after the former, and change "pick" to
727 -# "fixup"/"squash".
728 -#
729 -# Note that if the config has specified a custom instruction format
730 -# each log message will be re-retrieved in order to normalize the
731 -# autosquash arrangement
732 -rearrange_squash () {
733 - format=$(git config --get rebase.instructionFormat)
734 - # extract fixup!/squash! lines and resolve any referenced sha1's
735 - while read -r pick sha1 message
736 - do
737 - test -z "${format}" || message=$(git log -n 1 --format="%s" ${sha1})
738 - case "$message" in
739 - "squash! "*|"fixup! "*)
740 - action="${message%%!*}"
741 - rest=$message
742 - prefix=
743 - # skip all squash! or fixup! (but save for later)
744 - while :
745 - do
746 - case "$rest" in
747 - "squash! "*|"fixup! "*)
748 - prefix="$prefix${rest%%!*},"
749 - rest="${rest#*! }"
750 - ;;
751 - *)
752 - break
753 - ;;
754 - esac
755 - done
756 - printf '%s %s %s %s\n' "$sha1" "$action" "$prefix" "$rest"
757 - # if it's a single word, try to resolve to a full sha1 and
758 - # emit a second copy. This allows us to match on both message
759 - # and on sha1 prefix
760 - if test "${rest#* }" = "$rest"; then
761 - fullsha="$(git rev-parse -q --verify "$rest" 2>/dev/null)"
762 - if test -n "$fullsha"; then
763 - # prefix the action to uniquely identify this line as
764 - # intended for full sha1 match
765 - echo "$sha1 +$action $prefix $fullsha"
766 - fi
767 - fi
768 - esac
769 - done >"$1.sq" <"$1"
770 - test -s "$1.sq" || return
771 -
772 - used=
773 - while read -r pick sha1 message
774 - do
775 - case " $used" in
776 - *" $sha1 "*) continue ;;
777 - esac
778 - printf '%s\n' "$pick $sha1 $message"
779 - test -z "${format}" || message=$(git log -n 1 --format="%s" ${sha1})
780 - used="$used$sha1 "
781 - while read -r squash action msg_prefix msg_content
782 - do
783 - case " $used" in
784 - *" $squash "*) continue ;;
785 - esac
786 - emit=0
787 - case "$action" in
788 - +*)
789 - action="${action#+}"
790 - # full sha1 prefix test
791 - case "$msg_content" in "$sha1"*) emit=1;; esac ;;
792 - *)
793 - # message prefix test
794 - case "$message" in "$msg_content"*) emit=1;; esac ;;
795 - esac
796 - if test $emit = 1; then
797 - if test -n "${format}"
798 - then
799 - msg_content=$(git log -n 1 --format="${format}" ${squash})
800 - else
801 - msg_content="$(echo "$msg_prefix" | sed "s/,/! /g")$msg_content"
802 - fi
803 - printf '%s\n' "$action $squash $msg_content"
804 - used="$used$squash "
805 - fi
806 - done <"$1.sq"
807 - done >"$1.rearranged" <"$1"
808 - cat "$1.rearranged" >"$1"
809 - rm -f "$1.sq" "$1.rearranged"
810 -}
811 -
724 # Add commands after a pick or after a squash/fixup serie
725 # in the todo list.
726 add_exec_commands () {
@@ -1068,7 +980,7 @@ then
980 fi
981
982 test -s "$todo" || echo noop >> "$todo"
1071 -test -n "$autosquash" && rearrange_squash "$todo"
983 +test -z "$autosquash" || git rebase--helper --rearrange-squash || exit
984 test -n "$cmd" && add_exec_commands "$todo"
985
986 todocount=$(git stripspace --strip-comments <"$todo" | wc -l)
sequencer.c
+196
@@ -20,6 +20,7 @@
20 #include "trailer.h"
21 #include "log-tree.h"
22 #include "wt-status.h"
23 +#include "hashmap.h"
24
25 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
26
@@ -2748,3 +2749,198 @@ int skip_unnecessary_picks(void)
2749
2750 return 0;
2751 }
2752 +
2753 +struct subject2item_entry {
2754 + struct hashmap_entry entry;
2755 + int i;
2756 + char subject[FLEX_ARRAY];
2757 +};
2758 +
2759 +static int subject2item_cmp(const void *fndata,
2760 + const struct subject2item_entry *a,
2761 + const struct subject2item_entry *b, const void *key)
2762 +{
2763 + return key ? strcmp(a->subject, key) : strcmp(a->subject, b->subject);
2764 +}
2765 +
2766 +/*
2767 + * Rearrange the todo list that has both "pick commit-id msg" and "pick
2768 + * commit-id fixup!/squash! msg" in it so that the latter is put immediately
2769 + * after the former, and change "pick" to "fixup"/"squash".
2770 + *
2771 + * Note that if the config has specified a custom instruction format, each log
2772 + * message will have to be retrieved from the commit (as the oneline in the
2773 + * script cannot be trusted) in order to normalize the autosquash arrangement.
2774 + */
2775 +int rearrange_squash(void)
2776 +{
2777 + const char *todo_file = rebase_path_todo();
2778 + struct todo_list todo_list = TODO_LIST_INIT;
2779 + struct hashmap subject2item;
2780 + int res = 0, rearranged = 0, *next, *tail, fd, i;
2781 + char **subjects;
2782 +
2783 + fd = open(todo_file, O_RDONLY);
2784 + if (fd < 0)
2785 + return error_errno(_("could not open '%s'"), todo_file);
2786 + if (strbuf_read(&todo_list.buf, fd, 0) < 0) {
2787 + close(fd);
2788 + return error(_("could not read '%s'."), todo_file);
2789 + }
2790 + close(fd);
2791 + if (parse_insn_buffer(todo_list.buf.buf, &todo_list) < 0) {
2792 + todo_list_release(&todo_list);
2793 + return -1;
2794 + }
2795 +
2796 + /*
2797 + * The hashmap maps onelines to the respective todo list index.
2798 + *
2799 + * If any items need to be rearranged, the next[i] value will indicate
2800 + * which item was moved directly after the i'th.
2801 + *
2802 + * In that case, last[i] will indicate the index of the latest item to
2803 + * be moved to appear after the i'th.
2804 + */
2805 + hashmap_init(&subject2item, (hashmap_cmp_fn) subject2item_cmp,
2806 + NULL, todo_list.nr);
2807 + ALLOC_ARRAY(next, todo_list.nr);
2808 + ALLOC_ARRAY(tail, todo_list.nr);
2809 + ALLOC_ARRAY(subjects, todo_list.nr);
2810 + for (i = 0; i < todo_list.nr; i++) {
2811 + struct strbuf buf = STRBUF_INIT;
2812 + struct todo_item *item = todo_list.items + i;
2813 + const char *commit_buffer, *subject, *p;
2814 + size_t subject_len;
2815 + int i2 = -1;
2816 + struct subject2item_entry *entry;
2817 +
2818 + next[i] = tail[i] = -1;
2819 + if (item->command >= TODO_EXEC) {
2820 + subjects[i] = NULL;
2821 + continue;
2822 + }
2823 +
2824 + if (is_fixup(item->command)) {
2825 + todo_list_release(&todo_list);
2826 + return error(_("the script was already rearranged."));
2827 + }
2828 +
2829 + item->commit->util = item;
2830 +
2831 + parse_commit(item->commit);
2832 + commit_buffer = get_commit_buffer(item->commit, NULL);
2833 + find_commit_subject(commit_buffer, &subject);
2834 + format_subject(&buf, subject, " ");
2835 + subject = subjects[i] = strbuf_detach(&buf, &subject_len);
2836 + unuse_commit_buffer(item->commit, commit_buffer);
2837 + if ((skip_prefix(subject, "fixup! ", &p) ||
2838 + skip_prefix(subject, "squash! ", &p))) {
2839 + struct commit *commit2;
2840 +
2841 + for (;;) {
2842 + while (isspace(*p))
2843 + p++;
2844 + if (!skip_prefix(p, "fixup! ", &p) &&
2845 + !skip_prefix(p, "squash! ", &p))
2846 + break;
2847 + }
2848 +
2849 + if ((entry = hashmap_get_from_hash(&subject2item,
2850 + strhash(p), p)))
2851 + /* found by title */
2852 + i2 = entry->i;
2853 + else if (!strchr(p, ' ') &&
2854 + (commit2 =
2855 + lookup_commit_reference_by_name(p)) &&
2856 + commit2->util)
2857 + /* found by commit name */
2858 + i2 = (struct todo_item *)commit2->util
2859 + - todo_list.items;
2860 + else {
2861 + /* copy can be a prefix of the commit subject */
2862 + for (i2 = 0; i2 < i; i2++)
2863 + if (subjects[i2] &&
2864 + starts_with(subjects[i2], p))
2865 + break;
2866 + if (i2 == i)
2867 + i2 = -1;
2868 + }
2869 + }
2870 + if (i2 >= 0) {
2871 + rearranged = 1;
2872 + todo_list.items[i].command =
2873 + starts_with(subject, "fixup!") ?
2874 + TODO_FIXUP : TODO_SQUASH;
2875 + if (next[i2] < 0)
2876 + next[i2] = i;
2877 + else
2878 + next[tail[i2]] = i;
2879 + tail[i2] = i;
2880 + } else if (!hashmap_get_from_hash(&subject2item,
2881 + strhash(subject), subject)) {
2882 + FLEX_ALLOC_MEM(entry, subject, subject, subject_len);
2883 + entry->i = i;
2884 + hashmap_entry_init(entry, strhash(entry->subject));
2885 + hashmap_put(&subject2item, entry);
2886 + }
2887 + }
2888 +
2889 + if (rearranged) {
2890 + struct strbuf buf = STRBUF_INIT;
2891 +
2892 + for (i = 0; i < todo_list.nr; i++) {
2893 + enum todo_command command = todo_list.items[i].command;
2894 + int cur = i;
2895 +
2896 + /*
2897 + * Initially, all commands are 'pick's. If it is a
2898 + * fixup or a squash now, we have rearranged it.
2899 + */
2900 + if (is_fixup(command))
2901 + continue;
2902 +
2903 + while (cur >= 0) {
2904 + int offset = todo_list.items[cur].offset_in_buf;
2905 + int end_offset = cur + 1 < todo_list.nr ?
2906 + todo_list.items[cur + 1].offset_in_buf :
2907 + todo_list.buf.len;
2908 + char *bol = todo_list.buf.buf + offset;
2909 + char *eol = todo_list.buf.buf + end_offset;
2910 +
2911 + /* replace 'pick', by 'fixup' or 'squash' */
2912 + command = todo_list.items[cur].command;
2913 + if (is_fixup(command)) {
2914 + strbuf_addstr(&buf,
2915 + todo_command_info[command].str);
2916 + bol += strcspn(bol, " \t");
2917 + }
2918 +
2919 + strbuf_add(&buf, bol, eol - bol);
2920 +
2921 + cur = next[cur];
2922 + }
2923 + }
2924 +
2925 + fd = open(todo_file, O_WRONLY);
2926 + if (fd < 0)
2927 + res = error_errno(_("could not open '%s'"), todo_file);
2928 + else if (write(fd, buf.buf, buf.len) < 0)
2929 + res = error_errno(_("could not read '%s'."), todo_file);
2930 + else if (ftruncate(fd, buf.len) < 0)
2931 + res = error_errno(_("could not finish '%s'"),
2932 + todo_file);
2933 + close(fd);
2934 + strbuf_release(&buf);
2935 + }
2936 +
2937 + free(next);
2938 + free(tail);
2939 + for (i = 0; i < todo_list.nr; i++)
2940 + free(subjects[i]);
2941 + free(subjects);
2942 + hashmap_free(&subject2item, 1);
2943 + todo_list_release(&todo_list);
2944 +
2945 + return res;
2946 +}
sequencer.h
+1
@@ -51,6 +51,7 @@ int sequencer_make_script(int keep_empty, FILE *out,
51 int transform_todo_ids(int shorten_ids);
52 int check_todo_list(void);
53 int skip_unnecessary_picks(void);
54 +int rearrange_squash(void);
55
56 extern const char sign_off_header[];
57
t/t3415-rebase-autosquash.sh
+1 -1
@@ -290,7 +290,7 @@ set_backup_editor () {
290 test_set_editor "$PWD/backup-editor.sh"
291 }
292
293 -test_expect_failure 'autosquash with multiple empty patches' '
293 +test_expect_success 'autosquash with multiple empty patches' '
294 test_tick &&
295 git commit --allow-empty -m "empty" &&
296 test_tick &&