Merge branch 'ma/skip-writing-unchanged-index'
authorJunio C Hamano <gitster@pobox.com>
Wed, 21 Mar 2018 18:30:10 +0000 (11:30 -0700)
committerJunio C Hamano <gitster@pobox.com>
Wed, 21 Mar 2018 18:30:10 +0000 (11:30 -0700)
Internal API clean-up to allow write_locked_index() optionally skip
writing the in-core index when it is not modified.

* ma/skip-writing-unchanged-index:
write_locked_index(): add flag to avoid writing unchanged index

1  2 
builtin/add.c
builtin/commit.c
builtin/merge.c
builtin/mv.c
builtin/rm.c
cache.h
read-cache.c
sequencer.c
diff --combined builtin/add.c
index ac7c1c327766c6a729ab0377fd71c2f8e9042d00,9e5a80cc6f3a036e1d1f9e420825f990f6df53da..9ef7fb02d56aac94d104b50aed7d7dfda09cfc98
@@@ -294,7 -294,7 +294,7 @@@ static struct option builtin_add_option
        OPT_BOOL('i', "interactive", &add_interactive, N_("interactive picking")),
        OPT_BOOL('p', "patch", &patch_interactive, N_("select hunks interactively")),
        OPT_BOOL('e', "edit", &edit_interactive, N_("edit current diff and apply")),
 -      OPT__FORCE(&ignored_too, N_("allow adding otherwise ignored files")),
 +      OPT__FORCE(&ignored_too, N_("allow adding otherwise ignored files"), 0),
        OPT_BOOL('u', "update", &take_worktree_changes, N_("update tracked files")),
        OPT_BOOL(0, "renormalize", &add_renormalize, N_("renormalize EOL of tracked files (implies -u)")),
        OPT_BOOL('N', "intent-to-add", &intent_to_add, N_("record only the fact that the path will be added later")),
@@@ -534,10 -534,9 +534,9 @@@ int cmd_add(int argc, const char **argv
        unplug_bulk_checkin();
  
  finish:
-       if (active_cache_changed) {
-               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 | SKIP_IF_UNCHANGED))
+               die(_("Unable to write new index file"));
  
        UNLEAK(pathspec);
        UNLEAK(dir);
diff --combined builtin/commit.c
index 092077c3eef7a81b3d814cee951cee98e37eac02,ff6dac4b9c2a1df9b8ddee9bd2f7f9eee7537034..37fcb55ab0a03a5fdabaca1913bc700201fd8e10
@@@ -389,13 -389,9 +389,9 @@@ static const char *prepare_index(int ar
                if (active_cache_changed
                    || !cache_tree_fully_valid(active_cache_tree))
                        update_main_cache_tree(WRITE_TREE_SILENT);
-               if (active_cache_changed) {
-                       if (write_locked_index(&the_index, &index_lock,
-                                              COMMIT_LOCK))
-                               die(_("unable to write new_index file"));
-               } else {
-                       rollback_lock_file(&index_lock);
-               }
+               if (write_locked_index(&the_index, &index_lock,
+                                      COMMIT_LOCK | SKIP_IF_UNCHANGED))
+                       die(_("unable to write new_index file"));
                commit_style = COMMIT_AS_IS;
                ret = get_index_file();
                goto out;
@@@ -1061,9 -1057,6 +1057,9 @@@ static void finalize_deferred_config(st
                s->show_branch = status_deferred_config.show_branch;
        if (s->show_branch < 0)
                s->show_branch = 0;
 +
 +      if (s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
 +              s->ahead_behind_flags = AHEAD_BEHIND_FULL;
  }
  
  static int parse_and_validate_options(int argc, const char *argv[],
@@@ -1280,8 -1273,6 +1276,8 @@@ int cmd_status(int argc, const char **a
                         N_("show branch information")),
                OPT_BOOL(0, "show-stash", &s.show_stash,
                         N_("show stash information")),
 +              OPT_BOOL(0, "ahead-behind", &s.ahead_behind_flags,
 +                       N_("compute full ahead/behind values")),
                { OPTION_CALLBACK, 0, "porcelain", &status_format,
                  N_("version"), N_("machine-readable output"),
                  PARSE_OPT_OPTARG, opt_parse_porcelain },
@@@ -1407,7 -1398,6 +1403,7 @@@ int run_commit_hook(int editor_is_used
  
  int cmd_commit(int argc, const char **argv, const char *prefix)
  {
 +      const char *argv_gc_auto[] = {"gc", "--auto", NULL};
        static struct wt_status s;
        static struct option builtin_commit_options[] = {
                OPT__QUIET(&quiet, N_("suppress summary after successful commit")),
                OPT_SET_INT(0, "short", &status_format, N_("show status concisely"),
                            STATUS_FORMAT_SHORT),
                OPT_BOOL(0, "branch", &s.show_branch, N_("show branch information")),
 +              OPT_BOOL(0, "ahead-behind", &s.ahead_behind_flags,
 +                       N_("compute full ahead/behind values")),
                OPT_SET_INT(0, "porcelain", &status_format,
                            N_("machine-readable output"), STATUS_FORMAT_PORCELAIN),
                OPT_SET_INT(0, "long", &status_format,
                     "not exceeded, and then \"git reset HEAD\" to recover."));
  
        rerere(0);
 +      run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
        run_commit_hook(use_editor, get_index_file(), "post-commit", NULL);
        if (amend && !no_post_rewrite) {
                commit_post_rewrite(current_head, &oid);
diff --combined builtin/merge.c
index e8d9d4383ed69bd5783b0092a2ee221fc29ce439,7efa3c041d16fe9da504e967ef31dcd19b065d1e..ee050a47f34d7394d048f955baabb37a9e716ef8
@@@ -33,7 -33,6 +33,7 @@@
  #include "sequencer.h"
  #include "string-list.h"
  #include "packfile.h"
 +#include "tag.h"
  
  #define DEFAULT_TWOHEAD (1<<0)
  #define DEFAULT_OCTOPUS (1<<1)
@@@ -521,7 -520,7 +521,7 @@@ static void merge_name(const char *remo
                if (desc && desc->obj && desc->obj->type == OBJ_TAG) {
                        strbuf_addf(msg, "%s\t\t%s '%s'\n",
                                    oid_to_hex(&desc->obj->oid),
 -                                  typename(desc->obj->type),
 +                                  type_name(desc->obj->type),
                                    remote);
                        goto cleanup;
                }
@@@ -652,10 -651,9 +652,9 @@@ static int try_merge_strategy(const cha
  
        hold_locked_index(&lock, LOCK_DIE_ON_ERROR);
        refresh_cache(REFRESH_QUIET);
-       if (active_cache_changed &&
-           write_locked_index(&the_index, &lock, COMMIT_LOCK))
+       if (write_locked_index(&the_index, &lock,
+                              COMMIT_LOCK | SKIP_IF_UNCHANGED))
                return error(_("Unable to write index."));
-       rollback_lock_file(&lock);
  
        if (!strcmp(strategy, "recursive") || !strcmp(strategy, "subtree")) {
                int clean, x;
                                remoteheads->item, reversed, &result);
                if (clean < 0)
                        exit(128);
-               if (active_cache_changed &&
-                   write_locked_index(&the_index, &lock, COMMIT_LOCK))
+               if (write_locked_index(&the_index, &lock,
+                                      COMMIT_LOCK | SKIP_IF_UNCHANGED))
                        die (_("unable to write %s"), get_index_file());
-               rollback_lock_file(&lock);
                return clean ? 0 : 1;
        } else {
                return try_merge_command(strategy, xopts_nr, xopts,
@@@ -811,10 -808,9 +809,9 @@@ static int merge_trivial(struct commit 
  
        hold_locked_index(&lock, LOCK_DIE_ON_ERROR);
        refresh_cache(REFRESH_QUIET);
-       if (active_cache_changed &&
-           write_locked_index(&the_index, &lock, COMMIT_LOCK))
+       if (write_locked_index(&the_index, &lock,
+                              COMMIT_LOCK | SKIP_IF_UNCHANGED))
                return error(_("Unable to write index."));
-       rollback_lock_file(&lock);
  
        write_tree_trivial(&result_tree);
        printf(_("Wonderful.\n"));
@@@ -1126,43 -1122,6 +1123,43 @@@ static struct commit_list *collect_pare
        return remoteheads;
  }
  
 +static int merging_a_throwaway_tag(struct commit *commit)
 +{
 +      char *tag_ref;
 +      struct object_id oid;
 +      int is_throwaway_tag = 0;
 +
 +      /* Are we merging a tag? */
 +      if (!merge_remote_util(commit) ||
 +          !merge_remote_util(commit)->obj ||
 +          merge_remote_util(commit)->obj->type != OBJ_TAG)
 +              return is_throwaway_tag;
 +
 +      /*
 +       * Now we know we are merging a tag object.  Are we downstream
 +       * and following the tags from upstream?  If so, we must have
 +       * the tag object pointed at by "refs/tags/$T" where $T is the
 +       * tagname recorded in the tag object.  We want to allow such
 +       * a "just to catch up" merge to fast-forward.
 +       *
 +       * Otherwise, we are playing an integrator's role, making a
 +       * merge with a throw-away tag from a contributor with
 +       * something like "git pull $contributor $signed_tag".
 +       * We want to forbid such a merge from fast-forwarding
 +       * by default; otherwise we would not keep the signature
 +       * anywhere.
 +       */
 +      tag_ref = xstrfmt("refs/tags/%s",
 +                        ((struct tag *)merge_remote_util(commit)->obj)->tag);
 +      if (!read_ref(tag_ref, &oid) &&
 +          !oidcmp(&oid, &merge_remote_util(commit)->obj->oid))
 +              is_throwaway_tag = 0;
 +      else
 +              is_throwaway_tag = 1;
 +      free(tag_ref);
 +      return is_throwaway_tag;
 +}
 +
  int cmd_merge(int argc, const char **argv, const char *prefix)
  {
        struct object_id result_tree, stash, head_oid;
                            oid_to_hex(&commit->object.oid));
                setenv(buf.buf, merge_remote_util(commit)->name, 1);
                strbuf_reset(&buf);
 -              if (fast_forward != FF_ONLY &&
 -                  merge_remote_util(commit) &&
 -                  merge_remote_util(commit)->obj &&
 -                  merge_remote_util(commit)->obj->type == OBJ_TAG)
 +              if (fast_forward != FF_ONLY && merging_a_throwaway_tag(commit))
                        fast_forward = FF_NO;
        }
  
diff --combined builtin/mv.c
index 8eceb310aae5f8007475f571a0146832eab0be49,ae013d6d20030640e11ea53b09530fa37bebba00..6d141f7a532c08e52f1f5f82330d046c60073f93
@@@ -122,8 -122,7 +122,8 @@@ int cmd_mv(int argc, const char **argv
        struct option builtin_mv_options[] = {
                OPT__VERBOSE(&verbose, N_("be verbose")),
                OPT__DRY_RUN(&show_only, N_("dry run")),
 -              OPT__FORCE(&force, N_("force move/rename even if target exists")),
 +              OPT__FORCE(&force, N_("force move/rename even if target exists"),
 +                         PARSE_OPT_NOCOMPLETE),
                OPT_BOOL('k', NULL, &ignore_errors, N_("skip move/rename errors")),
                OPT_END(),
        };
  
                pos = cache_name_pos(src, strlen(src));
                assert(pos >= 0);
 -              if (!show_only)
 -                      rename_cache_entry_at(pos, dst);
 +              rename_cache_entry_at(pos, dst);
        }
  
        if (gitmodules_modified)
                stage_updated_gitmodules(&the_index);
  
-       if (active_cache_changed &&
-           write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
+       if (write_locked_index(&the_index, &lock_file,
+                              COMMIT_LOCK | SKIP_IF_UNCHANGED))
                die(_("Unable to write new index file"));
  
        return 0;
diff --combined builtin/rm.c
index a818efe230a0667b9e3e5f03efbb11b1db1df273,5d59a0aa65729a5d5c8e3c3c13ab743ce7b064b3..4447bb4d0faf8c34f3fc96361a651eaad396d6d4
@@@ -242,7 -242,7 +242,7 @@@ static struct option builtin_rm_options
        OPT__DRY_RUN(&show_only, N_("dry run")),
        OPT__QUIET(&quiet, N_("do not list removed files")),
        OPT_BOOL( 0 , "cached",         &index_only, N_("only remove from the index")),
 -      OPT__FORCE(&force, N_("override the up-to-date check")),
 +      OPT__FORCE(&force, N_("override the up-to-date check"), PARSE_OPT_NOCOMPLETE),
        OPT_BOOL('r', NULL,             &recursive,  N_("allow recursive removal")),
        OPT_BOOL( 0 , "ignore-unmatch", &ignore_unmatch,
                                N_("exit with a zero status even if nothing matched")),
@@@ -385,10 -385,9 +385,9 @@@ int cmd_rm(int argc, const char **argv
                        stage_updated_gitmodules(&the_index);
        }
  
-       if (active_cache_changed) {
-               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 | SKIP_IF_UNCHANGED))
+               die(_("Unable to write new index file"));
  
        return 0;
  }
diff --combined cache.h
index d06932ed0bd049c8d41906da3508725c8978aa1c,5c880318e0c016a6a5eca49d284963d4a1afcb9d..a61b2d3f0d79b0f56992e0343803811f5265d716
+++ b/cache.h
@@@ -599,6 -599,7 +599,7 @@@ extern int read_index_unmerged(struct i
  
  /* For use with `write_locked_index()`. */
  #define COMMIT_LOCK           (1 << 0)
+ #define SKIP_IF_UNCHANGED     (1 << 1)
  
  /*
   * Write the index while holding an already-taken lock. Close the lock,
   * With `COMMIT_LOCK`, the lock is always committed or rolled back.
   * Without it, the lock is closed, but neither committed nor rolled
   * back.
+  *
+  * If `SKIP_IF_UNCHANGED` is given and the index is unchanged, nothing
+  * is written (and the lock is rolled back if `COMMIT_LOCK` is given).
   */
  extern int write_locked_index(struct index_state *, struct lock_file *lock, unsigned flags);
  
@@@ -1661,7 -1665,7 +1665,7 @@@ struct pack_entry 
   * usual "XXXXXX" trailer, and the resulting filename is written into the
   * "template" buffer. Returns the open descriptor.
   */
 -extern int odb_mkstemp(struct strbuf *template, const char *pattern);
 +extern int odb_mkstemp(struct strbuf *temp_filename, const char *pattern);
  
  /*
   * Create a pack .keep file named "name" (which should generally be the output
@@@ -1732,7 -1736,7 +1736,7 @@@ struct object_info 
        unsigned long *sizep;
        off_t *disk_sizep;
        unsigned char *delta_base_sha1;
 -      struct strbuf *typename;
 +      struct strbuf *type_name;
        void **contentp;
  
        /* Response */
diff --combined read-cache.c
index 977921d90c65ea94b40cb7f8332246bf77c0f215,e2a939a5d2e1d7f16919bce540bceb2727cec4bb..d05eb725b533d3b70daca910df60d27de9ce947c
@@@ -70,20 -70,20 +70,20 @@@ static void replace_index_entry(struct 
  
  void rename_index_entry_at(struct index_state *istate, int nr, const char *new_name)
  {
 -      struct cache_entry *old = istate->cache[nr], *new;
 +      struct cache_entry *old_entry = istate->cache[nr], *new_entry;
        int namelen = strlen(new_name);
  
 -      new = xmalloc(cache_entry_size(namelen));
 -      copy_cache_entry(new, old);
 -      new->ce_flags &= ~CE_HASHED;
 -      new->ce_namelen = namelen;
 -      new->index = 0;
 -      memcpy(new->name, new_name, namelen + 1);
 +      new_entry = xmalloc(cache_entry_size(namelen));
 +      copy_cache_entry(new_entry, old_entry);
 +      new_entry->ce_flags &= ~CE_HASHED;
 +      new_entry->ce_namelen = namelen;
 +      new_entry->index = 0;
 +      memcpy(new_entry->name, new_name, namelen + 1);
  
 -      cache_tree_invalidate_path(istate, old->name);
 -      untracked_cache_remove_from_index(istate, old->name);
 +      cache_tree_invalidate_path(istate, old_entry->name);
 +      untracked_cache_remove_from_index(istate, old_entry->name);
        remove_index_entry_at(istate, nr);
 -      add_index_entry(istate, new, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);
 +      add_index_entry(istate, new_entry, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);
  }
  
  void fill_stat_data(struct stat_data *sd, struct stat *st)
@@@ -615,18 -615,18 +615,18 @@@ static struct cache_entry *create_alias
                                           struct cache_entry *alias)
  {
        int len;
 -      struct cache_entry *new;
 +      struct cache_entry *new_entry;
  
        if (alias->ce_flags & CE_ADDED)
                die("Will not add file alias '%s' ('%s' already exists in index)", ce->name, alias->name);
  
        /* Ok, create the new entry using the name of the existing alias */
        len = ce_namelen(alias);
 -      new = xcalloc(1, cache_entry_size(len));
 -      memcpy(new->name, alias->name, len);
 -      copy_cache_entry(new, ce);
 +      new_entry = xcalloc(1, cache_entry_size(len));
 +      memcpy(new_entry->name, alias->name, len);
 +      copy_cache_entry(new_entry, ce);
        save_or_free_index_entry(istate, ce);
 -      return new;
 +      return new_entry;
  }
  
  void set_object_name_for_intent_to_add_entry(struct cache_entry *ce)
@@@ -1379,7 -1379,7 +1379,7 @@@ int refresh_index(struct index_state *i
        added_fmt = (in_porcelain ? "A\t%s\n" : "%s needs update\n");
        unmerged_fmt = (in_porcelain ? "U\t%s\n" : "%s: needs merge\n");
        for (i = 0; i < istate->cache_nr; i++) {
 -              struct cache_entry *ce, *new;
 +              struct cache_entry *ce, *new_entry;
                int cache_errno = 0;
                int changed = 0;
                int filtered = 0;
                if (filtered)
                        continue;
  
 -              new = refresh_cache_ent(istate, ce, options, &cache_errno, &changed);
 -              if (new == ce)
 +              new_entry = refresh_cache_ent(istate, ce, options, &cache_errno, &changed);
 +              if (new_entry == ce)
                        continue;
 -              if (!new) {
 +              if (!new_entry) {
                        const char *fmt;
  
                        if (really && cache_errno == EINVAL) {
                        continue;
                }
  
 -              replace_index_entry(istate, i, new);
 +              replace_index_entry(istate, i, new_entry);
        }
        trace_performance_since(start, "refresh index");
        return has_errors;
@@@ -2538,6 -2538,12 +2538,12 @@@ int write_locked_index(struct index_sta
        int new_shared_index, ret;
        struct split_index *si = istate->split_index;
  
+       if ((flags & SKIP_IF_UNCHANGED) && !istate->cache_changed) {
+               if (flags & COMMIT_LOCK)
+                       rollback_lock_file(lock);
+               return 0;
+       }
        if (istate->fsmonitor_last_update)
                fill_fsmonitor_bitmap(istate);
  
diff --combined sequencer.c
index 091bd6bda5c55c27e2d0688a5e12988e22e6e431,6ee251849d7a45a989b28a9f87bd9c6ca95c22ba..f9d1001dee9ad10e243aaeafc46fbdd13597fce7
@@@ -517,15 -517,14 +517,14 @@@ static int do_recursive_merge(struct co
                return clean;
        }
  
-       if (active_cache_changed &&
-           write_locked_index(&the_index, &index_lock, COMMIT_LOCK))
+       if (write_locked_index(&the_index, &index_lock,
+                              COMMIT_LOCK | SKIP_IF_UNCHANGED))
                /*
                 * TRANSLATORS: %s will be "revert", "cherry-pick" or
                 * "rebase -i".
                 */
                return error(_("%s: Unable to write new index file"),
                        _(action_name(opts)));
-       rollback_lock_file(&index_lock);
  
        if (!clean)
                append_conflicts_hint(msgbuf);
@@@ -1713,13 -1712,13 +1712,13 @@@ static int read_and_refresh_cache(struc
                        _(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 (index_fd >= 0) {
+               if (write_locked_index(&the_index, &index_lock,
+                                      COMMIT_LOCK | SKIP_IF_UNCHANGED)) {
                        return error(_("git %s: failed to refresh the index"),
                                _(action_name(opts)));
                }
        }
-       rollback_lock_file(&index_lock);
        return 0;
  }
  
@@@ -1869,31 -1868,22 +1868,31 @@@ static int count_commands(struct todo_l
        return count;
  }
  
 +static ssize_t strbuf_read_file_or_whine(struct strbuf *sb, const char *path)
 +{
 +      int fd;
 +      ssize_t len;
 +
 +      fd = open(path, O_RDONLY);
 +      if (fd < 0)
 +              return error_errno(_("could not open '%s'"), path);
 +      len = strbuf_read(sb, fd, 0);
 +      close(fd);
 +      if (len < 0)
 +              return error(_("could not read '%s'."), path);
 +      return len;
 +}
 +
  static int read_populate_todo(struct todo_list *todo_list,
                        struct replay_opts *opts)
  {
        struct stat st;
        const char *todo_file = get_todo_path(opts);
 -      int fd, res;
 +      int res;
  
        strbuf_reset(&todo_list->buf);
 -      fd = open(todo_file, O_RDONLY);
 -      if (fd < 0)
 -              return error_errno(_("could not open '%s'"), todo_file);
 -      if (strbuf_read(&todo_list->buf, fd, 0) < 0) {
 -              close(fd);
 -              return error(_("could not read '%s'."), todo_file);
 -      }
 -      close(fd);
 +      if (strbuf_read_file_or_whine(&todo_list->buf, todo_file) < 0)
 +              return -1;
  
        res = stat(todo_file, &st);
        if (res)
@@@ -2319,9 -2309,6 +2318,9 @@@ static int make_patch(struct commit *co
        p = short_commit_name(commit);
        if (write_message(p, strlen(p), rebase_path_stopped_sha(), 1) < 0)
                return -1;
 +      if (update_ref("rebase", "REBASE_HEAD", &commit->object.oid,
 +                     NULL, REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR))
 +              res |= error(_("could not update %s"), "REBASE_HEAD");
  
        strbuf_addf(&buf, "%s/patch", get_dir(opts));
        memset(&log_tree_opt, 0, sizeof(log_tree_opt));
@@@ -2573,7 -2560,6 +2572,7 @@@ static int pick_commits(struct todo_lis
                        unlink(rebase_path_author_script());
                        unlink(rebase_path_stopped_sha());
                        unlink(rebase_path_amend());
 +                      delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
                }
                if (item->command <= TODO_SQUASH) {
                        if (is_rebase_i(opts))
@@@ -2879,7 -2865,7 +2878,7 @@@ int sequencer_pick_revisions(struct rep
                        if (!lookup_commit_reference_gently(&oid, 1)) {
                                enum object_type type = sha1_object_info(oid.hash, NULL);
                                return error(_("%s: can't cherry-pick a %s"),
 -                                      name, typename(type));
 +                                      name, type_name(type));
                        }
                } else
                        return error(_("%s: bad revision"), name);
@@@ -3160,13 -3146,20 +3159,13 @@@ int check_todo_list(void
        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, fd, i;
 +      int advise_to_edit_todo = 0, res = 0, i;
  
        strbuf_addstr(&todo_file, rebase_path_todo());
 -      fd = open(todo_file.buf, O_RDONLY);
 -      if (fd < 0) {
 -              res = error_errno(_("could not open '%s'"), todo_file.buf);
 -              goto leave_check;
 -      }
 -      if (strbuf_read(&todo_list.buf, fd, 0) < 0) {
 -              close(fd);
 -              res = error(_("could not read '%s'."), todo_file.buf);
 +      if (strbuf_read_file_or_whine(&todo_list.buf, todo_file.buf) < 0) {
 +              res = -1;
                goto leave_check;
        }
 -      close(fd);
        advise_to_edit_todo = res =
                parse_insn_buffer(todo_list.buf.buf, &todo_list);
  
  
        todo_list_release(&todo_list);
        strbuf_addstr(&todo_file, ".backup");
 -      fd = open(todo_file.buf, O_RDONLY);
 -      if (fd < 0) {
 -              res = error_errno(_("could not open '%s'"), todo_file.buf);
 -              goto leave_check;
 -      }
 -      if (strbuf_read(&todo_list.buf, fd, 0) < 0) {
 -              close(fd);
 -              res = error(_("could not read '%s'."), todo_file.buf);
 +      if (strbuf_read_file_or_whine(&todo_list.buf, todo_file.buf) < 0) {
 +              res = -1;
                goto leave_check;
        }
 -      close(fd);
        strbuf_release(&todo_file);
        res = !!parse_insn_buffer(todo_list.buf.buf, &todo_list);
  
@@@ -3266,8 -3266,15 +3265,8 @@@ int skip_unnecessary_picks(void
        }
        strbuf_release(&buf);
  
 -      fd = open(todo_file, O_RDONLY);
 -      if (fd < 0) {
 -              return error_errno(_("could not open '%s'"), todo_file);
 -      }
 -      if (strbuf_read(&todo_list.buf, fd, 0) < 0) {
 -              close(fd);
 -              return error(_("could not read '%s'."), todo_file);
 -      }
 -      close(fd);
 +      if (strbuf_read_file_or_whine(&todo_list.buf, todo_file) < 0)
 +              return -1;
        if (parse_insn_buffer(todo_list.buf.buf, &todo_list) < 0) {
                todo_list_release(&todo_list);
                return -1;
@@@ -3358,11 -3365,17 +3357,11 @@@ int rearrange_squash(void
        const char *todo_file = rebase_path_todo();
        struct todo_list todo_list = TODO_LIST_INIT;
        struct hashmap subject2item;
 -      int res = 0, rearranged = 0, *next, *tail, fd, i;
 +      int res = 0, rearranged = 0, *next, *tail, i;
        char **subjects;
  
 -      fd = open(todo_file, O_RDONLY);
 -      if (fd < 0)
 -              return error_errno(_("could not open '%s'"), todo_file);
 -      if (strbuf_read(&todo_list.buf, fd, 0) < 0) {
 -              close(fd);
 -              return error(_("could not read '%s'."), todo_file);
 -      }
 -      close(fd);
 +      if (strbuf_read_file_or_whine(&todo_list.buf, todo_file) < 0)
 +              return -1;
        if (parse_insn_buffer(todo_list.buf.buf, &todo_list) < 0) {
                todo_list_release(&todo_list);
                return -1;