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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Aug 26, 2014 at 2:38 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Ronnie Sahlberg <sahlberg@xxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]