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

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

[Josh Triplett]

[-- Attachment #1: Type: text/plain, Size: 1017 bytes --]

ETC_GITCONFIG defaults to $(prefix)/etc/gitconfig, so if you just set
prefix=/usr or prefix=/usr/local, you end up with a git that looks in
/usr/etc/gitconfig or /usr/local/etc/gitconfig.  That seems rather suboptimal.

Use ifeq in the Makefile to set ETC_GITCONFIG=/etc/gitconfig unless the prefix
points to $HOME .

The builder can always override ETC_GITCONFIG, so this just makes the default
saner.

Signed-off-by: Josh Triplett <josh@freedesktop.org>
---
 Makefile |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 65bd2db..6e1ae95 100644
--- a/Makefile
+++ b/Makefile
@@ -141,7 +141,11 @@ prefix = $(HOME)
 bindir = $(prefix)/bin
 gitexecdir = $(bindir)
 template_dir = $(prefix)/share/git-core/templates/
-ETC_GITCONFIG = $(prefix)/etc/gitconfig
+ifeq ($(prefix),$(HOME))
+ETC_GITCONFIG = $(HOME)/etc/gitconfig
+else
+ETC_GITCONFIG = /etc/gitconfig
+endif
 # DESTDIR=
 
 # default configuration for gitweb
-- 
1.5.1.1



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

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

[Junio C Hamano]

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.

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

[Josh Triplett]

[-- Attachment #1: Type: text/plain, Size: 701 bytes --]

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?

- Josh Triplett


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

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

[Junio C Hamano]

Josh Triplett <josh@freedesktop.org> 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.

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

[Josh Triplett]

[-- Attachment #1: Type: text/plain, Size: 4064 bytes --]

Junio C Hamano wrote:
> Josh Triplett <josh@freedesktop.org> 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@cox.net>
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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

[PATCH] Create a sysconfdir variable, and use it for ETC_GITCONFIG

[Josh Triplett]

[-- Attachment #1: Type: text/plain, Size: 1562 bytes --]

ETC_GITCONFIG defaults to $(prefix)/etc/gitconfig, so if you just set
prefix=/usr, you end up with a git that looks in /usr/etc/gitconfig, rather
than /etc/gitconfig as specified by the FHS.  Furthermore, setting
ETC_GITCONFIG does not fix the paths to any future system-wide configuration
files.

Factor out the path to the system-wide configuration directory into a variable
sysconfdir, normally set to $(prefix)/etc, but set to /etc when prefix=/usr .
This fixes the prefix=/usr problem for ETC_GITCONFIG, and allows centralized
configuration of any future system-wide configuration files without requiring
further action from package maintainers or other people building and
installing git.

Signed-off-by: Josh Triplett <josh@freedesktop.org>
---
 Makefile |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 65bd2db..60c41fd 100644
--- a/Makefile
+++ b/Makefile
@@ -141,7 +141,12 @@ prefix = $(HOME)
 bindir = $(prefix)/bin
 gitexecdir = $(bindir)
 template_dir = $(prefix)/share/git-core/templates/
-ETC_GITCONFIG = $(prefix)/etc/gitconfig
+ifeq ($(prefix),/usr)
+sysconfdir = /etc
+else
+sysconfdir = $(prefix)/etc
+endif
+ETC_GITCONFIG = $(sysconfdir)/gitconfig
 # DESTDIR=
 
 # default configuration for gitweb
@@ -160,7 +165,7 @@ GITWEB_FAVICON = git-favicon.png
 GITWEB_SITE_HEADER =
 GITWEB_SITE_FOOTER =
 
-export prefix bindir gitexecdir template_dir
+export prefix bindir gitexecdir template_dir sysconfdir
 
 CC = gcc
 AR = ar
-- 
1.5.1.1



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

Re: [PATCH] Create a sysconfdir variable, and use it for ETC_GITCONFIG

[Junio C Hamano]

I like this much better.  Let's have this in 'maint' then.

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

[Junio C Hamano]

Josh Triplett <josh@freedesktop.org> writes:

> ETC_GITCONFIG defaults to $(prefix)/etc/gitconfig, so if you just set
> prefix=/usr or prefix=/usr/local, you end up with a git that looks in
> /usr/etc/gitconfig or /usr/local/etc/gitconfig.  That seems rather suboptimal.
>
> Use ifeq in the Makefile to set ETC_GITCONFIG=/etc/gitconfig unless the prefix
> points to $HOME .

I personally have four installations of git under $HOME by
setting prefix to $HOME/git-{maint,master,next,pu}.  I would
rather not see this to break, as I suspect there are other
people who depend on this behaviour.

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

[Junio C Hamano]

Junio C Hamano <junkio@cox.net> writes:

> Josh Triplett <josh@freedesktop.org> writes:
>
>> ETC_GITCONFIG defaults to $(prefix)/etc/gitconfig, so if you just set
>> prefix=/usr or prefix=/usr/local, you end up with a git that looks in
>> /usr/etc/gitconfig or /usr/local/etc/gitconfig.  That seems rather suboptimal.
>>
>> Use ifeq in the Makefile to set ETC_GITCONFIG=/etc/gitconfig unless the prefix
>> points to $HOME .
>
> I personally have four installations of git under $HOME by
> setting prefix to $HOME/git-{maint,master,next,pu}.  I would
> rather not see this to break, as I suspect there are other
> people who depend on this behaviour.

Also what happens to people who use "prefix=/usr/local"?

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

[Sergio Callegari]

> Junio C Hamano <junkio <at> cox.net> writes:
> 
> > Josh Triplett <josh <at> freedesktop.org> writes:
> >
> >> ETC_GITCONFIG defaults to $(prefix)/etc/gitconfig, so if you just set
> >> prefix=/usr or prefix=/usr/local, you end up with a git that looks in
> >> /usr/etc/gitconfig or /usr/local/etc/gitconfig.  That seems rather
suboptimal.
> >>
> Also what happens to people who use "prefix=/usr/local"?
>  

Could it be better to set ETC_GITCONFIG=/etc/gitconfig /only/ if prefix is /usr?

These would possibly make all people who have git in /usr/local or in /opt/Git
or in /home happier...

If I am not wrong, /usr is the only prefix, where, for historical reasons
there is no /usr/etc but /etc is used.  Having /usr/local/etc does not seem that
bad. And /opt/Git/etc appears surely ok.

Sergio

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

[Shawn O. Pearce]

Junio C Hamano <junkio@cox.net> wrote:
> Josh Triplett <josh@freedesktop.org> writes:
> > Use ifeq in the Makefile to set ETC_GITCONFIG=/etc/gitconfig unless the prefix
> > points to $HOME .
> 
> I personally have four installations of git under $HOME by
> setting prefix to $HOME/git-{maint,master,next,pu}.  I would
> rather not see this to break, as I suspect there are other
> people who depend on this behaviour.

I second that.  :-)

$ cat config.mak
...
current_branch := \
	$(subst refs/heads/,,$(shell git-symbolic-ref HEAD 2>/dev/null))
ifeq ($(current_branch),)
	prefix := $(HOME)/sw/git-unknownbuild
else
	prefix := $(HOME)/sw/git-$(current_branch)
EOF

;-)

-- 
Shawn.