From 2f108821666a968940f171d52d4d0745a3002b35 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:20 +0200 Subject: add_packed_ref(): teach function to overwrite existing refs Teach `add_packed_ref()` to overwrite an existing entry if one already exists for the specified `refname`. This means that we can call it from `files_pack_refs()`, thereby reducing the amount that the latter function needs to know about the internals of packed-reference handling. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 40 ++++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 22 deletions(-) (limited to 'refs/files-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index b040bb3b0a..87cecde231 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -413,15 +413,16 @@ static struct ref_dir *get_packed_refs(struct files_ref_store *refs) } /* - * Add a reference to 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(). + * 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(). */ static void add_packed_ref(struct files_ref_store *refs, const char *refname, const struct object_id *oid) { - struct packed_ref_cache *packed_ref_cache = get_packed_ref_cache(refs); + struct ref_dir *packed_refs; + struct ref_entry *packed_entry; if (!is_lock_file_locked(&refs->packed_refs_lock)) die("BUG: packed refs not locked"); @@ -429,8 +430,17 @@ static void add_packed_ref(struct files_ref_store *refs, if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) die("Reference has invalid format: '%s'", refname); - add_ref_entry(get_packed_ref_dir(packed_ref_cache), - create_ref_entry(refname, oid, REF_ISPACKED)); + 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); + } } /* @@ -1526,12 +1536,10 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags) files_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB, "pack_refs"); struct ref_iterator *iter; - struct ref_dir *packed_refs; int ok; struct ref_to_prune *refs_to_prune = NULL; lock_packed_refs(refs, LOCK_DIE_ON_ERROR); - packed_refs = get_packed_refs(refs); iter = cache_ref_iterator_begin(get_loose_ref_cache(refs), NULL, 0); while ((ok = ref_iterator_advance(iter)) == ITER_OK) { @@ -1540,8 +1548,6 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags) * in the packed ref cache. If the reference should be * pruned, also add it to refs_to_prune. */ - struct ref_entry *packed_entry; - if (!should_pack_ref(iter->refname, iter->oid, iter->flags, flags)) continue; @@ -1552,17 +1558,7 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags) * we don't copy the peeled status, because we want it * to be re-peeled. */ - packed_entry = find_ref_entry(packed_refs, iter->refname); - if (packed_entry) { - /* Overwrite existing packed entry with info from loose entry */ - packed_entry->flag = REF_ISPACKED; - oidcpy(&packed_entry->u.value.oid, iter->oid); - } else { - packed_entry = create_ref_entry(iter->refname, iter->oid, - REF_ISPACKED); - add_ref_entry(packed_refs, packed_entry); - } - oidclr(&packed_entry->u.value.peeled); + add_packed_ref(refs, iter->refname, iter->oid); /* Schedule the loose reference for pruning if requested. */ if ((flags & PACK_REFS_PRUNE)) { -- cgit v1.2.1 From bdf55fa6b23b14ac8726fc29182739762b8f6011 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:21 +0200 Subject: packed_ref_store: new struct Start extracting the packed-refs-related data structures into a new class, `packed_ref_store`. It doesn't yet implement `ref_store`, but it will. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 42 +++++++++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 9 deletions(-) (limited to 'refs/files-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index 87cecde231..2efb71cee9 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -47,6 +47,28 @@ struct packed_ref_cache { struct stat_validity validity; }; +/* + * A container for `packed-refs`-related data. It is not (yet) a + * `ref_store`. + */ +struct packed_ref_store { + unsigned int store_flags; + + /* + * A cache of the values read from the `packed-refs` file, if + * it might still be current; otherwise, NULL. + */ + struct packed_ref_cache *cache; +}; + +static struct packed_ref_store *packed_ref_store_create(unsigned int store_flags) +{ + struct packed_ref_store *refs = xcalloc(1, sizeof(*refs)); + + refs->store_flags = store_flags; + return refs; +} + /* * Future: need to be in "struct repository" * when doing a full libification. @@ -60,13 +82,14 @@ struct files_ref_store { char *packed_refs_path; struct ref_cache *loose; - struct packed_ref_cache *packed; /* * Lock used for the "packed-refs" file. Note that this (and * thus the enclosing `files_ref_store`) must not be freed. */ struct lock_file packed_refs_lock; + + struct packed_ref_store *packed_ref_store; }; /* @@ -95,12 +118,12 @@ static int release_packed_ref_cache(struct packed_ref_cache *packed_refs) static void clear_packed_ref_cache(struct files_ref_store *refs) { - if (refs->packed) { - struct packed_ref_cache *packed_refs = refs->packed; + if (refs->packed_ref_store->cache) { + struct packed_ref_cache *packed_refs = refs->packed_ref_store->cache; if (is_lock_file_locked(&refs->packed_refs_lock)) die("BUG: packed-ref cache cleared while locked"); - refs->packed = NULL; + refs->packed_ref_store->cache = NULL; release_packed_ref_cache(packed_refs); } } @@ -132,6 +155,7 @@ static struct ref_store *files_ref_store_create(const char *gitdir, refs->gitcommondir = strbuf_detach(&sb, NULL); strbuf_addf(&sb, "%s/packed-refs", refs->gitcommondir); refs->packed_refs_path = strbuf_detach(&sb, NULL); + refs->packed_ref_store = packed_ref_store_create(flags); return ref_store; } @@ -375,8 +399,8 @@ static void files_ref_path(struct files_ref_store *refs, */ static void validate_packed_ref_cache(struct files_ref_store *refs) { - if (refs->packed && - !stat_validity_check(&refs->packed->validity, + if (refs->packed_ref_store->cache && + !stat_validity_check(&refs->packed_ref_store->cache->validity, files_packed_refs_path(refs))) clear_packed_ref_cache(refs); } @@ -396,10 +420,10 @@ static struct packed_ref_cache *get_packed_ref_cache(struct files_ref_store *ref if (!is_lock_file_locked(&refs->packed_refs_lock)) validate_packed_ref_cache(refs); - if (!refs->packed) - refs->packed = read_packed_refs(packed_refs_file); + if (!refs->packed_ref_store->cache) + refs->packed_ref_store->cache = read_packed_refs(packed_refs_file); - return refs->packed; + return refs->packed_ref_store->cache; } static struct ref_dir *get_packed_ref_dir(struct packed_ref_cache *packed_ref_cache) -- cgit v1.2.1 From e0d483970b0d666be7bbb74ee0bbf6d8baf7a0ea Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:22 +0200 Subject: packed_ref_store: move `packed_refs_path` here Move `packed_refs_path` from `files_ref_store` to `packed_ref_store`, and rename it to `path` since its meaning is clear from its new context. Inline `files_packed_refs_path()`. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) (limited to 'refs/files-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index 2efb71cee9..c4b8e2f63b 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -54,6 +54,9 @@ struct packed_ref_cache { 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. @@ -61,11 +64,13 @@ struct packed_ref_store { struct packed_ref_cache *cache; }; -static struct packed_ref_store *packed_ref_store_create(unsigned int store_flags) +static 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; } @@ -79,7 +84,6 @@ struct files_ref_store { char *gitdir; char *gitcommondir; - char *packed_refs_path; struct ref_cache *loose; @@ -154,8 +158,8 @@ static struct ref_store *files_ref_store_create(const char *gitdir, get_common_dir_noenv(&sb, gitdir); refs->gitcommondir = strbuf_detach(&sb, NULL); strbuf_addf(&sb, "%s/packed-refs", refs->gitcommondir); - refs->packed_refs_path = strbuf_detach(&sb, NULL); - refs->packed_ref_store = packed_ref_store_create(flags); + refs->packed_ref_store = packed_ref_store_create(sb.buf, flags); + strbuf_release(&sb); return ref_store; } @@ -343,11 +347,6 @@ static struct packed_ref_cache *read_packed_refs(const char *packed_refs_file) return packed_refs; } -static const char *files_packed_refs_path(struct files_ref_store *refs) -{ - return refs->packed_refs_path; -} - static void files_reflog_path(struct files_ref_store *refs, struct strbuf *sb, const char *refname) @@ -401,7 +400,7 @@ static void validate_packed_ref_cache(struct files_ref_store *refs) { if (refs->packed_ref_store->cache && !stat_validity_check(&refs->packed_ref_store->cache->validity, - files_packed_refs_path(refs))) + refs->packed_ref_store->path)) clear_packed_ref_cache(refs); } @@ -415,7 +414,7 @@ static void validate_packed_ref_cache(struct files_ref_store *refs) */ static struct packed_ref_cache *get_packed_ref_cache(struct files_ref_store *refs) { - const char *packed_refs_file = files_packed_refs_path(refs); + const char *packed_refs_file = refs->packed_ref_store->path; if (!is_lock_file_locked(&refs->packed_refs_lock)) validate_packed_ref_cache(refs); @@ -1352,7 +1351,7 @@ static int lock_packed_refs(struct files_ref_store *refs, int flags) } if (hold_lock_file_for_update_timeout( - &refs->packed_refs_lock, files_packed_refs_path(refs), + &refs->packed_refs_lock, refs->packed_ref_store->path, flags, timeout_value) < 0) return -1; @@ -1633,7 +1632,7 @@ static int repack_without_refs(struct files_ref_store *refs, return 0; /* no refname exists in packed refs */ if (lock_packed_refs(refs, 0)) { - unable_to_lock_message(files_packed_refs_path(refs), errno, err); + unable_to_lock_message(refs->packed_ref_store->path, errno, err); return -1; } packed = get_packed_refs(refs); -- cgit v1.2.1 From 139c4596ad6fc1585574f27fde6dea28ab1d3a54 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:23 +0200 Subject: packed_ref_store: move `packed_refs_lock` member here Move the `packed_refs_lock` member from `files_ref_store` to `packed_ref_store`, and rename it to `lock` since it's now more obvious what it is locking. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) (limited to 'refs/files-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index c4b8e2f63b..de8293493f 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -62,6 +62,12 @@ struct packed_ref_store { * 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; }; static struct packed_ref_store *packed_ref_store_create( @@ -87,12 +93,6 @@ struct files_ref_store { struct ref_cache *loose; - /* - * Lock used for the "packed-refs" file. Note that this (and - * thus the enclosing `files_ref_store`) must not be freed. - */ - struct lock_file packed_refs_lock; - struct packed_ref_store *packed_ref_store; }; @@ -125,7 +125,7 @@ static void clear_packed_ref_cache(struct files_ref_store *refs) if (refs->packed_ref_store->cache) { struct packed_ref_cache *packed_refs = refs->packed_ref_store->cache; - if (is_lock_file_locked(&refs->packed_refs_lock)) + if (is_lock_file_locked(&refs->packed_ref_store->lock)) die("BUG: packed-ref cache cleared while locked"); refs->packed_ref_store->cache = NULL; release_packed_ref_cache(packed_refs); @@ -416,7 +416,7 @@ static struct packed_ref_cache *get_packed_ref_cache(struct files_ref_store *ref { const char *packed_refs_file = refs->packed_ref_store->path; - if (!is_lock_file_locked(&refs->packed_refs_lock)) + if (!is_lock_file_locked(&refs->packed_ref_store->lock)) validate_packed_ref_cache(refs); if (!refs->packed_ref_store->cache) @@ -447,7 +447,7 @@ static void add_packed_ref(struct files_ref_store *refs, struct ref_dir *packed_refs; struct ref_entry *packed_entry; - if (!is_lock_file_locked(&refs->packed_refs_lock)) + if (!is_lock_file_locked(&refs->packed_ref_store->lock)) die("BUG: packed refs not locked"); if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) @@ -1351,7 +1351,8 @@ static int lock_packed_refs(struct files_ref_store *refs, int flags) } if (hold_lock_file_for_update_timeout( - &refs->packed_refs_lock, refs->packed_ref_store->path, + &refs->packed_ref_store->lock, + refs->packed_ref_store->path, flags, timeout_value) < 0) return -1; @@ -1388,10 +1389,10 @@ static int commit_packed_refs(struct files_ref_store *refs) files_assert_main_repository(refs, "commit_packed_refs"); - if (!is_lock_file_locked(&refs->packed_refs_lock)) + if (!is_lock_file_locked(&refs->packed_ref_store->lock)) die("BUG: packed-refs not locked"); - out = fdopen_lock_file(&refs->packed_refs_lock, "w"); + out = fdopen_lock_file(&refs->packed_ref_store->lock, "w"); if (!out) die_errno("unable to fdopen packed-refs descriptor"); @@ -1409,7 +1410,7 @@ static int commit_packed_refs(struct files_ref_store *refs) if (ok != ITER_DONE) die("error while iterating over references"); - if (commit_lock_file(&refs->packed_refs_lock)) { + if (commit_lock_file(&refs->packed_ref_store->lock)) { save_errno = errno; error = -1; } @@ -1430,9 +1431,9 @@ static void rollback_packed_refs(struct files_ref_store *refs) files_assert_main_repository(refs, "rollback_packed_refs"); - if (!is_lock_file_locked(&refs->packed_refs_lock)) + if (!is_lock_file_locked(&refs->packed_ref_store->lock)) die("BUG: packed-refs not locked"); - rollback_lock_file(&refs->packed_refs_lock); + rollback_lock_file(&refs->packed_ref_store->lock); release_packed_ref_cache(packed_ref_cache); clear_packed_ref_cache(refs); } -- cgit v1.2.1 From 9c4fe0ff95a2d6245f6f0e52988d59f4131293b4 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:24 +0200 Subject: clear_packed_ref_cache(): take a `packed_ref_store *` parameter It only cares about the packed-refs part of the reference store. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'refs/files-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index de8293493f..2b0db60b2b 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -120,15 +120,15 @@ static int release_packed_ref_cache(struct packed_ref_cache *packed_refs) } } -static void clear_packed_ref_cache(struct files_ref_store *refs) +static void clear_packed_ref_cache(struct packed_ref_store *refs) { - if (refs->packed_ref_store->cache) { - struct packed_ref_cache *packed_refs = refs->packed_ref_store->cache; + if (refs->cache) { + struct packed_ref_cache *cache = refs->cache; - if (is_lock_file_locked(&refs->packed_ref_store->lock)) + if (is_lock_file_locked(&refs->lock)) die("BUG: packed-ref cache cleared while locked"); - refs->packed_ref_store->cache = NULL; - release_packed_ref_cache(packed_refs); + refs->cache = NULL; + release_packed_ref_cache(cache); } } @@ -401,7 +401,7 @@ static void validate_packed_ref_cache(struct files_ref_store *refs) if (refs->packed_ref_store->cache && !stat_validity_check(&refs->packed_ref_store->cache->validity, refs->packed_ref_store->path)) - clear_packed_ref_cache(refs); + clear_packed_ref_cache(refs->packed_ref_store); } /* @@ -1435,7 +1435,7 @@ static void rollback_packed_refs(struct files_ref_store *refs) die("BUG: packed-refs not locked"); rollback_lock_file(&refs->packed_ref_store->lock); release_packed_ref_cache(packed_ref_cache); - clear_packed_ref_cache(refs); + clear_packed_ref_cache(refs->packed_ref_store); } struct ref_to_prune { -- cgit v1.2.1 From 25e0c5faf2628e3701b1b9b783c3ca4a5653ec4d Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:25 +0200 Subject: validate_packed_ref_cache(): take a `packed_ref_store *` parameter It only cares about the packed-refs part of the reference store. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'refs/files-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index 2b0db60b2b..f061506bf0 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -396,12 +396,11 @@ static void files_ref_path(struct files_ref_store *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 files_ref_store *refs) +static void validate_packed_ref_cache(struct packed_ref_store *refs) { - if (refs->packed_ref_store->cache && - !stat_validity_check(&refs->packed_ref_store->cache->validity, - refs->packed_ref_store->path)) - clear_packed_ref_cache(refs->packed_ref_store); + if (refs->cache && + !stat_validity_check(&refs->cache->validity, refs->path)) + clear_packed_ref_cache(refs); } /* @@ -417,7 +416,7 @@ static struct packed_ref_cache *get_packed_ref_cache(struct files_ref_store *ref const char *packed_refs_file = refs->packed_ref_store->path; if (!is_lock_file_locked(&refs->packed_ref_store->lock)) - validate_packed_ref_cache(refs); + validate_packed_ref_cache(refs->packed_ref_store); if (!refs->packed_ref_store->cache) refs->packed_ref_store->cache = read_packed_refs(packed_refs_file); @@ -1364,7 +1363,7 @@ static int lock_packed_refs(struct files_ref_store *refs, int flags) * 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); + validate_packed_ref_cache(refs->packed_ref_store); packed_ref_cache = get_packed_ref_cache(refs); /* Increment the reference count to prevent it from being freed: */ -- cgit v1.2.1 From 8e821c38f76c49e0e679c8d79c0b150d18aba155 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:26 +0200 Subject: get_packed_ref_cache(): take a `packed_ref_store *` parameter It only cares about the packed-refs part of the reference store. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) (limited to 'refs/files-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index f061506bf0..b2ef7b3bb9 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -404,24 +404,22 @@ static void validate_packed_ref_cache(struct packed_ref_store *refs) } /* - * Get the packed_ref_cache for the specified files_ref_store, + * 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 files_ref_store *refs) +static struct packed_ref_cache *get_packed_ref_cache(struct packed_ref_store *refs) { - const char *packed_refs_file = refs->packed_ref_store->path; + if (!is_lock_file_locked(&refs->lock)) + validate_packed_ref_cache(refs); - if (!is_lock_file_locked(&refs->packed_ref_store->lock)) - validate_packed_ref_cache(refs->packed_ref_store); - - if (!refs->packed_ref_store->cache) - refs->packed_ref_store->cache = read_packed_refs(packed_refs_file); + if (!refs->cache) + refs->cache = read_packed_refs(refs->path); - return refs->packed_ref_store->cache; + return refs->cache; } static struct ref_dir *get_packed_ref_dir(struct packed_ref_cache *packed_ref_cache) @@ -431,7 +429,7 @@ static struct ref_dir *get_packed_ref_dir(struct packed_ref_cache *packed_ref_ca static struct ref_dir *get_packed_refs(struct files_ref_store *refs) { - return get_packed_ref_dir(get_packed_ref_cache(refs)); + return get_packed_ref_dir(get_packed_ref_cache(refs->packed_ref_store)); } /* @@ -1151,7 +1149,7 @@ static struct ref_iterator *files_ref_iterator_begin( loose_iter = cache_ref_iterator_begin(get_loose_ref_cache(refs), prefix, 1); - iter->packed_ref_cache = get_packed_ref_cache(refs); + iter->packed_ref_cache = get_packed_ref_cache(refs->packed_ref_store); acquire_packed_ref_cache(iter->packed_ref_cache); packed_iter = cache_ref_iterator_begin(iter->packed_ref_cache->cache, prefix, 0); @@ -1365,7 +1363,7 @@ static int lock_packed_refs(struct files_ref_store *refs, int flags) */ validate_packed_ref_cache(refs->packed_ref_store); - packed_ref_cache = get_packed_ref_cache(refs); + packed_ref_cache = get_packed_ref_cache(refs->packed_ref_store); /* Increment the reference count to prevent it from being freed: */ acquire_packed_ref_cache(packed_ref_cache); return 0; @@ -1380,7 +1378,7 @@ static int lock_packed_refs(struct files_ref_store *refs, int flags) static int commit_packed_refs(struct files_ref_store *refs) { struct packed_ref_cache *packed_ref_cache = - get_packed_ref_cache(refs); + get_packed_ref_cache(refs->packed_ref_store); int ok, error = 0; int save_errno = 0; FILE *out; @@ -1426,7 +1424,7 @@ static int commit_packed_refs(struct files_ref_store *refs) static void rollback_packed_refs(struct files_ref_store *refs) { struct packed_ref_cache *packed_ref_cache = - get_packed_ref_cache(refs); + get_packed_ref_cache(refs->packed_ref_store); files_assert_main_repository(refs, "rollback_packed_refs"); -- cgit v1.2.1 From a9169f5dc2ca4ed1059d9733661058f9e5311724 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:27 +0200 Subject: get_packed_refs(): take a `packed_ref_store *` parameter It only cares about the packed-refs part of the reference store. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'refs/files-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index b2ef7b3bb9..bc5c0de84e 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -427,9 +427,9 @@ static struct ref_dir *get_packed_ref_dir(struct packed_ref_cache *packed_ref_ca return get_ref_dir(packed_ref_cache->cache->root); } -static struct ref_dir *get_packed_refs(struct files_ref_store *refs) +static struct ref_dir *get_packed_refs(struct packed_ref_store *refs) { - return get_packed_ref_dir(get_packed_ref_cache(refs->packed_ref_store)); + return get_packed_ref_dir(get_packed_ref_cache(refs)); } /* @@ -450,7 +450,7 @@ static void add_packed_ref(struct files_ref_store *refs, if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) die("Reference has invalid format: '%s'", refname); - packed_refs = get_packed_refs(refs); + packed_refs = get_packed_refs(refs->packed_ref_store); packed_entry = find_ref_entry(packed_refs, refname); if (packed_entry) { /* Overwrite the existing entry: */ @@ -592,7 +592,7 @@ static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs) static struct ref_entry *get_packed_ref(struct files_ref_store *refs, const char *refname) { - return find_ref_entry(get_packed_refs(refs), refname); + return find_ref_entry(get_packed_refs(refs->packed_ref_store), refname); } /* @@ -1633,7 +1633,7 @@ static int repack_without_refs(struct files_ref_store *refs, unable_to_lock_message(refs->packed_ref_store->path, errno, err); return -1; } - packed = get_packed_refs(refs); + packed = get_packed_refs(refs->packed_ref_store); /* Remove refnames from the cache */ for_each_string_list_item(refname, refnames) -- cgit v1.2.1 From e70b70294e7010c6281766b5229fb811df193870 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:28 +0200 Subject: add_packed_ref(): take a `packed_ref_store *` parameter It only cares about the packed-refs part of the reference store. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'refs/files-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index bc5c0de84e..4943207098 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -438,19 +438,19 @@ 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(). */ -static void add_packed_ref(struct files_ref_store *refs, +static 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->packed_ref_store->lock)) + 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_ref_store); + packed_refs = get_packed_refs(refs); packed_entry = find_ref_entry(packed_refs, refname); if (packed_entry) { /* Overwrite the existing entry: */ @@ -1579,7 +1579,7 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags) * we don't copy the peeled status, because we want it * to be re-peeled. */ - add_packed_ref(refs, iter->refname, iter->oid); + add_packed_ref(refs->packed_ref_store, iter->refname, iter->oid); /* Schedule the loose reference for pruning if requested. */ if ((flags & PACK_REFS_PRUNE)) { @@ -3210,7 +3210,7 @@ static int files_initial_transaction_commit(struct ref_store *ref_store, if ((update->flags & REF_HAVE_NEW) && !is_null_oid(&update->new_oid)) - add_packed_ref(refs, update->refname, + add_packed_ref(refs->packed_ref_store, update->refname, &update->new_oid); } -- cgit v1.2.1 From f512f0f32ced4832776ff78f5026ae87bf65f454 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:29 +0200 Subject: lock_packed_refs(): take a `packed_ref_store *` parameter It only cares about the packed-refs part of the reference store. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) (limited to 'refs/files-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index 4943207098..0d8f39089f 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -80,6 +80,19 @@ static struct packed_ref_store *packed_ref_store_create( 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); +} + /* * Future: need to be in "struct repository" * when doing a full libification. @@ -1334,13 +1347,13 @@ static void write_packed_entry(FILE *fh, const char *refname, * hold_lock_file_for_update(). Return 0 on success. On errors, set * errno appropriately and return a nonzero value. */ -static int lock_packed_refs(struct files_ref_store *refs, int flags) +static 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; - files_assert_main_repository(refs, "lock_packed_refs"); + packed_assert_main_repository(refs, "lock_packed_refs"); if (!timeout_configured) { git_config_get_int("core.packedrefstimeout", &timeout_value); @@ -1348,8 +1361,8 @@ static int lock_packed_refs(struct files_ref_store *refs, int flags) } if (hold_lock_file_for_update_timeout( - &refs->packed_ref_store->lock, - refs->packed_ref_store->path, + &refs->lock, + refs->path, flags, timeout_value) < 0) return -1; @@ -1361,9 +1374,9 @@ static int lock_packed_refs(struct files_ref_store *refs, int flags) * 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_store); + validate_packed_ref_cache(refs); - packed_ref_cache = get_packed_ref_cache(refs->packed_ref_store); + 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; @@ -1560,7 +1573,7 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags) int ok; struct ref_to_prune *refs_to_prune = NULL; - lock_packed_refs(refs, LOCK_DIE_ON_ERROR); + lock_packed_refs(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) { @@ -1629,7 +1642,7 @@ static int repack_without_refs(struct files_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->packed_ref_store, 0)) { unable_to_lock_message(refs->packed_ref_store->path, errno, err); return -1; } @@ -3198,7 +3211,7 @@ static int files_initial_transaction_commit(struct ref_store *ref_store, } } - if (lock_packed_refs(refs, 0)) { + if (lock_packed_refs(refs->packed_ref_store, 0)) { strbuf_addf(err, "unable to lock packed-refs file: %s", strerror(errno)); ret = TRANSACTION_GENERIC_ERROR; -- cgit v1.2.1 From cf30b3e88b0551f67ff4e0b8c35aedf6b03c41d7 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:30 +0200 Subject: commit_packed_refs(): take a `packed_ref_store *` parameter It only cares about the packed-refs part of the reference store. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'refs/files-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index 0d8f39089f..5d159620f0 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1388,21 +1388,21 @@ static int lock_packed_refs(struct packed_ref_store *refs, int flags) * lock_packed_refs()). Return zero on success. On errors, set errno * and return a nonzero value */ -static int commit_packed_refs(struct files_ref_store *refs) +static int commit_packed_refs(struct packed_ref_store *refs) { struct packed_ref_cache *packed_ref_cache = - get_packed_ref_cache(refs->packed_ref_store); + get_packed_ref_cache(refs); int ok, error = 0; int save_errno = 0; FILE *out; struct ref_iterator *iter; - files_assert_main_repository(refs, "commit_packed_refs"); + packed_assert_main_repository(refs, "commit_packed_refs"); - if (!is_lock_file_locked(&refs->packed_ref_store->lock)) + if (!is_lock_file_locked(&refs->lock)) die("BUG: packed-refs not locked"); - out = fdopen_lock_file(&refs->packed_ref_store->lock, "w"); + out = fdopen_lock_file(&refs->lock, "w"); if (!out) die_errno("unable to fdopen packed-refs descriptor"); @@ -1420,7 +1420,7 @@ static int commit_packed_refs(struct files_ref_store *refs) if (ok != ITER_DONE) die("error while iterating over references"); - if (commit_lock_file(&refs->packed_ref_store->lock)) { + if (commit_lock_file(&refs->lock)) { save_errno = errno; error = -1; } @@ -1606,7 +1606,7 @@ 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)) + if (commit_packed_refs(refs->packed_ref_store)) die_errno("unable to overwrite old ref-pack file"); prune_refs(refs, refs_to_prune); @@ -1662,7 +1662,7 @@ static int repack_without_refs(struct files_ref_store *refs, } /* Write what remains */ - ret = commit_packed_refs(refs); + ret = commit_packed_refs(refs->packed_ref_store); if (ret) strbuf_addf(err, "unable to overwrite old ref-pack file: %s", strerror(errno)); @@ -3227,7 +3227,7 @@ static int files_initial_transaction_commit(struct ref_store *ref_store, &update->new_oid); } - if (commit_packed_refs(refs)) { + if (commit_packed_refs(refs->packed_ref_store)) { strbuf_addf(err, "unable to commit packed-refs file: %s", strerror(errno)); ret = TRANSACTION_GENERIC_ERROR; -- cgit v1.2.1 From 38e3fe6decfb7c32eb08a58b192c428c18180508 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:31 +0200 Subject: rollback_packed_refs(): take a `packed_ref_store *` parameter It only cares about the packed-refs part of the reference store. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) (limited to 'refs/files-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index 5d159620f0..a08d3fbadf 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1434,18 +1434,17 @@ static int commit_packed_refs(struct packed_ref_store *refs) * 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 files_ref_store *refs) +static void rollback_packed_refs(struct packed_ref_store *refs) { - struct packed_ref_cache *packed_ref_cache = - get_packed_ref_cache(refs->packed_ref_store); + struct packed_ref_cache *packed_ref_cache = get_packed_ref_cache(refs); - files_assert_main_repository(refs, "rollback_packed_refs"); + packed_assert_main_repository(refs, "rollback_packed_refs"); - if (!is_lock_file_locked(&refs->packed_ref_store->lock)) + if (!is_lock_file_locked(&refs->lock)) die("BUG: packed-refs not locked"); - rollback_lock_file(&refs->packed_ref_store->lock); + rollback_lock_file(&refs->lock); release_packed_ref_cache(packed_ref_cache); - clear_packed_ref_cache(refs->packed_ref_store); + clear_packed_ref_cache(refs); } struct ref_to_prune { @@ -1657,7 +1656,7 @@ static int repack_without_refs(struct files_ref_store *refs, * All packed entries disappeared while we were * acquiring the lock. */ - rollback_packed_refs(refs); + rollback_packed_refs(refs->packed_ref_store); return 0; } -- cgit v1.2.1 From f3f97249402b74fd4cd3d0b008e0de2e14cd5b63 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:32 +0200 Subject: get_packed_ref(): take a `packed_ref_store *` parameter It only cares about the packed-refs part of the reference store. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'refs/files-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index a08d3fbadf..2b9d93d3b6 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -602,10 +602,10 @@ static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs) * 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 files_ref_store *refs, +static struct ref_entry *get_packed_ref(struct packed_ref_store *refs, const char *refname) { - return find_ref_entry(get_packed_refs(refs->packed_ref_store), refname); + return find_ref_entry(get_packed_refs(refs), refname); } /* @@ -621,7 +621,7 @@ static int resolve_packed_ref(struct files_ref_store *refs, * The loose reference file does not exist; check for a packed * reference. */ - entry = get_packed_ref(refs, refname); + entry = get_packed_ref(refs->packed_ref_store, refname); if (entry) { hashcpy(sha1, entry->u.value.oid.hash); *flags |= REF_ISPACKED; @@ -1044,7 +1044,9 @@ static int files_peel_ref(struct ref_store *ref_store, * have REF_KNOWS_PEELED. */ if (flag & REF_ISPACKED) { - struct ref_entry *r = get_packed_ref(refs, refname); + struct ref_entry *r = + get_packed_ref(refs->packed_ref_store, refname); + if (r) { if (peel_entry(r, 0)) return -1; @@ -1631,7 +1633,7 @@ static int repack_without_refs(struct files_ref_store *refs, /* Look for a packed ref */ for_each_string_list_item(refname, refnames) { - if (get_packed_ref(refs, refname->string)) { + if (get_packed_ref(refs->packed_ref_store, refname->string)) { needs_repacking = 1; break; } -- cgit v1.2.1 From 0f199b1ee051e3e6854268841335287c3ad94b88 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:33 +0200 Subject: repack_without_refs(): take a `packed_ref_store *` parameter It only cares about the packed-refs part of the reference store. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'refs/files-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index 2b9d93d3b6..c206791b91 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1621,19 +1621,19 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags) * * The refs in 'refnames' needn't be sorted. `err` must not be NULL. */ -static int repack_without_refs(struct files_ref_store *refs, +static 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; - files_assert_main_repository(refs, "repack_without_refs"); + 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->packed_ref_store, refname->string)) { + if (get_packed_ref(refs, refname->string)) { needs_repacking = 1; break; } @@ -1643,11 +1643,11 @@ static int repack_without_refs(struct files_ref_store *refs, if (!needs_repacking) return 0; /* no refname exists in packed refs */ - if (lock_packed_refs(refs->packed_ref_store, 0)) { - unable_to_lock_message(refs->packed_ref_store->path, errno, err); + if (lock_packed_refs(refs, 0)) { + unable_to_lock_message(refs->path, errno, err); return -1; } - packed = get_packed_refs(refs->packed_ref_store); + packed = get_packed_refs(refs); /* Remove refnames from the cache */ for_each_string_list_item(refname, refnames) @@ -1658,12 +1658,12 @@ static int repack_without_refs(struct files_ref_store *refs, * All packed entries disappeared while we were * acquiring the lock. */ - rollback_packed_refs(refs->packed_ref_store); + rollback_packed_refs(refs); return 0; } /* Write what remains */ - ret = commit_packed_refs(refs->packed_ref_store); + ret = commit_packed_refs(refs); if (ret) strbuf_addf(err, "unable to overwrite old ref-pack file: %s", strerror(errno)); @@ -1681,7 +1681,7 @@ static int files_delete_refs(struct ref_store *ref_store, const char *msg, if (!refnames->nr) return 0; - result = repack_without_refs(refs, refnames, &err); + result = repack_without_refs(refs->packed_ref_store, refnames, &err); if (result) { /* * If we failed to rewrite the packed-refs file, then @@ -3101,7 +3101,7 @@ static int files_transaction_finish(struct ref_store *ref_store, } } - if (repack_without_refs(refs, &refs_to_delete, err)) { + if (repack_without_refs(refs->packed_ref_store, &refs_to_delete, err)) { ret = TRANSACTION_GENERIC_ERROR; goto cleanup; } -- cgit v1.2.1 From 6dc6ba7092423dfd5b94b9dcb649f2905d456d94 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:34 +0200 Subject: packed_peel_ref(): new function, extracted from `files_peel_ref()` This will later become a method of `packed_ref_store`. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) (limited to 'refs/files-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index c206791b91..185d05e1d6 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1013,6 +1013,18 @@ out: return ret; } +static 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; +} + static int files_peel_ref(struct ref_store *ref_store, const char *refname, unsigned char *sha1) { @@ -1043,17 +1055,9 @@ static int files_peel_ref(struct ref_store *ref_store, * be expensive and (b) loose references anyway usually do not * have REF_KNOWS_PEELED. */ - if (flag & REF_ISPACKED) { - struct ref_entry *r = - get_packed_ref(refs->packed_ref_store, refname); - - if (r) { - if (peel_entry(r, 0)) - return -1; - hashcpy(sha1, r->u.value.peeled.hash); - return 0; - } - } + if (flag & REF_ISPACKED && + !packed_peel_ref(refs->packed_ref_store, refname, sha1)) + return 0; return peel_object(base, sha1); } -- cgit v1.2.1 From 38b86e81ae9d3fff3ce300e867f6cd2d1732c998 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:35 +0200 Subject: packed_ref_store: support iteration Add the infrastructure to iterate over a `packed_ref_store`. It's a lot of boilerplate, but it's all part of a campaign to make `packed_ref_store` implement `ref_store`. In the future, this iterator will work much differently. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 110 insertions(+), 9 deletions(-) (limited to 'refs/files-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index 185d05e1d6..0490cc087e 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1062,10 +1062,102 @@ static int files_peel_ref(struct ref_store *ref_store, return peel_object(base, sha1); } +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 +}; + +static 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; +} + struct files_ref_iterator { struct ref_iterator base; - struct packed_ref_cache *packed_ref_cache; struct ref_iterator *iter0; unsigned int flags; }; @@ -1118,7 +1210,6 @@ static int files_ref_iterator_abort(struct ref_iterator *ref_iterator) if (iter->iter0) ok = ref_iterator_abort(iter->iter0); - release_packed_ref_cache(iter->packed_ref_cache); base_ref_iterator_free(ref_iterator); return ok; } @@ -1160,18 +1251,28 @@ static struct ref_iterator *files_ref_iterator_begin( * (If they've already been read, that's OK; we only need to * guarantee that they're read before the packed refs, not * *how much* before.) After that, we call - * get_packed_ref_cache(), which internally checks whether the - * packed-ref cache is up to date with what is on disk, and - * re-reads it if not. + * packed_ref_iterator_begin(), which internally checks + * whether the packed-ref cache is up to date with what is on + * disk, and re-reads it if not. */ loose_iter = cache_ref_iterator_begin(get_loose_ref_cache(refs), prefix, 1); - iter->packed_ref_cache = get_packed_ref_cache(refs->packed_ref_store); - acquire_packed_ref_cache(iter->packed_ref_cache); - packed_iter = cache_ref_iterator_begin(iter->packed_ref_cache->cache, - prefix, 0); + /* + * The packed-refs file might contain broken references, for + * example an old version of a reference that points at an + * object that has since been garbage-collected. This is OK as + * long as there is a corresponding loose reference that + * overrides it, and we don't want to emit an error message in + * this case. So ask the packed_ref_store for all of its + * references, and (if needed) do our own check for broken + * 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, + DO_FOR_EACH_INCLUDE_BROKEN); iter->iter0 = overlay_ref_iterator_begin(loose_iter, packed_iter); iter->flags = flags; -- cgit v1.2.1 From d13fa1a9ba494fbf0e4c362ebe1f60ff464a8f43 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 23 Jun 2017 09:01:36 +0200 Subject: packed_read_raw_ref(): new function, replacing `resolve_packed_ref()` Add a new function, `packed_read_raw_ref()`, which is nearly a `read_raw_ref_fn`. Use it in place of `resolve_packed_ref()`. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) (limited to 'refs/files-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index 0490cc087e..346794cf7c 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -608,27 +608,23 @@ static struct ref_entry *get_packed_ref(struct packed_ref_store *refs, return find_ref_entry(get_packed_refs(refs), refname); } -/* - * A loose ref file doesn't exist; check for a packed ref. - */ -static int resolve_packed_ref(struct files_ref_store *refs, - const char *refname, - unsigned char *sha1, unsigned int *flags) +static 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; - /* - * The loose reference file does not exist; check for a packed - * reference. - */ - entry = get_packed_ref(refs->packed_ref_store, refname); - if (entry) { - hashcpy(sha1, entry->u.value.oid.hash); - *flags |= REF_ISPACKED; - return 0; + *type = 0; + + entry = get_packed_ref(refs, refname); + if (!entry) { + errno = ENOENT; + return -1; } - /* refname is not a packed reference. */ - return -1; + + hashcpy(sha1, entry->u.value.oid.hash); + *type = REF_ISPACKED; + return 0; } static int files_read_raw_ref(struct ref_store *ref_store, @@ -674,7 +670,8 @@ stat_ref: if (lstat(path, &st) < 0) { if (errno != ENOENT) goto out; - if (resolve_packed_ref(refs, refname, sha1, type)) { + if (packed_read_raw_ref(refs->packed_ref_store, refname, + sha1, referent, type)) { errno = ENOENT; goto out; } @@ -713,7 +710,8 @@ stat_ref: * ref is supposed to be, there could still be a * packed ref: */ - if (resolve_packed_ref(refs, refname, sha1, type)) { + if (packed_read_raw_ref(refs->packed_ref_store, refname, + sha1, referent, type)) { errno = EISDIR; goto out; } -- cgit v1.2.1 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/files-backend.c | 640 +-------------------------------------------------- 1 file changed, 1 insertion(+), 639 deletions(-) (limited to 'refs/files-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index 346794cf7c..7df9747798 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2,6 +2,7 @@ #include "../refs.h" #include "refs-internal.h" #include "ref-cache.h" +#include "packed-backend.h" #include "../iterator.h" #include "../dir-iterator.h" #include "../lockfile.h" @@ -14,85 +15,6 @@ struct ref_lock { struct object_id old_oid; }; -/* - * Return true if refname, which has the specified oid and flags, can - * be resolved to an object in the database. If the referred-to object - * does not exist, emit a warning and return false. - */ -static int ref_resolves_to_object(const char *refname, - const struct object_id *oid, - unsigned int flags) -{ - if (flags & REF_ISBROKEN) - return 0; - if (!has_sha1_file(oid->hash)) { - error("%s does not point to a valid object!", refname); - return 0; - } - return 1; -} - -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; -}; - -/* - * 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; -}; - -static 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); -} - /* * Future: need to be in "struct repository" * when doing a full libification. @@ -109,42 +31,6 @@ struct files_ref_store { struct packed_ref_store *packed_ref_store; }; -/* - * 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; - } -} - -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); - } -} - static void clear_loose_ref_cache(struct files_ref_store *refs) { if (refs->loose) { @@ -215,151 +101,6 @@ static struct files_ref_store *files_downcast(struct ref_store *ref_store, return refs; } -/* The length of a peeled reference line in packed-refs, including EOL: */ -#define PEELED_LINE_LENGTH 42 - -/* - * 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"; - -/* - * 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; -} - static void files_reflog_path(struct files_ref_store *refs, struct strbuf *sb, const char *refname) @@ -405,77 +146,6 @@ static void files_ref_path(struct files_ref_store *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(). - */ -static 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); - } -} - /* * Read the loose references from the namespace dirname into dir * (without recursing). dirname must end with '/'. dir must be the @@ -598,35 +268,6 @@ static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs) return refs->loose; } -/* - * 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); -} - -static 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; -} - static int files_read_raw_ref(struct ref_store *ref_store, const char *refname, unsigned char *sha1, struct strbuf *referent, unsigned int *type) @@ -1011,18 +652,6 @@ out: return ret; } -static 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; -} - static int files_peel_ref(struct ref_store *ref_store, const char *refname, unsigned char *sha1) { @@ -1060,99 +689,6 @@ static int files_peel_ref(struct ref_store *ref_store, return peel_object(base, sha1); } -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 -}; - -static 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; -} - struct files_ref_iterator { struct ref_iterator base; @@ -1434,124 +970,6 @@ static struct ref_lock *lock_ref_sha1_basic(struct files_ref_store *refs, return lock; } -/* - * 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)); -} - -/* - * 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. - */ -static 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; -} - -/* - * 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 - */ -static 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); -} - struct ref_to_prune { struct ref_to_prune *next; unsigned char sha1[20]; @@ -1717,62 +1135,6 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags) return 0; } -/* - * 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. - */ -static 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; -} - static int files_delete_refs(struct ref_store *ref_store, const char *msg, struct string_list *refnames, unsigned int flags) { -- cgit v1.2.1 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 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'refs/files-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); -- cgit v1.2.1 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 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'refs/files-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; } -- cgit v1.2.1 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 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'refs/files-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; -- cgit v1.2.1 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 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'refs/files-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; } -- cgit v1.2.1 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 ++ 1 file changed, 2 insertions(+) (limited to 'refs/files-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; -- cgit v1.2.1 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 +++++++++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 16 deletions(-) (limited to 'refs/files-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); -- cgit v1.2.1 From 8ec617c80cc37dcd66821f89678fc8797af96f10 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Thu, 17 Aug 2017 17:12:50 +0200 Subject: files-backend: cheapen refname_available check when locking refs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When locking references in preparation for updating them, we need to check that none of the newly added references D/F conflict with existing references (e.g., we don't allow `refs/foo` to be added if `refs/foo/bar` already exists, or vice versa). Prior to 524a9fdb51 (refs_verify_refname_available(): use function in more places, 2017-04-16), conflicts with existing loose references were checked by looking directly in the filesystem, and then conflicts with existing packed references were checked by running `verify_refname_available_dir()` against the packed-refs cache. But that commit changed the final check to call `refs_verify_refname_available()` against the *whole* files ref-store, including both loose and packed references, with the following comment: > This means that those callsites now check for conflicts with all > references rather than just packed refs, but the performance cost > shouldn't be significant (and will be regained later). That comment turned out to be too sanguine. User s@kazlauskas.me reported that fetches involving a very large number of references in neighboring directories were slowed down by that change. The problem is that when fetching, each reference is updated individually, within its own reference transaction. This is done because some reference updates might succeed even though others fail. But every time a reference update transaction is finished, `clear_loose_ref_cache()` is called. So when it is time to update the next reference, part of the loose ref cache has to be repopulated for the `refs_verify_refname_available()` call. If the references are all in neighboring directories, then the cost of repopulating the reference cache increases with the number of references, resulting in O(N²) effort. The comment above also claims that the performance cost "will be regained later". The idea was that once the packed-refs were finished being split out into a separate ref-store, we could limit the `refs_verify_refname_available()` call to the packed references again. That is what we do now. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'refs/files-backend.c') diff --git a/refs/files-backend.c b/refs/files-backend.c index e9b95592b6..f2a420c611 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -631,11 +631,11 @@ retry: /* * If the ref did not exist and we are creating it, - * make sure there is no existing ref that conflicts - * with refname: + * make sure there is no existing packed ref that + * conflicts with refname: */ if (refs_verify_refname_available( - &refs->base, refname, + refs->packed_ref_store, refname, extras, skip, err)) goto error_return; } @@ -938,7 +938,7 @@ static struct ref_lock *lock_ref_sha1_basic(struct files_ref_store *refs, * our refname. */ if (is_null_oid(&lock->old_oid) && - refs_verify_refname_available(&refs->base, refname, + refs_verify_refname_available(refs->packed_ref_store, refname, extras, skip, err)) { last_errno = ENOTDIR; goto error_return; -- cgit v1.2.1