sequencer: completely revamp the "todo" script parsing
When we came up with the "sequencer" idea, we really wanted to have kind of a plumbing equivalent of the interactive rebase. Hence the choice of words: the "todo" script, a "pick", etc. However, when it came time to implement the entire shebang, somehow this idea got lost and the sequencer was used as working horse for cherry-pick and revert instead. So as not to interfere with the interactive rebase, it even uses a separate directory to store its state. Furthermore, it also is stupidly strict about the "todo" script it accepts: while it parses commands in a way that was *designed* to be similar to the interactive rebase, it then goes on to *error out* if the commands disagree with the overall action (cherry-pick or revert). Finally, the sequencer code chose to deviate from the interactive rebase code insofar that when it comes to writing the file with the remaining commands, it *reformats* the "todo" script instead of just writing the part of the parsed script that were not yet processed. This is not only unnecessary churn, but might well lose information that is valuable to the user (i.e. comments after the commands). Let's just bite the bullet and rewrite the entire parser; the code now becomes not only more elegant: it allows us to go on and teach the sequencer how to parse *true* "todo" scripts as used by the interactive rebase itself. In a way, the sequencer is about to grow up to do its older brother's job. Better. In particular, we choose to maintain the list of commands in an array instead of a linked list: this is flexible enough to allow us later on to even implement rebase -i's reordering of fixup!/squash! commits very easily (and with a very nice speed bonus, at least on Windows). While at it, do not stop at the first problem, but list *all* of the problems. This will help the user when the sequencer will do `rebase -i`'s work by allowing to address all issues in one go rather than going back and forth until the todo list is valid. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Johannes Schindelin committed
Oct 21, 2016 at 14:24 UTC
004fefa754a4aa3a99a5954da9fb805dbba8dbeb
1 file changed
+163
-121
sequencer.c
+163
-121
@@ -470,7 +470,26 @@ static int allow_empty(struct replay_opts *opts, struct commit *commit)
470
return 1;
471
}
472
473
-static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
473
+enum todo_command {
474
+ TODO_PICK = 0,
475
+ TODO_REVERT
476
+};
477
+
478
+static const char *todo_command_strings[] = {
479
+ "pick",
480
+ "revert"
481
+};
482
+
483
+static const char *command_to_string(const enum todo_command command)
484
+{
485
+ if (command < ARRAY_SIZE(todo_command_strings))
486
+ return todo_command_strings[command];
487
+ die("Unknown command: %d", command);
488
+}
489
+
490
+
491
+static int do_pick_commit(enum todo_command command, struct commit *commit,
492
+ struct replay_opts *opts)
493
{
494
unsigned char head[20];
495
struct commit *base, *next, *parent;
@@ -529,10 +548,11 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
548
return fast_forward_to(commit->object.oid.hash, head, unborn, opts);
549
550
if (parent && parse_commit(parent) < 0)
532
- /* TRANSLATORS: The first %s will be "revert" or
533
- "cherry-pick", the second %s a SHA1 */
551
+ /* TRANSLATORS: The first %s will be a "todo" command like
552
+ "revert" or "pick", the second %s a SHA1. */
553
return error(_("%s: cannot parse parent commit %s"),
535
- action_name(opts), oid_to_hex(&parent->object.oid));
554
+ command_to_string(command),
555
+ oid_to_hex(&parent->object.oid));
556
557
if (get_message(commit, &msg) != 0)
558
return error(_("Cannot get commit message for %s"),
@@ -545,7 +565,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
565
* reverse of it if we are revert.
566
*/
567
548
- if (opts->action == REPLAY_REVERT) {
568
+ if (command == TODO_REVERT) {
569
base = commit;
570
base_label = msg.label;
571
next = parent;
@@ -586,7 +606,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
606
}
607
}
608
589
- if (!opts->strategy || !strcmp(opts->strategy, "recursive") || opts->action == REPLAY_REVERT) {
609
+ if (!opts->strategy || !strcmp(opts->strategy, "recursive") || command == TODO_REVERT) {
610
res = do_recursive_merge(base, next, base_label, next_label,
611
head, &msgbuf, opts);
612
if (res < 0)
@@ -613,17 +633,17 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
633
* However, if the merge did not even start, then we don't want to
634
* write it at all.
635
*/
616
- if (opts->action == REPLAY_PICK && !opts->no_commit && (res == 0 || res == 1) &&
636
+ if (command == TODO_PICK && !opts->no_commit && (res == 0 || res == 1) &&
637
update_ref(NULL, "CHERRY_PICK_HEAD", commit->object.oid.hash, NULL,
638
REF_NODEREF, UPDATE_REFS_MSG_ON_ERR))
639
res = -1;
620
- if (opts->action == REPLAY_REVERT && ((opts->no_commit && res == 0) || res == 1) &&
640
+ if (command == TODO_REVERT && ((opts->no_commit && res == 0) || res == 1) &&
641
update_ref(NULL, "REVERT_HEAD", commit->object.oid.hash, NULL,
642
REF_NODEREF, UPDATE_REFS_MSG_ON_ERR))
643
res = -1;
644
645
if (res) {
626
- error(opts->action == REPLAY_REVERT
646
+ error(command == TODO_REVERT
647
? _("could not revert %s... %s")
648
: _("could not apply %s... %s"),
649
short_commit_name(commit), msg.subject);
@@ -684,116 +704,122 @@ static int read_and_refresh_cache(struct replay_opts *opts)
704
return 0;
705
}
706
687
-static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
688
- struct replay_opts *opts)
707
+struct todo_item {
708
+ enum todo_command command;
709
+ struct commit *commit;
710
+ size_t offset_in_buf;
711
+};
712
+
713
+struct todo_list {
714
+ struct strbuf buf;
715
+ struct todo_item *items;
716
+ int nr, alloc, current;
717
+};
718
+
719
+#define TODO_LIST_INIT { STRBUF_INIT }
720
+
721
+static void todo_list_release(struct todo_list *todo_list)
722
{
690
- struct commit_list *cur = NULL;
691
- const char *sha1_abbrev = NULL;
692
- const char *action_str = opts->action == REPLAY_REVERT ? "revert" : "pick";
693
- const char *subject;
694
- int subject_len;
723
+ strbuf_release(&todo_list->buf);
724
+ free(todo_list->items);
725
+ todo_list->items = NULL;
726
+ todo_list->nr = todo_list->alloc = 0;
727
+}
728
696
- for (cur = todo_list; cur; cur = cur->next) {
697
- const char *commit_buffer = get_commit_buffer(cur->item, NULL);
698
- sha1_abbrev = find_unique_abbrev(cur->item->object.oid.hash, DEFAULT_ABBREV);
699
- subject_len = find_commit_subject(commit_buffer, &subject);
700
- strbuf_addf(buf, "%s %s %.*s\n", action_str, sha1_abbrev,
701
- subject_len, subject);
702
- unuse_commit_buffer(cur->item, commit_buffer);
703
- }
704
- return 0;
729
+static struct todo_item *append_new_todo(struct todo_list *todo_list)
730
+{
731
+ ALLOC_GROW(todo_list->items, todo_list->nr + 1, todo_list->alloc);
732
+ return todo_list->items + todo_list->nr++;
733
}
734
707
-static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *opts)
735
+static int parse_insn_line(struct todo_item *item, const char *bol, char *eol)
736
{
737
unsigned char commit_sha1[20];
710
- enum replay_action action;
738
char *end_of_object_name;
712
- int saved, status, padding;
713
-
714
- if (starts_with(bol, "pick")) {
715
- action = REPLAY_PICK;
716
- bol += strlen("pick");
717
- } else if (starts_with(bol, "revert")) {
718
- action = REPLAY_REVERT;
719
- bol += strlen("revert");
720
- } else
721
- return NULL;
739
+ int i, saved, status, padding;
740
+
741
+ for (i = 0; i < ARRAY_SIZE(todo_command_strings); i++)
742
+ if (skip_prefix(bol, todo_command_strings[i], &bol)) {
743
+ item->command = i;
744
+ break;
745
+ }
746
+ if (i >= ARRAY_SIZE(todo_command_strings))
747
+ return -1;
748
749
/* Eat up extra spaces/ tabs before object name */
750
padding = strspn(bol, " \t");
751
if (!padding)
726
- return NULL;
752
+ return -1;
753
bol += padding;
754
729
- end_of_object_name = bol + strcspn(bol, " \t\n");
755
+ end_of_object_name = (char *) bol + strcspn(bol, " \t\n");
756
saved = *end_of_object_name;
757
*end_of_object_name = '\0';
758
status = get_sha1(bol, commit_sha1);
759
*end_of_object_name = saved;
760
735
- /*
736
- * Verify that the action matches up with the one in
737
- * opts; we don't support arbitrary instructions
738
- */
739
- if (action != opts->action) {
740
- if (action == REPLAY_REVERT)
741
- error((opts->action == REPLAY_REVERT)
742
- ? _("Cannot revert during another revert.")
743
- : _("Cannot revert during a cherry-pick."));
744
- else
745
- error((opts->action == REPLAY_REVERT)
746
- ? _("Cannot cherry-pick during a revert.")
747
- : _("Cannot cherry-pick during another cherry-pick."));
748
- return NULL;
749
- }
750
-
761
if (status < 0)
752
- return NULL;
762
+ return -1;
763
754
- return lookup_commit_reference(commit_sha1);
764
+ item->commit = lookup_commit_reference(commit_sha1);
765
+ return !item->commit;
766
}
767
757
-static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
758
- struct replay_opts *opts)
768
+static int parse_insn_buffer(char *buf, struct todo_list *todo_list)
769
{
760
- struct commit_list **next = todo_list;
761
- struct commit *commit;
762
- char *p = buf;
763
- int i;
770
+ struct todo_item *item;
771
+ char *p = buf, *next_p;
772
+ int i, res = 0;
773
765
- for (i = 1; *p; i++) {
774
+ for (i = 1; *p; i++, p = next_p) {
775
char *eol = strchrnul(p, '\n');
767
- commit = parse_insn_line(p, eol, opts);
768
- if (!commit)
769
- return error(_("Could not parse line %d."), i);
770
- next = commit_list_append(commit, next);
771
- p = *eol ? eol + 1 : eol;
776
+
777
+ next_p = *eol ? eol + 1 /* skip LF */ : eol;
778
+
779
+ item = append_new_todo(todo_list);
780
+ item->offset_in_buf = p - todo_list->buf.buf;
781
+ if (parse_insn_line(item, p, eol)) {
782
+ res = error(_("Invalid line %d: %.*s"),
783
+ i, (int)(eol - p), p);
784
+ item->command = -1;
785
+ }
786
}
773
- if (!*todo_list)
787
+ if (!todo_list->nr)
788
return error(_("No commits parsed."));
775
- return 0;
789
+ return res;
790
}
791
778
-static int read_populate_todo(struct commit_list **todo_list,
792
+static int read_populate_todo(struct todo_list *todo_list,
793
struct replay_opts *opts)
794
{
795
const char *todo_file = get_todo_path(opts);
782
- struct strbuf buf = STRBUF_INIT;
796
int fd, res;
797
798
+ strbuf_reset(&todo_list->buf);
799
fd = open(todo_file, O_RDONLY);
800
if (fd < 0)
801
return error_errno(_("Could not open %s"), todo_file);
788
- if (strbuf_read(&buf, fd, 0) < 0) {
802
+ if (strbuf_read(&todo_list->buf, fd, 0) < 0) {
803
close(fd);
790
- strbuf_release(&buf);
804
return error(_("Could not read %s."), todo_file);
805
}
806
close(fd);
807
795
- res = parse_insn_buffer(buf.buf, todo_list, opts);
796
- strbuf_release(&buf);
808
+ res = parse_insn_buffer(todo_list->buf.buf, todo_list);
809
+ if (!res) {
810
+ enum todo_command valid =
811
+ opts->action == REPLAY_PICK ? TODO_PICK : TODO_REVERT;
812
+ int i;
813
+
814
+ for (i = 0; i < todo_list->nr; i++)
815
+ if (valid == todo_list->items[i].command)
816
+ continue;
817
+ else if (valid == TODO_PICK)
818
+ return error(_("Cannot cherry-pick during a revert."));
819
+ else
820
+ return error(_("Cannot revert during a cherry-pick."));
821
+ }
822
+
823
if (res)
824
return error(_("Unusable instruction sheet: %s"), todo_file);
825
return 0;
@@ -860,18 +886,31 @@ static int read_populate_opts(struct replay_opts *opts)
886
return 0;
887
}
888
863
-static int walk_revs_populate_todo(struct commit_list **todo_list,
889
+static int walk_revs_populate_todo(struct todo_list *todo_list,
890
struct replay_opts *opts)
891
{
892
+ enum todo_command command = opts->action == REPLAY_PICK ?
893
+ TODO_PICK : TODO_REVERT;
894
+ const char *command_string = todo_command_strings[command];
895
struct commit *commit;
867
- struct commit_list **next;
896
897
if (prepare_revs(opts))
898
return -1;
899
872
- next = todo_list;
873
- while ((commit = get_revision(opts->revs)))
874
- next = commit_list_append(commit, next);
900
+ while ((commit = get_revision(opts->revs))) {
901
+ struct todo_item *item = append_new_todo(todo_list);
902
+ const char *commit_buffer = get_commit_buffer(commit, NULL);
903
+ const char *subject;
904
+ int subject_len;
905
+
906
+ item->command = command;
907
+ item->commit = commit;
908
+ item->offset_in_buf = todo_list->buf.len;
909
+ subject_len = find_commit_subject(commit_buffer, &subject);
910
+ strbuf_addf(&todo_list->buf, "%s %s %.*s\n", command_string,
911
+ short_commit_name(commit), subject_len, subject);
912
+ unuse_commit_buffer(commit, commit_buffer);
913
+ }
914
return 0;
915
}
916
@@ -979,30 +1018,22 @@ fail:
1018
return -1;
1019
}
1020
982
-static int save_todo(struct commit_list *todo_list, struct replay_opts *opts)
1021
+static int save_todo(struct todo_list *todo_list, struct replay_opts *opts)
1022
{
1023
static struct lock_file todo_lock;
985
- struct strbuf buf = STRBUF_INIT;
986
- int fd;
1024
+ const char *todo_path = get_todo_path(opts);
1025
+ int next = todo_list->current, offset, fd;
1026
988
- fd = hold_lock_file_for_update(&todo_lock, git_path_todo_file(), 0);
1027
+ fd = hold_lock_file_for_update(&todo_lock, todo_path, 0);
1028
if (fd < 0)
990
- return error_errno(_("Could not lock '%s'"),
991
- git_path_todo_file());
992
- if (format_todo(&buf, todo_list, opts) < 0) {
993
- strbuf_release(&buf);
994
- return error(_("Could not format %s."), git_path_todo_file());
995
- }
996
- if (write_in_full(fd, buf.buf, buf.len) < 0) {
997
- strbuf_release(&buf);
998
- return error_errno(_("Could not write to %s"),
999
- git_path_todo_file());
1000
- }
1001
- if (commit_lock_file(&todo_lock) < 0) {
1002
- strbuf_release(&buf);
1003
- return error(_("Error wrapping up %s."), git_path_todo_file());
1004
- }
1005
- strbuf_release(&buf);
1029
+ return error_errno(_("Could not lock '%s'"), todo_path);
1030
+ offset = next < todo_list->nr ?
1031
+ todo_list->items[next].offset_in_buf : todo_list->buf.len;
1032
+ if (write_in_full(fd, todo_list->buf.buf + offset,
1033
+ todo_list->buf.len - offset) < 0)
1034
+ return error_errno(_("Could not write to '%s'"), todo_path);
1035
+ if (commit_lock_file(&todo_lock) < 0)
1036
+ return error(_("Error wrapping up %s."), todo_path);
1037
return 0;
1038
}
1039
@@ -1041,9 +1072,8 @@ static int save_opts(struct replay_opts *opts)
1072
return res;
1073
}
1074
1044
-static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
1075
+static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
1076
{
1046
- struct commit_list *cur;
1077
int res;
1078
1079
setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
@@ -1053,10 +1083,12 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
1083
if (read_and_refresh_cache(opts))
1084
return -1;
1085
1056
- for (cur = todo_list; cur; cur = cur->next) {
1057
- if (save_todo(cur, opts))
1086
+ while (todo_list->current < todo_list->nr) {
1087
+ struct todo_item *item = todo_list->items + todo_list->current;
1088
+ if (save_todo(todo_list, opts))
1089
return -1;
1059
- res = do_pick_commit(cur->item, opts);
1090
+ res = do_pick_commit(item->command, item->commit, opts);
1091
+ todo_list->current++;
1092
if (res)
1093
return res;
1094
}
@@ -1081,38 +1113,46 @@ static int continue_single_pick(void)
1113
1114
static int sequencer_continue(struct replay_opts *opts)
1115
{
1084
- struct commit_list *todo_list = NULL;
1116
+ struct todo_list todo_list = TODO_LIST_INIT;
1117
+ int res;
1118
1119
if (!file_exists(get_todo_path(opts)))
1120
return continue_single_pick();
1088
- if (read_populate_opts(opts) ||
1089
- read_populate_todo(&todo_list, opts))
1121
+ if (read_populate_opts(opts))
1122
return -1;
1123
+ if ((res = read_populate_todo(&todo_list, opts)))
1124
+ goto release_todo_list;
1125
1126
/* Verify that the conflict has been resolved */
1127
if (file_exists(git_path_cherry_pick_head()) ||
1128
file_exists(git_path_revert_head())) {
1095
- int ret = continue_single_pick();
1096
- if (ret)
1097
- return ret;
1129
+ res = continue_single_pick();
1130
+ if (res)
1131
+ goto release_todo_list;
1132
}
1099
- if (index_differs_from("HEAD", 0))
1100
- return error_dirty_index(opts);
1101
- todo_list = todo_list->next;
1102
- return pick_commits(todo_list, opts);
1133
+ if (index_differs_from("HEAD", 0)) {
1134
+ res = error_dirty_index(opts);
1135
+ goto release_todo_list;
1136
+ }
1137
+ todo_list.current++;
1138
+ res = pick_commits(&todo_list, opts);
1139
+release_todo_list:
1140
+ todo_list_release(&todo_list);
1141
+ return res;
1142
}
1143
1144
static int single_pick(struct commit *cmit, struct replay_opts *opts)
1145
{
1146
setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
1108
- return do_pick_commit(cmit, opts);
1147
+ return do_pick_commit(opts->action == REPLAY_PICK ?
1148
+ TODO_PICK : TODO_REVERT, cmit, opts);
1149
}
1150
1151
int sequencer_pick_revisions(struct replay_opts *opts)
1152
{
1113
- struct commit_list *todo_list = NULL;
1153
+ struct todo_list todo_list = TODO_LIST_INIT;
1154
unsigned char sha1[20];
1115
- int i;
1155
+ int i, res;
1156
1157
if (opts->subcommand == REPLAY_NONE)
1158
assert(opts->revs);
@@ -1187,7 +1227,9 @@ int sequencer_pick_revisions(struct replay_opts *opts)
1227
return -1;
1228
if (save_opts(opts))
1229
return -1;
1190
- return pick_commits(todo_list, opts);
1230
+ res = pick_commits(&todo_list, opts);
1231
+ todo_list_release(&todo_list);
1232
+ return res;
1233
}
1234
1235
void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)