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

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

 



On Thu, Mar 16, 2017 at 05:03:03PM +0000, Ramsay Jones wrote:

> > 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! ;-)

I think there's some double-slash normalization (though not consistent
or thorough). So yeah, I'm not really sure the normalization is of great
value. I am surprised that there isn't a call to "convert_slashes()" in
there on Windows, as it seems to be sprinkled other places (like
prefix_filename()).

<shrug> If nobody's complaining, I'm happy to leave it as-is for now,
and if somebody comes up with a case where git_path normalization makes
a difference, we can look at switching it.

-Peff



[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]