git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 9/9] rebase -i: rearrange fixup/squash lines using the rebase--helper

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Sep 2, 2016, 16:23 UTC
Message-ID
<3810bd4bddb3b850a78e5d960207157de64a2e56.1472833365.git.johannes.schindelin@gmx.de>
In-Reply-To
<cover.1472833365.git.johannes.schindelin@gmx.de>

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.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/rebase--helper.c     |   6 +-
 git-rebase--interactive.sh   |  90 +-------------------
 sequencer.c                  | 197 +++++++++++++++++++++++++++++++++++++++++++
 sequencer.h                  |   1 +
 t/t3415-rebase-autosquash.sh |   2 +-
 5 files changed, 205 insertions(+), 91 deletions(-)
diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index de3ccd9..e6591f0 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, SKIP_UNNECESSARY_PICKS
+		CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH
 	} command = 0;
 	struct option options[] = {
 		OPT_BOOL(0, "ff", &opts.allow_ff, N_("allow fast-forward")),
@@ -33,6 +33,8 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 			N_("check the todo list"), CHECK_TODO_LIST),
 		OPT_CMDMODE(0, "skip-unnecessary-picks", &command,
 			N_("skip unnecessary picks"), SKIP_UNNECESSARY_PICKS),
+		OPT_CMDMODE(0, "rearrange-squash", &command,
+			N_("rearrange fixup/squash lines"), REARRANGE_SQUASH),
 		OPT_END()
 	};
 
@@ -59,5 +61,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 		return !!check_todo_list();
 	if (command == SKIP_UNNECESSARY_PICKS && argc == 1)
 		return !!skip_unnecessary_picks();
+	if (command == REARRANGE_SQUASH && argc == 1)
+		return !!rearrange_squash();
 	usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index a34ebdc..68a6e6a 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -734,94 +734,6 @@ collapse_todo_ids() {
 	git rebase--helper --shorten-sha1s
 }
 
-# Rearrange the todo list that has both "pick sha1 msg" and
-# "pick sha1 fixup!/squash! msg" appears in it so that the latter
-# comes immediately after the former, and change "pick" to
-# "fixup"/"squash".
-#
-# Note that if the config has specified a custom instruction format
-# each log message will be re-retrieved in order to normalize the
-# autosquash arrangement
-rearrange_squash () {
-	format=$(git config --get rebase.instructionFormat)
-	# extract fixup!/squash! lines and resolve any referenced sha1's
-	while read -r pick sha1 message
-	do
-		test -z "${format}" || message=$(git log -n 1 --format="%s" ${sha1})
-		case "$message" in
-		"squash! "*|"fixup! "*)
-			action="${message%%!*}"
-			rest=$message
-			prefix=
-			# skip all squash! or fixup! (but save for later)
-			while :
-			do
-				case "$rest" in
-				"squash! "*|"fixup! "*)
-					prefix="$prefix${rest%%!*},"
-					rest="${rest#*! }"
-					;;
-				*)
-					break
-					;;
-				esac
-			done
-			printf '%s %s %s %s\n' "$sha1" "$action" "$prefix" "$rest"
-			# if it's a single word, try to resolve to a full sha1 and
-			# emit a second copy. This allows us to match on both message
-			# and on sha1 prefix
-			if test "${rest#* }" = "$rest"; then
-				fullsha="$(git rev-parse -q --verify "$rest" 2>/dev/null)"
-				if test -n "$fullsha"; then
-					# prefix the action to uniquely identify this line as
-					# intended for full sha1 match
-					echo "$sha1 +$action $prefix $fullsha"
-				fi
-			fi
-		esac
-	done >"$1.sq" <"$1"
-	test -s "$1.sq" || return
-
-	used=
-	while read -r pick sha1 message
-	do
-		case " $used" in
-		*" $sha1 "*) continue ;;
-		esac
-		printf '%s\n' "$pick $sha1 $message"
-		test -z "${format}" || message=$(git log -n 1 --format="%s" ${sha1})
-		used="$used$sha1 "
-		while read -r squash action msg_prefix msg_content
-		do
-			case " $used" in
-			*" $squash "*) continue ;;
-			esac
-			emit=0
-			case "$action" in
-			+*)
-				action="${action#+}"
-				# full sha1 prefix test
-				case "$msg_content" in "$sha1"*) emit=1;; esac ;;
-			*)
-				# message prefix test
-				case "$message" in "$msg_content"*) emit=1;; esac ;;
-			esac
-			if test $emit = 1; then
-				if test -n "${format}"
-				then
-					msg_content=$(git log -n 1 --format="${format}" ${squash})
-				else
-					msg_content="$(echo "$msg_prefix" | sed "s/,/! /g")$msg_content"
-				fi
-				printf '%s\n' "$action $squash $msg_content"
-				used="$used$squash "
-			fi
-		done <"$1.sq"
-	done >"$1.rearranged" <"$1"
-	cat "$1.rearranged" >"$1"
-	rm -f "$1.sq" "$1.rearranged"
-}
-
 # Add commands after a pick or after a squash/fixup serie
 # in the todo list.
 add_exec_commands () {
@@ -1084,7 +996,7 @@ then
 fi
 
 test -s "$todo" || echo noop >> "$todo"
-test -n "$autosquash" && rearrange_squash "$todo"
+test -z "$autosquash" || git rebase--helper --rearrange-squash || exit
 test -n "$cmd" && add_exec_commands "$todo"
 
 todocount=$(git stripspace --strip-comments <"$todo" | wc -l)
diff --git a/sequencer.c b/sequencer.c
index 6cc94c9..4b50a4c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -18,6 +18,7 @@
 #include "quote.h"
 #include "log-tree.h"
 #include "wt-status.h"
+#include "hashmap.h"
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
@@ -2664,3 +2665,199 @@ int skip_unnecessary_picks(void)
 
 	return 0;
 }
+
+struct subject2item_entry {
+	struct hashmap_entry entry;
+	int i;
+	char subject[FLEX_ARRAY];
+};
+
+static int subject2item_cmp(const struct subject2item_entry *a,
+	const struct subject2item_entry *b, const void *key)
+{
+	return key ? strcmp(a->subject, key) : strcmp(a->subject, b->subject);
+}
+
+/*
+ * Rearrange the todo list that has both "pick sha1 msg" and "pick sha1
+ * fixup!/squash! msg" in it so that the latter is put immediately after the
+ * former, and change "pick" to "fixup"/"squash".
+ *
+ * Note that if the config has specified a custom instruction format, each log
+ * message will have to be retrieved from the commit (as the oneline in the
+ * script cannot be trusted) in order to normalize the autosquash arrangement.
+ */
+int rearrange_squash(void)
+{
+	const char *todo_file = rebase_path_todo();
+	struct todo_list todo_list = TODO_LIST_INIT;
+	struct hashmap subject2item;
+	int res = 0, rearranged = 0, *next, *tail, fd, i;
+	char **subjects;
+
+	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;
+	}
+
+	/*
+	 * The hashmap maps onelines to the respective todo list index.
+	 *
+	 * If any items need to be rearranged, the next[i] value will indicate
+	 * which item was moved directly after the i'th.
+	 *
+	 * In that case, last[i] will indicate the index of the latest item to
+	 * be moved to appear after the i'th.
+	 */
+	hashmap_init(&subject2item, (hashmap_cmp_fn) subject2item_cmp,
+		     todo_list.nr);
+	ALLOC_ARRAY(next, todo_list.nr);
+	ALLOC_ARRAY(tail, todo_list.nr);
+	ALLOC_ARRAY(subjects, todo_list.nr);
+	for (i = 0; i < todo_list.nr; i++) {
+		struct strbuf buf = STRBUF_INIT;
+		struct todo_item *item = todo_list.items + i;
+		const char *commit_buffer, *subject, *p;
+		int i2 = -1;
+		struct subject2item_entry *entry;
+
+		next[i] = tail[i] = -1;
+		if (item->command >= TODO_EXEC) {
+			subjects[i] = NULL;
+			continue;
+		}
+
+		if (is_fixup(item->command)) {
+			todo_list_release(&todo_list);
+			return error(_("The script was already rearranged."));
+		}
+
+		item->commit->util = item;
+
+		parse_commit(item->commit);
+		commit_buffer = get_commit_buffer(item->commit, NULL);
+		find_commit_subject(commit_buffer, &subject);
+		format_subject(&buf, subject, " ");
+		subject = subjects[i] = buf.buf;
+		unuse_commit_buffer(item->commit, commit_buffer);
+		if ((skip_prefix(subject, "fixup! ", &p) ||
+		     skip_prefix(subject, "squash! ", &p))) {
+			struct commit *commit2;
+
+			for (;;) {
+				while (isspace(*p))
+					p++;
+				if (!skip_prefix(p, "fixup! ", &p) &&
+				    !skip_prefix(p, "squash! ", &p))
+					break;
+			}
+
+			if ((entry = hashmap_get_from_hash(&subject2item,
+							   strhash(p), p)))
+				/* found by title */
+				i2 = entry->i;
+			else if (!strchr(p, ' ') &&
+				 (commit2 =
+				  lookup_commit_reference_by_name(p)) &&
+				 commit2->util)
+				/* found by commit name */
+				i2 = (struct todo_item *)commit2->util
+					- todo_list.items;
+			else {
+				/* copy can be a prefix of the commit subject */
+				for (i2 = 0; i2 < i; i2++)
+					if (subjects[i2] &&
+					    starts_with(subjects[i2], p))
+						break;
+				if (i2 == i)
+					i2 = -1;
+			}
+		}
+		if (i2 >= 0) {
+			rearranged = 1;
+			todo_list.items[i].command =
+				starts_with(subject, "fixup!") ?
+				TODO_FIXUP : TODO_SQUASH;
+			if (next[i2] < 0)
+				next[i2] = i;
+			else
+				next[tail[i2]] = i;
+			tail[i2] = i;
+		}
+		else if (!hashmap_get_from_hash(&subject2item,
+						strhash(subject), subject)) {
+			FLEX_ALLOC_MEM(entry, subject, buf.buf, buf.len);
+			entry->i = i;
+			hashmap_entry_init(entry, strhash(entry->subject));
+			hashmap_put(&subject2item, entry);
+		}
+		strbuf_detach(&buf, NULL);
+	}
+
+	if (rearranged) {
+		struct strbuf buf = STRBUF_INIT;
+		char *format = NULL;
+
+		git_config_get_string("rebase.instructionFormat", &format);
+		for (i = 0; i < todo_list.nr; i++) {
+			enum todo_command command = todo_list.items[i].command;
+			int cur = i;
+
+			/*
+			 * Initially, all commands are 'pick's. If it is a
+			 * fixup or a squash now, we have rearranged it.
+			 */
+			if (is_fixup(command))
+				continue;
+
+			while (cur >= 0) {
+				int offset = todo_list.items[cur].offset_in_buf;
+				int end_offset = cur + 1 < todo_list.nr ?
+					todo_list.items[cur + 1].offset_in_buf :
+					todo_list.buf.len;
+				char *bol = todo_list.buf.buf + offset;
+				char *eol = todo_list.buf.buf + end_offset;
+
+				/* replace 'pick', by 'fixup' or 'squash' */
+				command = todo_list.items[cur].command;
+				if (is_fixup(command)) {
+					strbuf_addstr(&buf,
+						todo_command_info[command].str);
+					bol += strcspn(bol, " \t");
+				}
+
+				strbuf_add(&buf, bol, eol - bol);
+
+				cur = next[cur];
+			}
+		}
+
+		fd = open(todo_file, O_WRONLY);
+		if (fd < 0)
+			res |= error_errno(_("Could not open %s"), todo_file);
+		else if (write(fd, buf.buf, buf.len) < 0)
+			res |= error_errno(_("Could not read %s."), todo_file);
+		else if (ftruncate(fd, buf.len) < 0)
+			res |= error_errno(_("Could not finish %s"), todo_file);
+		close(fd);
+		strbuf_release(&buf);
+	}
+
+	free(next);
+	free(tail);
+	for (i = 0; i < todo_list.nr; i++)
+		free(subjects[i]);
+	free(subjects);
+	hashmap_free(&subject2item, 1);
+	todo_list_release(&todo_list);
+
+	return res;
+}
diff --git a/sequencer.h b/sequencer.h
index 4da215c..a3aa3cb 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -64,6 +64,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);
+int rearrange_squash(void);
 
 extern const char sign_off_header[];
 
diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index 9fd629a..b9e2600 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -278,7 +278,7 @@ set_backup_editor () {
 	test_set_editor "$PWD/backup-editor.sh"
 }
 
-test_expect_failure 'autosquash with multiple empty patches' '
+test_expect_success 'autosquash with multiple empty patches' '
 	test_tick &&
 	git commit --allow-empty -m "empty" &&
 	test_tick &&
-- 
2.9.3.windows.3
Previous: Johannes SchindelinNext: Josh Triplett
Message 13 of 100 in “The final building block for a faster rebase -i”
  1. 0/9 The final building block for a faster rebase -iJohannes Schindelin, Sep 2, 2016
  2. 1/9 rebase -i: generate the script via rebase--helperJohannes Schindelin, Sep 2, 2016
  3. 2/9 rebase -i: remove useless indentationJohannes Schindelin, Sep 2, 2016
  4. 3/9 rebase -i: do not invent onelines when expanding/collapsing SHA-1sJohannes Schindelin, Sep 2, 2016
  5. 4/9 rebase -i: also expand/collapse the SHA-1s via the rebase--helperJohannes Schindelin, Sep 2, 2016
  6. Dennis KaarsemakerSep 2, 2016
  7. Johannes SchindelinSep 3, 2016
  8. 5/9 t3404: relax rebase.missingCommitsCheck testsJohannes Schindelin, Sep 2, 2016
  9. 6/9 rebase -i: check for missing commits in the rebase--helperJohannes Schindelin, Sep 2, 2016
  10. Dennis KaarsemakerSep 2, 2016
  11. 8/9 t3415: test fixup with wrapped onelineJohannes Schindelin, Sep 2, 2016
  12. 7/9 rebase -i: skip unnecessary picks using the rebase--helperJohannes Schindelin, Sep 2, 2016
  13. 9/9 rebase -i: rearrange fixup/squash lines using the rebase--helperJohannes Schindelin, Sep 2, 2016
  14. Josh TriplettSep 3, 2016
  15. Johannes SchindelinSep 4, 2016
  16. 0/9 The final building block for a faster rebase -iJohannes Schindelin, Apr 25, 2017
  17. 3/9 rebase -i: do not invent onelines when expanding/collapsing SHA-1sJohannes Schindelin, Apr 25, 2017
  18. 1/9 rebase -i: generate the script via rebase--helperJohannes Schindelin, Apr 25, 2017
  19. Jeff KingApr 26, 2017
  20. Johannes SchindelinApr 26, 2017
  21. 2/9 rebase -i: remove useless indentationJohannes Schindelin, Apr 25, 2017
  22. 5/9 t3404: relax rebase.missingCommitsCheck testsJohannes Schindelin, Apr 25, 2017
  23. 4/9 rebase -i: also expand/collapse the SHA-1s via the rebase--helperJohannes Schindelin, Apr 25, 2017
  24. 8/9 t3415: test fixup with wrapped onelineJohannes Schindelin, Apr 25, 2017
  25. 6/9 rebase -i: check for missing commits in the rebase--helperJohannes Schindelin, Apr 25, 2017
  26. 9/9 rebase -i: rearrange fixup/squash lines using the rebase--helperJohannes Schindelin, Apr 25, 2017
  27. 7/9 rebase -i: skip unnecessary picks using the rebase--helperJohannes Schindelin, Apr 25, 2017
  28. Jeff KingApr 26, 2017
  29. Johannes SchindelinApr 26, 2017
  30. Junio C HamanoApr 26, 2017
  31. 0/9 The final building block for a faster rebase -iJohannes Schindelin, Apr 26, 2017
  32. 1/9 rebase -i: generate the script via rebase--helperJohannes Schindelin, Apr 26, 2017
  33. Junio C HamanoApr 27, 2017
  34. Johannes SchindelinApr 27, 2017
  35. Junio C HamanoApr 28, 2017
  36. Junio C HamanoApr 28, 2017
  37. Johannes SchindelinApr 28, 2017
  38. Junio C HamanoMay 1, 2017
  39. Johannes SchindelinMay 1, 2017
  40. Phillip WoodApr 28, 2017
  41. Johannes SchindelinApr 28, 2017
  42. Phillip WoodMay 1, 2017
  43. Johannes SchindelinMay 1, 2017
  44. Junio C HamanoMay 1, 2017
  45. Johannes SchindelinMay 1, 2017
  46. 2/9 rebase -i: remove useless indentationJohannes Schindelin, Apr 26, 2017
  47. 3/9 rebase -i: do not invent onelines when expanding/collapsing SHA-1sJohannes Schindelin, Apr 26, 2017
  48. 5/9 t3404: relax rebase.missingCommitsCheck testsJohannes Schindelin, Apr 26, 2017
  49. Junio C HamanoApr 27, 2017
  50. Johannes SchindelinApr 27, 2017
  51. 4/9 rebase -i: also expand/collapse the SHA-1s via the rebase--helperJohannes Schindelin, Apr 26, 2017
  52. Junio C HamanoApr 27, 2017
  53. Junio C HamanoApr 27, 2017
  54. Johannes SchindelinApr 27, 2017
  55. Junio C HamanoApr 28, 2017
  56. Johannes SchindelinApr 28, 2017
  57. 6/9 rebase -i: check for missing commits in the rebase--helperJohannes Schindelin, Apr 26, 2017
  58. Junio C HamanoApr 27, 2017
  59. Johannes SchindelinApr 28, 2017
  60. 7/9 rebase -i: skip unnecessary picks using the rebase--helperJohannes Schindelin, Apr 26, 2017
  61. 8/9 t3415: test fixup with wrapped onelineJohannes Schindelin, Apr 26, 2017
  62. 9/9 rebase -i: rearrange fixup/squash lines using the rebase--helperJohannes Schindelin, Apr 26, 2017
  63. 00/10 The final building block for a faster rebase -iJohannes Schindelin, Apr 28, 2017
  64. 01/10 t3415: verify that an empty instructionFormat is handled as beforeJohannes Schindelin, Apr 28, 2017
  65. 02/10 rebase -i: generate the script via rebase--helperJohannes Schindelin, Apr 28, 2017
  66. 02/10 rebase -i: generate the script via rebase--helperLiam Beguin, May 26, 2017
  67. Johannes SchindelinMay 29, 2017
  68. liam BeguinMay 30, 2017
  69. liam BeguinMay 30, 2017
  70. Junio C HamanoMay 29, 2017
  71. Johannes SchindelinMay 29, 2017
  72. Junio C HamanoMay 30, 2017
  73. Johannes SchindelinMay 30, 2017
  74. revision API design, was Re: [PATCH v4 02/10] rebase -i: generate the script via rebase--helperJohannes Schindelin, May 30, 2017
  75. Junio C HamanoMay 30, 2017
  76. Junio C HamanoJun 1, 2017
  77. 03/10 rebase -i: remove useless indentationJohannes Schindelin, Apr 28, 2017
  78. 03/10 rebase -i: remove useless indentationLiam Beguin, May 26, 2017
  79. Stefan BellerMay 26, 2017
  80. liam BeguinMay 27, 2017
  81. 04/10 rebase -i: do not invent onelines when expanding/collapsing SHA-1sJohannes Schindelin, Apr 28, 2017
  82. 05/10 rebase -i: also expand/collapse the SHA-1s via the rebase--helperJohannes Schindelin, Apr 28, 2017
  83. 05/10 rebase -i: also expand/collapse the SHA-1s via the rebase--helperLiam Beguin, May 26, 2017
  84. Johannes SchindelinMay 29, 2017
  85. 06/10 t3404: relax rebase.missingCommitsCheck testsJohannes Schindelin, Apr 28, 2017
  86. 07/10 rebase -i: check for missing commits in the rebase--helperJohannes Schindelin, Apr 28, 2017
  87. 08/10 rebase -i: skip unnecessary picks using the rebase--helperJohannes Schindelin, Apr 28, 2017
  88. 09/10 t3415: test fixup with wrapped onelineJohannes Schindelin, Apr 28, 2017
  89. 10/10 rebase -i: rearrange fixup/squash lines using the rebase--helperJohannes Schindelin, Apr 28, 2017
  90. 10/10 rebase -i: rearrange fixup/squash lines using the rebase--helperLiam Beguin, May 26, 2017
  91. Johannes SchindelinMay 29, 2017
  92. 00/10 The final building block for a faster rebase -iLiam Beguin, May 26, 2017
  93. René ScharfeMay 27, 2017
  94. Johannes SchindelinMay 29, 2017
  95. Ævar Arnfjörð BjarmasonMay 29, 2017
  96. Johannes SchindelinMay 30, 2017
  97. Ævar Arnfjörð BjarmasonMay 30, 2017
  98. Ævar Arnfjörð BjarmasonMay 31, 2017
  99. Johannes SchindelinMay 29, 2017
  100. Junio C HamanoMay 29, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.