Re: [PATCH] Respect definition of prefix from autotools in ETC_GITCONFIG and ETC_GITATTRIBUTES

[Kacper Kornet <kornet@camk.edu.pl>]

On Thu, Apr 28, 2011 at 09:54:02AM -0700, Junio C Hamano wrote:
> Kacper Kornet <kornet@camk.edu.pl> writes:

> > Definitions of ETC_GITCONFIG and ETC_GITATTRIBUTES depend on value of
> > prefix. As prefix can be changed in config.mak.autogen, all if blocks
> > with conditions based on prefix should be placed after the file is
> > included in Makefile.

> This is _not_ just about autogen, is it?  The same issue exists if the
> user wants to manually tweak prefix in config.mak, no?

> If so, perhaps the patch needs to be retitled to avoid confusion,
> something like:

>     Subject: Honor $(prefix) set in config.mak* when defining ETC_GIT* variables

You are right.

> > diff --git a/Makefile b/Makefile
> > index cbc3fce..5b4ae40 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -291,15 +291,8 @@ sharedir = $(prefix)/share
> >  gitwebdir = $(sharedir)/gitweb
> >  template_dir = share/git-core/templates
> >  htmldir = share/doc/git-doc
> > -ifeq ($(prefix),/usr)
> > -sysconfdir = /etc
> >  ETC_GITCONFIG = $(sysconfdir)/gitconfig
> >  ETC_GITATTRIBUTES = $(sysconfdir)/gitattributes
> > -else
> > -sysconfdir = $(prefix)/etc
> > -ETC_GITCONFIG = etc/gitconfig
> > -ETC_GITATTRIBUTES = etc/gitattributes
> > -endif
> >  lib = lib
> >  # DESTDIR=
> >  pathsep = :
> > @@ -1192,6 +1185,12 @@ endif
> >  -include config.mak.autogen
> >  -include config.mak

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

Actually I have made a mistake here. I meant the last hunk to be:

@@ -1192,6 +1185,12 @@ endif
 -include config.mak.autogen
 -include config.mak

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

> But this part in the Makefile outside the context of the patch bothers
> me.  It seems to imply that sysconfdir is _not_ that variable you want to
> define later.

>    # Among the variables below, these:
>    #   gitexecdir
>    #   template_dir
>    #   mandir
>    #   infodir
>    #   htmldir
>    #   ETC_GITCONFIG (but not sysconfdir)
>    #   ETC_GITATTRIBUTES
>    # can be specified as a relative path some/where/else;

> So I have a suspicion that your patch as is will break when prefix is set
> to something other than /usr directory.  I don't think anybody in-tree
> currently uses sysconfdir, but that does not mean nobody will ever do.

See the corrected hunk above. I will be prepare the corrected patch.

-- 
  Kacper Kornet