On Thu, Apr 28, 2011 at 09:54:02AM -0700, Junio C Hamano wrote: > Kacper Kornet <kornet@xxxxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html