From: Michael Haggerty <mhagger@alum.mit.edu>
To: David Turner <dturner@twopensource.com>, git@vger.kernel.org
Subject: Re: [PATCH 08/16] refs: add methods to init refs backend and db
Date: Wed, 23 Dec 2015 06:33:28 +0100 [thread overview]
Message-ID: <567A3228.2060407@alum.mit.edu> (raw)
In-Reply-To: <1449102921-7707-9-git-send-email-dturner@twopensource.com>
On 12/03/2015 01:35 AM, David Turner wrote:
> Alternate refs backends might not need the refs/heads directory and so
> on, so we make ref db initialization part of the backend. We also
> might need to initialize ref backends themselves, so we'll add a
> method for that as well.
>
> Signed-off-by: David Turner <dturner@twopensource.com>
> ---
> builtin/init-db.c | 14 ++++----------
> refs.c | 8 +++++++-
> refs.h | 4 +++-
> refs/files-backend.c | 23 +++++++++++++++++++++++
> refs/refs-internal.h | 4 ++++
> 5 files changed, 41 insertions(+), 12 deletions(-)
>
> [...]
> diff --git a/refs.c b/refs.c
> index 9a2fed7..bdeb276 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -22,13 +22,14 @@ struct ref_be *refs_backends = &refs_be_files;
> /*
> * This function is used to switch to an alternate backend.
> */
> -int set_refs_backend(const char *name)
> +int set_refs_backend(const char *name, void *data)
The purpose of the data argument is rather mysterious at this point,
especially since set_refs_backend() still doesn't have any callers. I
hope that will become clearer later in the series. Nevertheless, it
would be nice for its use to be described in the docstring (which should
preferably be moved to the header file).
> [...]
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index e769242..6600c02 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -3313,6 +3313,11 @@ static int ref_present(const char *refname,
> return string_list_has_string(affected_refnames, refname);
> }
>
> +void files_init_backend(void *data)
> +{
> + /* do nothing */
> +}
> +
> static int files_initial_transaction_commit(struct ref_transaction *transaction,
> struct strbuf *err)
> {
> @@ -3534,9 +3539,27 @@ static int files_reflog_expire(const char *refname, const unsigned char *sha1,
> return -1;
> }
>
> +static int files_init_db(struct strbuf *err, int shared)
> +{
> + /*
> + * Create .git/refs/{heads,tags}
> + */
> + safe_create_dir(git_path("refs"), 1);
> + safe_create_dir(git_path("refs/heads"), 1);
> + safe_create_dir(git_path("refs/tags"), 1);
> + if (shared) {
> + adjust_shared_perm(git_path("refs"));
> + adjust_shared_perm(git_path("refs/heads"));
> + adjust_shared_perm(git_path("refs/tags"));
> + }
> + return 0;
> +}
> +
> struct ref_be refs_be_files = {
> NULL,
> "files",
> + files_init_backend,
> + files_init_db,
> files_transaction_commit,
> files_initial_transaction_commit,
>
> diff --git a/refs/refs-internal.h b/refs/refs-internal.h
> index 478ad54..85a0b91 100644
> --- a/refs/refs-internal.h
> +++ b/refs/refs-internal.h
> @@ -208,6 +208,8 @@ const char *find_descendant_ref(const char *dirname,
> int rename_ref_available(const char *oldname, const char *newname);
>
> /* refs backends */
> +typedef void ref_backend_init_fn(void *data);
> +typedef int ref_backend_init_db_fn(struct strbuf *err, int shared);
> typedef int ref_transaction_commit_fn(struct ref_transaction *transaction,
> struct strbuf *err);
>
> @@ -267,6 +269,8 @@ typedef int for_each_replace_ref_fn(each_ref_fn fn, void *cb_data);
> struct ref_be {
> struct ref_be *next;
> const char *name;
> + ref_backend_init_fn *init_backend;
> + ref_backend_init_db_fn *init_db;
> ref_transaction_commit_fn *transaction_commit;
> ref_transaction_commit_fn *initial_transaction_commit;
>
>
Your naming seems inconsistent here. I would have expected a
"files_init_backend()" function to satisfy the typedef
"ref_backend_init_backend_fn" or "ref_init_backend_fn", not
"ref_backend_init_fn". Or, conversely, for the function implementing
"ref_backend_init_fn" to be called "files_init()".
More generally, it would be nice to have a consistent naming pattern
between (1) the name of the typedef, (2) the name of the member in
"struct ref_be", (3) the names of concrete, backend-specific
implementations of the functions.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
next prev parent reply other threads:[~2015-12-23 5:40 UTC|newest]
Thread overview: 73+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-03 0:35 [PATCH 00/16] LMDB refs backend atop pre-vtable David Turner
2015-12-03 0:35 ` [PATCH 01/16] refs: add a backend method structure with transaction functions David Turner
2015-12-05 0:07 ` Junio C Hamano
2015-12-03 0:35 ` [PATCH 02/16] refs: add methods for misc ref operations David Turner
2015-12-11 23:33 ` Junio C Hamano
2015-12-11 23:49 ` David Turner
2015-12-11 23:39 ` Junio C Hamano
2015-12-11 23:49 ` David Turner
2015-12-12 0:23 ` Junio C Hamano
2015-12-12 0:48 ` David Turner
2015-12-18 4:06 ` Howard Chu
2015-12-03 0:35 ` [PATCH 03/16] refs: add methods for the ref iterators David Turner
2016-01-03 0:06 ` David Aguilar
2016-01-04 19:01 ` Junio C Hamano
2016-01-05 13:43 ` Michael Haggerty
2016-01-05 18:56 ` Junio C Hamano
2016-01-04 19:12 ` Ronnie Sahlberg
2016-01-04 20:26 ` Junio C Hamano
2016-01-05 1:17 ` Jeff King
2016-01-05 3:29 ` Junio C Hamano
2015-12-03 0:35 ` [PATCH 04/16] refs: add do_for_each_per_worktree_ref David Turner
2015-12-11 23:52 ` Junio C Hamano
2015-12-12 0:01 ` David Turner
2015-12-03 0:35 ` [PATCH 05/16] refs: add methods for reflog David Turner
2015-12-03 0:35 ` [PATCH 06/16] refs: add method for initial ref transaction commit David Turner
2015-12-03 0:35 ` [PATCH 07/16] refs: add method for delete_refs David Turner
2015-12-03 0:35 ` [PATCH 08/16] refs: add methods to init refs backend and db David Turner
2015-12-23 5:33 ` Michael Haggerty [this message]
2015-12-23 6:54 ` David Turner
2015-12-03 0:35 ` [PATCH 09/16] refs: add method to rename refs David Turner
2015-12-03 0:35 ` [PATCH 10/16] refs: make lock generic David Turner
2015-12-03 0:35 ` [PATCH 11/16] refs: move duplicate check to common code David Turner
2015-12-23 6:27 ` Michael Haggerty
2016-01-05 16:42 ` David Turner
2015-12-03 0:35 ` [PATCH 12/16] refs: always handle non-normal refs in files backend David Turner
2015-12-23 8:02 ` Michael Haggerty
2016-01-06 0:13 ` David Turner
2016-01-06 23:41 ` [PATCH/RFC v2 1/3] refs: allow log-only updates David Turner
2016-01-06 23:41 ` [PATCH/RFC v2 2/3] refs: resolve symbolic refs first David Turner
2016-01-06 23:41 ` [PATCH/RFC v2 3/3] refs: always handle non-normal refs in files backend David Turner
2016-01-08 12:52 ` David Turner
2016-01-06 23:42 ` [PATCH 12/16] " David Turner
2015-12-03 0:35 ` [PATCH 13/16] init: allow alternate backends to be set for new repos David Turner
2015-12-05 0:07 ` Junio C Hamano
2015-12-05 6:30 ` Duy Nguyen
2015-12-05 7:44 ` Jeff King
2015-12-08 0:38 ` David Turner
2015-12-23 9:52 ` Michael Haggerty
2015-12-23 20:01 ` Jeff King
2015-12-10 18:02 ` Jeff King
2015-12-10 19:36 ` David Turner
2015-12-23 11:30 ` [PATCH] clone: use child_process for recursive checkouts Michael Haggerty
2016-01-06 23:41 ` David Turner
2015-12-23 13:34 ` [PATCH 13/16] init: allow alternate backends to be set for new repos Michael Haggerty
2016-01-05 17:26 ` David Turner
2016-01-05 18:03 ` Junio C Hamano
2016-01-05 18:24 ` David Turner
2016-01-06 12:02 ` Michael Haggerty
2016-01-06 12:52 ` Duy Nguyen
2016-01-07 3:31 ` Shawn Pearce
2015-12-03 0:35 ` [PATCH 14/16] refs: allow ref backend to be set for clone David Turner
2015-12-23 13:51 ` Michael Haggerty
2015-12-23 20:23 ` Eric Sunshine
2015-12-03 0:35 ` [PATCH 15/16] refs: add LMDB refs backend David Turner
2015-12-05 0:08 ` Junio C Hamano
2015-12-05 0:25 ` David Turner
2015-12-17 1:00 ` Jonathan Nieder
2015-12-17 2:31 ` David Turner
2015-12-17 20:49 ` Jonathan Nieder
2015-12-23 14:32 ` Michael Haggerty
2016-01-08 16:05 ` David Turner
2015-12-03 0:35 ` [PATCH 16/16] refs: tests for lmdb backend David Turner
2015-12-22 23:56 ` [PATCH 00/16] LMDB refs backend atop pre-vtable David Turner
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=567A3228.2060407@alum.mit.edu \
--to=mhagger@alum.mit.edu \
--cc=dturner@twopensource.com \
--cc=git@vger.kernel.org \
/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).