Re: [PATCH 22/27] sha1_file: allow sha1_file_name to handle arbitrary repositories

2018-02-23 Thread Stefan Beller
On Wed, Feb 21, 2018 at 4:44 PM, Jonathan Tan  wrote:
> On Tue, 20 Feb 2018 17:54:25 -0800
> Stefan Beller  wrote:
>
>> Signed-off-by: Stefan Beller 
>> Signed-off-by: Jonathan Nieder 
>
> Reviewed-by: Jonathan Tan 
>
>> -void sha1_file_name_the_repository(struct strbuf *buf, const unsigned char 
>> *sha1)
>> +void sha1_file_name(struct repository *r, struct strbuf *buf, const 
>> unsigned char *sha1)
>>  {
>> - strbuf_addstr(buf, get_object_directory());
>> + strbuf_addstr(buf, r->objects.objectdir);
>>   strbuf_addch(buf, '/');
>>   fill_sha1_path(buf, sha1);
>>  }
>
> In the future, we should probably have:
>  - a function to get the object store out of a repo (so that it can
>lazily initialize the object store struct if necessary)
>  - when the object store is obtained, its objectdir field is guaranteed
>to be populated
>  - sha1_file_name should take the object store struct, not the repo
>struct

We had that in v2, but I reverted it as the consensus seemed that we prefer
the_repository to be passed around.

>
> but this is outside the scope of this patch.


Re: [PATCH 22/27] sha1_file: allow sha1_file_name to handle arbitrary repositories

2018-02-21 Thread Jonathan Tan
On Tue, 20 Feb 2018 17:54:25 -0800
Stefan Beller  wrote:

> Signed-off-by: Stefan Beller 
> Signed-off-by: Jonathan Nieder 

Reviewed-by: Jonathan Tan 

> -void sha1_file_name_the_repository(struct strbuf *buf, const unsigned char 
> *sha1)
> +void sha1_file_name(struct repository *r, struct strbuf *buf, const unsigned 
> char *sha1)
>  {
> - strbuf_addstr(buf, get_object_directory());
> + strbuf_addstr(buf, r->objects.objectdir);
>   strbuf_addch(buf, '/');
>   fill_sha1_path(buf, sha1);
>  }

In the future, we should probably have:
 - a function to get the object store out of a repo (so that it can
   lazily initialize the object store struct if necessary)
 - when the object store is obtained, its objectdir field is guaranteed
   to be populated
 - sha1_file_name should take the object store struct, not the repo
   struct

but this is outside the scope of this patch.