This patch series is one of the half dozen patch series left to move the
bulk of rebase -i into a builtin.

The purpose of this patch series is to switch the functions in
sequencer.c from die()ing to returning errors instead, as proper library
functions should do, to give callers a chance to clean up after an
error.

Changes since v2:

- the commit message of the read_populate_opts() patch now clarifies why
  we do not take care of git_config_from_file() possibly die()ing.

- the save_head() and the save_opts() conditionals are now separate, for
  improved readability.

- the fast_forward_to() libification now happens in its own commit.

- checkout_fast_forward_to() is now libified, too.

- added a code comment to clarify why we don't care about
  git_parse_source() being able to die().

- added rollbacks in case of failure in read_and_refresh_cache() and
  save_head().


Johannes Schindelin (17):
  sequencer: lib'ify sequencer_pick_revisions()
  sequencer: do not die() in do_pick_commit()
  sequencer: lib'ify write_message()
  sequencer: lib'ify do_recursive_merge()
  sequencer: lib'ify do_pick_commit()
  sequencer: lib'ify walk_revs_populate_todo()
  sequencer: lib'ify prepare_revs()
  sequencer: lib'ify read_and_refresh_cache()
  sequencer: lib'ify read_populate_todo()
  sequencer: lib'ify read_populate_opts()
  sequencer: lib'ify create_seq_dir()
  sequencer: lib'ify save_head()
  sequencer: lib'ify save_todo()
  sequencer: lib'ify save_opts()
  sequencer: lib'ify fast_forward_to()
  lib'ify checkout_fast_forward_to()
  sequencer: ensure to release the lock when we could not read the index

 merge.c     |   9 ++-
 sequencer.c | 197 ++++++++++++++++++++++++++++++++++++++----------------------
 2 files changed, 131 insertions(+), 75 deletions(-)

Published-As: https://github.com/dscho/git/releases/tag/libify-sequencer-v3
Fetch-It-Via: git fetch https://github.com/dscho/git libify-sequencer-v3

Interdiff vs v2:

 diff --git a/merge.c b/merge.c
 index 5db7d56..23866c9 100644
 --- a/merge.c
 +++ b/merge.c
 @@ -57,7 +57,8 @@ int checkout_fast_forward(const unsigned char *head,
  
        refresh_cache(REFRESH_QUIET);
  
 -      hold_locked_index(lock_file, 1);
 +      if (hold_locked_index(lock_file, 0) < 0)
 +              return -1;
  
        memset(&trees, 0, sizeof(trees));
        memset(&opts, 0, sizeof(opts));
 @@ -90,7 +91,9 @@ int checkout_fast_forward(const unsigned char *head,
        }
        if (unpack_trees(nr_trees, t, &opts))
                return -1;
 -      if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
 -              die(_("unable to write new index file"));
 +      if (write_locked_index(&the_index, lock_file, COMMIT_LOCK)) {
 +              rollback_lock_file(lock_file);
 +              return error(_("unable to write new index file"));
 +      }
        return 0;
  }
 diff --git a/sequencer.c b/sequencer.c
 index b6481bb..eec8a60 100644
 --- a/sequencer.c
 +++ b/sequencer.c
 @@ -644,14 +644,18 @@ static int read_and_refresh_cache(struct replay_opts 
*opts)
  {
        static struct lock_file index_lock;
        int index_fd = hold_locked_index(&index_lock, 0);
 -      if (read_index_preload(&the_index, NULL) < 0)
 +      if (read_index_preload(&the_index, NULL) < 0) {
 +              rollback_lock_file(&index_lock);
                return error(_("git %s: failed to read the index"),
                        action_name(opts));
 +      }
        refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, 
NULL);
        if (the_index.cache_changed && index_fd >= 0) {
 -              if (write_locked_index(&the_index, &index_lock, COMMIT_LOCK))
 +              if (write_locked_index(&the_index, &index_lock, COMMIT_LOCK)) {
 +                      rollback_lock_file(&index_lock);
                        return error(_("git %s: failed to refresh the index"),
                                action_name(opts));
 +              }
        }
        rollback_lock_file(&index_lock);
        return 0;
 @@ -812,6 +816,12 @@ static int read_populate_opts(struct replay_opts **opts)
  {
        if (!file_exists(git_path_opts_file()))
                return 0;
 +      /*
 +       * The function git_parse_source(), called from git_config_from_file(),
 +       * may die() in case of a syntactically incorrect file. We do not care
 +       * about this case, though, because we wrote that file ourselves, so we
 +       * are pretty certain that it is syntactically correct.
 +       */
        if (git_config_from_file(populate_opts_cb, git_path_opts_file(), *opts) 
< 0)
                return error(_("Malformed options sheet: %s"),
                        git_path_opts_file());
 @@ -853,14 +863,20 @@ static int save_head(const char *head)
        int fd;
  
        fd = hold_lock_file_for_update(&head_lock, git_path_head_file(), 0);
 -      if (fd < 0)
 +      if (fd < 0) {
 +              rollback_lock_file(&head_lock);
                return error_errno(_("Could not lock HEAD"));
 +      }
        strbuf_addf(&buf, "%s\n", head);
 -      if (write_in_full(fd, buf.buf, buf.len) < 0)
 +      if (write_in_full(fd, buf.buf, buf.len) < 0) {
 +              rollback_lock_file(&head_lock);
                return error_errno(_("Could not write to %s"),
                                   git_path_head_file());
 -      if (commit_lock_file(&head_lock) < 0)
 +      }
 +      if (commit_lock_file(&head_lock) < 0) {
 +              rollback_lock_file(&head_lock);
                return error(_("Error wrapping up %s."), git_path_head_file());
 +      }
        return 0;
  }
  
 @@ -1135,8 +1151,9 @@ int sequencer_pick_revisions(struct replay_opts *opts)
                return -1;
        if (get_sha1("HEAD", sha1) && (opts->action == REPLAY_REVERT))
                return error(_("Can't revert as initial commit"));
 -      if (save_head(sha1_to_hex(sha1)) ||
 -                      save_opts(opts))
 +      if (save_head(sha1_to_hex(sha1)))
 +              return -1;
 +      if (save_opts(opts))
                return -1;
        return pick_commits(todo_list, opts);
  }

-- 
2.10.0.windows.1.10.g803177d

base-commit: 6ebdac1bab966b720d776aa43ca188fe378b1f4b

Reply via email to