sequencer: refactor rearrange_squash() to work on a todo_list

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

Alban Gruin committed Mar 5, 2019 at 20:17 UTC f2a04904be6584f1ec783ed5d3c425026bcf908f
3 files changed +49 -47
builtin/rebase--interactive.c
+1 -1
@@ -268,7 +268,7 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
268 ret = check_todo_list_from_file(the_repository);
269 break;
270 case REARRANGE_SQUASH:
271 - ret = rearrange_squash(the_repository);
271 + ret = rearrange_squash_in_todo_file(the_repository);
272 break;
273 case ADD_EXEC:
274 ret = sequencer_add_exec_commands(the_repository, &commands);
sequencer.c
+47 -45
@@ -4840,7 +4840,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
4840 write_message("noop\n", 5, todo_file, 0))
4841 return -1;
4842
4843 - if (autosquash && rearrange_squash(r))
4843 + if (autosquash && rearrange_squash_in_todo_file(r))
4844 return -1;
4845
4846 if (commands->nr)
@@ -4946,21 +4946,13 @@ define_commit_slab(commit_todo_item, struct todo_item *);
4946 * message will have to be retrieved from the commit (as the oneline in the
4947 * script cannot be trusted) in order to normalize the autosquash arrangement.
4948 */
4949 -int rearrange_squash(struct repository *r)
4949 +static int todo_list_rearrange_squash(struct todo_list *todo_list)
4950 {
4951 - const char *todo_file = rebase_path_todo();
4952 - struct todo_list todo_list = TODO_LIST_INIT;
4951 struct hashmap subject2item;
4954 - int res = 0, rearranged = 0, *next, *tail, i;
4952 + int rearranged = 0, *next, *tail, i, nr = 0, alloc = 0;
4953 char **subjects;
4954 struct commit_todo_item commit_todo;
4957 -
4958 - if (strbuf_read_file_or_whine(&todo_list.buf, todo_file) < 0)
4959 - return -1;
4960 - if (todo_list_parse_insn_buffer(r, todo_list.buf.buf, &todo_list) < 0) {
4961 - todo_list_release(&todo_list);
4962 - return -1;
4963 - }
4955 + struct todo_item *items = NULL;
4956
4957 init_commit_todo_item(&commit_todo);
4958 /*
@@ -4973,13 +4965,13 @@ int rearrange_squash(struct repository *r)
4965 * be moved to appear after the i'th.
4966 */
4967 hashmap_init(&subject2item, (hashmap_cmp_fn) subject2item_cmp,
4976 - NULL, todo_list.nr);
4977 - ALLOC_ARRAY(next, todo_list.nr);
4978 - ALLOC_ARRAY(tail, todo_list.nr);
4979 - ALLOC_ARRAY(subjects, todo_list.nr);
4980 - for (i = 0; i < todo_list.nr; i++) {
4968 + NULL, todo_list->nr);
4969 + ALLOC_ARRAY(next, todo_list->nr);
4970 + ALLOC_ARRAY(tail, todo_list->nr);
4971 + ALLOC_ARRAY(subjects, todo_list->nr);
4972 + for (i = 0; i < todo_list->nr; i++) {
4973 struct strbuf buf = STRBUF_INIT;
4982 - struct todo_item *item = todo_list.items + i;
4974 + struct todo_item *item = todo_list->items + i;
4975 const char *commit_buffer, *subject, *p;
4976 size_t subject_len;
4977 int i2 = -1;
@@ -4992,7 +4984,6 @@ int rearrange_squash(struct repository *r)
4984 }
4985
4986 if (is_fixup(item->command)) {
4995 - todo_list_release(&todo_list);
4987 clear_commit_todo_item(&commit_todo);
4988 return error(_("the script was already rearranged."));
4989 }
@@ -5027,7 +5018,7 @@ int rearrange_squash(struct repository *r)
5018 *commit_todo_item_at(&commit_todo, commit2))
5019 /* found by commit name */
5020 i2 = *commit_todo_item_at(&commit_todo, commit2)
5030 - - todo_list.items;
5021 + - todo_list->items;
5022 else {
5023 /* copy can be a prefix of the commit subject */
5024 for (i2 = 0; i2 < i; i2++)
@@ -5040,7 +5031,7 @@ int rearrange_squash(struct repository *r)
5031 }
5032 if (i2 >= 0) {
5033 rearranged = 1;
5043 - todo_list.items[i].command =
5034 + todo_list->items[i].command =
5035 starts_with(subject, "fixup!") ?
5036 TODO_FIXUP : TODO_SQUASH;
5037 if (next[i2] < 0)
@@ -5058,10 +5049,8 @@ int rearrange_squash(struct repository *r)
5049 }
5050
5051 if (rearranged) {
5061 - struct strbuf buf = STRBUF_INIT;
5062 -
5063 - for (i = 0; i < todo_list.nr; i++) {
5064 - enum todo_command command = todo_list.items[i].command;
5052 + for (i = 0; i < todo_list->nr; i++) {
5053 + enum todo_command command = todo_list->items[i].command;
5054 int cur = i;
5055
5056 /*
@@ -5072,37 +5061,50 @@ int rearrange_squash(struct repository *r)
5061 continue;
5062
5063 while (cur >= 0) {
5075 - const char *bol =
5076 - get_item_line(&todo_list, cur);
5077 - const char *eol =
5078 - get_item_line(&todo_list, cur + 1);
5079 -
5080 - /* replace 'pick', by 'fixup' or 'squash' */
5081 - command = todo_list.items[cur].command;
5082 - if (is_fixup(command)) {
5083 - strbuf_addstr(&buf,
5084 - todo_command_info[command].str);
5085 - bol += strcspn(bol, " \t");
5086 - }
5087 -
5088 - strbuf_add(&buf, bol, eol - bol);
5089 -
5064 + ALLOC_GROW(items, nr + 1, alloc);
5065 + items[nr++] = todo_list->items[cur];
5066 cur = next[cur];
5067 }
5068 }
5069
5094 - res = rewrite_file(todo_file, buf.buf, buf.len);
5095 - strbuf_release(&buf);
5070 + FREE_AND_NULL(todo_list->items);
5071 + todo_list->items = items;
5072 + todo_list->nr = nr;
5073 + todo_list->alloc = alloc;
5074 }
5075
5076 free(next);
5077 free(tail);
5100 - for (i = 0; i < todo_list.nr; i++)
5078 + for (i = 0; i < todo_list->nr; i++)
5079 free(subjects[i]);
5080 free(subjects);
5081 hashmap_free(&subject2item, 1);
5104 - todo_list_release(&todo_list);
5082
5083 clear_commit_todo_item(&commit_todo);
5107 - return res;
5084 +
5085 + return 0;
5086 +}
5087 +
5088 +int rearrange_squash_in_todo_file(struct repository *r)
5089 +{
5090 + const char *todo_file = rebase_path_todo();
5091 + struct todo_list todo_list = TODO_LIST_INIT;
5092 + int res = 0;
5093 +
5094 + if (strbuf_read_file_or_whine(&todo_list.buf, todo_file) < 0)
5095 + return -1;
5096 + if (todo_list_parse_insn_buffer(r, todo_list.buf.buf, &todo_list) < 0) {
5097 + todo_list_release(&todo_list);
5098 + return -1;
5099 + }
5100 +
5101 + res = todo_list_rearrange_squash(&todo_list);
5102 + if (!res)
5103 + res = todo_list_write_to_file(r, &todo_list, todo_file, NULL, NULL, -1, 0);
5104 +
5105 + todo_list_release(&todo_list);
5106 +
5107 + if (res)
5108 + return error_errno(_("could not write '%s'."), todo_file);
5109 + return 0;
5110 }
sequencer.h
+1 -1
@@ -154,7 +154,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
154 const char *shortrevisions, const char *onto_name,
155 const char *onto, const char *orig_head, struct string_list *commands,
156 unsigned autosquash);
157 -int rearrange_squash(struct repository *r);
157 +int rearrange_squash_in_todo_file(struct repository *r);
158
159 extern const char sign_off_header[];
160