Merge branch 'mh/reporting-broken-refs-from-for-each-ref'
authorJunio C Hamano <gitster@pobox.com>
Wed, 24 Jun 2015 19:21:51 +0000 (12:21 -0700)
committerJunio C Hamano <gitster@pobox.com>
Wed, 24 Jun 2015 19:21:52 +0000 (12:21 -0700)
"git for-each-ref" reported "missing object" for 0{40} when it
encounters a broken ref. The lack of object whose name is 0{40} is
not the problem; the ref being broken is.

* mh/reporting-broken-refs-from-for-each-ref:
read_loose_refs(): treat NULL_SHA1 loose references as broken
read_loose_refs(): simplify function logic
for-each-ref: report broken references correctly
t6301: new tests of for-each-ref error handling

1  2 
builtin/for-each-ref.c
refs.c
diff --combined builtin/for-each-ref.c
index f7e51a7fadc40b1e4484a657fc55af8a3043967c,4a15f568f116fe797d257274ce0782be9e4e8aeb..cb7db230d3ef76ee6a8b73971cbdabcc6d6590a9
@@@ -74,7 -74,6 +74,7 @@@ static struct 
        { "contents:body" },
        { "contents:signature" },
        { "upstream" },
 +      { "push" },
        { "symref" },
        { "flag" },
        { "HEAD" },
@@@ -660,26 -659,15 +660,26 @@@ static void populate_value(struct refin
                else if (starts_with(name, "symref"))
                        refname = ref->symref ? ref->symref : "";
                else if (starts_with(name, "upstream")) {
 +                      const char *branch_name;
                        /* only local branches may have an upstream */
 -                      if (!starts_with(ref->refname, "refs/heads/"))
 +                      if (!skip_prefix(ref->refname, "refs/heads/",
 +                                       &branch_name))
                                continue;
 -                      branch = branch_get(ref->refname + 11);
 +                      branch = branch_get(branch_name);
  
 -                      if (!branch || !branch->merge || !branch->merge[0] ||
 -                          !branch->merge[0]->dst)
 +                      refname = branch_get_upstream(branch, NULL);
 +                      if (!refname)
 +                              continue;
 +              } else if (starts_with(name, "push")) {
 +                      const char *branch_name;
 +                      if (!skip_prefix(ref->refname, "refs/heads/",
 +                                       &branch_name))
 +                              continue;
 +                      branch = branch_get(branch_name);
 +
 +                      refname = branch_get_push(branch, NULL);
 +                      if (!refname)
                                continue;
 -                      refname = branch->merge[0]->dst;
                } else if (starts_with(name, "color:")) {
                        char color[COLOR_MAXLEN] = "";
  
                                refname = shorten_unambiguous_ref(refname,
                                                      warn_ambiguous_refs);
                        else if (!strcmp(formatp, "track") &&
 -                               starts_with(name, "upstream")) {
 +                               (starts_with(name, "upstream") ||
 +                                starts_with(name, "push"))) {
                                char buf[40];
  
                                if (stat_tracking_info(branch, &num_ours,
 -                                                     &num_theirs) != 1)
 +                                                     &num_theirs, NULL))
                                        continue;
  
                                if (!num_ours && !num_theirs)
                                }
                                continue;
                        } else if (!strcmp(formatp, "trackshort") &&
 -                                 starts_with(name, "upstream")) {
 +                                 (starts_with(name, "upstream") ||
 +                                  starts_with(name, "push"))) {
                                assert(branch);
  
                                if (stat_tracking_info(branch, &num_ours,
 -                                                      &num_theirs) != 1)
 +                                                      &num_theirs, NULL))
                                        continue;
  
                                if (!num_ours && !num_theirs)
@@@ -854,8 -840,7 +854,8 @@@ struct grab_ref_cbdata 
   * A call-back given to for_each_ref().  Filter refs and keep them for
   * later object processing.
   */
 -static int grab_single_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 +static int grab_single_ref(const char *refname, const struct object_id *oid,
 +                         int flag, void *cb_data)
  {
        struct grab_ref_cbdata *cb = cb_data;
        struct refinfo *ref;
                  return 0;
        }
  
+       if (flag & REF_ISBROKEN) {
+                 warning("ignoring broken ref %s", refname);
+                 return 0;
+       }
        if (*cb->grab_pattern) {
                const char **pattern;
                int namelen = strlen(refname);
         */
        ref = xcalloc(1, sizeof(*ref));
        ref->refname = xstrdup(refname);
 -      hashcpy(ref->objectname, sha1);
 +      hashcpy(ref->objectname, oid->hash);
        ref->flag = flag;
  
        cnt = cb->grab_cnt;
@@@ -1075,7 -1065,7 +1080,7 @@@ static int opt_parse_sort(const struct 
  }
  
  static char const * const for_each_ref_usage[] = {
 -      N_("git for-each-ref [options] [<pattern>]"),
 +      N_("git for-each-ref [<options>] [<pattern>]"),
        NULL
  };
  
diff --combined refs.c
index 26d1ac1e32eb4fc4c6729abcf0354dbad6177d1b,07f8847e6d78423a4b70b9e66d6ddfe97845e421..7ac05cf21a25802f8e16d45ef75e37b7de2cda8a
--- 1/refs.c
--- 2/refs.c
+++ b/refs.c
@@@ -6,13 -6,6 +6,13 @@@
  #include "dir.h"
  #include "string-list.h"
  
 +struct ref_lock {
 +      char *ref_name;
 +      char *orig_ref_name;
 +      struct lock_file *lk;
 +      struct object_id old_oid;
 +};
 +
  /*
   * How to handle various characters in refnames:
   * 0: An acceptable character for refs
@@@ -33,35 -26,10 +33,35 @@@ static unsigned char refname_dispositio
  };
  
  /*
 - * Used as a flag to ref_transaction_delete when a loose ref is being
 + * Flag passed to lock_ref_sha1_basic() telling it to tolerate broken
 + * refs (i.e., because the reference is about to be deleted anyway).
 + */
 +#define REF_DELETING  0x02
 +
 +/*
 + * Used as a flag in ref_update::flags when a loose ref is being
   * pruned.
   */
 -#define REF_ISPRUNING 0x0100
 +#define REF_ISPRUNING 0x04
 +
 +/*
 + * Used as a flag in ref_update::flags when the reference should be
 + * updated to new_sha1.
 + */
 +#define REF_HAVE_NEW  0x08
 +
 +/*
 + * Used as a flag in ref_update::flags when old_sha1 should be
 + * checked.
 + */
 +#define REF_HAVE_OLD  0x10
 +
 +/*
 + * Used as a flag in ref_update::flags when the lockfile needs to be
 + * committed.
 + */
 +#define REF_NEEDS_COMMIT 0x20
 +
  /*
   * Try to read one refname component from the front of refname.
   * Return the length of the component found, or -1 if the component is
@@@ -161,7 -129,7 +161,7 @@@ struct ref_value 
         * null.  If REF_ISSYMREF, then this is the name of the object
         * referred to by the last reference in the symlink chain.
         */
 -      unsigned char sha1[20];
 +      struct object_id oid;
  
        /*
         * If REF_KNOWS_PEELED, then this field holds the peeled value
         * be peelable.  See the documentation for peel_ref() for an
         * exact definition of "peelable".
         */
 -      unsigned char peeled[20];
 +      struct object_id peeled;
  };
  
  struct ref_cache;
@@@ -268,7 -236,7 +268,7 @@@ struct ref_dir 
   * presence of an empty subdirectory does not block the creation of a
   * similarly-named reference.  (The fact that reference names with the
   * same leading components can conflict *with each other* is a
 - * separate issue that is regulated by is_refname_available().)
 + * separate issue that is regulated by verify_refname_available().)
   *
   * Please note that the name field contains the fully-qualified
   * reference (or subdirectory) name.  Space could be saved by only
@@@ -349,10 -317,12 +349,10 @@@ static struct ref_entry *create_ref_ent
        if (check_name &&
            check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))
                die("Reference has invalid format: '%s'", refname);
 -      if (!check_name && !refname_is_safe(refname))
 -              die("Reference has invalid name: '%s'", refname);
        len = strlen(refname) + 1;
        ref = xmalloc(sizeof(struct ref_entry) + len);
 -      hashcpy(ref->u.value.sha1, sha1);
 -      hashclr(ref->u.value.peeled);
 +      hashcpy(ref->u.value.oid.hash, sha1);
 +      oidclr(&ref->u.value.peeled);
        memcpy(ref->name, refname, len);
        ref->flag = flag;
        return ref;
@@@ -626,7 -596,7 +626,7 @@@ static int is_dup_ref(const struct ref_
                /* This is impossible by construction */
                die("Reference directory conflict: %s", ref1->name);
  
 -      if (hashcmp(ref1->u.value.sha1, ref2->u.value.sha1))
 +      if (oidcmp(&ref1->u.value.oid, &ref2->u.value.oid))
                die("Duplicated ref, and SHA1s don't match: %s", ref1->name);
  
        warning("Duplicated ref: %s", ref1->name);
@@@ -674,7 -644,7 +674,7 @@@ static int ref_resolves_to_object(struc
  {
        if (entry->flag & REF_ISBROKEN)
                return 0;
 -      if (!has_sha1_file(entry->u.value.sha1)) {
 +      if (!has_sha1_file(entry->u.value.oid.hash)) {
                error("%s does not point to a valid object!", entry->name);
                return 0;
        }
@@@ -722,7 -692,7 +722,7 @@@ static int do_one_ref(struct ref_entry 
        /* Store the old value, in case this is a recursive call: */
        old_current_ref = current_ref;
        current_ref = entry;
 -      retval = data->fn(entry->name + data->trim, entry->u.value.sha1,
 +      retval = data->fn(entry->name + data->trim, &entry->u.value.oid,
                          entry->flag, data->cb_data);
        current_ref = old_current_ref;
        return retval;
@@@ -844,181 -814,121 +844,181 @@@ static void prime_ref_dir(struct ref_di
        }
  }
  
 -static int entry_matches(struct ref_entry *entry, const struct string_list *list)
 -{
 -      return list && string_list_has_string(list, entry->name);
 -}
 -
  struct nonmatching_ref_data {
        const struct string_list *skip;
 -      struct ref_entry *found;
 +      const char *conflicting_refname;
  };
  
  static int nonmatching_ref_fn(struct ref_entry *entry, void *vdata)
  {
        struct nonmatching_ref_data *data = vdata;
  
 -      if (entry_matches(entry, data->skip))
 +      if (data->skip && string_list_has_string(data->skip, entry->name))
                return 0;
  
 -      data->found = entry;
 +      data->conflicting_refname = entry->name;
        return 1;
  }
  
 -static void report_refname_conflict(struct ref_entry *entry,
 -                                  const char *refname)
 -{
 -      error("'%s' exists; cannot create '%s'", entry->name, refname);
 -}
 -
  /*
 - * Return true iff a reference named refname could be created without
 - * conflicting with the name of an existing reference in dir.  If
 - * skip is non-NULL, ignore potential conflicts with refs in skip
 - * (e.g., because they are scheduled for deletion in the same
 - * operation).
 + * Return 0 if a reference named refname could be created without
 + * conflicting with the name of an existing reference in dir.
 + * Otherwise, return a negative value and write an explanation to err.
 + * If extras is non-NULL, it is a list of additional refnames with
 + * which refname is not allowed to conflict. If skip is non-NULL,
 + * ignore potential conflicts with refs in skip (e.g., because they
 + * are scheduled for deletion in the same operation). Behavior is
 + * undefined if the same name is listed in both extras and skip.
   *
   * Two reference names conflict if one of them exactly matches the
 - * leading components of the other; e.g., "foo/bar" conflicts with
 - * both "foo" and with "foo/bar/baz" but not with "foo/bar" or
 - * "foo/barbados".
 + * leading components of the other; e.g., "refs/foo/bar" conflicts
 + * with both "refs/foo" and with "refs/foo/bar/baz" but not with
 + * "refs/foo/bar" or "refs/foo/barbados".
   *
 - * skip must be sorted.
 + * extras and skip must be sorted.
   */
 -static int is_refname_available(const char *refname,
 -                              const struct string_list *skip,
 -                              struct ref_dir *dir)
 +static int verify_refname_available(const char *refname,
 +                                  const struct string_list *extras,
 +                                  const struct string_list *skip,
 +                                  struct ref_dir *dir,
 +                                  struct strbuf *err)
  {
        const char *slash;
 -      size_t len;
        int pos;
 -      char *dirname;
 +      struct strbuf dirname = STRBUF_INIT;
 +      int ret = -1;
 +
 +      /*
 +       * For the sake of comments in this function, suppose that
 +       * refname is "refs/foo/bar".
 +       */
 +
 +      assert(err);
  
 +      strbuf_grow(&dirname, strlen(refname) + 1);
        for (slash = strchr(refname, '/'); slash; slash = strchr(slash + 1, '/')) {
 +              /* Expand dirname to the new prefix, not including the trailing slash: */
 +              strbuf_add(&dirname, refname + dirname.len, slash - refname - dirname.len);
 +
                /*
 -               * We are still at a leading dir of the refname; we are
 -               * looking for a conflict with a leaf entry.
 -               *
 -               * If we find one, we still must make sure it is
 -               * not in "skip".
 +               * We are still at a leading dir of the refname (e.g.,
 +               * "refs/foo"; if there is a reference with that name,
 +               * it is a conflict, *unless* it is in skip.
                 */
 -              pos = search_ref_dir(dir, refname, slash - refname);
 -              if (pos >= 0) {
 -                      struct ref_entry *entry = dir->entries[pos];
 -                      if (entry_matches(entry, skip))
 -                              return 1;
 -                      report_refname_conflict(entry, refname);
 -                      return 0;
 +              if (dir) {
 +                      pos = search_ref_dir(dir, dirname.buf, dirname.len);
 +                      if (pos >= 0 &&
 +                          (!skip || !string_list_has_string(skip, dirname.buf))) {
 +                              /*
 +                               * We found a reference whose name is
 +                               * a proper prefix of refname; e.g.,
 +                               * "refs/foo", and is not in skip.
 +                               */
 +                              strbuf_addf(err, "'%s' exists; cannot create '%s'",
 +                                          dirname.buf, refname);
 +                              goto cleanup;
 +                      }
                }
  
 +              if (extras && string_list_has_string(extras, dirname.buf) &&
 +                  (!skip || !string_list_has_string(skip, dirname.buf))) {
 +                      strbuf_addf(err, "cannot process '%s' and '%s' at the same time",
 +                                  refname, dirname.buf);
 +                      goto cleanup;
 +              }
  
                /*
                 * Otherwise, we can try to continue our search with
 -               * the next component; if we come up empty, we know
 -               * there is nothing under this whole prefix.
 +               * the next component. So try to look up the
 +               * directory, e.g., "refs/foo/". If we come up empty,
 +               * we know there is nothing under this whole prefix,
 +               * but even in that case we still have to continue the
 +               * search for conflicts with extras.
                 */
 -              pos = search_ref_dir(dir, refname, slash + 1 - refname);
 -              if (pos < 0)
 -                      return 1;
 -
 -              dir = get_ref_dir(dir->entries[pos]);
 +              strbuf_addch(&dirname, '/');
 +              if (dir) {
 +                      pos = search_ref_dir(dir, dirname.buf, dirname.len);
 +                      if (pos < 0) {
 +                              /*
 +                               * There was no directory "refs/foo/",
 +                               * so there is nothing under this
 +                               * whole prefix. So there is no need
 +                               * to continue looking for conflicting
 +                               * references. But we need to continue
 +                               * looking for conflicting extras.
 +                               */
 +                              dir = NULL;
 +                      } else {
 +                              dir = get_ref_dir(dir->entries[pos]);
 +                      }
 +              }
        }
  
        /*
 -       * We are at the leaf of our refname; we want to
 -       * make sure there are no directories which match it.
 +       * We are at the leaf of our refname (e.g., "refs/foo/bar").
 +       * There is no point in searching for a reference with that
 +       * name, because a refname isn't considered to conflict with
 +       * itself. But we still need to check for references whose
 +       * names are in the "refs/foo/bar/" namespace, because they
 +       * *do* conflict.
         */
 -      len = strlen(refname);
 -      dirname = xmallocz(len + 1);
 -      sprintf(dirname, "%s/", refname);
 -      pos = search_ref_dir(dir, dirname, len + 1);
 -      free(dirname);
 +      strbuf_addstr(&dirname, refname + dirname.len);
 +      strbuf_addch(&dirname, '/');
 +
 +      if (dir) {
 +              pos = search_ref_dir(dir, dirname.buf, dirname.len);
  
 -      if (pos >= 0) {
 +              if (pos >= 0) {
 +                      /*
 +                       * We found a directory named "$refname/"
 +                       * (e.g., "refs/foo/bar/"). It is a problem
 +                       * iff it contains any ref that is not in
 +                       * "skip".
 +                       */
 +                      struct nonmatching_ref_data data;
 +
 +                      data.skip = skip;
 +                      data.conflicting_refname = NULL;
 +                      dir = get_ref_dir(dir->entries[pos]);
 +                      sort_ref_dir(dir);
 +                      if (do_for_each_entry_in_dir(dir, 0, nonmatching_ref_fn, &data)) {
 +                              strbuf_addf(err, "'%s' exists; cannot create '%s'",
 +                                          data.conflicting_refname, refname);
 +                              goto cleanup;
 +                      }
 +              }
 +      }
 +
 +      if (extras) {
                /*
 -               * We found a directory named "refname". It is a
 -               * problem iff it contains any ref that is not
 -               * in "skip".
 +               * Check for entries in extras that start with
 +               * "$refname/". We do that by looking for the place
 +               * where "$refname/" would be inserted in extras. If
 +               * there is an entry at that position that starts with
 +               * "$refname/" and is not in skip, then we have a
 +               * conflict.
                 */
 -              struct ref_entry *entry = dir->entries[pos];
 -              struct ref_dir *dir = get_ref_dir(entry);
 -              struct nonmatching_ref_data data;
 +              for (pos = string_list_find_insert_index(extras, dirname.buf, 0);
 +                   pos < extras->nr; pos++) {
 +                      const char *extra_refname = extras->items[pos].string;
  
 -              data.skip = skip;
 -              sort_ref_dir(dir);
 -              if (!do_for_each_entry_in_dir(dir, 0, nonmatching_ref_fn, &data))
 -                      return 1;
 +                      if (!starts_with(extra_refname, dirname.buf))
 +                              break;
  
 -              report_refname_conflict(data.found, refname);
 -              return 0;
 +                      if (!skip || !string_list_has_string(skip, extra_refname)) {
 +                              strbuf_addf(err, "cannot process '%s' and '%s' at the same time",
 +                                          refname, extra_refname);
 +                              goto cleanup;
 +                      }
 +              }
        }
  
 -      /*
 -       * There is no point in searching for another leaf
 -       * node which matches it; such an entry would be the
 -       * ref we are looking for, not a conflict.
 -       */
 -      return 1;
 +      /* No conflicts were found */
 +      ret = 0;
 +
 +cleanup:
 +      strbuf_release(&dirname);
 +      return ret;
  }
  
  struct packed_ref_cache {
@@@ -1241,8 -1151,6 +1241,8 @@@ static void read_packed_refs(FILE *f, s
                        int flag = REF_ISPACKED;
  
                        if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
 +                              if (!refname_is_safe(refname))
 +                                      die("packed refname is dangerous: %s", refname);
                                hashclr(sha1);
                                flag |= REF_BAD_NAME | REF_ISBROKEN;
                        }
                    line.len == PEELED_LINE_LENGTH &&
                    line.buf[PEELED_LINE_LENGTH - 1] == '\n' &&
                    !get_sha1_hex(line.buf + 1, sha1)) {
 -                      hashcpy(last->u.value.peeled, sha1);
 +                      hashcpy(last->u.value.peeled.hash, sha1);
                        /*
                         * Regardless of what the file header said,
                         * we definitely know the value of *this*
@@@ -1373,23 -1281,36 +1373,38 @@@ static void read_loose_refs(const char 
                                         create_dir_entry(refs, refname.buf,
                                                          refname.len, 1));
                } else {
+                       int read_ok;
                        if (*refs->name) {
                                hashclr(sha1);
                                flag = 0;
-                               if (resolve_gitlink_ref(refs->name, refname.buf, sha1) < 0) {
-                                       hashclr(sha1);
-                                       flag |= REF_ISBROKEN;
-                               }
-                       } else if (read_ref_full(refname.buf,
-                                                RESOLVE_REF_READING,
-                                                sha1, &flag)) {
+                               read_ok = !resolve_gitlink_ref(refs->name,
+                                                              refname.buf, sha1);
+                       } else {
+                               read_ok = !read_ref_full(refname.buf,
+                                                        RESOLVE_REF_READING,
+                                                        sha1, &flag);
+                       }
+                       if (!read_ok) {
                                hashclr(sha1);
                                flag |= REF_ISBROKEN;
+                       } else if (is_null_sha1(sha1)) {
+                               /*
+                                * It is so astronomically unlikely
+                                * that NULL_SHA1 is the SHA-1 of an
+                                * actual object that we consider its
+                                * appearance in a loose reference
+                                * file to be repo corruption
+                                * (probably due to a software bug).
+                                */
+                               flag |= REF_ISBROKEN;
                        }
                        if (check_refname_format(refname.buf,
                                                 REFNAME_ALLOW_ONELEVEL)) {
 +                              if (!refname_is_safe(refname.buf))
 +                                      die("loose refname is dangerous: %s", refname.buf);
                                hashclr(sha1);
                                flag |= REF_BAD_NAME | REF_ISBROKEN;
                        }
@@@ -1439,7 -1360,7 +1454,7 @@@ static int resolve_gitlink_packed_ref(s
        if (ref == NULL)
                return -1;
  
 -      hashcpy(sha1, ref->u.value.sha1);
 +      hashcpy(sha1, ref->u.value.oid.hash);
        return 0;
  }
  
@@@ -1449,7 -1370,7 +1464,7 @@@ static int resolve_gitlink_ref_recursiv
  {
        int fd, len;
        char buffer[128], *p;
 -      char *path;
 +      const char *path;
  
        if (recursion > MAXDEPTH || strlen(refname) > MAXREFLEN)
                return -1;
@@@ -1526,7 -1447,7 +1541,7 @@@ static int resolve_missing_loose_ref(co
         */
        entry = get_packed_ref(refname);
        if (entry) {
 -              hashcpy(sha1, entry->u.value.sha1);
 +              hashcpy(sha1, entry->u.value.oid.hash);
                if (flags)
                        *flags |= REF_ISPACKED;
                return 0;
  }
  
  /* This function needs to return a meaningful errno on failure */
 -const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned char *sha1, int *flags)
 +static const char *resolve_ref_unsafe_1(const char *refname,
 +                                      int resolve_flags,
 +                                      unsigned char *sha1,
 +                                      int *flags,
 +                                      struct strbuf *sb_path)
  {
        int depth = MAXDEPTH;
        ssize_t len;
                bad_name = 1;
        }
        for (;;) {
 -              char path[PATH_MAX];
 +              const char *path;
                struct stat st;
                char *buf;
                int fd;
                        return NULL;
                }
  
 -              git_snpath(path, sizeof(path), "%s", refname);
 +              strbuf_reset(sb_path);
 +              strbuf_git_path(sb_path, "%s", refname);
 +              path = sb_path->buf;
  
                /*
                 * We might have to loop back here to avoid a race
        }
  }
  
 +const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
 +                             unsigned char *sha1, int *flags)
 +{
 +      struct strbuf sb_path = STRBUF_INIT;
 +      const char *ret = resolve_ref_unsafe_1(refname, resolve_flags,
 +                                             sha1, flags, &sb_path);
 +      strbuf_release(&sb_path);
 +      return ret;
 +}
 +
  char *resolve_refdup(const char *ref, int resolve_flags, unsigned char *sha1, int *flags)
  {
        return xstrdup_or_null(resolve_ref_unsafe(ref, resolve_flags, sha1, flags));
@@@ -1756,14 -1661,13 +1771,14 @@@ int ref_exists(const char *refname
        return !!resolve_ref_unsafe(refname, RESOLVE_REF_READING, sha1, NULL);
  }
  
 -static int filter_refs(const char *refname, const unsigned char *sha1, int flags,
 -                     void *data)
 +static int filter_refs(const char *refname, const struct object_id *oid,
 +                         int flags, void *data)
  {
        struct ref_filter *filter = (struct ref_filter *)data;
 +
        if (wildmatch(filter->pattern, refname, 0, NULL))
                return 0;
 -      return filter->fn(refname, sha1, flags, filter->cb_data);
 +      return filter->fn(refname, oid, flags, filter->cb_data);
  }
  
  enum peel_status {
@@@ -1837,9 -1741,9 +1852,9 @@@ static enum peel_status peel_entry(stru
        if (entry->flag & REF_KNOWS_PEELED) {
                if (repeel) {
                        entry->flag &= ~REF_KNOWS_PEELED;
 -                      hashclr(entry->u.value.peeled);
 +                      oidclr(&entry->u.value.peeled);
                } else {
 -                      return is_null_sha1(entry->u.value.peeled) ?
 +                      return is_null_oid(&entry->u.value.peeled) ?
                                PEEL_NON_TAG : PEEL_PEELED;
                }
        }
        if (entry->flag & REF_ISSYMREF)
                return PEEL_IS_SYMREF;
  
 -      status = peel_object(entry->u.value.sha1, entry->u.value.peeled);
 +      status = peel_object(entry->u.value.oid.hash, entry->u.value.peeled.hash);
        if (status == PEEL_PEELED || status == PEEL_NON_TAG)
                entry->flag |= REF_KNOWS_PEELED;
        return status;
@@@ -1863,7 -1767,7 +1878,7 @@@ int peel_ref(const char *refname, unsig
                            || !strcmp(current_ref->name, refname))) {
                if (peel_entry(current_ref, 0))
                        return -1;
 -              hashcpy(sha1, current_ref->u.value.peeled);
 +              hashcpy(sha1, current_ref->u.value.peeled.hash);
                return 0;
        }
  
                if (r) {
                        if (peel_entry(r, 0))
                                return -1;
 -                      hashcpy(sha1, r->u.value.peeled);
 +                      hashcpy(sha1, r->u.value.peeled.hash);
                        return 0;
                }
        }
@@@ -1898,17 -1802,17 +1913,17 @@@ struct warn_if_dangling_data 
        const char *msg_fmt;
  };
  
 -static int warn_if_dangling_symref(const char *refname, const unsigned char *sha1,
 +static int warn_if_dangling_symref(const char *refname, const struct object_id *oid,
                                   int flags, void *cb_data)
  {
        struct warn_if_dangling_data *d = cb_data;
        const char *resolves_to;
 -      unsigned char junk[20];
 +      struct object_id junk;
  
        if (!(flags & REF_ISSYMREF))
                return 0;
  
 -      resolves_to = resolve_ref_unsafe(refname, 0, junk, NULL);
 +      resolves_to = resolve_ref_unsafe(refname, 0, junk.hash, NULL);
        if (!resolves_to
            || (d->refname
                ? strcmp(resolves_to, d->refname)
@@@ -2028,18 -1932,18 +2043,18 @@@ static int do_for_each_ref(struct ref_c
  
  static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data)
  {
 -      unsigned char sha1[20];
 +      struct object_id oid;
        int flag;
  
        if (submodule) {
 -              if (resolve_gitlink_ref(submodule, "HEAD", sha1) == 0)
 -                      return fn("HEAD", sha1, 0, cb_data);
 +              if (resolve_gitlink_ref(submodule, "HEAD", oid.hash) == 0)
 +                      return fn("HEAD", &oid, 0, cb_data);
  
                return 0;
        }
  
 -      if (!read_ref_full("HEAD", RESOLVE_REF_READING, sha1, &flag))
 -              return fn("HEAD", sha1, flag, cb_data);
 +      if (!read_ref_full("HEAD", RESOLVE_REF_READING, oid.hash, &flag))
 +              return fn("HEAD", &oid, flag, cb_data);
  
        return 0;
  }
@@@ -2114,12 -2018,12 +2129,12 @@@ int head_ref_namespaced(each_ref_fn fn
  {
        struct strbuf buf = STRBUF_INIT;
        int ret = 0;
 -      unsigned char sha1[20];
 +      struct object_id oid;
        int flag;
  
        strbuf_addf(&buf, "%sHEAD", get_git_namespace());
 -      if (!read_ref_full(buf.buf, RESOLVE_REF_READING, sha1, &flag))
 -              ret = fn(buf.buf, sha1, flag, cb_data);
 +      if (!read_ref_full(buf.buf, RESOLVE_REF_READING, oid.hash, &flag))
 +              ret = fn(buf.buf, &oid, flag, cb_data);
        strbuf_release(&buf);
  
        return ret;
@@@ -2209,45 -2113,27 +2224,45 @@@ int refname_match(const char *abbrev_na
        return 0;
  }
  
 -/* This function should make sure errno is meaningful on error */
 -static struct ref_lock *verify_lock(struct ref_lock *lock,
 -      const unsigned char *old_sha1, int mustexist)
 +static void unlock_ref(struct ref_lock *lock)
 +{
 +      /* Do not free lock->lk -- atexit() still looks at them */
 +      if (lock->lk)
 +              rollback_lock_file(lock->lk);
 +      free(lock->ref_name);
 +      free(lock->orig_ref_name);
 +      free(lock);
 +}
 +
 +/*
 + * Verify that the reference locked by lock has the value old_sha1.
 + * Fail if the reference doesn't exist and mustexist is set. Return 0
 + * on success. On error, write an error message to err, set errno, and
 + * return a negative value.
 + */
 +static int verify_lock(struct ref_lock *lock,
 +                     const unsigned char *old_sha1, int mustexist,
 +                     struct strbuf *err)
  {
 +      assert(err);
 +
        if (read_ref_full(lock->ref_name,
                          mustexist ? RESOLVE_REF_READING : 0,
 -                        lock->old_sha1, NULL)) {
 +                        lock->old_oid.hash, NULL)) {
                int save_errno = errno;
 -              error("Can't verify ref %s", lock->ref_name);
 -              unlock_ref(lock);
 +              strbuf_addf(err, "can't verify ref %s", lock->ref_name);
                errno = save_errno;
 -              return NULL;
 +              return -1;
        }
 -      if (hashcmp(lock->old_sha1, old_sha1)) {
 -              error("Ref %s is at %s but expected %s", lock->ref_name,
 -                      sha1_to_hex(lock->old_sha1), sha1_to_hex(old_sha1));
 -              unlock_ref(lock);
 +      if (hashcmp(lock->old_oid.hash, old_sha1)) {
 +              strbuf_addf(err, "ref %s is at %s but expected %s",
 +                          lock->ref_name,
 +                          sha1_to_hex(lock->old_oid.hash),
 +                          sha1_to_hex(old_sha1));
                errno = EBUSY;
 -              return NULL;
 +              return -1;
        }
 -      return lock;
 +      return 0;
  }
  
  static int remove_empty_directories(const char *file)
@@@ -2363,23 -2249,21 +2378,23 @@@ int dwim_log(const char *str, int len, 
   */
  static struct ref_lock *lock_ref_sha1_basic(const char *refname,
                                            const unsigned char *old_sha1,
 +                                          const struct string_list *extras,
                                            const struct string_list *skip,
 -                                          int flags, int *type_p)
 +                                          unsigned int flags, int *type_p,
 +                                          struct strbuf *err)
  {
 -      char *ref_file;
 +      const char *ref_file;
        const char *orig_refname = refname;
        struct ref_lock *lock;
        int last_errno = 0;
        int type, lflags;
        int mustexist = (old_sha1 && !is_null_sha1(old_sha1));
        int resolve_flags = 0;
 -      int missing = 0;
        int attempts_remaining = 3;
  
 +      assert(err);
 +
        lock = xcalloc(1, sizeof(struct ref_lock));
 -      lock->lock_fd = -1;
  
        if (mustexist)
                resolve_flags |= RESOLVE_REF_READING;
        }
  
        refname = resolve_ref_unsafe(refname, resolve_flags,
 -                                   lock->old_sha1, &type);
 +                                   lock->old_oid.hash, &type);
        if (!refname && errno == EISDIR) {
                /* we are trying to lock foo but we used to
                 * have foo/bar which now does not exist;
                ref_file = git_path("%s", orig_refname);
                if (remove_empty_directories(ref_file)) {
                        last_errno = errno;
 -                      error("there are still refs under '%s'", orig_refname);
 +
 +                      if (!verify_refname_available(orig_refname, extras, skip,
 +                                                    get_loose_refs(&ref_cache), err))
 +                              strbuf_addf(err, "there are still refs under '%s'",
 +                                          orig_refname);
 +
                        goto error_return;
                }
                refname = resolve_ref_unsafe(orig_refname, resolve_flags,
 -                                           lock->old_sha1, &type);
 +                                           lock->old_oid.hash, &type);
        }
        if (type_p)
            *type_p = type;
        if (!refname) {
                last_errno = errno;
 -              error("unable to resolve reference %s: %s",
 -                      orig_refname, strerror(errno));
 +              if (last_errno != ENOTDIR ||
 +                  !verify_refname_available(orig_refname, extras, skip,
 +                                            get_loose_refs(&ref_cache), err))
 +                      strbuf_addf(err, "unable to resolve reference %s: %s",
 +                                  orig_refname, strerror(last_errno));
 +
                goto error_return;
        }
 -      missing = is_null_sha1(lock->old_sha1);
 -      /* When the ref did not exist and we are creating it,
 -       * make sure there is no existing ref that is packed
 -       * whose name begins with our refname, nor a ref whose
 -       * name is a proper prefix of our refname.
 +      /*
 +       * If the ref did not exist and we are creating it, make sure
 +       * there is no existing packed ref whose name begins with our
 +       * refname, nor a packed ref whose name is a proper prefix of
 +       * our refname.
         */
 -      if (missing &&
 -           !is_refname_available(refname, skip, get_packed_refs(&ref_cache))) {
 +      if (is_null_oid(&lock->old_oid) &&
 +          verify_refname_available(refname, extras, skip,
 +                                   get_packed_refs(&ref_cache), err)) {
                last_errno = ENOTDIR;
                goto error_return;
        }
        lock->ref_name = xstrdup(refname);
        lock->orig_ref_name = xstrdup(orig_refname);
        ref_file = git_path("%s", refname);
 -      if (missing)
 -              lock->force_write = 1;
 -      if ((flags & REF_NODEREF) && (type & REF_ISSYMREF))
 -              lock->force_write = 1;
  
   retry:
 -      switch (safe_create_leading_directories(ref_file)) {
 +      switch (safe_create_leading_directories_const(ref_file)) {
        case SCLD_OK:
                break; /* success */
        case SCLD_VANISHED:
                /* fall through */
        default:
                last_errno = errno;
 -              error("unable to create directory for %s", ref_file);
 +              strbuf_addf(err, "unable to create directory for %s", ref_file);
                goto error_return;
        }
  
 -      lock->lock_fd = hold_lock_file_for_update(lock->lk, ref_file, lflags);
 -      if (lock->lock_fd < 0) {
 +      if (hold_lock_file_for_update(lock->lk, ref_file, lflags) < 0) {
                last_errno = errno;
                if (errno == ENOENT && --attempts_remaining > 0)
                        /*
                         */
                        goto retry;
                else {
 -                      struct strbuf err = STRBUF_INIT;
 -                      unable_to_lock_message(ref_file, errno, &err);
 -                      error("%s", err.buf);
 -                      strbuf_release(&err);
 +                      unable_to_lock_message(ref_file, errno, err);
                        goto error_return;
                }
        }
 -      return old_sha1 ? verify_lock(lock, old_sha1, mustexist) : lock;
 +      if (old_sha1 && verify_lock(lock, old_sha1, mustexist, err)) {
 +              last_errno = errno;
 +              goto error_return;
 +      }
 +      return lock;
  
   error_return:
        unlock_ref(lock);
        return NULL;
  }
  
 -struct ref_lock *lock_any_ref_for_update(const char *refname,
 -                                       const unsigned char *old_sha1,
 -                                       int flags, int *type_p)
 -{
 -      return lock_ref_sha1_basic(refname, old_sha1, NULL, flags, type_p);
 -}
 -
  /*
   * Write an entry to the packed-refs file for the specified refname.
   * If peeled is non-NULL, write it as the entry's peeled value.
@@@ -2509,28 -2394,18 +2524,28 @@@ static int write_packed_entry_fn(struc
        if (peel_status != PEEL_PEELED && peel_status != PEEL_NON_TAG)
                error("internal error: %s is not a valid packed reference!",
                      entry->name);
 -      write_packed_entry(cb_data, entry->name, entry->u.value.sha1,
 +      write_packed_entry(cb_data, entry->name, entry->u.value.oid.hash,
                           peel_status == PEEL_PEELED ?
 -                         entry->u.value.peeled : NULL);
 +                         entry->u.value.peeled.hash : NULL);
        return 0;
  }
  
  /* This should return a meaningful errno on failure */
  int lock_packed_refs(int flags)
  {
 +      static int timeout_configured = 0;
 +      static int timeout_value = 1000;
 +
        struct packed_ref_cache *packed_ref_cache;
  
 -      if (hold_lock_file_for_update(&packlock, git_path("packed-refs"), flags) < 0)
 +      if (!timeout_configured) {
 +              git_config_get_int("core.packedrefstimeout", &timeout_value);
 +              timeout_configured = 1;
 +      }
 +
 +      if (hold_lock_file_for_update_timeout(
 +                          &packlock, git_path("packed-refs"),
 +                          flags, timeout_value) < 0)
                return -1;
        /*
         * Get the current packed-refs while holding the lock.  If the
@@@ -2628,24 -2503,24 +2643,24 @@@ static int pack_if_possible_fn(struct r
        peel_status = peel_entry(entry, 1);
        if (peel_status != PEEL_PEELED && peel_status != PEEL_NON_TAG)
                die("internal error peeling reference %s (%s)",
 -                  entry->name, sha1_to_hex(entry->u.value.sha1));
 +                  entry->name, oid_to_hex(&entry->u.value.oid));
        packed_entry = find_ref(cb->packed_refs, entry->name);
        if (packed_entry) {
                /* Overwrite existing packed entry with info from loose entry */
                packed_entry->flag = REF_ISPACKED | REF_KNOWS_PEELED;
 -              hashcpy(packed_entry->u.value.sha1, entry->u.value.sha1);
 +              oidcpy(&packed_entry->u.value.oid, &entry->u.value.oid);
        } else {
 -              packed_entry = create_ref_entry(entry->name, entry->u.value.sha1,
 +              packed_entry = create_ref_entry(entry->name, entry->u.value.oid.hash,
                                                REF_ISPACKED | REF_KNOWS_PEELED, 0);
                add_ref(cb->packed_refs, packed_entry);
        }
 -      hashcpy(packed_entry->u.value.peeled, entry->u.value.peeled);
 +      oidcpy(&packed_entry->u.value.peeled, &entry->u.value.peeled);
  
        /* Schedule the loose reference for pruning if requested. */
        if ((cb->flags & PACK_REFS_PRUNE)) {
                int namelen = strlen(entry->name) + 1;
                struct ref_to_prune *n = xcalloc(1, sizeof(*n) + namelen);
 -              hashcpy(n->sha1, entry->u.value.sha1);
 +              hashcpy(n->sha1, entry->u.value.oid.hash);
                strcpy(n->name, entry->name);
                n->next = cb->ref_to_prune;
                cb->ref_to_prune = n;
@@@ -2696,7 -2571,7 +2711,7 @@@ static void prune_ref(struct ref_to_pru
        transaction = ref_transaction_begin(&err);
        if (!transaction ||
            ref_transaction_delete(transaction, r->name, r->sha1,
 -                                 REF_ISPRUNING, 1, NULL, &err) ||
 +                                 REF_ISPRUNING, NULL, &err) ||
            ref_transaction_commit(transaction, &err)) {
                ref_transaction_free(transaction);
                error("%s", err.buf);
@@@ -2801,16 -2676,15 +2816,16 @@@ static int delete_ref_loose(struct ref_
        return 0;
  }
  
 -int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
 +int delete_ref(const char *refname, const unsigned char *sha1, unsigned int flags)
  {
        struct ref_transaction *transaction;
        struct strbuf err = STRBUF_INIT;
  
        transaction = ref_transaction_begin(&err);
        if (!transaction ||
 -          ref_transaction_delete(transaction, refname, sha1, delopt,
 -                                 sha1 && !is_null_sha1(sha1), NULL, &err) ||
 +          ref_transaction_delete(transaction, refname,
 +                                 (sha1 && !is_null_sha1(sha1)) ? sha1 : NULL,
 +                                 flags, NULL, &err) ||
            ref_transaction_commit(transaction, &err)) {
                error("%s", err.buf);
                ref_transaction_free(transaction);
@@@ -2836,7 -2710,7 +2851,7 @@@ static int rename_tmp_log(const char *n
        int attempts_remaining = 4;
  
   retry:
 -      switch (safe_create_leading_directories(git_path("logs/%s", newrefname))) {
 +      switch (safe_create_leading_directories_const(git_path("logs/%s", newrefname))) {
        case SCLD_OK:
                break; /* success */
        case SCLD_VANISHED:
  static int rename_ref_available(const char *oldname, const char *newname)
  {
        struct string_list skip = STRING_LIST_INIT_NODUP;
 +      struct strbuf err = STRBUF_INIT;
        int ret;
  
        string_list_insert(&skip, oldname);
 -      ret = is_refname_available(newname, &skip, get_packed_refs(&ref_cache))
 -          && is_refname_available(newname, &skip, get_loose_refs(&ref_cache));
 +      ret = !verify_refname_available(newname, NULL, &skip,
 +                                      get_packed_refs(&ref_cache), &err)
 +              && !verify_refname_available(newname, NULL, &skip,
 +                                           get_loose_refs(&ref_cache), &err);
 +      if (!ret)
 +              error("%s", err.buf);
 +
        string_list_clear(&skip, 0);
 +      strbuf_release(&err);
        return ret;
  }
  
 -static int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1,
 -                        const char *logmsg);
 +static int write_ref_to_lockfile(struct ref_lock *lock, const unsigned char *sha1);
 +static int commit_ref_update(struct ref_lock *lock,
 +                           const unsigned char *sha1, const char *logmsg);
  
  int rename_ref(const char *oldrefname, const char *newrefname, const char *logmsg)
  {
        struct stat loginfo;
        int log = !lstat(git_path("logs/%s", oldrefname), &loginfo);
        const char *symref = NULL;
 +      struct strbuf err = STRBUF_INIT;
  
        if (log && S_ISLNK(loginfo.st_mode))
                return error("reflog for %s is a symlink", oldrefname);
  
        logmoved = log;
  
 -      lock = lock_ref_sha1_basic(newrefname, NULL, NULL, 0, NULL);
 +      lock = lock_ref_sha1_basic(newrefname, NULL, NULL, NULL, 0, NULL, &err);
        if (!lock) {
 -              error("unable to lock %s for update", newrefname);
 +              error("unable to rename '%s' to '%s': %s", oldrefname, newrefname, err.buf);
 +              strbuf_release(&err);
                goto rollback;
        }
 -      lock->force_write = 1;
 -      hashcpy(lock->old_sha1, orig_sha1);
 -      if (write_ref_sha1(lock, orig_sha1, logmsg)) {
 +      hashcpy(lock->old_oid.hash, orig_sha1);
 +
 +      if (write_ref_to_lockfile(lock, orig_sha1) ||
 +          commit_ref_update(lock, orig_sha1, logmsg)) {
                error("unable to write current sha1 into %s", newrefname);
                goto rollback;
        }
        return 0;
  
   rollback:
 -      lock = lock_ref_sha1_basic(oldrefname, NULL, NULL, 0, NULL);
 +      lock = lock_ref_sha1_basic(oldrefname, NULL, NULL, NULL, 0, NULL, &err);
        if (!lock) {
 -              error("unable to lock %s for rollback", oldrefname);
 +              error("unable to lock %s for rollback: %s", oldrefname, err.buf);
 +              strbuf_release(&err);
                goto rollbacklog;
        }
  
 -      lock->force_write = 1;
        flag = log_all_ref_updates;
        log_all_ref_updates = 0;
 -      if (write_ref_sha1(lock, orig_sha1, NULL))
 +      if (write_ref_to_lockfile(lock, orig_sha1) ||
 +          commit_ref_update(lock, orig_sha1, NULL))
                error("unable to write current sha1 into %s", oldrefname);
        log_all_ref_updates = flag;
  
        return 1;
  }
  
 -int close_ref(struct ref_lock *lock)
 +static int close_ref(struct ref_lock *lock)
  {
        if (close_lock_file(lock->lk))
                return -1;
 -      lock->lock_fd = -1;
        return 0;
  }
  
 -int commit_ref(struct ref_lock *lock)
 +static int commit_ref(struct ref_lock *lock)
  {
        if (commit_lock_file(lock->lk))
                return -1;
 -      lock->lock_fd = -1;
        return 0;
  }
  
 -void unlock_ref(struct ref_lock *lock)
 -{
 -      /* Do not free lock->lk -- atexit() still looks at them */
 -      if (lock->lk)
 -              rollback_lock_file(lock->lk);
 -      free(lock->ref_name);
 -      free(lock->orig_ref_name);
 -      free(lock);
 -}
 -
  /*
   * copy the reflog message msg to buf, which has been allocated sufficiently
   * large, while cleaning up the whitespaces.  Especially, convert LF to space,
@@@ -3034,15 -2908,11 +3049,15 @@@ static int copy_msg(char *buf, const ch
  }
  
  /* This function must set a meaningful errno on failure */
 -int log_ref_setup(const char *refname, char *logfile, int bufsize)
 +int log_ref_setup(const char *refname, struct strbuf *sb_logfile)
  {
        int logfd, oflags = O_APPEND | O_WRONLY;
 +      char *logfile;
  
 -      git_snpath(logfile, bufsize, "logs/%s", refname);
 +      strbuf_git_path(sb_logfile, "logs/%s", refname);
 +      logfile = sb_logfile->buf;
 +      /* make sure the rest of the function can't change "logfile" */
 +      sb_logfile = NULL;
        if (log_all_ref_updates &&
            (starts_with(refname, "refs/heads/") ||
             starts_with(refname, "refs/remotes/") ||
        return 0;
  }
  
 -static int log_ref_write(const char *refname, const unsigned char *old_sha1,
 -                       const unsigned char *new_sha1, const char *msg)
 +static int log_ref_write_fd(int fd, const unsigned char *old_sha1,
 +                          const unsigned char *new_sha1,
 +                          const char *committer, const char *msg)
  {
 -      int logfd, result, written, oflags = O_APPEND | O_WRONLY;
 +      int msglen, written;
        unsigned maxlen, len;
 -      int msglen;
 -      char log_file[PATH_MAX];
        char *logrec;
 -      const char *committer;
 -
 -      if (log_all_ref_updates < 0)
 -              log_all_ref_updates = !is_bare_repository();
 -
 -      result = log_ref_setup(refname, log_file, sizeof(log_file));
 -      if (result)
 -              return result;
  
 -      logfd = open(log_file, oflags);
 -      if (logfd < 0)
 -              return 0;
        msglen = msg ? strlen(msg) : 0;
 -      committer = git_committer_info(0);
        maxlen = strlen(committer) + msglen + 100;
        logrec = xmalloc(maxlen);
        len = sprintf(logrec, "%s %s %s\n",
                      committer);
        if (msglen)
                len += copy_msg(logrec + len - 1, msg) - 1;
 -      written = len <= maxlen ? write_in_full(logfd, logrec, len) : -1;
 +
 +      written = len <= maxlen ? write_in_full(fd, logrec, len) : -1;
        free(logrec);
 -      if (written != len) {
 +      if (written != len)
 +              return -1;
 +
 +      return 0;
 +}
 +
 +static int log_ref_write_1(const char *refname, const unsigned char *old_sha1,
 +                         const unsigned char *new_sha1, const char *msg,
 +                         struct strbuf *sb_log_file)
 +{
 +      int logfd, result, oflags = O_APPEND | O_WRONLY;
 +      char *log_file;
 +
 +      if (log_all_ref_updates < 0)
 +              log_all_ref_updates = !is_bare_repository();
 +
 +      result = log_ref_setup(refname, sb_log_file);
 +      if (result)
 +              return result;
 +      log_file = sb_log_file->buf;
 +      /* make sure the rest of the function can't change "log_file" */
 +      sb_log_file = NULL;
 +
 +      logfd = open(log_file, oflags);
 +      if (logfd < 0)
 +              return 0;
 +      result = log_ref_write_fd(logfd, old_sha1, new_sha1,
 +                                git_committer_info(0), msg);
 +      if (result) {
                int save_errno = errno;
                close(logfd);
                error("Unable to append to %s", log_file);
        return 0;
  }
  
 +static int log_ref_write(const char *refname, const unsigned char *old_sha1,
 +                       const unsigned char *new_sha1, const char *msg)
 +{
 +      struct strbuf sb = STRBUF_INIT;
 +      int ret = log_ref_write_1(refname, old_sha1, new_sha1, msg, &sb);
 +      strbuf_release(&sb);
 +      return ret;
 +}
 +
  int is_branch(const char *refname)
  {
        return !strcmp(refname, "HEAD") || starts_with(refname, "refs/heads/");
  }
  
  /*
 - * Write sha1 into the ref specified by the lock. Make sure that errno
 - * is sane on error.
 + * Write sha1 into the open lockfile, then close the lockfile. On
 + * errors, rollback the lockfile and set errno to reflect the problem.
   */
 -static int write_ref_sha1(struct ref_lock *lock,
 -      const unsigned char *sha1, const char *logmsg)
 +static int write_ref_to_lockfile(struct ref_lock *lock,
 +                               const unsigned char *sha1)
  {
        static char term = '\n';
        struct object *o;
  
 -      if (!lock) {
 -              errno = EINVAL;
 -              return -1;
 -      }
 -      if (!lock->force_write && !hashcmp(lock->old_sha1, sha1)) {
 -              unlock_ref(lock);
 -              return 0;
 -      }
        o = parse_object(sha1);
        if (!o) {
                error("Trying to write ref %s with nonexistent object %s",
                errno = EINVAL;
                return -1;
        }
 -      if (write_in_full(lock->lock_fd, sha1_to_hex(sha1), 40) != 40 ||
 -          write_in_full(lock->lock_fd, &term, 1) != 1 ||
 +      if (write_in_full(lock->lk->fd, sha1_to_hex(sha1), 40) != 40 ||
 +          write_in_full(lock->lk->fd, &term, 1) != 1 ||
            close_ref(lock) < 0) {
                int save_errno = errno;
                error("Couldn't write %s", lock->lk->filename.buf);
                errno = save_errno;
                return -1;
        }
 +      return 0;
 +}
 +
 +/*
 + * Commit a change to a loose reference that has already been written
 + * to the loose reference lockfile. Also update the reflogs if
 + * necessary, using the specified lockmsg (which can be NULL).
 + */
 +static int commit_ref_update(struct ref_lock *lock,
 +                           const unsigned char *sha1, const char *logmsg)
 +{
        clear_loose_ref_cache(&ref_cache);
 -      if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||
 +      if (log_ref_write(lock->ref_name, lock->old_oid.hash, sha1, logmsg) < 0 ||
            (strcmp(lock->ref_name, lock->orig_ref_name) &&
 -           log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) {
 +           log_ref_write(lock->orig_ref_name, lock->old_oid.hash, sha1, logmsg) < 0)) {
                unlock_ref(lock);
                return -1;
        }
                                              head_sha1, &head_flag);
                if (head_ref && (head_flag & REF_ISSYMREF) &&
                    !strcmp(head_ref, lock->ref_name))
 -                      log_ref_write("HEAD", lock->old_sha1, sha1, logmsg);
 +                      log_ref_write("HEAD", lock->old_oid.hash, sha1, logmsg);
        }
        if (commit_ref(lock)) {
                error("Couldn't set %s", lock->ref_name);
@@@ -3629,12 -3471,11 +3644,12 @@@ static int do_for_each_reflog(struct st
                                strbuf_addch(name, '/');
                                retval = do_for_each_reflog(name, fn, cb_data);
                        } else {
 -                              unsigned char sha1[20];
 -                              if (read_ref_full(name->buf, 0, sha1, NULL))
 +                              struct object_id oid;
 +
 +                              if (read_ref_full(name->buf, 0, oid.hash, NULL))
                                        retval = error("bad ref for %s", name->buf);
                                else
 -                                      retval = fn(name->buf, sha1, 0, cb_data);
 +                                      retval = fn(name->buf, &oid, 0, cb_data);
                        }
                        if (retval)
                                break;
@@@ -3656,27 -3497,16 +3671,27 @@@ int for_each_reflog(each_ref_fn fn, voi
  }
  
  /**
 - * Information needed for a single ref update.  Set new_sha1 to the
 - * new value or to zero to delete the ref.  To check the old value
 - * while locking the ref, set have_old to 1 and set old_sha1 to the
 - * value or to zero to ensure the ref does not exist before update.
 + * Information needed for a single ref update. Set new_sha1 to the new
 + * value or to null_sha1 to delete the ref. To check the old value
 + * while the ref is locked, set (flags & REF_HAVE_OLD) and set
 + * old_sha1 to the old value, or to null_sha1 to ensure the ref does
 + * not exist before update.
   */
  struct ref_update {
 +      /*
 +       * If (flags & REF_HAVE_NEW), set the reference to this value:
 +       */
        unsigned char new_sha1[20];
 +      /*
 +       * If (flags & REF_HAVE_OLD), check that the reference
 +       * previously had this value:
 +       */
        unsigned char old_sha1[20];
 -      int flags; /* REF_NODEREF? */
 -      int have_old; /* 1 if old_sha1 is valid, 0 otherwise */
 +      /*
 +       * One or more of REF_HAVE_NEW, REF_HAVE_OLD, REF_NODEREF,
 +       * REF_DELETING, and REF_ISPRUNING:
 +       */
 +      unsigned int flags;
        struct ref_lock *lock;
        int type;
        char *msg;
@@@ -3748,7 -3578,7 +3763,7 @@@ int ref_transaction_update(struct ref_t
                           const char *refname,
                           const unsigned char *new_sha1,
                           const unsigned char *old_sha1,
 -                         int flags, int have_old, const char *msg,
 +                         unsigned int flags, const char *msg,
                           struct strbuf *err)
  {
        struct ref_update *update;
        if (transaction->state != REF_TRANSACTION_OPEN)
                die("BUG: update called for transaction that is not open");
  
 -      if (have_old && !old_sha1)
 -              die("BUG: have_old is true but old_sha1 is NULL");
 -
 -      if (!is_null_sha1(new_sha1) &&
 +      if (new_sha1 && !is_null_sha1(new_sha1) &&
            check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
                strbuf_addf(err, "refusing to update ref with bad name %s",
                            refname);
        }
  
        update = add_update(transaction, refname);
 -      hashcpy(update->new_sha1, new_sha1);
 -      update->flags = flags;
 -      update->have_old = have_old;
 -      if (have_old)
 +      if (new_sha1) {
 +              hashcpy(update->new_sha1, new_sha1);
 +              flags |= REF_HAVE_NEW;
 +      }
 +      if (old_sha1) {
                hashcpy(update->old_sha1, old_sha1);
 +              flags |= REF_HAVE_OLD;
 +      }
 +      update->flags = flags;
        if (msg)
                update->msg = xstrdup(msg);
        return 0;
  int ref_transaction_create(struct ref_transaction *transaction,
                           const char *refname,
                           const unsigned char *new_sha1,
 -                         int flags, const char *msg,
 +                         unsigned int flags, const char *msg,
                           struct strbuf *err)
  {
 -      struct ref_update *update;
 -
 -      assert(err);
 -
 -      if (transaction->state != REF_TRANSACTION_OPEN)
 -              die("BUG: create called for transaction that is not open");
 -
        if (!new_sha1 || is_null_sha1(new_sha1))
 -              die("BUG: create ref with null new_sha1");
 -
 -      if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
 -              strbuf_addf(err, "refusing to create ref with bad name %s",
 -                          refname);
 -              return -1;
 -      }
 -
 -      update = add_update(transaction, refname);
 -
 -      hashcpy(update->new_sha1, new_sha1);
 -      hashclr(update->old_sha1);
 -      update->flags = flags;
 -      update->have_old = 1;
 -      if (msg)
 -              update->msg = xstrdup(msg);
 -      return 0;
 +              die("BUG: create called without valid new_sha1");
 +      return ref_transaction_update(transaction, refname, new_sha1,
 +                                    null_sha1, flags, msg, err);
  }
  
  int ref_transaction_delete(struct ref_transaction *transaction,
                           const char *refname,
                           const unsigned char *old_sha1,
 -                         int flags, int have_old, const char *msg,
 +                         unsigned int flags, const char *msg,
                           struct strbuf *err)
  {
 -      struct ref_update *update;
 -
 -      assert(err);
 -
 -      if (transaction->state != REF_TRANSACTION_OPEN)
 -              die("BUG: delete called for transaction that is not open");
 -
 -      if (have_old && !old_sha1)
 -              die("BUG: have_old is true but old_sha1 is NULL");
 +      if (old_sha1 && is_null_sha1(old_sha1))
 +              die("BUG: delete called with old_sha1 set to zeros");
 +      return ref_transaction_update(transaction, refname,
 +                                    null_sha1, old_sha1,
 +                                    flags, msg, err);
 +}
  
 -      update = add_update(transaction, refname);
 -      update->flags = flags;
 -      update->have_old = have_old;
 -      if (have_old) {
 -              assert(!is_null_sha1(old_sha1));
 -              hashcpy(update->old_sha1, old_sha1);
 -      }
 -      if (msg)
 -              update->msg = xstrdup(msg);
 -      return 0;
 +int ref_transaction_verify(struct ref_transaction *transaction,
 +                         const char *refname,
 +                         const unsigned char *old_sha1,
 +                         unsigned int flags,
 +                         struct strbuf *err)
 +{
 +      if (!old_sha1)
 +              die("BUG: verify called with old_sha1 set to NULL");
 +      return ref_transaction_update(transaction, refname,
 +                                    NULL, old_sha1,
 +                                    flags, NULL, err);
  }
  
 -int update_ref(const char *action, const char *refname,
 -             const unsigned char *sha1, const unsigned char *oldval,
 -             int flags, enum action_on_err onerr)
 +int update_ref(const char *msg, const char *refname,
 +             const unsigned char *new_sha1, const unsigned char *old_sha1,
 +             unsigned int flags, enum action_on_err onerr)
  {
        struct ref_transaction *t;
        struct strbuf err = STRBUF_INIT;
  
        t = ref_transaction_begin(&err);
        if (!t ||
 -          ref_transaction_update(t, refname, sha1, oldval, flags,
 -                                 !!oldval, action, &err) ||
 +          ref_transaction_update(t, refname, new_sha1, old_sha1,
 +                                 flags, msg, &err) ||
            ref_transaction_commit(t, &err)) {
                const char *str = "update_ref failed for ref '%s': %s";
  
        return 0;
  }
  
 -static int ref_update_compare(const void *r1, const void *r2)
 -{
 -      const struct ref_update * const *u1 = r1;
 -      const struct ref_update * const *u2 = r2;
 -      return strcmp((*u1)->refname, (*u2)->refname);
 -}
 -
 -static int ref_update_reject_duplicates(struct ref_update **updates, int n,
 +static int ref_update_reject_duplicates(struct string_list *refnames,
                                        struct strbuf *err)
  {
 -      int i;
 +      int i, n = refnames->nr;
  
        assert(err);
  
        for (i = 1; i < n; i++)
 -              if (!strcmp(updates[i - 1]->refname, updates[i]->refname)) {
 +              if (!strcmp(refnames->items[i - 1].string, refnames->items[i].string)) {
                        strbuf_addf(err,
                                    "Multiple updates for ref '%s' not allowed.",
 -                                  updates[i]->refname);
 +                                  refnames->items[i].string);
                        return 1;
                }
        return 0;
@@@ -3876,7 -3735,6 +3891,7 @@@ int ref_transaction_commit(struct ref_t
        struct ref_update **updates = transaction->updates;
        struct string_list refs_to_delete = STRING_LIST_INIT_NODUP;
        struct string_list_item *ref_to_delete;
 +      struct string_list affected_refnames = STRING_LIST_INIT_NODUP;
  
        assert(err);
  
                return 0;
        }
  
 -      /* Copy, sort, and reject duplicate refs */
 -      qsort(updates, n, sizeof(*updates), ref_update_compare);
 -      if (ref_update_reject_duplicates(updates, n, err)) {
 +      /* Fail if a refname appears more than once in the transaction: */
 +      for (i = 0; i < n; i++)
 +              string_list_append(&affected_refnames, updates[i]->refname);
 +      string_list_sort(&affected_refnames);
 +      if (ref_update_reject_duplicates(&affected_refnames, err)) {
                ret = TRANSACTION_GENERIC_ERROR;
                goto cleanup;
        }
  
 -      /* Acquire all locks while verifying old values */
 +      /*
 +       * Acquire all locks, verify old values if provided, check
 +       * that new values are valid, and write new values to the
 +       * lockfiles, ready to be activated. Only keep one lockfile
 +       * open at a time to avoid running out of file descriptors.
 +       */
        for (i = 0; i < n; i++) {
                struct ref_update *update = updates[i];
 -              int flags = update->flags;
 -
 -              if (is_null_sha1(update->new_sha1))
 -                      flags |= REF_DELETING;
 -              update->lock = lock_ref_sha1_basic(update->refname,
 -                                                 (update->have_old ?
 -                                                  update->old_sha1 :
 -                                                  NULL),
 -                                                 NULL,
 -                                                 flags,
 -                                                 &update->type);
 +
 +              if ((update->flags & REF_HAVE_NEW) &&
 +                  is_null_sha1(update->new_sha1))
 +                      update->flags |= REF_DELETING;
 +              update->lock = lock_ref_sha1_basic(
 +                              update->refname,
 +                              ((update->flags & REF_HAVE_OLD) ?
 +                               update->old_sha1 : NULL),
 +                              &affected_refnames, NULL,
 +                              update->flags,
 +                              &update->type,
 +                              err);
                if (!update->lock) {
 +                      char *reason;
 +
                        ret = (errno == ENOTDIR)
                                ? TRANSACTION_NAME_CONFLICT
                                : TRANSACTION_GENERIC_ERROR;
 -                      strbuf_addf(err, "Cannot lock the ref '%s'.",
 -                                  update->refname);
 +                      reason = strbuf_detach(err, NULL);
 +                      strbuf_addf(err, "cannot lock ref '%s': %s",
 +                                  update->refname, reason);
 +                      free(reason);
                        goto cleanup;
                }
 +              if ((update->flags & REF_HAVE_NEW) &&
 +                  !(update->flags & REF_DELETING)) {
 +                      int overwriting_symref = ((update->type & REF_ISSYMREF) &&
 +                                                (update->flags & REF_NODEREF));
 +
 +                      if (!overwriting_symref &&
 +                          !hashcmp(update->lock->old_oid.hash, update->new_sha1)) {
 +                              /*
 +                               * The reference already has the desired
 +                               * value, so we don't need to write it.
 +                               */
 +                      } else if (write_ref_to_lockfile(update->lock,
 +                                                       update->new_sha1)) {
 +                              /*
 +                               * The lock was freed upon failure of
 +                               * write_ref_to_lockfile():
 +                               */
 +                              update->lock = NULL;
 +                              strbuf_addf(err, "cannot update the ref '%s'.",
 +                                          update->refname);
 +                              ret = TRANSACTION_GENERIC_ERROR;
 +                              goto cleanup;
 +                      } else {
 +                              update->flags |= REF_NEEDS_COMMIT;
 +                      }
 +              }
 +              if (!(update->flags & REF_NEEDS_COMMIT)) {
 +                      /*
 +                       * We didn't have to write anything to the lockfile.
 +                       * Close it to free up the file descriptor:
 +                       */
 +                      if (close_ref(update->lock)) {
 +                              strbuf_addf(err, "Couldn't close %s.lock",
 +                                          update->refname);
 +                              goto cleanup;
 +                      }
 +              }
        }
  
        /* Perform updates first so live commits remain referenced */
        for (i = 0; i < n; i++) {
                struct ref_update *update = updates[i];
  
 -              if (!is_null_sha1(update->new_sha1)) {
 -                      if (write_ref_sha1(update->lock, update->new_sha1,
 -                                         update->msg)) {
 -                              update->lock = NULL; /* freed by write_ref_sha1 */
 +              if (update->flags & REF_NEEDS_COMMIT) {
 +                      if (commit_ref_update(update->lock,
 +                                            update->new_sha1, update->msg)) {
 +                              /* freed by commit_ref_update(): */
 +                              update->lock = NULL;
                                strbuf_addf(err, "Cannot update the ref '%s'.",
                                            update->refname);
                                ret = TRANSACTION_GENERIC_ERROR;
                                goto cleanup;
 +                      } else {
 +                              /* freed by commit_ref_update(): */
 +                              update->lock = NULL;
                        }
 -                      update->lock = NULL; /* freed by write_ref_sha1 */
                }
        }
  
        for (i = 0; i < n; i++) {
                struct ref_update *update = updates[i];
  
 -              if (update->lock) {
 +              if (update->flags & REF_DELETING) {
                        if (delete_ref_loose(update->lock, update->type, err)) {
                                ret = TRANSACTION_GENERIC_ERROR;
                                goto cleanup;
@@@ -4019,7 -3825,6 +4034,7 @@@ cleanup
                if (updates[i]->lock)
                        unlock_ref(updates[i]->lock);
        string_list_clear(&refs_to_delete, 0);
 +      string_list_clear(&affected_refnames, 0);
        return ret;
  }
  
@@@ -4158,145 -3963,3 +4173,145 @@@ int ref_is_hidden(const char *refname
        }
        return 0;
  }
 +
 +struct expire_reflog_cb {
 +      unsigned int flags;
 +      reflog_expiry_should_prune_fn *should_prune_fn;
 +      void *policy_cb;
 +      FILE *newlog;
 +      unsigned char last_kept_sha1[20];
 +};
 +
 +static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
 +                           const char *email, unsigned long timestamp, int tz,
 +                           const char *message, void *cb_data)
 +{
 +      struct expire_reflog_cb *cb = cb_data;
 +      struct expire_reflog_policy_cb *policy_cb = cb->policy_cb;
 +
 +      if (cb->flags & EXPIRE_REFLOGS_REWRITE)
 +              osha1 = cb->last_kept_sha1;
 +
 +      if ((*cb->should_prune_fn)(osha1, nsha1, email, timestamp, tz,
 +                                 message, policy_cb)) {
 +              if (!cb->newlog)
 +                      printf("would prune %s", message);
 +              else if (cb->flags & EXPIRE_REFLOGS_VERBOSE)
 +                      printf("prune %s", message);
 +      } else {
 +              if (cb->newlog) {
 +                      fprintf(cb->newlog, "%s %s %s %lu %+05d\t%s",
 +                              sha1_to_hex(osha1), sha1_to_hex(nsha1),
 +                              email, timestamp, tz, message);
 +                      hashcpy(cb->last_kept_sha1, nsha1);
 +              }
 +              if (cb->flags & EXPIRE_REFLOGS_VERBOSE)
 +                      printf("keep %s", message);
 +      }
 +      return 0;
 +}
 +
 +int reflog_expire(const char *refname, const unsigned char *sha1,
 +               unsigned int flags,
 +               reflog_expiry_prepare_fn prepare_fn,
 +               reflog_expiry_should_prune_fn should_prune_fn,
 +               reflog_expiry_cleanup_fn cleanup_fn,
 +               void *policy_cb_data)
 +{
 +      static struct lock_file reflog_lock;
 +      struct expire_reflog_cb cb;
 +      struct ref_lock *lock;
 +      char *log_file;
 +      int status = 0;
 +      int type;
 +      struct strbuf err = STRBUF_INIT;
 +
 +      memset(&cb, 0, sizeof(cb));
 +      cb.flags = flags;
 +      cb.policy_cb = policy_cb_data;
 +      cb.should_prune_fn = should_prune_fn;
 +
 +      /*
 +       * The reflog file is locked by holding the lock on the
 +       * reference itself, plus we might need to update the
 +       * reference if --updateref was specified:
 +       */
 +      lock = lock_ref_sha1_basic(refname, sha1, NULL, NULL, 0, &type, &err);
 +      if (!lock) {
 +              error("cannot lock ref '%s': %s", refname, err.buf);
 +              strbuf_release(&err);
 +              return -1;
 +      }
 +      if (!reflog_exists(refname)) {
 +              unlock_ref(lock);
 +              return 0;
 +      }
 +
 +      log_file = git_pathdup("logs/%s", refname);
 +      if (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {
 +              /*
 +               * Even though holding $GIT_DIR/logs/$reflog.lock has
 +               * no locking implications, we use the lock_file
 +               * machinery here anyway because it does a lot of the
 +               * work we need, including cleaning up if the program
 +               * exits unexpectedly.
 +               */
 +              if (hold_lock_file_for_update(&reflog_lock, log_file, 0) < 0) {
 +                      struct strbuf err = STRBUF_INIT;
 +                      unable_to_lock_message(log_file, errno, &err);
 +                      error("%s", err.buf);
 +                      strbuf_release(&err);
 +                      goto failure;
 +              }
 +              cb.newlog = fdopen_lock_file(&reflog_lock, "w");
 +              if (!cb.newlog) {
 +                      error("cannot fdopen %s (%s)",
 +                            reflog_lock.filename.buf, strerror(errno));
 +                      goto failure;
 +              }
 +      }
 +
 +      (*prepare_fn)(refname, sha1, cb.policy_cb);
 +      for_each_reflog_ent(refname, expire_reflog_ent, &cb);
 +      (*cleanup_fn)(cb.policy_cb);
 +
 +      if (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {
 +              /*
 +               * It doesn't make sense to adjust a reference pointed
 +               * to by a symbolic ref based on expiring entries in
 +               * the symbolic reference's reflog. Nor can we update
 +               * a reference if there are no remaining reflog
 +               * entries.
 +               */
 +              int update = (flags & EXPIRE_REFLOGS_UPDATE_REF) &&
 +                      !(type & REF_ISSYMREF) &&
 +                      !is_null_sha1(cb.last_kept_sha1);
 +
 +              if (close_lock_file(&reflog_lock)) {
 +                      status |= error("couldn't write %s: %s", log_file,
 +                                      strerror(errno));
 +              } else if (update &&
 +                         (write_in_full(lock->lk->fd,
 +                              sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
 +                       write_str_in_full(lock->lk->fd, "\n") != 1 ||
 +                       close_ref(lock) < 0)) {
 +                      status |= error("couldn't write %s",
 +                                      lock->lk->filename.buf);
 +                      rollback_lock_file(&reflog_lock);
 +              } else if (commit_lock_file(&reflog_lock)) {
 +                      status |= error("unable to commit reflog '%s' (%s)",
 +                                      log_file, strerror(errno));
 +              } else if (update && commit_ref(lock)) {
 +                      status |= error("couldn't set %s", lock->ref_name);
 +              }
 +      }
 +      free(log_file);
 +      unlock_ref(lock);
 +      return status;
 +
 + failure:
 +      rollback_lock_file(&reflog_lock);
 +      free(log_file);
 +      unlock_ref(lock);
 +      return -1;
 +}