git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
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

  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).