Merge branch 'pw/status-with-corrupt-sequencer-state'
authorJunio C Hamano <gitster@pobox.com>
Fri, 19 Jul 2019 18:30:20 +0000 (11:30 -0700)
committerJunio C Hamano <gitster@pobox.com>
Fri, 19 Jul 2019 18:30:20 +0000 (11:30 -0700)
The code to read state files used by the sequencer machinery for
"git status" has been made more robust against a corrupt or stale
state files.

* pw/status-with-corrupt-sequencer-state:
status: do not report errors in sequencer/todo
sequencer: factor out todo command name parsing
sequencer: always allow tab after command name

1  2 
sequencer.c
t/t7512-status-help.sh
diff --combined sequencer.c
index cf262701e8e7df668b2c659bfb4806f0c7e90a9e,f8eab1697e7fcb83b54a902888cf469d710bd728..b91c981b326a16122ec1a5cbbd9f2c545115fa8c
@@@ -279,7 -279,7 +279,7 @@@ static const char *gpg_sign_opt_quoted(
  int sequencer_remove_state(struct replay_opts *opts)
  {
        struct strbuf buf = STRBUF_INIT;
 -      int i;
 +      int i, ret = 0;
  
        if (is_rebase_i(opts) &&
            strbuf_read_file(&buf, rebase_path_refs_to_delete(), 0) > 0) {
                        char *eol = strchr(p, '\n');
                        if (eol)
                                *eol = '\0';
 -                      if (delete_ref("(rebase -i) cleanup", p, NULL, 0) < 0)
 +                      if (delete_ref("(rebase -i) cleanup", p, NULL, 0) < 0) {
                                warning(_("could not delete '%s'"), p);
 +                              ret = -1;
 +                      }
                        if (!eol)
                                break;
                        p = eol + 1;
  
        strbuf_reset(&buf);
        strbuf_addstr(&buf, get_dir(opts));
 -      remove_dir_recursively(&buf, 0);
 +      if (remove_dir_recursively(&buf, 0))
 +              ret = error(_("could not remove '%s'"), buf.buf);
        strbuf_release(&buf);
  
 -      return 0;
 +      return ret;
  }
  
  static const char *action_name(const struct replay_opts *opts)
@@@ -770,7 -767,7 +770,7 @@@ static int parse_key_value_squoted(cha
   *    GIT_AUTHOR_DATE='$author_date'
   *
   * where $author_name, $author_email and $author_date are quoted. We are strict
 - * with our parsing, as the file was meant to be eval'd in the old
 + * with our parsing, as the file was meant to be eval'd in the now-removed
   * git-am.sh/git-rebase--interactive.sh scripts, and thus if the file differs
   * from what this function expects, it is better to bail out than to do
   * something that the user does not expect.
@@@ -2079,6 -2076,18 +2079,18 @@@ const char *todo_item_get_arg(struct to
        return todo_list->buf.buf + item->arg_offset;
  }
  
+ static int is_command(enum todo_command command, const char **bol)
+ {
+       const char *str = todo_command_info[command].str;
+       const char nick = todo_command_info[command].c;
+       const char *p = *bol + 1;
+       return skip_prefix(*bol, str, bol) ||
+               ((nick && **bol == nick) &&
+                (*p == ' ' || *p == '\t' || *p == '\n' || *p == '\r' || !*p) &&
+                (*bol = p));
+ }
  static int parse_insn_line(struct repository *r, struct todo_item *item,
                           const char *buf, const char *bol, char *eol)
  {
        }
  
        for (i = 0; i < TODO_COMMENT; i++)
-               if (skip_prefix(bol, todo_command_info[i].str, &bol)) {
-                       item->command = i;
-                       break;
-               } else if ((bol + 1 == eol || bol[1] == ' ') &&
-                          *bol == todo_command_info[i].c) {
-                       bol++;
+               if (is_command(i, &bol)) {
                        item->command = i;
                        break;
                }
  
  int sequencer_get_last_command(struct repository *r, enum replay_action *action)
  {
-       struct todo_item item;
-       char *eol;
-       const char *todo_file;
+       const char *todo_file, *bol;
        struct strbuf buf = STRBUF_INIT;
-       int ret = -1;
+       int ret = 0;
  
        todo_file = git_path_todo_file();
        if (strbuf_read_file(&buf, todo_file, 0) < 0) {
-               if (errno == ENOENT)
+               if (errno == ENOENT || errno == ENOTDIR)
                        return -1;
                else
                        return error_errno("unable to open '%s'", todo_file);
        }
-       eol = strchrnul(buf.buf, '\n');
-       if (buf.buf != eol && eol[-1] == '\r')
-               eol--; /* strip Carriage Return */
-       if (parse_insn_line(r, &item, buf.buf, buf.buf, eol))
-               goto fail;
-       if (item.command == TODO_PICK)
+       bol = buf.buf + strspn(buf.buf, " \t\r\n");
+       if (is_command(TODO_PICK, &bol) && (*bol == ' ' || *bol == '\t'))
                *action = REPLAY_PICK;
-       else if (item.command == TODO_REVERT)
+       else if (is_command(TODO_REVERT, &bol) &&
+                (*bol == ' ' || *bol == '\t'))
                *action = REPLAY_REVERT;
        else
-               goto fail;
-       ret = 0;
+               ret = -1;
  
-  fail:
        strbuf_release(&buf);
  
        return ret;
@@@ -2314,21 -2310,19 +2313,21 @@@ static int have_finished_the_last_pick(
        return ret;
  }
  
 -void sequencer_post_commit_cleanup(struct repository *r)
 +void sequencer_post_commit_cleanup(struct repository *r, int verbose)
  {
        struct replay_opts opts = REPLAY_OPTS_INIT;
        int need_cleanup = 0;
  
        if (file_exists(git_path_cherry_pick_head(r))) {
 -              unlink(git_path_cherry_pick_head(r));
 +              if (!unlink(git_path_cherry_pick_head(r)) && verbose)
 +                      warning(_("cancelling a cherry picking in progress"));
                opts.action = REPLAY_PICK;
                need_cleanup = 1;
        }
  
        if (file_exists(git_path_revert_head(r))) {
 -              unlink(git_path_revert_head(r));
 +              if (!unlink(git_path_revert_head(r)) && verbose)
 +                      warning(_("cancelling a revert in progress"));
                opts.action = REPLAY_REVERT;
                need_cleanup = 1;
        }
@@@ -3406,10 -3400,6 +3405,10 @@@ static int do_merge(struct repository *
                rollback_lock_file(&lock);
                ret = fast_forward_to(r, &commit->object.oid,
                                      &head_commit->object.oid, 0, opts);
 +              if (flags & TODO_EDIT_MERGE_MSG) {
 +                      run_commit_flags |= AMEND_MSG;
 +                      goto fast_forward_edit;
 +              }
                goto leave_merge;
        }
  
                 * value (a negative one would indicate that the `merge`
                 * command needs to be rescheduled).
                 */
 +      fast_forward_edit:
                ret = !!run_git_commit(r, git_path_merge_msg(r), opts,
                                       run_commit_flags);
  
@@@ -3741,11 -3730,8 +3740,11 @@@ static int pick_commits(struct reposito
                        unlink(git_path_merge_head(the_repository));
                        delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
  
 -                      if (item->command == TODO_BREAK)
 +                      if (item->command == TODO_BREAK) {
 +                              if (!opts->verbose)
 +                                      term_clear_line();
                                return stopped_at_head(r);
 +                      }
                }
                if (item->command <= TODO_SQUASH) {
                        if (is_rebase_i(opts))
                        }
                        if (item->command == TODO_EDIT) {
                                struct commit *commit = item->commit;
 -                              if (!res)
 +                              if (!res) {
 +                                      if (!opts->verbose)
 +                                              term_clear_line();
                                        fprintf(stderr,
                                                _("Stopped at %s...  %.*s\n"),
                                                short_commit_name(commit),
                                                item->arg_len, arg);
 +                              }
                                return error_with_patch(r, commit,
                                        arg, item->arg_len, opts, res, !res);
                        }
                        int saved = *end_of_arg;
                        struct stat st;
  
 +                      if (!opts->verbose)
 +                              term_clear_line();
                        *end_of_arg = '\0';
                        res = do_exec(r, arg);
                        *end_of_arg = saved;
@@@ -3972,13 -3953,10 +3971,13 @@@ cleanup_head_ref
                }
                apply_autostash(opts);
  
 -              if (!opts->quiet)
 +              if (!opts->quiet) {
 +                      if (!opts->verbose)
 +                              term_clear_line();
                        fprintf(stderr,
                                "Successfully rebased and updated %s.\n",
                                head_ref.buf);
 +              }
  
                strbuf_release(&buf);
                strbuf_release(&head_ref);
diff --combined t/t7512-status-help.sh
index b9f5d73423cfec8e61e10b6b42e7f8dc237a37b2,3c9308709abe430cd056cde6af2107b947d337b2..60eea88c41d6046a85d4c4f5741e44750da4387b
@@@ -85,7 -85,7 +85,7 @@@ You are currently rebasing branch '\''r
    (use "git rebase --abort" to check out the original branch)
  
  Unmerged paths:
 -  (use "git reset HEAD <file>..." to unstage)
 +  (use "git restore --staged <file>..." to unstage)
    (use "git add <file>..." to mark resolution)
  
        both modified:   main.txt
@@@ -110,7 -110,7 +110,7 @@@ You are currently rebasing branch '\''r
    (all conflicts fixed: run "git rebase --continue")
  
  Changes to be committed:
 -  (use "git reset HEAD <file>..." to unstage)
 +  (use "git restore --staged <file>..." to unstage)
  
        modified:   main.txt
  
@@@ -148,7 -148,7 +148,7 @@@ You are currently rebasing branch '\''r
    (use "git rebase --abort" to check out the original branch)
  
  Unmerged paths:
 -  (use "git reset HEAD <file>..." to unstage)
 +  (use "git restore --staged <file>..." to unstage)
    (use "git add <file>..." to mark resolution)
  
        both modified:   main.txt
@@@ -176,7 -176,7 +176,7 @@@ You are currently rebasing branch '\''r
    (all conflicts fixed: run "git rebase --continue")
  
  Changes to be committed:
 -  (use "git reset HEAD <file>..." to unstage)
 +  (use "git restore --staged <file>..." to unstage)
  
        modified:   main.txt
  
@@@ -246,7 -246,7 +246,7 @@@ You are currently splitting a commit wh
  
  Changes not staged for commit:
    (use "git add <file>..." to update what will be committed)
 -  (use "git checkout -- <file>..." to discard changes in working directory)
 +  (use "git restore <file>..." to discard changes in working directory)
  
        modified:   main.txt
  
@@@ -354,7 -354,7 +354,7 @@@ You are currently splitting a commit wh
  
  Changes not staged for commit:
    (use "git add <file>..." to update what will be committed)
 -  (use "git checkout -- <file>..." to discard changes in working directory)
 +  (use "git restore <file>..." to discard changes in working directory)
  
        modified:   main.txt
  
@@@ -453,7 -453,7 +453,7 @@@ You are currently splitting a commit wh
  
  Changes not staged for commit:
    (use "git add <file>..." to update what will be committed)
 -  (use "git checkout -- <file>..." to discard changes in working directory)
 +  (use "git restore <file>..." to discard changes in working directory)
  
        modified:   main.txt
  
@@@ -557,7 -557,7 +557,7 @@@ You are currently splitting a commit wh
  
  Changes not staged for commit:
    (use "git add <file>..." to update what will be committed)
 -  (use "git checkout -- <file>..." to discard changes in working directory)
 +  (use "git restore <file>..." to discard changes in working directory)
  
        modified:   main.txt
  
@@@ -798,6 -798,22 +798,22 @@@ EO
        test_i18ncmp expected actual
  '
  
+ test_expect_success 'status shows cherry-pick with invalid oid' '
+       mkdir .git/sequencer &&
+       test_write_lines "pick invalid-oid" >.git/sequencer/todo &&
+       git status --untracked-files=no >actual 2>err &&
+       git cherry-pick --quit &&
+       test_must_be_empty err &&
+       test_i18ncmp expected actual
+ '
+ test_expect_success 'status does not show error if .git/sequencer is a file' '
+       test_when_finished "rm .git/sequencer" &&
+       test_write_lines hello >.git/sequencer &&
+       git status --untracked-files=no 2>err &&
+       test_must_be_empty err
+ '
  test_expect_success 'status showing detached at and from a tag' '
        test_commit atag tagging &&
        git checkout atag &&
@@@ -834,7 -850,7 +850,7 @@@ You are currently reverting commit $TO_
    (use "git revert --abort" to cancel the revert operation)
  
  Unmerged paths:
 -  (use "git reset HEAD <file>..." to unstage)
 +  (use "git restore --staged <file>..." to unstage)
    (use "git add <file>..." to mark resolution)
  
        both modified:   to-revert.txt
@@@ -855,7 -871,7 +871,7 @@@ You are currently reverting commit $TO_
    (use "git revert --abort" to cancel the revert operation)
  
  Changes to be committed:
 -  (use "git reset HEAD <file>..." to unstage)
 +  (use "git restore --staged <file>..." to unstage)
  
        modified:   to-revert.txt