From: Brandon Williams <bmwill@google.com>
To: Michael Haggerty <mhagger@alum.mit.edu>
Cc: "Junio C Hamano" <gitster@pobox.com>,
"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
"Stefan Beller" <sbeller@google.com>, "Jeff King" <peff@peff.net>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
git@vger.kernel.org
Subject: Re: [PATCH 09/10] packed-backend: rip out some now-unused code
Date: Tue, 29 Aug 2017 11:24:28 -0700 [thread overview]
Message-ID: <20170829182428.GB131745@google.com> (raw)
In-Reply-To: <824ad441f8531f1bbcc8800a14fe71466721066d.1503993268.git.mhagger@alum.mit.edu>
On 08/29, Michael Haggerty wrote:
> Now the outside world interacts with the packed ref store only via the
> generic refs API plus a few lock-related functions. This allows us to
> delete some functions that are no longer used, thereby completing the
> encapsulation of the packed ref store.
Love seeing patches which only remove old code :)
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
> refs/packed-backend.c | 193 --------------------------------------------------
> refs/packed-backend.h | 8 ---
> 2 files changed, 201 deletions(-)
>
> diff --git a/refs/packed-backend.c b/refs/packed-backend.c
> index 83a088118f..90f44c1fbb 100644
> --- a/refs/packed-backend.c
> +++ b/refs/packed-backend.c
> @@ -91,19 +91,6 @@ struct ref_store *packed_ref_store_create(const char *path,
> return ref_store;
> }
>
> -/*
> - * 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);
> -}
> -
> /*
> * 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
> @@ -321,40 +308,6 @@ 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 packed_refs_lock()). To actually write the packed-refs file,
> - * call commit_packed_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;
> -
> - 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.
> @@ -592,152 +545,6 @@ int packed_refs_is_locked(struct ref_store *ref_store)
> 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
> - * packed_refs_lock()). 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, 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;
> - 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");
> -
> - /*
> - * 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);
> - 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));
> - goto error;
> - }
> -
> - if (fprintf(out, "%s", PACKED_REFS_HEADER) < 0) {
> - strbuf_addf(err, "error writing to %s: %s",
> - get_tempfile_path(&refs->tempfile), 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);
> -
> - if (write_packed_entry(out, iter->refname, iter->oid->hash,
> - peel_error ? NULL : peeled.hash)) {
> - strbuf_addf(err, "error writing to %s: %s",
> - get_tempfile_path(&refs->tempfile),
> - strerror(errno));
> - ref_iterator_abort(iter);
> - goto error;
> - }
> - }
> -
> - if (ok != ITER_DONE) {
> - strbuf_addf(err, "unable to rewrite packed-refs file: "
> - "error iterating over old contents");
> - goto error;
> - }
> -
> - if (rename_tempfile(&refs->tempfile, packed_refs_path)) {
> - strbuf_addf(err, "error replacing %s: %s",
> - refs->path, strerror(errno));
> - goto out;
> - }
> -
> - ret = 0;
> - goto out;
> -
> -error:
> - delete_tempfile(&refs->tempfile);
> -
> -out:
> - free(packed_refs_path);
> - return ret;
> -}
> -
> -/*
> - * 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 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.
> - */
> -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 needs_repacking = 0, removed = 0;
> -
> - 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)) {
> - needs_repacking = 1;
> - break;
> - }
> - }
> -
> - /* Avoid locking if we have nothing to do */
> - if (!needs_repacking)
> - return 0; /* no refname exists in packed refs */
> -
> - 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.
> - */
> - clear_packed_ref_cache(refs);
> - return 0;
> - }
> -
> - /* Write what remains */
> - return commit_packed_refs(&refs->base, err);
> -}
> -
> static int packed_init_db(struct ref_store *ref_store, struct strbuf *err)
> {
> /* Nothing to do. */
> diff --git a/refs/packed-backend.h b/refs/packed-backend.h
> index 7af2897757..61687e408a 100644
> --- a/refs/packed-backend.h
> +++ b/refs/packed-backend.h
> @@ -23,12 +23,4 @@ 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);
> -
> -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);
> -
> #endif /* REFS_PACKED_BACKEND_H */
> --
> 2.14.1
>
--
Brandon Williams
next prev parent reply other threads:[~2017-08-29 18:24 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-29 8:20 [PATCH 00/10] Implement transactions for the packed ref store Michael Haggerty
2017-08-29 8:20 ` [PATCH 01/10] packed-backend: don't adjust the reference count on lock/unlock Michael Haggerty
2017-09-08 6:52 ` Jeff King
2017-09-08 10:02 ` Michael Haggerty
2017-08-29 8:20 ` [PATCH 02/10] struct ref_transaction: add a place for backends to store data Michael Haggerty
2017-09-08 7:02 ` Jeff King
2017-09-08 8:19 ` Michael Haggerty
2017-09-08 8:33 ` Jeff King
2017-08-29 8:20 ` [PATCH 03/10] packed_ref_store: implement reference transactions Michael Haggerty
2017-08-29 8:20 ` [PATCH 04/10] packed_delete_refs(): implement method Michael Haggerty
2017-08-29 18:07 ` Brandon Williams
2017-08-30 3:00 ` Michael Haggerty
2017-08-29 8:20 ` [PATCH 05/10] files_pack_refs(): use a reference transaction to write packed refs Michael Haggerty
2017-08-29 8:20 ` [PATCH 06/10] files_initial_transaction_commit(): use a transaction for " Michael Haggerty
2017-09-08 7:27 ` Jeff King
2017-09-08 10:04 ` Michael Haggerty
2017-08-29 8:20 ` [PATCH 07/10] t1404: demonstrate two problems with reference transactions Michael Haggerty
2017-08-30 17:21 ` Stefan Beller
2017-08-31 3:42 ` Michael Haggerty
2017-09-08 4:44 ` Junio C Hamano
2017-09-08 7:45 ` Jeff King
2017-09-08 10:06 ` Michael Haggerty
2017-08-29 8:20 ` [PATCH 08/10] files_ref_store: use a transaction to update packed refs Michael Haggerty
2017-09-08 7:38 ` Jeff King
2017-09-08 12:44 ` Michael Haggerty
2017-09-08 12:57 ` Jeff King
2017-08-29 8:20 ` [PATCH 09/10] packed-backend: rip out some now-unused code Michael Haggerty
2017-08-29 18:24 ` Brandon Williams [this message]
2017-08-29 8:20 ` [PATCH 10/10] files_transaction_finish(): delete reflogs before references Michael Haggerty
2017-08-29 18:30 ` [PATCH 00/10] Implement transactions for the packed ref store Brandon Williams
2017-09-08 7:42 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170829182428.GB131745@google.com \
--to=bmwill@google.com \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=mhagger@alum.mit.edu \
--cc=pclouds@gmail.com \
--cc=peff@peff.net \
--cc=sbeller@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).