Re: [PATCH] Only use ETC_GITCONFIG=$(prefix)/etc/gitconfig ifeq ($(prefix),$(HOME))

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

 



Junio C Hamano wrote:
> Josh Triplett <josh@xxxxxxxxxxxxxxx> writes:
> 
>> Junio C Hamano wrote:
>>> Is this really necessary?
>>>
>>> I personally think distro people (or anybody who configures git
>>> for system-wide deployment for that matter) already has Makefile
>>> wrapper (a la debian/rules) to take care of this and other
>>> issues.
>> So did I.  Then I noticed (while stracing git to diagnose the issue with
>> git-add searching the whole working copy for .gitignore files) that the
>> Debian-packaged git looked for /usr/etc/gitconfig.  See
>> <http://bugs.debian.org/420675>.  Apparently few enough people use
>> /etc/gitconfig that nobody noticed and reported that it didn't work. :)
>>
>> Why not make the defaults more resistant to broken configuration?
> 
> Hmm.
> 
> It's tempting to leave this as is to gauge which distribution
> has more competent package maintainer and which ones have sloppy
> ones ;-).
>
> Sympathetic, but still not entirely convinced.

commit cc58fc0684396c5298b21c97f00a568e46224258
Merge: 8a13bec... 8565d2d...
Author: Junio C Hamano <junkio@xxxxxxx>
Date:   Sat Feb 24 01:43:28 2007 -0800

    Merge branch 'js/etc-config'

    * js/etc-config:
      Make tests independent of global config files
      config: read system-wide defaults from /etc/gitconfig


Fairly recent feature, only a few Debian package releases have come out since
then, and I strongly suspect that few people use /etc/gitconfig.  I didn't
even know it existed until the strace, and none of the other Git users I know
use it.

It seems harder to make the mistake *now*, while looking at the Makefile to
see what variables need setting, but it seems easy to have not noticed the
necessary change for existing packaging.

Furthermore, people often forget the same thing with autofoo-based programs;
--prefix=/usr or --prefix=/usr/local works for many programs (since a small
fraction of autofoo-using programs have systemwide configuration, compared to
all autoconf programs), and it seems easy enough to forget about
--sysconfdir=/etc.  Most of the time, however, the configuration file gets
used more often, and often a default version exists that would get installed
to the wrong place, so this mistake gets more readily noticed.

git does not install a default systemwide configuration file, and it does not
attempt to create the target etc directory, making it easy to not notice the
mistake.  In order to notice, you would have to re-read the Makefile in
detail, read the git-config manpage in detail (it shows up in no other
documentation), read the source code, or notice a line in an strace.  Or you
would have to get a bugreport from someone using /etc/gitconfig, but again,
few do.

Finally, more people install Git than just experienced and observant package
maintainers.  Random sysadmins install Git too (I know some of them), and
making life easier for them seems like a good idea.

Overall, this seems like an easy change, with good benefits, and no drawbacks
that I can see.


That said, I can see another way to make it slightly more forward-looking:
create a sysconfdir variable in Makefile, like the existing bindir and
gitexecdir, and apply the appropriate logic to it instead.  Then, any future
configuration files can take advantage of the same logic.

Furthermore, I think my original logic may have a flaw in it: someone
installing with prefix=$HOME/local (which I do for other software, though not
Git because I use the Debian package) would likely expect $(prefix)/etc as
well.  For /usr/local, it can go either way; some people apparently do expect
/usr/local/etc and some people expect /etc.  Overall, someone installing in a
strange nonstandard location probably wants $(prefix)/etc.  So, I think the
right logic for sysconfdir looks like:

ifeq ($(prefix),/usr)
sysconfdir=/etc
else
sysconfdir=$(prefix)/etc
endif

Personally, I wish automake and autoconf had exactly the same logic; it just
makes sense.

- Josh Triplett

Attachment: signature.asc
Description: OpenPGP digital signature


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