git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: Alban Gruin <alban.gruin@gmail.com>
To: git@vger.kernel.org
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Phillip Wood <phillip.wood@talktalk.net>,
	Junio C Hamano <gitster@pobox.com>,
	Alban Gruin <alban.gruin@gmail.com>
Subject: [PATCH v3 07/16] sequencer: refactor rearrange_squash() to work on a todo_list
Date: Fri,  9 Nov 2018 09:07:56 +0100
Message-ID: <20181109080805.6350-8-alban.gruin@gmail.com> (raw)
In-Reply-To: <20181109080805.6350-1-alban.gruin@gmail.com>

This refactors rearrange_squash() to work on a todo_list to avoid
redundant reads and writes.  The function is renamed
todo_list_rearrange_squash().

The old version created a new buffer, which was directly written to the
disk.  This new version creates a new item list by just memcpying items
from the old item list, without creating a new buffer.  This eliminates
the need to reparse the todo list, but this also means its buffer cannot
be directly written to the disk.

As rebase -p still need to check the todo list from the disk, a new
function is introduced, rearrange_squash_in_todo_file().

complete_action() still uses rearrange_squash_in_todo_file() for now.
This will be changed in a future commit.

Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
---
 builtin/rebase--interactive.c |  2 +-
 sequencer.c                   | 87 +++++++++++++++++------------------
 sequencer.h                   |  2 +-
 3 files changed, 45 insertions(+), 46 deletions(-)

diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c
index 1fb5a43c0d..a6d83a684e 100644
--- a/builtin/rebase--interactive.c
+++ b/builtin/rebase--interactive.c
@@ -265,7 +265,7 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
 		ret = check_todo_list_from_file();
 		break;
 	case REARRANGE_SQUASH:
-		ret = rearrange_squash();
+		ret = rearrange_squash_in_todo_file();
 		break;
 	case ADD_EXEC:
 		ret = sequencer_add_exec_commands(&commands);
diff --git a/sequencer.c b/sequencer.c
index 11692d0b98..fce97e5f11 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4722,7 +4722,7 @@ int complete_action(struct replay_opts *opts, unsigned flags,
 	    write_message("noop\n", 5, todo_file, 0))
 		return -1;
 
-	if (autosquash && rearrange_squash())
+	if (autosquash && rearrange_squash_in_todo_file())
 		return -1;
 
 	if (commands->nr)
@@ -4828,21 +4828,13 @@ define_commit_slab(commit_todo_item, struct todo_item *);
  * 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)
+static int todo_list_rearrange_squash(struct todo_list *todo_list)
 {
-	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, i;
+	int rearranged = 0, *next, *tail, i, nr = 0, alloc = 0;
 	char **subjects;
 	struct commit_todo_item commit_todo;
-
-	if (strbuf_read_file_or_whine(&todo_list.buf, todo_file) < 0)
-		return -1;
-	if (todo_list_parse_insn_buffer(todo_list.buf.buf, &todo_list) < 0) {
-		todo_list_release(&todo_list);
-		return -1;
-	}
+	struct todo_item *items = NULL;
 
 	init_commit_todo_item(&commit_todo);
 	/*
@@ -4855,13 +4847,13 @@ int rearrange_squash(void)
 	 * be moved to appear after the i'th.
 	 */
 	hashmap_init(&subject2item, (hashmap_cmp_fn) subject2item_cmp,
-		     NULL, 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++) {
+		     NULL, 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;
+		struct todo_item *item = todo_list->items + i;
 		const char *commit_buffer, *subject, *p;
 		size_t subject_len;
 		int i2 = -1;
@@ -4874,7 +4866,6 @@ int rearrange_squash(void)
 		}
 
 		if (is_fixup(item->command)) {
-			todo_list_release(&todo_list);
 			clear_commit_todo_item(&commit_todo);
 			return error(_("the script was already rearranged."));
 		}
@@ -4909,7 +4900,7 @@ int rearrange_squash(void)
 				 *commit_todo_item_at(&commit_todo, commit2))
 				/* found by commit name */
 				i2 = *commit_todo_item_at(&commit_todo, commit2)
-					- todo_list.items;
+					- todo_list->items;
 			else {
 				/* copy can be a prefix of the commit subject */
 				for (i2 = 0; i2 < i; i2++)
@@ -4922,7 +4913,7 @@ int rearrange_squash(void)
 		}
 		if (i2 >= 0) {
 			rearranged = 1;
-			todo_list.items[i].command =
+			todo_list->items[i].command =
 				starts_with(subject, "fixup!") ?
 				TODO_FIXUP : TODO_SQUASH;
 			if (next[i2] < 0)
@@ -4940,10 +4931,8 @@ int rearrange_squash(void)
 	}
 
 	if (rearranged) {
-		struct strbuf buf = STRBUF_INIT;
-
-		for (i = 0; i < todo_list.nr; i++) {
-			enum todo_command command = todo_list.items[i].command;
+		for (i = 0; i < todo_list->nr; i++) {
+			enum todo_command command = todo_list->items[i].command;
 			int cur = i;
 
 			/*
@@ -4954,37 +4943,47 @@ int rearrange_squash(void)
 				continue;
 
 			while (cur >= 0) {
-				const char *bol =
-					get_item_line(&todo_list, cur);
-				const char *eol =
-					get_item_line(&todo_list, cur + 1);
-
-				/* 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);
-
+				ALLOC_GROW(items, nr + 1, alloc);
+				items[nr++] = todo_list->items[cur];
 				cur = next[cur];
 			}
 		}
 
-		res = rewrite_file(todo_file, buf.buf, buf.len);
-		strbuf_release(&buf);
+		FREE_AND_NULL(todo_list->items);
+		todo_list->items = items;
+		todo_list->nr = nr;
+		todo_list->alloc = alloc;
 	}
 
 	free(next);
 	free(tail);
-	for (i = 0; i < todo_list.nr; i++)
+	for (i = 0; i < todo_list->nr; i++)
 		free(subjects[i]);
 	free(subjects);
 	hashmap_free(&subject2item, 1);
-	todo_list_release(&todo_list);
 
 	clear_commit_todo_item(&commit_todo);
+
+	return 0;
+}
+
+int rearrange_squash_in_todo_file(void)
+{
+	const char *todo_file = rebase_path_todo();
+	struct todo_list todo_list = TODO_LIST_INIT;
+	int res = 0;
+
+	if (strbuf_read_file_or_whine(&todo_list.buf, todo_file) < 0)
+		return -1;
+	if (todo_list_parse_insn_buffer(todo_list.buf.buf, &todo_list) < 0) {
+		todo_list_release(&todo_list);
+		return -1;
+	}
+
+	res = todo_list_rearrange_squash(&todo_list);
+	if (!res)
+		res = todo_list_write_to_file(&todo_list, todo_file, NULL, NULL, -1, 0);
+
+	todo_list_release(&todo_list);
 	return res;
 }
diff --git a/sequencer.h b/sequencer.h
index d7b52044bd..1cd9d7f1e2 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -147,7 +147,7 @@ int complete_action(struct replay_opts *opts, unsigned flags,
 		    const char *shortrevisions, const char *onto_name,
 		    const char *onto, const char *orig_head, struct string_list *commands,
 		    unsigned autosquash);
-int rearrange_squash(void);
+int rearrange_squash_in_todo_file(void);
 
 extern const char sign_off_header[];
 
-- 
2.19.1.872.ga867da739e


  parent reply index

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-07 19:54 [PATCH 00/15] sequencer: refactor functions working " Alban Gruin
2018-10-07 19:54 ` [PATCH 01/15] sequencer: clear the number of items of a todo_list before parsing Alban Gruin
2018-10-07 19:54 ` [PATCH 02/15] sequencer: make the todo_list structure public Alban Gruin
2018-10-07 19:54 ` [PATCH 03/15] sequencer: refactor check_todo_list() to work on a todo_list Alban Gruin
2018-10-11 11:23   ` Phillip Wood
2018-10-07 19:54 ` [PATCH 04/15] sequencer: refactor sequencer_add_exec_commands() " Alban Gruin
2018-10-11 11:25   ` Phillip Wood
2018-10-11 16:57     ` Alban Gruin
2018-10-12  9:54       ` Phillip Wood
2018-10-12 12:23         ` Alban Gruin
2018-10-07 19:54 ` [PATCH 05/15] sequencer: refactor rearrange_squash() " Alban Gruin
2018-10-07 19:54 ` [PATCH 06/15] sequencer: refactor transform_todos() " Alban Gruin
2018-10-07 19:54 ` [PATCH 07/15] sequencer: make sequencer_make_script() write its script to a strbuf Alban Gruin
2018-10-12 10:01   ` SZEDER Gábor
2018-10-19  8:16     ` Junio C Hamano
2018-10-19  9:27       ` SZEDER Gábor
2018-10-07 19:54 ` [PATCH 08/15] sequencer: change complete_action() to use the refactored functions Alban Gruin
2018-10-11 13:51   ` Phillip Wood
2018-10-11 17:06     ` Alban Gruin
2018-10-07 19:54 ` [PATCH 09/15] sequencer: refactor skip_unnecessary_picks() to work on a todo_list Alban Gruin
2018-10-07 19:54 ` [PATCH 10/15] rebase-interactive: use todo_list_transform() in edit_todo_list() Alban Gruin
2018-10-11 15:16   ` Phillip Wood
2018-10-11 19:58     ` Alban Gruin
2018-10-07 19:54 ` [PATCH 11/15] rebase-interactive: append_todo_help() changes Alban Gruin
2018-10-07 19:54 ` [PATCH 12/15] rebase-interactive: rewrite edit_todo_list() to handle the initial edit Alban Gruin
2018-10-07 19:54 ` [PATCH 13/15] sequencer: use edit_todo_list() in complete_action() Alban Gruin
2018-10-07 19:54 ` [PATCH 14/15] sequencer: fix a call to error() in transform_todo_file() Alban Gruin
2018-10-07 19:54 ` [PATCH 15/15] rebase--interactive: move transform_todo_file() to rebase--interactive.c Alban Gruin
2018-10-07 20:51 ` [PATCH 00/15] sequencer: refactor functions working on a todo_list Alban Gruin
2018-10-27 21:29 ` [PATCH v2 00/16] " Alban Gruin
2018-10-27 21:29   ` [PATCH v2 01/16] sequencer: changes in parse_insn_buffer() Alban Gruin
2018-10-27 21:29   ` [PATCH v2 02/16] sequencer: make the todo_list structure public Alban Gruin
2018-10-27 21:29   ` [PATCH v2 03/16] sequencer: refactor transform_todos() to work on a todo_list Alban Gruin
2018-10-27 21:29   ` [PATCH v2 04/16] sequencer: introduce todo_list_write_to_file() Alban Gruin
2018-10-30 16:28     ` Phillip Wood
2018-11-01 23:31       ` Alban Gruin
2018-10-27 21:29   ` [PATCH v2 05/16] sequencer: refactor check_todo_list() to work on a todo_list Alban Gruin
2018-10-27 21:29   ` [PATCH v2 06/16] sequencer: refactor sequencer_add_exec_commands() " Alban Gruin
2018-10-30 16:47     ` Phillip Wood
2018-11-01 23:31       ` Alban Gruin
2018-11-02 10:09         ` Phillip Wood
2018-11-02 16:26           ` Alban Gruin
2018-11-02 17:09             ` Phillip Wood
2018-10-27 21:29   ` [PATCH v2 07/16] sequencer: refactor rearrange_squash() " Alban Gruin
2018-10-27 21:29   ` [PATCH v2 08/16] sequencer: make sequencer_make_script() write its script to a strbuf Alban Gruin
2018-10-27 21:29   ` [PATCH v2 09/16] sequencer: change complete_action() to use the refactored functions Alban Gruin
2018-10-27 21:29   ` [PATCH v2 10/16] sequencer: refactor skip_unnecessary_picks() to work on a todo_list Alban Gruin
2018-10-27 21:29   ` [PATCH v2 11/16] rebase-interactive: use todo_list_write_to_file() in edit_todo_list() Alban Gruin
2018-10-27 21:29   ` [PATCH v2 12/16] rebase-interactive: append_todo_help() changes Alban Gruin
2018-10-27 21:29   ` [PATCH v2 13/16] rebase-interactive: rewrite edit_todo_list() to handle the initial edit Alban Gruin
2018-10-27 21:29   ` [PATCH v2 14/16] sequencer: use edit_todo_list() in complete_action() Alban Gruin
2018-10-27 21:29   ` [PATCH v2 15/16] sequencer: fix a call to error() in transform_todo_file() Alban Gruin
2018-10-27 21:29   ` [PATCH v2 16/16] rebase--interactive: move transform_todo_file() to rebase--interactive.c Alban Gruin
2018-10-29  3:05   ` [PATCH v2 00/16] sequencer: refactor functions working on a todo_list Junio C Hamano
2018-10-29 15:34     ` Alban Gruin
2018-11-09  8:07   ` [PATCH v3 " Alban Gruin
2018-11-09  8:07     ` [PATCH v3 01/16] sequencer: changes in parse_insn_buffer() Alban Gruin
2018-11-09  8:07     ` [PATCH v3 02/16] sequencer: make the todo_list structure public Alban Gruin
2018-11-09  8:07     ` [PATCH v3 03/16] sequencer: refactor transform_todos() to work on a todo_list Alban Gruin
2018-11-09  8:07     ` [PATCH v3 04/16] sequencer: introduce todo_list_write_to_file() Alban Gruin
2018-11-09  8:07     ` [PATCH v3 05/16] sequencer: refactor check_todo_list() to work on a todo_list Alban Gruin
2018-11-09  8:07     ` [PATCH v3 06/16] sequencer: refactor sequencer_add_exec_commands() " Alban Gruin
2018-11-09  8:07     ` Alban Gruin [this message]
2018-11-09  8:07     ` [PATCH v3 08/16] sequencer: make sequencer_make_script() write its script to a strbuf Alban Gruin
2018-11-09  8:07     ` [PATCH v3 09/16] sequencer: change complete_action() to use the refactored functions Alban Gruin
2018-11-09  8:07     ` [PATCH v3 10/16] sequencer: refactor skip_unnecessary_picks() to work on a todo_list Alban Gruin
2018-11-09  8:08     ` [PATCH v3 11/16] rebase-interactive: use todo_list_write_to_file() in edit_todo_list() Alban Gruin
2018-11-09  8:08     ` [PATCH v3 12/16] rebase-interactive: append_todo_help() changes Alban Gruin
2018-11-09  8:08     ` [PATCH v3 13/16] rebase-interactive: rewrite edit_todo_list() to handle the initial edit Alban Gruin
2018-11-09  8:08     ` [PATCH v3 14/16] sequencer: use edit_todo_list() in complete_action() Alban Gruin
2018-11-09  8:08     ` [PATCH v3 15/16] sequencer: fix a call to error() in transform_todo_file() Alban Gruin
2018-11-09  8:08     ` [PATCH v3 16/16] rebase--interactive: move transform_todo_file() to rebase--interactive.c Alban Gruin

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181109080805.6350-8-alban.gruin@gmail.com \
    --to=alban.gruin@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=phillip.wood@talktalk.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox