sequencer: refactor check_todo_list() to work on a todo_list
authorAlban Gruin <alban.gruin@gmail.com>
Tue, 29 Jan 2019 15:01:49 +0000 (16:01 +0100)
committerJunio C Hamano <gitster@pobox.com>
Tue, 29 Jan 2019 20:09:24 +0000 (12:09 -0800)
This refactors check_todo_list() to work on a todo_list to avoid
redundant reads and writes to the disk.  The function is renamed
todo_list_check().  The parsing of the two todo lists is left to the
caller.

As rebase -p still need to check the todo list from the disk, a new
function is introduced, check_todo_list_from_file().  It reads the file
from the disk, parses it, pass the todo_list to todo_list_check(), and
writes it back to the disk.

As get_missing_commit_check_level() and the enum
missing_commit_check_level are no longer needed inside of sequencer.c,
they are moved to rebase-interactive.c, and made static again.

Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/rebase--interactive.c
rebase-interactive.c
rebase-interactive.h
sequencer.c
sequencer.h

index 0898eb4..df19cca 100644 (file)
@@ -256,7 +256,7 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
                ret = transform_todo_file(the_repository, flags);
                break;
        case CHECK_TODO_LIST:
                ret = transform_todo_file(the_repository, flags);
                break;
        case CHECK_TODO_LIST:
-               ret = check_todo_list(the_repository);
+               ret = check_todo_list_from_file(the_repository);
                break;
        case REARRANGE_SQUASH:
                ret = rearrange_squash(the_repository);
                break;
        case REARRANGE_SQUASH:
                ret = rearrange_squash(the_repository);
index 842fa07..dfa6dd5 100644 (file)
@@ -1,8 +1,32 @@
 #include "cache.h"
 #include "commit.h"
 #include "cache.h"
 #include "commit.h"
-#include "rebase-interactive.h"
 #include "sequencer.h"
 #include "sequencer.h"
+#include "rebase-interactive.h"
 #include "strbuf.h"
 #include "strbuf.h"
+#include "commit-slab.h"
+#include "config.h"
+
+enum missing_commit_check_level {
+       MISSING_COMMIT_CHECK_IGNORE = 0,
+       MISSING_COMMIT_CHECK_WARN,
+       MISSING_COMMIT_CHECK_ERROR
+};
+
+static enum missing_commit_check_level get_missing_commit_check_level(void)
+{
+       const char *value;
+
+       if (git_config_get_value("rebase.missingcommitscheck", &value) ||
+                       !strcasecmp("ignore", value))
+               return MISSING_COMMIT_CHECK_IGNORE;
+       if (!strcasecmp("warn", value))
+               return MISSING_COMMIT_CHECK_WARN;
+       if (!strcasecmp("error", value))
+               return MISSING_COMMIT_CHECK_ERROR;
+       warning(_("unrecognized setting %s for option "
+                 "rebase.missingCommitsCheck. Ignoring."), value);
+       return MISSING_COMMIT_CHECK_IGNORE;
+}
 
 void append_todo_help(unsigned edit_todo, unsigned keep_empty,
                      struct strbuf *buf)
 
 void append_todo_help(unsigned edit_todo, unsigned keep_empty,
                      struct strbuf *buf)
@@ -89,3 +113,68 @@ int edit_todo_list(struct repository *r, unsigned flags)
 
        return 0;
 }
 
        return 0;
 }
+
+define_commit_slab(commit_seen, unsigned char);
+/*
+ * Check if the user dropped some commits by mistake
+ * Behaviour determined by rebase.missingCommitsCheck.
+ * Check if there is an unrecognized command or a
+ * bad SHA-1 in a command.
+ */
+int todo_list_check(struct todo_list *old_todo, struct todo_list *new_todo)
+{
+       enum missing_commit_check_level check_level = get_missing_commit_check_level();
+       struct strbuf missing = STRBUF_INIT;
+       int res = 0, i;
+       struct commit_seen commit_seen;
+
+       init_commit_seen(&commit_seen);
+
+       if (check_level == MISSING_COMMIT_CHECK_IGNORE)
+               goto leave_check;
+
+       /* Mark the commits in git-rebase-todo as seen */
+       for (i = 0; i < new_todo->nr; i++) {
+               struct commit *commit = new_todo->items[i].commit;
+               if (commit)
+                       *commit_seen_at(&commit_seen, commit) = 1;
+       }
+
+       /* Find commits in git-rebase-todo.backup yet unseen */
+       for (i = old_todo->nr - 1; i >= 0; i--) {
+               struct todo_item *item = old_todo->items + i;
+               struct commit *commit = item->commit;
+               if (commit && !*commit_seen_at(&commit_seen, commit)) {
+                       strbuf_addf(&missing, " - %s %.*s\n",
+                                   find_unique_abbrev(&commit->object.oid, DEFAULT_ABBREV),
+                                   item->arg_len,
+                                   todo_item_get_arg(old_todo, item));
+                       *commit_seen_at(&commit_seen, commit) = 1;
+               }
+       }
+
+       /* Warn about missing commits */
+       if (!missing.len)
+               goto leave_check;
+
+       if (check_level == MISSING_COMMIT_CHECK_ERROR)
+               res = 1;
+
+       fprintf(stderr,
+               _("Warning: some commits may have been dropped accidentally.\n"
+               "Dropped commits (newer to older):\n"));
+
+       /* Make the list user-friendly and display */
+       fputs(missing.buf, stderr);
+       strbuf_release(&missing);
+
+       fprintf(stderr, _("To avoid this message, use \"drop\" to "
+               "explicitly remove a commit.\n\n"
+               "Use 'git config rebase.missingCommitsCheck' to change "
+               "the level of warnings.\n"
+               "The possible behaviours are: ignore, warn, error.\n\n"));
+
+leave_check:
+       clear_commit_seen(&commit_seen);
+       return res;
+}
index 17b6c9f..187b503 100644 (file)
@@ -3,9 +3,11 @@
 
 struct strbuf;
 struct repository;
 
 struct strbuf;
 struct repository;
+struct todo_list;
 
 void append_todo_help(unsigned edit_todo, unsigned keep_empty,
                      struct strbuf *buf);
 int edit_todo_list(struct repository *r, unsigned flags);
 
 void append_todo_help(unsigned edit_todo, unsigned keep_empty,
                      struct strbuf *buf);
 int edit_todo_list(struct repository *r, unsigned flags);
+int todo_list_check(struct todo_list *old_todo, struct todo_list *new_todo);
 
 #endif
 
 #endif
index 4809b22..99e12c7 100644 (file)
@@ -4660,112 +4660,37 @@ int transform_todo_file(struct repository *r, unsigned flags)
        return 0;
 }
 
        return 0;
 }
 
-enum missing_commit_check_level get_missing_commit_check_level(void)
-{
-       const char *value;
-
-       if (git_config_get_value("rebase.missingcommitscheck", &value) ||
-                       !strcasecmp("ignore", value))
-               return MISSING_COMMIT_CHECK_IGNORE;
-       if (!strcasecmp("warn", value))
-               return MISSING_COMMIT_CHECK_WARN;
-       if (!strcasecmp("error", value))
-               return MISSING_COMMIT_CHECK_ERROR;
-       warning(_("unrecognized setting %s for option "
-                 "rebase.missingCommitsCheck. Ignoring."), value);
-       return MISSING_COMMIT_CHECK_IGNORE;
-}
+static const char edit_todo_list_advice[] =
+N_("You can fix this with 'git rebase --edit-todo' "
+"and then run 'git rebase --continue'.\n"
+"Or you can abort the rebase with 'git rebase"
+" --abort'.\n");
 
 
-define_commit_slab(commit_seen, unsigned char);
-/*
- * Check if the user dropped some commits by mistake
- * Behaviour determined by rebase.missingCommitsCheck.
- * Check if there is an unrecognized command or a
- * bad SHA-1 in a command.
- */
-int check_todo_list(struct repository *r)
+int check_todo_list_from_file(struct repository *r)
 {
 {
-       enum missing_commit_check_level check_level = get_missing_commit_check_level();
-       struct strbuf todo_file = STRBUF_INIT;
-       struct todo_list todo_list = TODO_LIST_INIT;
-       struct strbuf missing = STRBUF_INIT;
-       int advise_to_edit_todo = 0, res = 0, i;
-       struct commit_seen commit_seen;
-
-       init_commit_seen(&commit_seen);
+       struct todo_list old_todo = TODO_LIST_INIT, new_todo = TODO_LIST_INIT;
+       int res = 0;
 
 
-       strbuf_addstr(&todo_file, rebase_path_todo());
-       if (strbuf_read_file_or_whine(&todo_list.buf, todo_file.buf) < 0) {
+       if (strbuf_read_file_or_whine(&new_todo.buf, rebase_path_todo()) < 0) {
                res = -1;
                res = -1;
-               goto leave_check;
-       }
-       advise_to_edit_todo = res =
-               todo_list_parse_insn_buffer(r, todo_list.buf.buf, &todo_list);
-
-       if (res || check_level == MISSING_COMMIT_CHECK_IGNORE)
-               goto leave_check;
-
-       /* Mark the commits in git-rebase-todo as seen */
-       for (i = 0; i < todo_list.nr; i++) {
-               struct commit *commit = todo_list.items[i].commit;
-               if (commit)
-                       *commit_seen_at(&commit_seen, commit) = 1;
+               goto out;
        }
 
        }
 
-       todo_list_release(&todo_list);
-       strbuf_addstr(&todo_file, ".backup");
-       if (strbuf_read_file_or_whine(&todo_list.buf, todo_file.buf) < 0) {
+       if (strbuf_read_file_or_whine(&old_todo.buf, rebase_path_todo_backup()) < 0) {
                res = -1;
                res = -1;
-               goto leave_check;
-       }
-       strbuf_release(&todo_file);
-       res = !!todo_list_parse_insn_buffer(r, todo_list.buf.buf, &todo_list);
-
-       /* Find commits in git-rebase-todo.backup yet unseen */
-       for (i = todo_list.nr - 1; i >= 0; i--) {
-               struct todo_item *item = todo_list.items + i;
-               struct commit *commit = item->commit;
-               if (commit && !*commit_seen_at(&commit_seen, commit)) {
-                       strbuf_addf(&missing, " - %s %.*s\n",
-                                   short_commit_name(commit),
-                                   item->arg_len,
-                                   todo_item_get_arg(&todo_list, item));
-                       *commit_seen_at(&commit_seen, commit) = 1;
-               }
+               goto out;
        }
 
        }
 
-       /* Warn about missing commits */
-       if (!missing.len)
-               goto leave_check;
-
-       if (check_level == MISSING_COMMIT_CHECK_ERROR)
-               advise_to_edit_todo = res = 1;
-
-       fprintf(stderr,
-               _("Warning: some commits may have been dropped accidentally.\n"
-               "Dropped commits (newer to older):\n"));
-
-       /* Make the list user-friendly and display */
-       fputs(missing.buf, stderr);
-       strbuf_release(&missing);
-
-       fprintf(stderr, _("To avoid this message, use \"drop\" to "
-               "explicitly remove a commit.\n\n"
-               "Use 'git config rebase.missingCommitsCheck' to change "
-               "the level of warnings.\n"
-               "The possible behaviours are: ignore, warn, error.\n\n"));
-
-leave_check:
-       clear_commit_seen(&commit_seen);
-       strbuf_release(&todo_file);
-       todo_list_release(&todo_list);
-
-       if (advise_to_edit_todo)
-               fprintf(stderr,
-                       _("You can fix this with 'git rebase --edit-todo' "
-                         "and then run 'git rebase --continue'.\n"
-                         "Or you can abort the rebase with 'git rebase"
-                         " --abort'.\n"));
+       res = todo_list_parse_insn_buffer(r, old_todo.buf.buf, &old_todo);
+       if (!res)
+               res = todo_list_parse_insn_buffer(r, new_todo.buf.buf, &new_todo);
+       if (!res)
+               res = todo_list_check(&old_todo, &new_todo);
+       if (res)
+               fprintf(stderr, _(edit_todo_list_advice));
+out:
+       todo_list_release(&old_todo);
+       todo_list_release(&new_todo);
 
        return res;
 }
 
        return res;
 }
@@ -4943,7 +4868,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
 
        todo_list_release(&todo_list);
 
 
        todo_list_release(&todo_list);
 
-       if (check_todo_list(r)) {
+       if (check_todo_list_from_file(r)) {
                checkout_onto(opts, onto_name, onto, orig_head);
                return -1;
        }
                checkout_onto(opts, onto_name, onto, orig_head);
                return -1;
        }
index 7278f96..217353e 100644 (file)
@@ -64,12 +64,6 @@ struct replay_opts {
 };
 #define REPLAY_OPTS_INIT { .action = -1, .current_fixups = STRBUF_INIT }
 
 };
 #define REPLAY_OPTS_INIT { .action = -1, .current_fixups = STRBUF_INIT }
 
-enum missing_commit_check_level {
-       MISSING_COMMIT_CHECK_IGNORE = 0,
-       MISSING_COMMIT_CHECK_WARN,
-       MISSING_COMMIT_CHECK_ERROR
-};
-
 int write_message(const void *buf, size_t len, const char *filename,
                  int append_eol);
 
 int write_message(const void *buf, size_t len, const char *filename,
                  int append_eol);
 
@@ -154,8 +148,7 @@ int sequencer_make_script(struct repository *r, FILE *out, int argc,
 
 int sequencer_add_exec_commands(struct repository *r, const char *command);
 int transform_todo_file(struct repository *r, unsigned flags);
 
 int sequencer_add_exec_commands(struct repository *r, const char *command);
 int transform_todo_file(struct repository *r, unsigned flags);
-enum missing_commit_check_level get_missing_commit_check_level(void);
-int check_todo_list(struct repository *r);
+int check_todo_list_from_file(struct repository *r);
 int complete_action(struct repository *r, struct replay_opts *opts, unsigned flags,
                    const char *shortrevisions, const char *onto_name,
                    const char *onto, const char *orig_head, const char *cmd,
 int complete_action(struct repository *r, struct replay_opts *opts, unsigned flags,
                    const char *shortrevisions, const char *onto_name,
                    const char *onto, const char *orig_head, const char *cmd,