We will want to be able to hold the lockfile for `packed-refs` even
after we have activated the new values. So use a separate tempfile,
`packed-refs.new`, as a place to stage the new contents of the
`packed-refs` file. For now this is all done within
`commit_packed_refs()`, but that will change shortly.

Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu>
---
 refs/packed-backend.c | 40 ++++++++++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 5bee49d497..6619052e96 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -68,6 +68,13 @@ struct packed_ref_store {
         * thus the enclosing `packed_ref_store`) must not be freed.
         */
        struct lock_file lock;
+
+       /*
+        * Temporary file used when rewriting new contents to the
+        * "packed-refs" file. Note that this (and thus the enclosing
+        * `packed_ref_store`) must not be freed.
+        */
+       struct tempfile tempfile;
 };
 
 struct ref_store *packed_ref_store_create(const char *path,
@@ -522,10 +529,16 @@ int lock_packed_refs(struct ref_store *ref_store, int 
flags)
                timeout_configured = 1;
        }
 
+       /*
+        * Note that we close the lockfile immediately because we
+        * don't write new content to it, but rather to a separate
+        * tempfile.
+        */
        if (hold_lock_file_for_update_timeout(
                            &refs->lock,
                            refs->path,
-                           flags, timeout_value) < 0)
+                           flags, timeout_value) < 0 ||
+           close_lock_file(&refs->lock))
                return -1;
 
        /*
@@ -567,13 +580,23 @@ int commit_packed_refs(struct ref_store *ref_store, 
struct strbuf *err)
                get_packed_ref_cache(refs);
        int ok;
        int ret = -1;
+       struct strbuf sb = STRBUF_INIT;
        FILE *out;
        struct ref_iterator *iter;
 
        if (!is_lock_file_locked(&refs->lock))
                die("BUG: commit_packed_refs() called when unlocked");
 
-       out = fdopen_lock_file(&refs->lock, "w");
+       strbuf_addf(&sb, "%s.new", refs->path);
+       if (create_tempfile(&refs->tempfile, sb.buf) < 0) {
+               strbuf_addf(err, "unable to create file %s: %s",
+                           sb.buf, strerror(errno));
+               strbuf_release(&sb);
+               goto out;
+       }
+       strbuf_release(&sb);
+
+       out = fdopen_tempfile(&refs->tempfile, "w");
        if (!out) {
                strbuf_addf(err, "unable to fdopen packed-refs tempfile: %s",
                            strerror(errno));
@@ -582,7 +605,7 @@ int commit_packed_refs(struct ref_store *ref_store, struct 
strbuf *err)
 
        if (fprintf(out, "%s", PACKED_REFS_HEADER) < 0) {
                strbuf_addf(err, "error writing to %s: %s",
-                           get_lock_file_path(&refs->lock), strerror(errno));
+                           get_tempfile_path(&refs->tempfile), 
strerror(errno));
                goto error;
        }
 
@@ -594,7 +617,7 @@ int commit_packed_refs(struct ref_store *ref_store, struct 
strbuf *err)
                if (write_packed_entry(out, iter->refname, iter->oid->hash,
                                       peel_error ? NULL : peeled.hash)) {
                        strbuf_addf(err, "error writing to %s: %s",
-                                   get_lock_file_path(&refs->lock),
+                                   get_tempfile_path(&refs->tempfile),
                                    strerror(errno));
                        ref_iterator_abort(iter);
                        goto error;
@@ -602,13 +625,13 @@ int commit_packed_refs(struct ref_store *ref_store, 
struct strbuf *err)
        }
 
        if (ok != ITER_DONE) {
-               strbuf_addf(err, "unable to write packed-refs file: "
+               strbuf_addf(err, "unable to rewrite packed-refs file: "
                            "error iterating over old contents");
                goto error;
        }
 
-       if (commit_lock_file(&refs->lock)) {
-               strbuf_addf(err, "error overwriting %s: %s",
+       if (rename_tempfile(&refs->tempfile, refs->path)) {
+               strbuf_addf(err, "error replacing %s: %s",
                            refs->path, strerror(errno));
                goto out;
        }
@@ -617,9 +640,10 @@ int commit_packed_refs(struct ref_store *ref_store, struct 
strbuf *err)
        goto out;
 
 error:
-       rollback_lock_file(&refs->lock);
+       delete_tempfile(&refs->tempfile);
 
 out:
+       rollback_lock_file(&refs->lock);
        release_packed_ref_cache(packed_ref_cache);
        return ret;
 }
-- 
2.11.0

Reply via email to