Re: [PATCH v3 19/23] refs-be-files.c: add a backend method structure with transaction functions

2014-08-26 Thread Junio C Hamano 
Ronnie Sahlberg sahlb...@google.com writes:

 diff --git a/refs-be-files.c b/refs-be-files.c
 index e58a7e1..27eafd0 100644
 --- a/refs-be-files.c
 +++ b/refs-be-files.c
 ...
 +struct ref_be refs_files = {
 + files_transaction_begin,
 + files_transaction_update_sha1,
 + files_transaction_create_sha1,
 + files_transaction_delete_sha1,
 + files_transaction_update_reflog,
 + files_transaction_commit,
 + files_transaction_free,
 +};
 +
 +struct ref_be *refs = refs_files;

 diff --git a/refs.c b/refs.c
 index 6b434ad..b8c942f 100644
 --- a/refs.c
 +++ b/refs.c
 ...
 +void transaction_free(struct ref_transaction *transaction)
 +{
 + return refs-transaction_free(transaction);
 +}
 diff --git a/refs.h b/refs.h
 index a14fc5d..4b669f5 100644
 --- a/refs.h
 +++ b/refs.h
 ...
 +struct ref_be {
 + transaction_begin_fn transaction_begin;
 + transaction_update_sha1_fn transaction_update_sha1;
 + transaction_create_sha1_fn transaction_create_sha1;
 + transaction_delete_sha1_fn transaction_delete_sha1;
 + transaction_update_reflog_fn transaction_update_reflog;
 + transaction_commit_fn transaction_commit;
 + transaction_free_fn transaction_free;
 +};
 +
 +extern struct ref_be *refs;
 +
  #endif /* REFS_H */

The overall structure is certainly nice, but this means you only can
LINK with one backend.  Is that what we really want?

I would have expected something like this:

  * In refs.c, there is a static struct ref_be *the_refs_backend
that points at the chosen singleton backend;

  * Upon start-up, set_refs_backend() function that is exported from
refs.c can be used to set the_refs_backend;

  * Each refs-be-frotz.c will export struct ref_be refs_frotz (or
perhaps struct refs_be refs_be_frotz) to the outside world, so
that the start-up code can call set_refs_backend() with it.

  * It is probably sensible to keep the_refs_backend default to
refs_be_files.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 19/23] refs-be-files.c: add a backend method structure with transaction functions

2014-08-26 Thread Ronnie Sahlberg 
On Tue, Aug 26, 2014 at 2:38 PM, Junio C Hamano gits...@pobox.com wrote:
 Ronnie Sahlberg sahlb...@google.com writes:

 diff --git a/refs-be-files.c b/refs-be-files.c
 index e58a7e1..27eafd0 100644
 --- a/refs-be-files.c
 +++ b/refs-be-files.c
 ...
 +struct ref_be refs_files = {
 + files_transaction_begin,
 + files_transaction_update_sha1,
 + files_transaction_create_sha1,
 + files_transaction_delete_sha1,
 + files_transaction_update_reflog,
 + files_transaction_commit,
 + files_transaction_free,
 +};
 +
 +struct ref_be *refs = refs_files;

 diff --git a/refs.c b/refs.c
 index 6b434ad..b8c942f 100644
 --- a/refs.c
 +++ b/refs.c
 ...
 +void transaction_free(struct ref_transaction *transaction)
 +{
 + return refs-transaction_free(transaction);
 +}
 diff --git a/refs.h b/refs.h
 index a14fc5d..4b669f5 100644
 --- a/refs.h
 +++ b/refs.h
 ...
 +struct ref_be {
 + transaction_begin_fn transaction_begin;
 + transaction_update_sha1_fn transaction_update_sha1;
 + transaction_create_sha1_fn transaction_create_sha1;
 + transaction_delete_sha1_fn transaction_delete_sha1;
 + transaction_update_reflog_fn transaction_update_reflog;
 + transaction_commit_fn transaction_commit;
 + transaction_free_fn transaction_free;
 +};
 +
 +extern struct ref_be *refs;
 +
  #endif /* REFS_H */

 The overall structure is certainly nice, but this means you only can
 LINK with one backend.  Is that what we really want?

 I would have expected something like this:

   * In refs.c, there is a static struct ref_be *the_refs_backend
 that points at the chosen singleton backend;

Done.
It is also initialized to default to the files backend :
refs.c:

  /* We always have a files backend and it is the default */
  struct ref_be *the_refs_backend = refs_be_files;


This does make refs_be_files and later refs_be_db public symbols instead of
the singleton. But we probably want/need these structures to be public anyway
if we at some stage want to be able to switch between backends at runtime.
refs.h:

   extern struct ref_be refs_be_files;
   void set_refs_backend(struct ref_be *ref_be);

Thus allowing us to
   set_refs_backend(refs_be_files) to switch back to the files backend.





   * Upon start-up, set_refs_backend() function that is exported from
 refs.c can be used to set the_refs_backend;

   * Each refs-be-frotz.c will export struct ref_be refs_frotz (or
 perhaps struct refs_be refs_be_frotz) to the outside world, so
 that the start-up code can call set_refs_backend() with it.

Yepp.
refs-be-db.c: does this.


   * It is probably sensible to keep the_refs_backend default to
 refs_be_files.


Yepp.

https://github.com/rsahlberg/git/tree/backend-struct-db
https://github.com/rsahlberg/git/tree/backend-struct-db-2 (adds a db
backend and daemon)


Thanks. Good suggestions.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 19/23] refs-be-files.c: add a backend method structure with transaction functions

2014-08-19 Thread Ronnie Sahlberg 
Add a ref structure for backend methods. Start by adding method pointers
for the transaction functions.

Rename the existing transaction functions to files_* and make them static.
Add new transaction functions that just pass through to the appropriate
methods for the backend.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-be-files.c | 68 ++---
 refs.c  | 55 ++
 refs.h  | 35 +
 3 files changed, 131 insertions(+), 27 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index e58a7e1..27eafd0 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -2777,12 +2777,12 @@ struct ref_transaction {
enum ref_transaction_state state;
 };
 
-struct ref_transaction *transaction_begin(struct strbuf *err)
+static struct ref_transaction *files_transaction_begin(struct strbuf *err)
 {
return xcalloc(1, sizeof(struct ref_transaction));
 }
 
-void transaction_free(struct ref_transaction *transaction)
+static void files_transaction_free(struct ref_transaction *transaction)
 {
int i;
 
@@ -2812,13 +2812,13 @@ static struct ref_update *add_update(struct 
ref_transaction *transaction,
return update;
 }
 
-int transaction_update_reflog(struct ref_transaction *transaction,
- const char *refname,
- const unsigned char *new_sha1,
- const unsigned char *old_sha1,
- struct reflog_committer_info *ci,
- const char *msg, int flags,
- struct strbuf *err)
+static int files_transaction_update_reflog(struct ref_transaction *transaction,
+  const char *refname,
+  const unsigned char *new_sha1,
+  const unsigned char *old_sha1,
+  struct reflog_committer_info *ci,
+  const char *msg, int flags,
+  struct strbuf *err)
 {
struct ref_update *update;
int i;
@@ -2865,12 +2865,13 @@ int transaction_update_reflog(struct ref_transaction 
*transaction,
return 0;
 }
 
-int transaction_update_sha1(struct ref_transaction *transaction,
-   const char *refname,
-   const unsigned char *new_sha1,
-   const unsigned char *old_sha1,
-   int flags, int have_old, const char *msg,
-   struct strbuf *err)
+static int files_transaction_update_sha1(struct ref_transaction *transaction,
+const char *refname,
+const unsigned char *new_sha1,
+const unsigned char *old_sha1,
+int flags, int have_old,
+const char *msg,
+struct strbuf *err)
 {
struct ref_update *update;
 
@@ -2897,11 +2898,11 @@ int transaction_update_sha1(struct ref_transaction 
*transaction,
return 0;
 }
 
-int transaction_create_sha1(struct ref_transaction *transaction,
-   const char *refname,
-   const unsigned char *new_sha1,
-   int flags, const char *msg,
-   struct strbuf *err)
+static int files_transaction_create_sha1(struct ref_transaction *transaction,
+const char *refname,
+const unsigned char *new_sha1,
+int flags, const char *msg,
+struct strbuf *err)
 {
if (transaction-state != REF_TRANSACTION_OPEN)
die(BUG: create called for transaction that is not open);
@@ -2913,11 +2914,12 @@ int transaction_create_sha1(struct ref_transaction 
*transaction,
   null_sha1, flags, 1, msg, err);
 }
 
-int transaction_delete_sha1(struct ref_transaction *transaction,
-   const char *refname,
-   const unsigned char *old_sha1,
-   int flags, int have_old, const char *msg,
-   struct strbuf *err)
+static int files_transaction_delete_sha1(struct ref_transaction *transaction,
+const char *refname,
+const unsigned char *old_sha1,
+int flags, int have_old,
+const char *msg,
+struct strbuf *err)
 {
if (transaction-state !=