Web lists-archives.com

Re: [PATCH v2 2/5] sha1_file.c: make pack-name helper globally accessible





On 16/03/17 14:31, Jeff King wrote:
> On Thu, Mar 16, 2017 at 10:27:06AM -0400, Jeff King wrote:
> 
>> -/*
>> - * Return the name of the pack or index file with the specified sha1
>> - * in its filename.  *base and *name are scratch space that must be
>> - * provided by the caller.  which should be "pack" or "idx".
>> - */
>> -static char *sha1_get_pack_name(const unsigned char *sha1,
>> -				struct strbuf *buf,
>> -				const char *which)
>> + char *odb_pack_name(struct strbuf *buf,
>> +		     const unsigned char *sha1,
>> +		     const char *ext)
>>  {
>>  	strbuf_reset(buf);
>>  	strbuf_addf(buf, "%s/pack/pack-%s.%s", get_object_directory(),
>> -		    sha1_to_hex(sha1), which);
>> +		    sha1_to_hex(sha1), ext);
>>  	return buf->buf;
>>  }
> 
> Incidentally, this entire function could be implemented as:
> 
>   return git_path_buf(buf, "objects/pack/pack-%s.%s",
>                       sha1_to_hex(sha1), ext);
> 
> as the git_path() functions are smart enough to replace "objects/" with
> the true object directory when necessary. I don't know if people find
> that more or less readable. Since it's buried in a helper function, I
> doubt it matters much either way. The git_path functions do also do some
> path normalization, which might be of value.

Hmm, I don't have strong feelings either way.

However, I note that the only normalization going on (that I can see)
is to remove .//* from the beginning of the resulting string. I don't
know why, but I guess it is to cater to people using the various
GIT_ environment variables doing things like:

   $ GIT_OBJECT_DIRECTORY=./my-objects git ....

It has always puzzled me slightly, why the git_path functions do this
normalization, but (for example) setup_git_env(), git_path_from_env(),
get_common_dir(), ... don't! ;-)

ATB,
Ramsay Jones