From 67be7c5a593da8905added96723448d28b4a1812 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:37 +0200 Subject: packed-backend: new module for handling packed references Now that the interface between `files_ref_store` and `packed_ref_store` is relatively narrow, move the latter into a new module, "refs/packed-backend.h" and "refs/packed-backend.c". It still doesn't quite implement the `ref_store` interface, but it will soon. This commit moves code around and adjusts its visibility, but doesn't change anything. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/packed-backend.c | 623 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 623 insertions(+) create mode 100644 refs/packed-backend.c (limited to 'refs/packed-backend.c') diff --git a/refs/packed-backend.c b/refs/packed-backend.c new file mode 100644 index 0000000000..e468421a40 --- /dev/null +++ b/refs/packed-backend.c @@ -0,0 +1,623 @@ +#include "../cache.h" +#include "../refs.h" +#include "refs-internal.h" +#include "ref-cache.h" +#include "packed-backend.h" +#include "../iterator.h" +#include "../lockfile.h" + +struct packed_ref_cache { + struct ref_cache *cache; + + /* + * Count of references to the data structure in this instance, + * including the pointer from files_ref_store::packed if any. + * The data will not be freed as long as the reference count + * is nonzero. + */ + unsigned int referrers; + + /* The metadata from when this packed-refs cache was read */ + struct stat_validity validity; +}; + +/* + * Increment the reference count of *packed_refs. + */ +static void acquire_packed_ref_cache(struct packed_ref_cache *packed_refs) +{ + packed_refs->referrers++; +} + +/* + * Decrease the reference count of *packed_refs. If it goes to zero, + * free *packed_refs and return true; otherwise return false. + */ +static int release_packed_ref_cache(struct packed_ref_cache *packed_refs) +{ + if (!--packed_refs->referrers) { + free_ref_cache(packed_refs->cache); + stat_validity_clear(&packed_refs->validity); + free(packed_refs); + return 1; + } else { + return 0; + } +} + +/* + * A container for `packed-refs`-related data. It is not (yet) a + * `ref_store`. + */ +struct packed_ref_store { + unsigned int store_flags; + + /* The path of the "packed-refs" file: */ + char *path; + + /* + * A cache of the values read from the `packed-refs` file, if + * it might still be current; otherwise, NULL. + */ + struct packed_ref_cache *cache; + + /* + * Lock used for the "packed-refs" file. Note that this (and + * thus the enclosing `packed_ref_store`) must not be freed. + */ + struct lock_file lock; +}; + +struct packed_ref_store *packed_ref_store_create( + const char *path, unsigned int store_flags) +{ + struct packed_ref_store *refs = xcalloc(1, sizeof(*refs)); + + refs->store_flags = store_flags; + refs->path = xstrdup(path); + return refs; +} + +/* + * Die if refs is not the main ref store. caller is used in any + * necessary error messages. + */ +static void packed_assert_main_repository(struct packed_ref_store *refs, + const char *caller) +{ + if (refs->store_flags & REF_STORE_MAIN) + return; + + die("BUG: operation %s only allowed for main ref store", caller); +} + +static void clear_packed_ref_cache(struct packed_ref_store *refs) +{ + if (refs->cache) { + struct packed_ref_cache *cache = refs->cache; + + if (is_lock_file_locked(&refs->lock)) + die("BUG: packed-ref cache cleared while locked"); + refs->cache = NULL; + release_packed_ref_cache(cache); + } +} + +/* The length of a peeled reference line in packed-refs, including EOL: */ +#define PEELED_LINE_LENGTH 42 + +/* + * Parse one line from a packed-refs file. Write the SHA1 to sha1. + * Return a pointer to the refname within the line (null-terminated), + * or NULL if there was a problem. + */ +static const char *parse_ref_line(struct strbuf *line, struct object_id *oid) +{ + const char *ref; + + if (parse_oid_hex(line->buf, oid, &ref) < 0) + return NULL; + if (!isspace(*ref++)) + return NULL; + + if (isspace(*ref)) + return NULL; + + if (line->buf[line->len - 1] != '\n') + return NULL; + line->buf[--line->len] = 0; + + return ref; +} + +/* + * Read from `packed_refs_file` into a newly-allocated + * `packed_ref_cache` and return it. The return value will already + * have its reference count incremented. + * + * A comment line of the form "# pack-refs with: " may contain zero or + * more traits. We interpret the traits as follows: + * + * No traits: + * + * Probably no references are peeled. But if the file contains a + * peeled value for a reference, we will use it. + * + * peeled: + * + * References under "refs/tags/", if they *can* be peeled, *are* + * peeled in this file. References outside of "refs/tags/" are + * probably not peeled even if they could have been, but if we find + * a peeled value for such a reference we will use it. + * + * fully-peeled: + * + * All references in the file that can be peeled are peeled. + * Inversely (and this is more important), any references in the + * file for which no peeled value is recorded is not peelable. This + * trait should typically be written alongside "peeled" for + * compatibility with older clients, but we do not require it + * (i.e., "peeled" is a no-op if "fully-peeled" is set). + */ +static struct packed_ref_cache *read_packed_refs(const char *packed_refs_file) +{ + FILE *f; + struct packed_ref_cache *packed_refs = xcalloc(1, sizeof(*packed_refs)); + struct ref_entry *last = NULL; + struct strbuf line = STRBUF_INIT; + enum { PEELED_NONE, PEELED_TAGS, PEELED_FULLY } peeled = PEELED_NONE; + struct ref_dir *dir; + + acquire_packed_ref_cache(packed_refs); + packed_refs->cache = create_ref_cache(NULL, NULL); + packed_refs->cache->root->flag &= ~REF_INCOMPLETE; + + f = fopen(packed_refs_file, "r"); + if (!f) { + if (errno == ENOENT) { + /* + * This is OK; it just means that no + * "packed-refs" file has been written yet, + * which is equivalent to it being empty. + */ + return packed_refs; + } else { + die_errno("couldn't read %s", packed_refs_file); + } + } + + stat_validity_update(&packed_refs->validity, fileno(f)); + + dir = get_ref_dir(packed_refs->cache->root); + while (strbuf_getwholeline(&line, f, '\n') != EOF) { + struct object_id oid; + const char *refname; + const char *traits; + + if (skip_prefix(line.buf, "# pack-refs with:", &traits)) { + if (strstr(traits, " fully-peeled ")) + peeled = PEELED_FULLY; + else if (strstr(traits, " peeled ")) + peeled = PEELED_TAGS; + /* perhaps other traits later as well */ + continue; + } + + refname = parse_ref_line(&line, &oid); + if (refname) { + int flag = REF_ISPACKED; + + if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) { + if (!refname_is_safe(refname)) + die("packed refname is dangerous: %s", refname); + oidclr(&oid); + flag |= REF_BAD_NAME | REF_ISBROKEN; + } + last = create_ref_entry(refname, &oid, flag); + if (peeled == PEELED_FULLY || + (peeled == PEELED_TAGS && starts_with(refname, "refs/tags/"))) + last->flag |= REF_KNOWS_PEELED; + add_ref_entry(dir, last); + continue; + } + if (last && + line.buf[0] == '^' && + line.len == PEELED_LINE_LENGTH && + line.buf[PEELED_LINE_LENGTH - 1] == '\n' && + !get_oid_hex(line.buf + 1, &oid)) { + oidcpy(&last->u.value.peeled, &oid); + /* + * Regardless of what the file header said, + * we definitely know the value of *this* + * reference: + */ + last->flag |= REF_KNOWS_PEELED; + } + } + + fclose(f); + strbuf_release(&line); + + return packed_refs; +} + +/* + * Check that the packed refs cache (if any) still reflects the + * contents of the file. If not, clear the cache. + */ +static void validate_packed_ref_cache(struct packed_ref_store *refs) +{ + if (refs->cache && + !stat_validity_check(&refs->cache->validity, refs->path)) + clear_packed_ref_cache(refs); +} + +/* + * Get the packed_ref_cache for the specified packed_ref_store, + * creating and populating it if it hasn't been read before or if the + * file has been changed (according to its `validity` field) since it + * was last read. On the other hand, if we hold the lock, then assume + * that the file hasn't been changed out from under us, so skip the + * extra `stat()` call in `stat_validity_check()`. + */ +static struct packed_ref_cache *get_packed_ref_cache(struct packed_ref_store *refs) +{ + if (!is_lock_file_locked(&refs->lock)) + validate_packed_ref_cache(refs); + + if (!refs->cache) + refs->cache = read_packed_refs(refs->path); + + return refs->cache; +} + +static struct ref_dir *get_packed_ref_dir(struct packed_ref_cache *packed_ref_cache) +{ + return get_ref_dir(packed_ref_cache->cache->root); +} + +static struct ref_dir *get_packed_refs(struct packed_ref_store *refs) +{ + return get_packed_ref_dir(get_packed_ref_cache(refs)); +} + +/* + * Add or overwrite a reference in the in-memory packed reference + * cache. This may only be called while the packed-refs file is locked + * (see lock_packed_refs()). To actually write the packed-refs file, + * call commit_packed_refs(). + */ +void add_packed_ref(struct packed_ref_store *refs, + const char *refname, const struct object_id *oid) +{ + struct ref_dir *packed_refs; + struct ref_entry *packed_entry; + + if (!is_lock_file_locked(&refs->lock)) + die("BUG: packed refs not locked"); + + if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) + die("Reference has invalid format: '%s'", refname); + + packed_refs = get_packed_refs(refs); + packed_entry = find_ref_entry(packed_refs, refname); + if (packed_entry) { + /* Overwrite the existing entry: */ + oidcpy(&packed_entry->u.value.oid, oid); + packed_entry->flag = REF_ISPACKED; + oidclr(&packed_entry->u.value.peeled); + } else { + packed_entry = create_ref_entry(refname, oid, REF_ISPACKED); + add_ref_entry(packed_refs, packed_entry); + } +} + +/* + * Return the ref_entry for the given refname from the packed + * references. If it does not exist, return NULL. + */ +static struct ref_entry *get_packed_ref(struct packed_ref_store *refs, + const char *refname) +{ + return find_ref_entry(get_packed_refs(refs), refname); +} + +int packed_read_raw_ref(struct packed_ref_store *refs, + const char *refname, unsigned char *sha1, + struct strbuf *referent, unsigned int *type) +{ + struct ref_entry *entry; + + *type = 0; + + entry = get_packed_ref(refs, refname); + if (!entry) { + errno = ENOENT; + return -1; + } + + hashcpy(sha1, entry->u.value.oid.hash); + *type = REF_ISPACKED; + return 0; +} + +int packed_peel_ref(struct packed_ref_store *refs, + const char *refname, unsigned char *sha1) +{ + struct ref_entry *r = get_packed_ref(refs, refname); + + if (!r || peel_entry(r, 0)) + return -1; + + hashcpy(sha1, r->u.value.peeled.hash); + return 0; +} + +struct packed_ref_iterator { + struct ref_iterator base; + + struct packed_ref_cache *cache; + struct ref_iterator *iter0; + unsigned int flags; +}; + +static int packed_ref_iterator_advance(struct ref_iterator *ref_iterator) +{ + struct packed_ref_iterator *iter = + (struct packed_ref_iterator *)ref_iterator; + int ok; + + while ((ok = ref_iterator_advance(iter->iter0)) == ITER_OK) { + if (iter->flags & DO_FOR_EACH_PER_WORKTREE_ONLY && + ref_type(iter->iter0->refname) != REF_TYPE_PER_WORKTREE) + continue; + + if (!(iter->flags & DO_FOR_EACH_INCLUDE_BROKEN) && + !ref_resolves_to_object(iter->iter0->refname, + iter->iter0->oid, + iter->iter0->flags)) + continue; + + iter->base.refname = iter->iter0->refname; + iter->base.oid = iter->iter0->oid; + iter->base.flags = iter->iter0->flags; + return ITER_OK; + } + + iter->iter0 = NULL; + if (ref_iterator_abort(ref_iterator) != ITER_DONE) + ok = ITER_ERROR; + + return ok; +} + +static int packed_ref_iterator_peel(struct ref_iterator *ref_iterator, + struct object_id *peeled) +{ + struct packed_ref_iterator *iter = + (struct packed_ref_iterator *)ref_iterator; + + return ref_iterator_peel(iter->iter0, peeled); +} + +static int packed_ref_iterator_abort(struct ref_iterator *ref_iterator) +{ + struct packed_ref_iterator *iter = + (struct packed_ref_iterator *)ref_iterator; + int ok = ITER_DONE; + + if (iter->iter0) + ok = ref_iterator_abort(iter->iter0); + + release_packed_ref_cache(iter->cache); + base_ref_iterator_free(ref_iterator); + return ok; +} + +static struct ref_iterator_vtable packed_ref_iterator_vtable = { + packed_ref_iterator_advance, + packed_ref_iterator_peel, + packed_ref_iterator_abort +}; + +struct ref_iterator *packed_ref_iterator_begin( + struct packed_ref_store *refs, + const char *prefix, unsigned int flags) +{ + struct packed_ref_iterator *iter; + struct ref_iterator *ref_iterator; + + iter = xcalloc(1, sizeof(*iter)); + ref_iterator = &iter->base; + base_ref_iterator_init(ref_iterator, &packed_ref_iterator_vtable); + + /* + * Note that get_packed_ref_cache() internally checks whether + * the packed-ref cache is up to date with what is on disk, + * and re-reads it if not. + */ + + iter->cache = get_packed_ref_cache(refs); + acquire_packed_ref_cache(iter->cache); + iter->iter0 = cache_ref_iterator_begin(iter->cache->cache, prefix, 0); + + iter->flags = flags; + + return ref_iterator; +} + +/* + * 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. + */ +static void write_packed_entry(FILE *fh, const char *refname, + const unsigned char *sha1, + const unsigned char *peeled) +{ + fprintf_or_die(fh, "%s %s\n", sha1_to_hex(sha1), refname); + if (peeled) + fprintf_or_die(fh, "^%s\n", sha1_to_hex(peeled)); +} + +int lock_packed_refs(struct packed_ref_store *refs, int flags) +{ + static int timeout_configured = 0; + static int timeout_value = 1000; + struct packed_ref_cache *packed_ref_cache; + + packed_assert_main_repository(refs, "lock_packed_refs"); + + if (!timeout_configured) { + git_config_get_int("core.packedrefstimeout", &timeout_value); + timeout_configured = 1; + } + + if (hold_lock_file_for_update_timeout( + &refs->lock, + refs->path, + flags, timeout_value) < 0) + return -1; + + /* + * Now that we hold the `packed-refs` lock, make sure that our + * cache matches the current version of the file. Normally + * `get_packed_ref_cache()` does that for us, but that + * function assumes that when the file is locked, any existing + * cache is still valid. We've just locked the file, but it + * might have changed the moment *before* we locked it. + */ + validate_packed_ref_cache(refs); + + packed_ref_cache = get_packed_ref_cache(refs); + /* Increment the reference count to prevent it from being freed: */ + acquire_packed_ref_cache(packed_ref_cache); + return 0; +} + +/* + * The packed-refs header line that we write out. Perhaps other + * traits will be added later. The trailing space is required. + */ +static const char PACKED_REFS_HEADER[] = + "# pack-refs with: peeled fully-peeled \n"; + +/* + * Write the current version of the packed refs cache from memory to + * disk. The packed-refs file must already be locked for writing (see + * lock_packed_refs()). Return zero on success. On errors, set errno + * and return a nonzero value. + */ +int commit_packed_refs(struct packed_ref_store *refs) +{ + struct packed_ref_cache *packed_ref_cache = + get_packed_ref_cache(refs); + int ok, error = 0; + int save_errno = 0; + FILE *out; + struct ref_iterator *iter; + + packed_assert_main_repository(refs, "commit_packed_refs"); + + if (!is_lock_file_locked(&refs->lock)) + die("BUG: packed-refs not locked"); + + out = fdopen_lock_file(&refs->lock, "w"); + if (!out) + die_errno("unable to fdopen packed-refs descriptor"); + + fprintf_or_die(out, "%s", PACKED_REFS_HEADER); + + iter = cache_ref_iterator_begin(packed_ref_cache->cache, NULL, 0); + while ((ok = ref_iterator_advance(iter)) == ITER_OK) { + struct object_id peeled; + int peel_error = ref_iterator_peel(iter, &peeled); + + write_packed_entry(out, iter->refname, iter->oid->hash, + peel_error ? NULL : peeled.hash); + } + + if (ok != ITER_DONE) + die("error while iterating over references"); + + if (commit_lock_file(&refs->lock)) { + save_errno = errno; + error = -1; + } + release_packed_ref_cache(packed_ref_cache); + errno = save_errno; + return error; +} + +/* + * Rollback the lockfile for the packed-refs file, and discard the + * in-memory packed reference cache. (The packed-refs file will be + * read anew if it is needed again after this function is called.) + */ +static void rollback_packed_refs(struct packed_ref_store *refs) +{ + struct packed_ref_cache *packed_ref_cache = get_packed_ref_cache(refs); + + packed_assert_main_repository(refs, "rollback_packed_refs"); + + if (!is_lock_file_locked(&refs->lock)) + die("BUG: packed-refs not locked"); + rollback_lock_file(&refs->lock); + release_packed_ref_cache(packed_ref_cache); + clear_packed_ref_cache(refs); +} + +/* + * Rewrite the packed-refs file, omitting any refs listed in + * 'refnames'. On error, leave packed-refs unchanged, write an error + * message to 'err', and return a nonzero value. + * + * The refs in 'refnames' needn't be sorted. `err` must not be NULL. + */ +int repack_without_refs(struct packed_ref_store *refs, + struct string_list *refnames, struct strbuf *err) +{ + struct ref_dir *packed; + struct string_list_item *refname; + int ret, needs_repacking = 0, removed = 0; + + packed_assert_main_repository(refs, "repack_without_refs"); + assert(err); + + /* Look for a packed ref */ + for_each_string_list_item(refname, refnames) { + if (get_packed_ref(refs, refname->string)) { + needs_repacking = 1; + break; + } + } + + /* Avoid locking if we have nothing to do */ + if (!needs_repacking) + return 0; /* no refname exists in packed refs */ + + if (lock_packed_refs(refs, 0)) { + unable_to_lock_message(refs->path, errno, err); + return -1; + } + packed = get_packed_refs(refs); + + /* Remove refnames from the cache */ + for_each_string_list_item(refname, refnames) + if (remove_entry_from_dir(packed, refname->string) != -1) + removed = 1; + if (!removed) { + /* + * All packed entries disappeared while we were + * acquiring the lock. + */ + rollback_packed_refs(refs); + return 0; + } + + /* Write what remains */ + ret = commit_packed_refs(refs); + if (ret) + strbuf_addf(err, "unable to overwrite old ref-pack file: %s", + strerror(errno)); + return ret; +} -- cgit v1.3-5-g9baa From e0cc8ac8202f7d6a721cc87fd5346a6c7f453302 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:38 +0200 Subject: packed_ref_store: make class into a subclass of `ref_store` Add the infrastructure to make `packed_ref_store` implement `ref_store`, at least formally (few of the methods are actually implemented yet). Change the functions in its interface to take `ref_store *` arguments. Change `files_ref_store` to store a pointer to `ref_store *` and to call functions via the virtual `ref_store` interface where possible. This also means that a few `packed_ref_store` functions can become static. This is a work in progress. Some more `ref_store` methods will soon be implemented (e.g., those having to do with reference transactions). But some of them will never be implemented (e.g., those having to do with symrefs or reflogs). Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 16 ++-- refs/packed-backend.c | 232 +++++++++++++++++++++++++++++++++++++++++++++----- refs/packed-backend.h | 23 ++--- refs/refs-internal.h | 1 + 4 files changed, 227 insertions(+), 45 deletions(-) (limited to 'refs/packed-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index 7df9747798..60f4fa5e7a 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -28,7 +28,7 @@ struct files_ref_store { struct ref_cache *loose; - struct packed_ref_store *packed_ref_store; + struct ref_store *packed_ref_store; }; static void clear_loose_ref_cache(struct files_ref_store *refs) @@ -311,8 +311,8 @@ stat_ref: if (lstat(path, &st) < 0) { if (errno != ENOENT) goto out; - if (packed_read_raw_ref(refs->packed_ref_store, refname, - sha1, referent, type)) { + if (refs_read_raw_ref(refs->packed_ref_store, refname, + sha1, referent, type)) { errno = ENOENT; goto out; } @@ -351,8 +351,8 @@ stat_ref: * ref is supposed to be, there could still be a * packed ref: */ - if (packed_read_raw_ref(refs->packed_ref_store, refname, - sha1, referent, type)) { + if (refs_read_raw_ref(refs->packed_ref_store, refname, + sha1, referent, type)) { errno = EISDIR; goto out; } @@ -683,7 +683,7 @@ static int files_peel_ref(struct ref_store *ref_store, * have REF_KNOWS_PEELED. */ if (flag & REF_ISPACKED && - !packed_peel_ref(refs->packed_ref_store, refname, sha1)) + !refs_peel_ref(refs->packed_ref_store, refname, sha1)) return 0; return peel_object(base, sha1); @@ -804,8 +804,8 @@ static struct ref_iterator *files_ref_iterator_begin( * ones in files_ref_iterator_advance(), after we have merged * the packed and loose references. */ - packed_iter = packed_ref_iterator_begin( - refs->packed_ref_store, prefix, + packed_iter = refs_ref_iterator_begin( + refs->packed_ref_store, prefix, 0, DO_FOR_EACH_INCLUDE_BROKEN); iter->iter0 = overlay_ref_iterator_begin(loose_iter, packed_iter); diff --git a/refs/packed-backend.c b/refs/packed-backend.c index e468421a40..4676dc3959 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -50,6 +50,8 @@ static int release_packed_ref_cache(struct packed_ref_cache *packed_refs) * `ref_store`. */ struct packed_ref_store { + struct ref_store base; + unsigned int store_flags; /* The path of the "packed-refs" file: */ @@ -68,14 +70,17 @@ struct packed_ref_store { struct lock_file lock; }; -struct packed_ref_store *packed_ref_store_create( - const char *path, unsigned int store_flags) +struct ref_store *packed_ref_store_create(const char *path, + unsigned int store_flags) { struct packed_ref_store *refs = xcalloc(1, sizeof(*refs)); + struct ref_store *ref_store = (struct ref_store *)refs; + base_ref_store_init(ref_store, &refs_be_packed); refs->store_flags = store_flags; + refs->path = xstrdup(path); - return refs; + return ref_store; } /* @@ -91,6 +96,31 @@ static void packed_assert_main_repository(struct packed_ref_store *refs, die("BUG: operation %s only allowed for main ref store", caller); } +/* + * Downcast `ref_store` to `packed_ref_store`. Die if `ref_store` is + * not a `packed_ref_store`. Also die if `packed_ref_store` doesn't + * support at least the flags specified in `required_flags`. `caller` + * is used in any necessary error messages. + */ +static struct packed_ref_store *packed_downcast(struct ref_store *ref_store, + unsigned int required_flags, + const char *caller) +{ + struct packed_ref_store *refs; + + if (ref_store->be != &refs_be_packed) + die("BUG: ref_store is type \"%s\" not \"packed\" in %s", + ref_store->be->name, caller); + + refs = (struct packed_ref_store *)ref_store; + + if ((refs->store_flags & required_flags) != required_flags) + die("BUG: unallowed operation (%s), requires %x, has %x\n", + caller, required_flags, refs->store_flags); + + return refs; +} + static void clear_packed_ref_cache(struct packed_ref_store *refs) { if (refs->cache) { @@ -287,9 +317,12 @@ static struct ref_dir *get_packed_refs(struct packed_ref_store *refs) * (see lock_packed_refs()). To actually write the packed-refs file, * call commit_packed_refs(). */ -void add_packed_ref(struct packed_ref_store *refs, +void add_packed_ref(struct ref_store *ref_store, const char *refname, const struct object_id *oid) { + struct packed_ref_store *refs = + packed_downcast(ref_store, REF_STORE_WRITE, + "add_packed_ref"); struct ref_dir *packed_refs; struct ref_entry *packed_entry; @@ -322,10 +355,13 @@ static struct ref_entry *get_packed_ref(struct packed_ref_store *refs, return find_ref_entry(get_packed_refs(refs), refname); } -int packed_read_raw_ref(struct packed_ref_store *refs, - const char *refname, unsigned char *sha1, - struct strbuf *referent, unsigned int *type) +static int packed_read_raw_ref(struct ref_store *ref_store, + const char *refname, unsigned char *sha1, + struct strbuf *referent, unsigned int *type) { + struct packed_ref_store *refs = + packed_downcast(ref_store, REF_STORE_READ, "read_raw_ref"); + struct ref_entry *entry; *type = 0; @@ -341,9 +377,12 @@ int packed_read_raw_ref(struct packed_ref_store *refs, return 0; } -int packed_peel_ref(struct packed_ref_store *refs, - const char *refname, unsigned char *sha1) +static int packed_peel_ref(struct ref_store *ref_store, + const char *refname, unsigned char *sha1) { + struct packed_ref_store *refs = + packed_downcast(ref_store, REF_STORE_READ | REF_STORE_ODB, + "peel_ref"); struct ref_entry *r = get_packed_ref(refs, refname); if (!r || peel_entry(r, 0)) @@ -420,12 +459,18 @@ static struct ref_iterator_vtable packed_ref_iterator_vtable = { packed_ref_iterator_abort }; -struct ref_iterator *packed_ref_iterator_begin( - struct packed_ref_store *refs, +static struct ref_iterator *packed_ref_iterator_begin( + struct ref_store *ref_store, const char *prefix, unsigned int flags) { + struct packed_ref_store *refs; struct packed_ref_iterator *iter; struct ref_iterator *ref_iterator; + unsigned int required_flags = REF_STORE_READ; + + if (!(flags & DO_FOR_EACH_INCLUDE_BROKEN)) + required_flags |= REF_STORE_ODB; + refs = packed_downcast(ref_store, required_flags, "ref_iterator_begin"); iter = xcalloc(1, sizeof(*iter)); ref_iterator = &iter->base; @@ -459,14 +504,15 @@ static void write_packed_entry(FILE *fh, const char *refname, fprintf_or_die(fh, "^%s\n", sha1_to_hex(peeled)); } -int lock_packed_refs(struct packed_ref_store *refs, int flags) +int lock_packed_refs(struct ref_store *ref_store, int flags) { + struct packed_ref_store *refs = + packed_downcast(ref_store, REF_STORE_WRITE | REF_STORE_MAIN, + "lock_packed_refs"); static int timeout_configured = 0; static int timeout_value = 1000; struct packed_ref_cache *packed_ref_cache; - packed_assert_main_repository(refs, "lock_packed_refs"); - if (!timeout_configured) { git_config_get_int("core.packedrefstimeout", &timeout_value); timeout_configured = 1; @@ -507,8 +553,11 @@ static const char PACKED_REFS_HEADER[] = * lock_packed_refs()). Return zero on success. On errors, set errno * and return a nonzero value. */ -int commit_packed_refs(struct packed_ref_store *refs) +int commit_packed_refs(struct ref_store *ref_store) { + struct packed_ref_store *refs = + packed_downcast(ref_store, REF_STORE_WRITE | REF_STORE_MAIN, + "commit_packed_refs"); struct packed_ref_cache *packed_ref_cache = get_packed_ref_cache(refs); int ok, error = 0; @@ -516,8 +565,6 @@ int commit_packed_refs(struct packed_ref_store *refs) FILE *out; struct ref_iterator *iter; - packed_assert_main_repository(refs, "commit_packed_refs"); - if (!is_lock_file_locked(&refs->lock)) die("BUG: packed-refs not locked"); @@ -573,9 +620,12 @@ static void rollback_packed_refs(struct packed_ref_store *refs) * * The refs in 'refnames' needn't be sorted. `err` must not be NULL. */ -int repack_without_refs(struct packed_ref_store *refs, +int repack_without_refs(struct ref_store *ref_store, struct string_list *refnames, struct strbuf *err) { + struct packed_ref_store *refs = + packed_downcast(ref_store, REF_STORE_WRITE | REF_STORE_MAIN, + "repack_without_refs"); struct ref_dir *packed; struct string_list_item *refname; int ret, needs_repacking = 0, removed = 0; @@ -595,7 +645,7 @@ int repack_without_refs(struct packed_ref_store *refs, if (!needs_repacking) return 0; /* no refname exists in packed refs */ - if (lock_packed_refs(refs, 0)) { + if (lock_packed_refs(&refs->base, 0)) { unable_to_lock_message(refs->path, errno, err); return -1; } @@ -615,9 +665,151 @@ int repack_without_refs(struct packed_ref_store *refs, } /* Write what remains */ - ret = commit_packed_refs(refs); + ret = commit_packed_refs(&refs->base); if (ret) strbuf_addf(err, "unable to overwrite old ref-pack file: %s", strerror(errno)); return ret; } + +static int packed_init_db(struct ref_store *ref_store, struct strbuf *err) +{ + /* Nothing to do. */ + return 0; +} + +static int packed_transaction_prepare(struct ref_store *ref_store, + struct ref_transaction *transaction, + struct strbuf *err) +{ + die("BUG: not implemented yet"); +} + +static int packed_transaction_abort(struct ref_store *ref_store, + struct ref_transaction *transaction, + struct strbuf *err) +{ + die("BUG: not implemented yet"); +} + +static int packed_transaction_finish(struct ref_store *ref_store, + struct ref_transaction *transaction, + struct strbuf *err) +{ + die("BUG: not implemented yet"); +} + +static int packed_initial_transaction_commit(struct ref_store *ref_store, + struct ref_transaction *transaction, + struct strbuf *err) +{ + return ref_transaction_commit(transaction, err); +} + +static int packed_delete_refs(struct ref_store *ref_store, const char *msg, + struct string_list *refnames, unsigned int flags) +{ + die("BUG: not implemented yet"); +} + +static int packed_pack_refs(struct ref_store *ref_store, unsigned int flags) +{ + /* + * Packed refs are already packed. It might be that loose refs + * are packed *into* a packed refs store, but that is done by + * updating the packed references via a transaction. + */ + return 0; +} + +static int packed_create_symref(struct ref_store *ref_store, + const char *refname, const char *target, + const char *logmsg) +{ + die("BUG: packed reference store does not support symrefs"); +} + +static int packed_rename_ref(struct ref_store *ref_store, + const char *oldrefname, const char *newrefname, + const char *logmsg) +{ + die("BUG: packed reference store does not support renaming references"); +} + +static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_store) +{ + return empty_ref_iterator_begin(); +} + +static int packed_for_each_reflog_ent(struct ref_store *ref_store, + const char *refname, + each_reflog_ent_fn fn, void *cb_data) +{ + return 0; +} + +static int packed_for_each_reflog_ent_reverse(struct ref_store *ref_store, + const char *refname, + each_reflog_ent_fn fn, + void *cb_data) +{ + return 0; +} + +static int packed_reflog_exists(struct ref_store *ref_store, + const char *refname) +{ + return 0; +} + +static int packed_create_reflog(struct ref_store *ref_store, + const char *refname, int force_create, + struct strbuf *err) +{ + die("BUG: packed reference store does not support reflogs"); +} + +static int packed_delete_reflog(struct ref_store *ref_store, + const char *refname) +{ + return 0; +} + +static int packed_reflog_expire(struct ref_store *ref_store, + 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) +{ + return 0; +} + +struct ref_storage_be refs_be_packed = { + NULL, + "packed", + packed_ref_store_create, + packed_init_db, + packed_transaction_prepare, + packed_transaction_finish, + packed_transaction_abort, + packed_initial_transaction_commit, + + packed_pack_refs, + packed_peel_ref, + packed_create_symref, + packed_delete_refs, + packed_rename_ref, + + packed_ref_iterator_begin, + packed_read_raw_ref, + + packed_reflog_iterator_begin, + packed_for_each_reflog_ent, + packed_for_each_reflog_ent_reverse, + packed_reflog_exists, + packed_create_reflog, + packed_delete_reflog, + packed_reflog_expire +}; diff --git a/refs/packed-backend.h b/refs/packed-backend.h index 22e8817ac4..beea9c14b5 100644 --- a/refs/packed-backend.h +++ b/refs/packed-backend.h @@ -1,33 +1,22 @@ #ifndef REFS_PACKED_BACKEND_H #define REFS_PACKED_BACKEND_H -struct packed_ref_store *packed_ref_store_create( - const char *path, unsigned int store_flags); - -int packed_read_raw_ref(struct packed_ref_store *refs, - const char *refname, unsigned char *sha1, - struct strbuf *referent, unsigned int *type); - -int packed_peel_ref(struct packed_ref_store *refs, - const char *refname, unsigned char *sha1); - -struct ref_iterator *packed_ref_iterator_begin( - struct packed_ref_store *refs, - const char *prefix, unsigned int flags); +struct ref_store *packed_ref_store_create(const char *path, + unsigned int store_flags); /* * Lock the packed-refs file for writing. Flags is passed to * hold_lock_file_for_update(). Return 0 on success. On errors, set * errno appropriately and return a nonzero value. */ -int lock_packed_refs(struct packed_ref_store *refs, int flags); +int lock_packed_refs(struct ref_store *ref_store, int flags); -void add_packed_ref(struct packed_ref_store *refs, +void add_packed_ref(struct ref_store *ref_store, const char *refname, const struct object_id *oid); -int commit_packed_refs(struct packed_ref_store *refs); +int commit_packed_refs(struct ref_store *ref_store); -int repack_without_refs(struct packed_ref_store *refs, +int repack_without_refs(struct ref_store *ref_store, struct string_list *refnames, struct strbuf *err); #endif /* REFS_PACKED_BACKEND_H */ diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 6f8f9f5619..4789106fc0 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -664,6 +664,7 @@ struct ref_storage_be { }; extern struct ref_storage_be refs_be_files; +extern struct ref_storage_be refs_be_packed; /* * A representation of the reference store for the main repository or -- cgit v1.3-5-g9baa From 3478983b517bd62cf2a5c9523815e5e5318a9477 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:39 +0200 Subject: commit_packed_refs(): report errors rather than dying Report errors via a `struct strbuf *err` rather than by calling `die()`. To enable this goal, change `write_packed_entry()` to report errors via a return value and `errno` rather than dying. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 10 +++--- refs/packed-backend.c | 85 +++++++++++++++++++++++++++++++++------------------ refs/packed-backend.h | 2 +- 3 files changed, 61 insertions(+), 36 deletions(-) (limited to 'refs/packed-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index 60f4fa5e7a..2810785efc 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1094,6 +1094,7 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags) struct ref_iterator *iter; int ok; struct ref_to_prune *refs_to_prune = NULL; + struct strbuf err = STRBUF_INIT; lock_packed_refs(refs->packed_ref_store, LOCK_DIE_ON_ERROR); @@ -1128,10 +1129,11 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags) if (ok != ITER_DONE) die("error while iterating over references"); - if (commit_packed_refs(refs->packed_ref_store)) - die_errno("unable to overwrite old ref-pack file"); + if (commit_packed_refs(refs->packed_ref_store, &err)) + die("unable to overwrite old ref-pack file: %s", err.buf); prune_refs(refs, refs_to_prune); + strbuf_release(&err); return 0; } @@ -2693,9 +2695,7 @@ static int files_initial_transaction_commit(struct ref_store *ref_store, &update->new_oid); } - if (commit_packed_refs(refs->packed_ref_store)) { - strbuf_addf(err, "unable to commit packed-refs file: %s", - strerror(errno)); + if (commit_packed_refs(refs->packed_ref_store, err)) { ret = TRANSACTION_GENERIC_ERROR; goto cleanup; } diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 4676dc3959..18ce47fcb7 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -493,15 +493,19 @@ static struct ref_iterator *packed_ref_iterator_begin( /* * 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. + * If peeled is non-NULL, write it as the entry's peeled value. On + * error, return a nonzero value and leave errno set at the value left + * by the failing call to `fprintf()`. */ -static void write_packed_entry(FILE *fh, const char *refname, - const unsigned char *sha1, - const unsigned char *peeled) +static int write_packed_entry(FILE *fh, const char *refname, + const unsigned char *sha1, + const unsigned char *peeled) { - fprintf_or_die(fh, "%s %s\n", sha1_to_hex(sha1), refname); - if (peeled) - fprintf_or_die(fh, "^%s\n", sha1_to_hex(peeled)); + if (fprintf(fh, "%s %s\n", sha1_to_hex(sha1), refname) < 0 || + (peeled && fprintf(fh, "^%s\n", sha1_to_hex(peeled)) < 0)) + return -1; + + return 0; } int lock_packed_refs(struct ref_store *ref_store, int flags) @@ -550,49 +554,74 @@ static const char PACKED_REFS_HEADER[] = /* * Write the current version of the packed refs cache from memory to * disk. The packed-refs file must already be locked for writing (see - * lock_packed_refs()). Return zero on success. On errors, set errno - * and return a nonzero value. + * lock_packed_refs()). Return zero on success. On errors, rollback + * the lockfile, write an error message to `err`, and return a nonzero + * value. */ -int commit_packed_refs(struct ref_store *ref_store) +int commit_packed_refs(struct ref_store *ref_store, struct strbuf *err) { struct packed_ref_store *refs = packed_downcast(ref_store, REF_STORE_WRITE | REF_STORE_MAIN, "commit_packed_refs"); struct packed_ref_cache *packed_ref_cache = get_packed_ref_cache(refs); - int ok, error = 0; - int save_errno = 0; + int ok; + int ret = -1; FILE *out; struct ref_iterator *iter; if (!is_lock_file_locked(&refs->lock)) - die("BUG: packed-refs not locked"); + die("BUG: commit_packed_refs() called when unlocked"); out = fdopen_lock_file(&refs->lock, "w"); - if (!out) - die_errno("unable to fdopen packed-refs descriptor"); + if (!out) { + strbuf_addf(err, "unable to fdopen packed-refs tempfile: %s", + strerror(errno)); + goto error; + } - fprintf_or_die(out, "%s", PACKED_REFS_HEADER); + if (fprintf(out, "%s", PACKED_REFS_HEADER) < 0) { + strbuf_addf(err, "error writing to %s: %s", + get_lock_file_path(&refs->lock), strerror(errno)); + goto error; + } iter = cache_ref_iterator_begin(packed_ref_cache->cache, NULL, 0); while ((ok = ref_iterator_advance(iter)) == ITER_OK) { struct object_id peeled; int peel_error = ref_iterator_peel(iter, &peeled); - write_packed_entry(out, iter->refname, iter->oid->hash, - peel_error ? NULL : peeled.hash); + 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), + strerror(errno)); + ref_iterator_abort(iter); + goto error; + } } - if (ok != ITER_DONE) - die("error while iterating over references"); + if (ok != ITER_DONE) { + strbuf_addf(err, "unable to write packed-refs file: " + "error iterating over old contents"); + goto error; + } if (commit_lock_file(&refs->lock)) { - save_errno = errno; - error = -1; + strbuf_addf(err, "error overwriting %s: %s", + refs->path, strerror(errno)); + goto out; } + + ret = 0; + goto out; + +error: + rollback_lock_file(&refs->lock); + +out: release_packed_ref_cache(packed_ref_cache); - errno = save_errno; - return error; + return ret; } /* @@ -628,7 +657,7 @@ int repack_without_refs(struct ref_store *ref_store, "repack_without_refs"); struct ref_dir *packed; struct string_list_item *refname; - int ret, needs_repacking = 0, removed = 0; + int needs_repacking = 0, removed = 0; packed_assert_main_repository(refs, "repack_without_refs"); assert(err); @@ -665,11 +694,7 @@ int repack_without_refs(struct ref_store *ref_store, } /* Write what remains */ - ret = commit_packed_refs(&refs->base); - if (ret) - strbuf_addf(err, "unable to overwrite old ref-pack file: %s", - strerror(errno)); - return ret; + return commit_packed_refs(&refs->base, err); } static int packed_init_db(struct ref_store *ref_store, struct strbuf *err) diff --git a/refs/packed-backend.h b/refs/packed-backend.h index beea9c14b5..3d4057b65b 100644 --- a/refs/packed-backend.h +++ b/refs/packed-backend.h @@ -14,7 +14,7 @@ int lock_packed_refs(struct ref_store *ref_store, int flags); void add_packed_ref(struct ref_store *ref_store, const char *refname, const struct object_id *oid); -int commit_packed_refs(struct ref_store *ref_store); +int commit_packed_refs(struct ref_store *ref_store, struct strbuf *err); int repack_without_refs(struct ref_store *ref_store, struct string_list *refnames, struct strbuf *err); -- cgit v1.3-5-g9baa From 42dfa7ecef22191b004862fb56074b408c94fc97 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:40 +0200 Subject: commit_packed_refs(): use a staging file separate from the lockfile 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 Signed-off-by: Junio C Hamano --- refs/packed-backend.c | 40 ++++++++++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 8 deletions(-) (limited to 'refs/packed-backend.c') diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 18ce47fcb7..71f92ed6f0 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; } -- cgit v1.3-5-g9baa From b7de57d8d18a08ab517b4d01151129f521185271 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:41 +0200 Subject: packed_refs_lock(): function renamed from lock_packed_refs() Rename `lock_packed_refs()` to `packed_refs_lock()` for consistency with how other methods are named. Also, it's about to get some companions. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 4 ++-- refs/packed-backend.c | 10 +++++----- refs/packed-backend.h | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) (limited to 'refs/packed-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index 2810785efc..88de907148 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1096,7 +1096,7 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags) struct ref_to_prune *refs_to_prune = NULL; struct strbuf err = STRBUF_INIT; - lock_packed_refs(refs->packed_ref_store, LOCK_DIE_ON_ERROR); + packed_refs_lock(refs->packed_ref_store, LOCK_DIE_ON_ERROR); iter = cache_ref_iterator_begin(get_loose_ref_cache(refs), NULL, 0); while ((ok = ref_iterator_advance(iter)) == ITER_OK) { @@ -2679,7 +2679,7 @@ static int files_initial_transaction_commit(struct ref_store *ref_store, } } - if (lock_packed_refs(refs->packed_ref_store, 0)) { + if (packed_refs_lock(refs->packed_ref_store, 0)) { strbuf_addf(err, "unable to lock packed-refs file: %s", strerror(errno)); ret = TRANSACTION_GENERIC_ERROR; diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 71f92ed6f0..cd214e07a1 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -321,7 +321,7 @@ static struct ref_dir *get_packed_refs(struct packed_ref_store *refs) /* * Add or overwrite a reference in the in-memory packed reference * cache. This may only be called while the packed-refs file is locked - * (see lock_packed_refs()). To actually write the packed-refs file, + * (see packed_refs_lock()). To actually write the packed-refs file, * call commit_packed_refs(). */ void add_packed_ref(struct ref_store *ref_store, @@ -515,11 +515,11 @@ static int write_packed_entry(FILE *fh, const char *refname, return 0; } -int lock_packed_refs(struct ref_store *ref_store, int flags) +int packed_refs_lock(struct ref_store *ref_store, int flags) { struct packed_ref_store *refs = packed_downcast(ref_store, REF_STORE_WRITE | REF_STORE_MAIN, - "lock_packed_refs"); + "packed_refs_lock"); static int timeout_configured = 0; static int timeout_value = 1000; struct packed_ref_cache *packed_ref_cache; @@ -567,7 +567,7 @@ static const char PACKED_REFS_HEADER[] = /* * Write the current version of the packed refs cache from memory to * disk. The packed-refs file must already be locked for writing (see - * lock_packed_refs()). Return zero on success. On errors, rollback + * packed_refs_lock()). Return zero on success. On errors, rollback * the lockfile, write an error message to `err`, and return a nonzero * value. */ @@ -698,7 +698,7 @@ int repack_without_refs(struct ref_store *ref_store, if (!needs_repacking) return 0; /* no refname exists in packed refs */ - if (lock_packed_refs(&refs->base, 0)) { + if (packed_refs_lock(&refs->base, 0)) { unable_to_lock_message(refs->path, errno, err); return -1; } diff --git a/refs/packed-backend.h b/refs/packed-backend.h index 3d4057b65b..dbc00d3396 100644 --- a/refs/packed-backend.h +++ b/refs/packed-backend.h @@ -9,7 +9,7 @@ struct ref_store *packed_ref_store_create(const char *path, * hold_lock_file_for_update(). Return 0 on success. On errors, set * errno appropriately and return a nonzero value. */ -int lock_packed_refs(struct ref_store *ref_store, int flags); +int packed_refs_lock(struct ref_store *ref_store, int flags); void add_packed_ref(struct ref_store *ref_store, const char *refname, const struct object_id *oid); -- cgit v1.3-5-g9baa From c8bed835c2a37056aa8f61769c6d8cc7f57bc4d3 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:42 +0200 Subject: packed_refs_lock(): report errors via a `struct strbuf *err` That way the callers don't have to come up with error messages themselves. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 6 ++---- refs/packed-backend.c | 17 +++++++++++------ refs/packed-backend.h | 6 +++--- 3 files changed, 16 insertions(+), 13 deletions(-) (limited to 'refs/packed-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index 88de907148..8ea4e9ab05 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1096,7 +1096,7 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags) struct ref_to_prune *refs_to_prune = NULL; struct strbuf err = STRBUF_INIT; - packed_refs_lock(refs->packed_ref_store, LOCK_DIE_ON_ERROR); + packed_refs_lock(refs->packed_ref_store, LOCK_DIE_ON_ERROR, &err); iter = cache_ref_iterator_begin(get_loose_ref_cache(refs), NULL, 0); while ((ok = ref_iterator_advance(iter)) == ITER_OK) { @@ -2679,9 +2679,7 @@ static int files_initial_transaction_commit(struct ref_store *ref_store, } } - if (packed_refs_lock(refs->packed_ref_store, 0)) { - strbuf_addf(err, "unable to lock packed-refs file: %s", - strerror(errno)); + if (packed_refs_lock(refs->packed_ref_store, 0, err)) { ret = TRANSACTION_GENERIC_ERROR; goto cleanup; } diff --git a/refs/packed-backend.c b/refs/packed-backend.c index cd214e07a1..78e877a9e3 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -515,7 +515,7 @@ static int write_packed_entry(FILE *fh, const char *refname, return 0; } -int packed_refs_lock(struct ref_store *ref_store, int flags) +int packed_refs_lock(struct ref_store *ref_store, int flags, struct strbuf *err) { struct packed_ref_store *refs = packed_downcast(ref_store, REF_STORE_WRITE | REF_STORE_MAIN, @@ -537,9 +537,15 @@ int packed_refs_lock(struct ref_store *ref_store, int flags) if (hold_lock_file_for_update_timeout( &refs->lock, refs->path, - flags, timeout_value) < 0 || - close_lock_file(&refs->lock)) + flags, timeout_value) < 0) { + unable_to_lock_message(refs->path, errno, err); + return -1; + } + + if (close_lock_file(&refs->lock)) { + strbuf_addf(err, "unable to close %s: %s", refs->path, strerror(errno)); return -1; + } /* * Now that we hold the `packed-refs` lock, make sure that our @@ -698,10 +704,9 @@ int repack_without_refs(struct ref_store *ref_store, if (!needs_repacking) return 0; /* no refname exists in packed refs */ - if (packed_refs_lock(&refs->base, 0)) { - unable_to_lock_message(refs->path, errno, err); + if (packed_refs_lock(&refs->base, 0, err)) return -1; - } + packed = get_packed_refs(refs); /* Remove refnames from the cache */ diff --git a/refs/packed-backend.h b/refs/packed-backend.h index dbc00d3396..210e3f35ce 100644 --- a/refs/packed-backend.h +++ b/refs/packed-backend.h @@ -6,10 +6,10 @@ struct ref_store *packed_ref_store_create(const char *path, /* * Lock the packed-refs file for writing. Flags is passed to - * hold_lock_file_for_update(). Return 0 on success. On errors, set - * errno appropriately and return a nonzero value. + * hold_lock_file_for_update(). Return 0 on success. On errors, write + * an error message to `err` and return a nonzero value. */ -int packed_refs_lock(struct ref_store *ref_store, int flags); +int packed_refs_lock(struct ref_store *ref_store, int flags, struct strbuf *err); void add_packed_ref(struct ref_store *ref_store, const char *refname, const struct object_id *oid); -- cgit v1.3-5-g9baa From 49aebcf4328d96bb984672213de76cd4c45197f2 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:43 +0200 Subject: packed_refs_unlock(), packed_refs_is_locked(): new functions Add two new public functions, `packed_refs_unlock()` and `packed_refs_is_locked()`, with which callers can manage and query the `packed-refs` lock externally. Call `packed_refs_unlock()` from `commit_packed_refs()` and `rollback_packed_refs()`. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/packed-backend.c | 31 +++++++++++++++++++++++++------ refs/packed-backend.h | 3 +++ 2 files changed, 28 insertions(+), 6 deletions(-) (limited to 'refs/packed-backend.c') diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 78e877a9e3..f27943f9a1 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -563,6 +563,29 @@ int packed_refs_lock(struct ref_store *ref_store, int flags, struct strbuf *err) return 0; } +void packed_refs_unlock(struct ref_store *ref_store) +{ + struct packed_ref_store *refs = packed_downcast( + ref_store, + REF_STORE_READ | REF_STORE_WRITE, + "packed_refs_unlock"); + + if (!is_lock_file_locked(&refs->lock)) + die("BUG: packed_refs_unlock() called when not locked"); + rollback_lock_file(&refs->lock); + release_packed_ref_cache(refs->cache); +} + +int packed_refs_is_locked(struct ref_store *ref_store) +{ + struct packed_ref_store *refs = packed_downcast( + ref_store, + REF_STORE_READ | REF_STORE_WRITE, + "packed_refs_is_locked"); + + return is_lock_file_locked(&refs->lock); +} + /* * The packed-refs header line that we write out. Perhaps other * traits will be added later. The trailing space is required. @@ -649,8 +672,7 @@ error: delete_tempfile(&refs->tempfile); out: - rollback_lock_file(&refs->lock); - release_packed_ref_cache(packed_ref_cache); + packed_refs_unlock(ref_store); return ret; } @@ -661,14 +683,11 @@ out: */ static void rollback_packed_refs(struct packed_ref_store *refs) { - struct packed_ref_cache *packed_ref_cache = get_packed_ref_cache(refs); - packed_assert_main_repository(refs, "rollback_packed_refs"); if (!is_lock_file_locked(&refs->lock)) die("BUG: packed-refs not locked"); - rollback_lock_file(&refs->lock); - release_packed_ref_cache(packed_ref_cache); + packed_refs_unlock(&refs->base); clear_packed_ref_cache(refs); } diff --git a/refs/packed-backend.h b/refs/packed-backend.h index 210e3f35ce..03b7c1de95 100644 --- a/refs/packed-backend.h +++ b/refs/packed-backend.h @@ -11,6 +11,9 @@ struct ref_store *packed_ref_store_create(const char *path, */ int packed_refs_lock(struct ref_store *ref_store, int flags, struct strbuf *err); +void packed_refs_unlock(struct ref_store *ref_store); +int packed_refs_is_locked(struct ref_store *ref_store); + void add_packed_ref(struct ref_store *ref_store, const char *refname, const struct object_id *oid); -- cgit v1.3-5-g9baa From 9051198214d3e5ec325917d93116bec55addaf5b Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:44 +0200 Subject: clear_packed_ref_cache(): don't protest if the lock is held The existing callers already check that the lock isn't held just before calling `clear_packed_ref_cache()`, and in the near future we want to be able to call this function when the lock is held. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/packed-backend.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'refs/packed-backend.c') diff --git a/refs/packed-backend.c b/refs/packed-backend.c index f27943f9a1..96d92a5eea 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -133,8 +133,6 @@ static void clear_packed_ref_cache(struct packed_ref_store *refs) if (refs->cache) { struct packed_ref_cache *cache = refs->cache; - if (is_lock_file_locked(&refs->lock)) - die("BUG: packed-ref cache cleared while locked"); refs->cache = NULL; release_packed_ref_cache(cache); } -- cgit v1.3-5-g9baa From 42c7f7ff96850a608023c21a5ea05d801e4e5030 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:45 +0200 Subject: commit_packed_refs(): remove call to `packed_refs_unlock()` Instead, change the callers of `commit_packed_refs()` to call `packed_refs_unlock()`. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 2 ++ refs/packed-backend.c | 18 ++++++++---------- 2 files changed, 10 insertions(+), 10 deletions(-) (limited to 'refs/packed-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index 8ea4e9ab05..93bdc8f0c8 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1131,6 +1131,7 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags) if (commit_packed_refs(refs->packed_ref_store, &err)) die("unable to overwrite old ref-pack file: %s", err.buf); + packed_refs_unlock(refs->packed_ref_store); prune_refs(refs, refs_to_prune); strbuf_release(&err); @@ -2699,6 +2700,7 @@ static int files_initial_transaction_commit(struct ref_store *ref_store, } cleanup: + packed_refs_unlock(refs->packed_ref_store); transaction->state = REF_TRANSACTION_CLOSED; string_list_clear(&affected_refnames, 0); return ret; diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 96d92a5eea..5cf6b3d40e 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -606,7 +606,6 @@ int commit_packed_refs(struct ref_store *ref_store, struct strbuf *err) struct packed_ref_cache *packed_ref_cache = get_packed_ref_cache(refs); int ok; - int ret = -1; struct strbuf sb = STRBUF_INIT; FILE *out; struct ref_iterator *iter; @@ -619,7 +618,7 @@ int commit_packed_refs(struct ref_store *ref_store, struct strbuf *err) strbuf_addf(err, "unable to create file %s: %s", sb.buf, strerror(errno)); strbuf_release(&sb); - goto out; + return -1; } strbuf_release(&sb); @@ -660,18 +659,14 @@ int commit_packed_refs(struct ref_store *ref_store, struct strbuf *err) if (rename_tempfile(&refs->tempfile, refs->path)) { strbuf_addf(err, "error replacing %s: %s", refs->path, strerror(errno)); - goto out; + return -1; } - ret = 0; - goto out; + return 0; error: delete_tempfile(&refs->tempfile); - -out: - packed_refs_unlock(ref_store); - return ret; + return -1; } /* @@ -705,6 +700,7 @@ int repack_without_refs(struct ref_store *ref_store, struct ref_dir *packed; struct string_list_item *refname; int needs_repacking = 0, removed = 0; + int ret; packed_assert_main_repository(refs, "repack_without_refs"); assert(err); @@ -740,7 +736,9 @@ int repack_without_refs(struct ref_store *ref_store, } /* Write what remains */ - return commit_packed_refs(&refs->base, err); + ret = commit_packed_refs(&refs->base, err); + packed_refs_unlock(ref_store); + return ret; } static int packed_init_db(struct ref_store *ref_store, struct strbuf *err) -- cgit v1.3-5-g9baa From e5cc7d7d2bf2fbb3dfba81fe0c0c2981607fbc84 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sat, 1 Jul 2017 20:31:06 +0200 Subject: repack_without_refs(): don't lock or unlock the packed refs Change `repack_without_refs()` to expect the packed-refs lock to be held already, and not to release the lock before returning. Change the callers to deal with lock management. This change makes it possible for callers to hold the packed-refs lock for a longer span of time, a possibility that will eventually make it possible to fix some longstanding races. The only semantic change here is that `repack_without_refs()` used to forget to release the lock in the `if (!removed)` exit path. That omission is now fixed. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 47 +++++++++++++++++++++++++++++++---------------- refs/packed-backend.c | 32 ++++++++------------------------ 2 files changed, 39 insertions(+), 40 deletions(-) (limited to 'refs/packed-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index 93bdc8f0c8..e9b95592b6 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1149,24 +1149,16 @@ static int files_delete_refs(struct ref_store *ref_store, const char *msg, if (!refnames->nr) return 0; - result = repack_without_refs(refs->packed_ref_store, refnames, &err); - if (result) { - /* - * If we failed to rewrite the packed-refs file, then - * it is unsafe to try to remove loose refs, because - * doing so might expose an obsolete packed value for - * a reference that might even point at an object that - * has been garbage collected. - */ - if (refnames->nr == 1) - error(_("could not delete reference %s: %s"), - refnames->items[0].string, err.buf); - else - error(_("could not delete references: %s"), err.buf); + if (packed_refs_lock(refs->packed_ref_store, 0, &err)) + goto error; - goto out; + if (repack_without_refs(refs->packed_ref_store, refnames, &err)) { + packed_refs_unlock(refs->packed_ref_store); + goto error; } + packed_refs_unlock(refs->packed_ref_store); + for (i = 0; i < refnames->nr; i++) { const char *refname = refnames->items[i].string; @@ -1174,9 +1166,24 @@ static int files_delete_refs(struct ref_store *ref_store, const char *msg, result |= error(_("could not remove reference %s"), refname); } -out: strbuf_release(&err); return result; + +error: + /* + * If we failed to rewrite the packed-refs file, then it is + * unsafe to try to remove loose refs, because doing so might + * expose an obsolete packed value for a reference that might + * even point at an object that has been garbage collected. + */ + if (refnames->nr == 1) + error(_("could not delete reference %s: %s"), + refnames->items[0].string, err.buf); + else + error(_("could not delete references: %s"), err.buf); + + strbuf_release(&err); + return -1; } /* @@ -2569,11 +2576,19 @@ static int files_transaction_finish(struct ref_store *ref_store, } } + if (packed_refs_lock(refs->packed_ref_store, 0, err)) { + ret = TRANSACTION_GENERIC_ERROR; + goto cleanup; + } + if (repack_without_refs(refs->packed_ref_store, &refs_to_delete, err)) { ret = TRANSACTION_GENERIC_ERROR; + packed_refs_unlock(refs->packed_ref_store); goto cleanup; } + packed_refs_unlock(refs->packed_ref_store); + /* Delete the reflogs of any references that were deleted: */ for_each_string_list_item(ref_to_delete, &refs_to_delete) { strbuf_reset(&sb); diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 5cf6b3d40e..377c775adb 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -669,25 +669,12 @@ error: return -1; } -/* - * Rollback the lockfile for the packed-refs file, and discard the - * in-memory packed reference cache. (The packed-refs file will be - * read anew if it is needed again after this function is called.) - */ -static void rollback_packed_refs(struct packed_ref_store *refs) -{ - packed_assert_main_repository(refs, "rollback_packed_refs"); - - if (!is_lock_file_locked(&refs->lock)) - die("BUG: packed-refs not locked"); - packed_refs_unlock(&refs->base); - clear_packed_ref_cache(refs); -} - /* * Rewrite the packed-refs file, omitting any refs listed in * 'refnames'. On error, leave packed-refs unchanged, write an error - * message to 'err', and return a nonzero value. + * message to 'err', and return a nonzero value. The packed refs lock + * must be held when calling this function; it will still be held when + * the function returns. * * The refs in 'refnames' needn't be sorted. `err` must not be NULL. */ @@ -700,11 +687,13 @@ int repack_without_refs(struct ref_store *ref_store, struct ref_dir *packed; struct string_list_item *refname; int needs_repacking = 0, removed = 0; - int ret; packed_assert_main_repository(refs, "repack_without_refs"); assert(err); + if (!is_lock_file_locked(&refs->lock)) + die("BUG: repack_without_refs called without holding lock"); + /* Look for a packed ref */ for_each_string_list_item(refname, refnames) { if (get_packed_ref(refs, refname->string)) { @@ -717,9 +706,6 @@ int repack_without_refs(struct ref_store *ref_store, if (!needs_repacking) return 0; /* no refname exists in packed refs */ - if (packed_refs_lock(&refs->base, 0, err)) - return -1; - packed = get_packed_refs(refs); /* Remove refnames from the cache */ @@ -731,14 +717,12 @@ int repack_without_refs(struct ref_store *ref_store, * All packed entries disappeared while we were * acquiring the lock. */ - rollback_packed_refs(refs); + clear_packed_ref_cache(refs); return 0; } /* Write what remains */ - ret = commit_packed_refs(&refs->base, err); - packed_refs_unlock(ref_store); - return ret; + return commit_packed_refs(&refs->base, err); } static int packed_init_db(struct ref_store *ref_store, struct strbuf *err) -- cgit v1.3-5-g9baa From 9308b7f3ca9bbe7e76b16c832617a8c6aea5ade3 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sat, 1 Jul 2017 20:31:08 +0200 Subject: read_packed_refs(): die if `packed-refs` contains bogus data The old code ignored any lines that it didn't understand, including unterminated lines. This is dangerous. Instead, `die()` if the `packed-refs` file contains any unterminated lines or lines that we don't know how to handle. This fixes the tests added in the last commit. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/packed-backend.c | 10 +++++++--- t/t3210-pack-refs.sh | 6 +++--- 2 files changed, 10 insertions(+), 6 deletions(-) (limited to 'refs/packed-backend.c') diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 377c775adb..a28befbfa3 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -229,6 +229,9 @@ static struct packed_ref_cache *read_packed_refs(const char *packed_refs_file) const char *refname; const char *traits; + if (!line.len || line.buf[line.len - 1] != '\n') + die("unterminated line in %s: %s", packed_refs_file, line.buf); + if (skip_prefix(line.buf, "# pack-refs with:", &traits)) { if (strstr(traits, " fully-peeled ")) peeled = PEELED_FULLY; @@ -253,9 +256,7 @@ static struct packed_ref_cache *read_packed_refs(const char *packed_refs_file) (peeled == PEELED_TAGS && starts_with(refname, "refs/tags/"))) last->flag |= REF_KNOWS_PEELED; add_ref_entry(dir, last); - continue; - } - if (last && + } else if (last && line.buf[0] == '^' && line.len == PEELED_LINE_LENGTH && line.buf[PEELED_LINE_LENGTH - 1] == '\n' && @@ -267,6 +268,9 @@ static struct packed_ref_cache *read_packed_refs(const char *packed_refs_file) * reference: */ last->flag |= REF_KNOWS_PEELED; + } else { + strbuf_setlen(&line, line.len - 1); + die("unexpected line in %s: %s", packed_refs_file, line.buf); } } diff --git a/t/t3210-pack-refs.sh b/t/t3210-pack-refs.sh index 4b65836283..2bb4b25ed9 100755 --- a/t/t3210-pack-refs.sh +++ b/t/t3210-pack-refs.sh @@ -194,7 +194,7 @@ test_expect_success 'notice d/f conflict with existing ref' ' test_must_fail git branch foo/bar/baz/lots/of/extra/components ' -test_expect_failure 'reject packed-refs with unterminated line' ' +test_expect_success 'reject packed-refs with unterminated line' ' cp .git/packed-refs .git/packed-refs.bak && test_when_finished "mv .git/packed-refs.bak .git/packed-refs" && printf "%s" "$HEAD refs/zzzzz" >>.git/packed-refs && @@ -203,7 +203,7 @@ test_expect_failure 'reject packed-refs with unterminated line' ' test_cmp expected_err err ' -test_expect_failure 'reject packed-refs containing junk' ' +test_expect_success 'reject packed-refs containing junk' ' cp .git/packed-refs .git/packed-refs.bak && test_when_finished "mv .git/packed-refs.bak .git/packed-refs" && printf "%s\n" "bogus content" >>.git/packed-refs && @@ -212,7 +212,7 @@ test_expect_failure 'reject packed-refs containing junk' ' test_cmp expected_err err ' -test_expect_failure 'reject packed-refs with a short SHA-1' ' +test_expect_success 'reject packed-refs with a short SHA-1' ' cp .git/packed-refs .git/packed-refs.bak && test_when_finished "mv .git/packed-refs.bak .git/packed-refs" && printf "%.7s %s\n" $HEAD refs/zzzzz >>.git/packed-refs && -- cgit v1.3-5-g9baa From 198b808e207e35ba390abe362c75040500997cea Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 26 Jul 2017 16:39:42 -0700 Subject: packed_ref_store: handle a packed-refs file that is a symlink One of the tricks that `contrib/workdir/git-new-workdir` plays is to making `packed-refs` in the new workdir a symlink to the `packed-refs` file in the original repository. Before 42dfa7ecef ("commit_packed_refs(): use a staging file separate from the lockfile", 2017-06-23), a lockfile was used as the staging file, and because the `LOCK_NO_DEREF` was not used, the pointed-to file was locked and modified. But after that commit, the staging file was created using a tempfile, with the end result that rewriting the `packed-refs` file in the workdir overwrote the symlink rather than the original `packed-refs` file. Change `commit_packed_refs()` to use `get_locked_file_path()` to find the path of the file that it should overwrite. Since that path was properly resolved when the lockfile was created, this restores the pre-42dfa7ecef behavior. Also add a test case to document this use case and prevent a regression like this from recurring. Signed-off-by: Michael Haggerty Reviewed-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- refs/packed-backend.c | 24 ++++++++++++++++++------ t/t3210-pack-refs.sh | 15 +++++++++++++++ 2 files changed, 33 insertions(+), 6 deletions(-) (limited to 'refs/packed-backend.c') diff --git a/refs/packed-backend.c b/refs/packed-backend.c index a28befbfa3..59e7d1a509 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -610,19 +610,27 @@ int commit_packed_refs(struct ref_store *ref_store, struct strbuf *err) struct packed_ref_cache *packed_ref_cache = get_packed_ref_cache(refs); int ok; + int ret = -1; struct strbuf sb = STRBUF_INIT; FILE *out; struct ref_iterator *iter; + char *packed_refs_path; if (!is_lock_file_locked(&refs->lock)) die("BUG: commit_packed_refs() called when unlocked"); - strbuf_addf(&sb, "%s.new", refs->path); + /* + * If packed-refs is a symlink, we want to overwrite the + * symlinked-to file, not the symlink itself. Also, put the + * staging file next to it: + */ + packed_refs_path = get_locked_file_path(&refs->lock); + strbuf_addf(&sb, "%s.new", packed_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); - return -1; + goto out; } strbuf_release(&sb); @@ -660,17 +668,21 @@ int commit_packed_refs(struct ref_store *ref_store, struct strbuf *err) goto error; } - if (rename_tempfile(&refs->tempfile, refs->path)) { + if (rename_tempfile(&refs->tempfile, packed_refs_path)) { strbuf_addf(err, "error replacing %s: %s", refs->path, strerror(errno)); - return -1; + goto out; } - return 0; + ret = 0; + goto out; error: delete_tempfile(&refs->tempfile); - return -1; + +out: + free(packed_refs_path); + return ret; } /* diff --git a/t/t3210-pack-refs.sh b/t/t3210-pack-refs.sh index 2bb4b25ed9..afa27ffe2d 100755 --- a/t/t3210-pack-refs.sh +++ b/t/t3210-pack-refs.sh @@ -238,4 +238,19 @@ test_expect_success 'retry acquiring packed-refs.lock' ' git -c core.packedrefstimeout=3000 pack-refs --all --prune ' +test_expect_success SYMLINKS 'pack symlinked packed-refs' ' + # First make sure that symlinking works when reading: + git update-ref refs/heads/loosy refs/heads/master && + git for-each-ref >all-refs-before && + mv .git/packed-refs .git/my-deviant-packed-refs && + ln -s my-deviant-packed-refs .git/packed-refs && + git for-each-ref >all-refs-linked && + test_cmp all-refs-before all-refs-linked && + git pack-refs --all --prune && + git for-each-ref >all-refs-packed && + test_cmp all-refs-before all-refs-packed && + test -h .git/packed-refs && + test "$(readlink .git/packed-refs)" = "my-deviant-packed-refs" +' + test_done -- cgit v1.3-5-g9baa