pull: handle git-fetch's options as well
[gitweb.git] / refs.c
diff --git a/refs.c b/refs.c
index ad4b6d520281579c9814c6b8cfdb7f238f645299..8480d8dbf5c78c28d895c9c1a2ac7aa5305f4e5e 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -11,7 +11,6 @@ struct ref_lock {
        char *orig_ref_name;
        struct lock_file *lk;
        unsigned char old_sha1[20];
-       int lock_fd;
 };
 
 /*
@@ -57,6 +56,12 @@ static unsigned char refname_disposition[256] = {
  */
 #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
@@ -263,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
@@ -344,8 +349,6 @@ static struct ref_entry *create_ref_entry(const char *refname,
        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);
@@ -841,121 +844,181 @@ static void prime_ref_dir(struct ref_dir *dir)
        }
 }
 
-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 {
@@ -1178,6 +1241,8 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
                        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;
                        }
@@ -1323,6 +1388,8 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
                        }
                        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;
                        }
@@ -1382,7 +1449,7 @@ static int resolve_gitlink_ref_recursive(struct ref_cache *refs,
 {
        int fd, len;
        char buffer[128], *p;
-       char *path;
+       const char *path;
 
        if (recursion > MAXDEPTH || strlen(refname) > MAXREFLEN)
                return -1;
@@ -1475,7 +1542,11 @@ static int resolve_missing_loose_ref(const char *refname,
 }
 
 /* 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;
@@ -1506,7 +1577,7 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned
                bad_name = 1;
        }
        for (;;) {
-               char path[PATH_MAX];
+               const char *path;
                struct stat st;
                char *buf;
                int fd;
@@ -1516,7 +1587,9 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned
                        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
@@ -1643,6 +1716,16 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned
        }
 }
 
+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));
@@ -2271,10 +2354,12 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
  */
 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,
-                                           unsigned 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;
@@ -2283,8 +2368,9 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
        int resolve_flags = 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;
@@ -2305,7 +2391,12 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
                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,
@@ -2315,8 +2406,12 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
            *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;
        }
        /*
@@ -2326,7 +2421,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
         * our refname.
         */
        if (is_null_sha1(lock->old_sha1) &&
-            !is_refname_available(refname, skip, get_packed_refs(&ref_cache))) {
+           verify_refname_available(refname, extras, skip,
+                                    get_packed_refs(&ref_cache), err)) {
                last_errno = ENOTDIR;
                goto error_return;
        }
@@ -2343,7 +2439,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
        ref_file = git_path("%s", refname);
 
  retry:
-       switch (safe_create_leading_directories(ref_file)) {
+       switch (safe_create_leading_directories_const(ref_file)) {
        case SCLD_OK:
                break; /* success */
        case SCLD_VANISHED:
@@ -2352,12 +2448,11 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
                /* 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)
                        /*
@@ -2367,10 +2462,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
                         */
                        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;
                }
        }
@@ -2413,9 +2505,19 @@ static int write_packed_entry_fn(struct ref_entry *entry, void *cb_data)
 /* 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
@@ -2721,7 +2823,7 @@ static int rename_tmp_log(const char *newrefname)
        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:
@@ -2764,12 +2866,19 @@ static int rename_tmp_log(const char *newrefname)
 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;
 }
 
@@ -2785,6 +2894,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
        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);
@@ -2827,9 +2937,10 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
 
        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;
        }
        hashcpy(lock->old_sha1, orig_sha1);
@@ -2843,9 +2954,10 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
        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;
        }
 
@@ -2872,7 +2984,6 @@ static int close_ref(struct ref_lock *lock)
 {
        if (close_lock_file(lock->lk))
                return -1;
-       lock->lock_fd = -1;
        return 0;
 }
 
@@ -2880,7 +2991,6 @@ static int commit_ref(struct ref_lock *lock)
 {
        if (commit_lock_file(lock->lk))
                return -1;
-       lock->lock_fd = -1;
        return 0;
 }
 
@@ -2911,11 +3021,15 @@ static int copy_msg(char *buf, const char *msg)
 }
 
 /* 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/") ||
@@ -2986,18 +3100,22 @@ static int log_ref_write_fd(int fd, const unsigned char *old_sha1,
        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_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[PATH_MAX];
+       char *log_file;
 
        if (log_all_ref_updates < 0)
                log_all_ref_updates = !is_bare_repository();
 
-       result = log_ref_setup(refname, log_file, sizeof(log_file));
+       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)
@@ -3020,6 +3138,15 @@ static int log_ref_write(const char *refname, const unsigned char *old_sha1,
        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/");
@@ -3050,8 +3177,8 @@ static int write_ref_to_lockfile(struct ref_lock *lock,
                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);
@@ -3108,21 +3235,6 @@ static int commit_ref_update(struct ref_lock *lock,
        return 0;
 }
 
-/*
- * Write sha1 as the new value of the reference specified by the
- * (open) lock. On error, roll back the lockfile and set errno
- * appropriately.
- */
-static int write_ref_sha1(struct ref_lock *lock,
-                         const unsigned char *sha1, const char *logmsg)
-{
-       if (write_ref_to_lockfile(lock, sha1) ||
-           commit_ref_update(lock, sha1, logmsg))
-               return -1;
-
-       return 0;
-}
-
 int create_symref(const char *ref_target, const char *refs_heads_master,
                  const char *logmsg)
 {
@@ -3725,25 +3837,18 @@ int update_ref(const char *msg, const char *refname,
        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;
@@ -3757,6 +3862,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,
        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);
 
@@ -3768,63 +3874,101 @@ int ref_transaction_commit(struct ref_transaction *transaction,
                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];
-               unsigned int flags = update->flags;
 
-               if ((flags & REF_HAVE_NEW) && is_null_sha1(update->new_sha1))
-                       flags |= REF_DELETING;
+               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),
-                               NULL,
-                               flags,
-                               &update->type);
+                               &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;
                }
-       }
-
-       /* Perform updates first so live commits remain referenced */
-       for (i = 0; i < n; i++) {
-               struct ref_update *update = updates[i];
-               int flags = update->flags;
-
-               if ((flags & REF_HAVE_NEW) && !is_null_sha1(update->new_sha1)) {
+               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_sha1, update->new_sha1)) {
+                       if (!overwriting_symref &&
+                           !hashcmp(update->lock->old_sha1, update->new_sha1)) {
                                /*
                                 * The reference already has the desired
                                 * value, so we don't need to write it.
                                 */
-                               unlock_ref(update->lock);
+                       } 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 (update->flags & REF_NEEDS_COMMIT) {
+                       if (commit_ref_update(update->lock,
+                                             update->new_sha1, update->msg)) {
+                               /* freed by commit_ref_update(): */
                                update->lock = NULL;
-                       } else if (write_ref_sha1(update->lock, update->new_sha1,
-                                                 update->msg)) {
-                               update->lock = NULL; /* freed by write_ref_sha1 */
                                strbuf_addf(err, "Cannot update the ref '%s'.",
                                            update->refname);
                                ret = TRANSACTION_GENERIC_ERROR;
                                goto cleanup;
                        } else {
-                               /* freed by write_ref_sha1(): */
+                               /* freed by commit_ref_update(): */
                                update->lock = NULL;
                        }
                }
@@ -3833,15 +3977,14 @@ int ref_transaction_commit(struct ref_transaction *transaction,
        /* Perform deletes now that updates are safely completed */
        for (i = 0; i < n; i++) {
                struct ref_update *update = updates[i];
-               int flags = update->flags;
 
-               if ((flags & REF_HAVE_NEW) && is_null_sha1(update->new_sha1)) {
+               if (update->flags & REF_DELETING) {
                        if (delete_ref_loose(update->lock, update->type, err)) {
                                ret = TRANSACTION_GENERIC_ERROR;
                                goto cleanup;
                        }
 
-                       if (!(flags & REF_ISPRUNING))
+                       if (!(update->flags & REF_ISPRUNING))
                                string_list_append(&refs_to_delete,
                                                   update->lock->ref_name);
                }
@@ -3862,6 +4005,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,
                if (updates[i]->lock)
                        unlock_ref(updates[i]->lock);
        string_list_clear(&refs_to_delete, 0);
+       string_list_clear(&affected_refnames, 0);
        return ret;
 }
 
@@ -4051,6 +4195,7 @@ int reflog_expire(const char *refname, const unsigned char *sha1,
        char *log_file;
        int status = 0;
        int type;
+       struct strbuf err = STRBUF_INIT;
 
        memset(&cb, 0, sizeof(cb));
        cb.flags = flags;
@@ -4062,9 +4207,12 @@ int reflog_expire(const char *refname, const unsigned char *sha1,
         * reference itself, plus we might need to update the
         * reference if --updateref was specified:
         */
-       lock = lock_ref_sha1_basic(refname, sha1, NULL, 0, &type);
-       if (!lock)
-               return error("cannot lock ref '%s'", refname);
+       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;
@@ -4114,9 +4262,9 @@ int reflog_expire(const char *refname, const unsigned char *sha1,
                        status |= error("couldn't write %s: %s", log_file,
                                        strerror(errno));
                } else if (update &&
-                       (write_in_full(lock->lock_fd,
+                          (write_in_full(lock->lk->fd,
                                sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
-                        write_str_in_full(lock->lock_fd, "\n") != 1 ||
+                        write_str_in_full(lock->lk->fd, "\n") != 1 ||
                         close_ref(lock) < 0)) {
                        status |= error("couldn't write %s",
                                        lock->lk->filename.buf);