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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 28, 2011, 16:54 UTC
Message-ID: <7v62py5nbp.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20110428022922.GC4833@camk.edu.pl>

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

Show 29 quoted lines

> 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

It makes sense to change the definition of ETC_GIT* variables to a form that depends on a variable like your patch did, i.e.

    ETC_GITCONFIG = $(some_etc_prefix)/gitconfig
    ETC_GITATTRIBUTES = $(some_etc_prefix)/gitattributes

and define that variable, whose definition depends on $(prefix), after we have read config.mak* files. So I like the general direction of this patch.

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.