From: Junio C Hamano <hidden> Date: 2016-06-15 22:51:08
Johannes Sixt [off-list ref] writes:
Please do not set Mail-Followup-To!! It makes communication on this list
extremely inconvenient.
Am 4/28/2011 21:27, schrieb Kacper Kornet:
quoted
Definitions of ETC_GITCONFIG, ETC_GITATTRIBUTES and sysconfdir 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.
...
Does this patch do anything useful? After the patch is applied, sysconfdir
is set-but-not-used. Therefore, you can remove the assignments. But then
you lose the reference to $(prefix) that the commit message claims is so
important. Puzzled...
The only thing it does is to to allow you to set prefix in config.mak and
then have it propaget to the selection of ETC_GITCONFIG (if prefix is /usr,
then it is always /etc/gitconfig, otherwise it is always etc/gitconfig).
The importance of prefix is not that the value is prefixed to ETC_GIT*,
but it is used in the conditional to choose between the two.
We can get rid of assignments to sysconfdir in that sense. But you spotted
a regression. If sysconfdir is set to somewhere else, even if you set prefix
to /usr, we should set ETC_GIT* using the value given to sysconfdir. The
original code did so, but the patch lost it.
From: Johannes Sixt <hidden> Date: 2016-06-15 22:51:09
Am 5/3/2011 19:32, schrieb Junio C Hamano:
Johannes Sixt [off-list ref] writes:
quoted
Please do not set Mail-Followup-To!! It makes communication on this list
extremely inconvenient.
Am 4/28/2011 21:27, schrieb Kacper Kornet:
quoted
Definitions of ETC_GITCONFIG, ETC_GITATTRIBUTES and sysconfdir 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.
...
Does this patch do anything useful? After the patch is applied, sysconfdir
is set-but-not-used. Therefore, you can remove the assignments. But then
you lose the reference to $(prefix) that the commit message claims is so
important. Puzzled...
The only thing it does is to to allow you to set prefix in config.mak and
then have it propaget to the selection of ETC_GITCONFIG (if prefix is /usr,
then it is always /etc/gitconfig, otherwise it is always etc/gitconfig).
The importance of prefix is not that the value is prefixed to ETC_GIT*,
but it is used in the conditional to choose between the two.
Fair enough.
We can get rid of assignments to sysconfdir in that sense. But you spotted
a regression. If sysconfdir is set to somewhere else, even if you set prefix
to /usr, we should set ETC_GIT* using the value given to sysconfdir. The
original code did so, but the patch lost it.
Looking closer, the patch introduces git_etcdir for no good reason, IIUC.
It should just re-use sysconfdir (the meaning of this variable is to point
to the etc directory).
-- Hannes
On Wed, May 04, 2011 at 07:52:30AM +0200, Johannes Sixt wrote:
Looking closer, the patch introduces git_etcdir for no good reason,
IIUC.
It should just re-use sysconfdir (the meaning of this variable is to
point
to the etc directory).
And the first version of my patch did it. However Junio has written:
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.
From that I understood that he prefers sysconfdir to be always an
On Tue, May 03, 2011 at 10:32:14AM -0700, Junio C Hamano wrote:
We can get rid of assignments to sysconfdir in that sense. But you spotted
a regression. If sysconfdir is set to somewhere else, even if you set prefix
to /usr, we should set ETC_GIT* using the value given to sysconfdir. The
original code did so, but the patch lost it.
Actually syconfdir was respected only when prefix was /usr. When it was
not /usr, ETC_GIT* were always set to etc/... The following version of
patch reproduces that behaviour:
From 1a438bc0ac71a96398260b73b3c24c5e752a02f5 Mon Sep 17 00:00:00 2001
From: Kacper Kornet <redacted>
Date: Thu, 28 Apr 2011 02:42:48 +0100
Subject: [PATCH] Honor $(prefix) set in config.mak* when defining ETC_GIT*
and sysconfdir
Definitions of ETC_GITCONFIG, ETC_GITATTRIBUTES and sysconfdir 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.
Signed-off-by: Kacper Kornet <redacted>
---
Makefile | 19 ++++++++++---------
1 files changed, 10 insertions(+), 9 deletions(-)
From: Johannes Sixt <hidden> Date: 2016-06-15 22:51:09
Am 5/4/2011 15:58, schrieb Kacper Kornet:
On Wed, May 04, 2011 at 07:52:30AM +0200, Johannes Sixt wrote:
quoted
Looking closer, the patch introduces git_etcdir for no good reason,
IIUC.
It should just re-use sysconfdir (the meaning of this variable is to
point
to the etc directory).
And the first version of my patch did it. However Junio has written:
quoted
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.
quoted
From that I understood that he prefers sysconfdir to be always an
absolute path.
Junio's worries should not be discarded lightly. But in this case they are
unfounded. Digging the history shows:
b51b8bbf (Create a sysconfdir variable, and use it for ETC_GITCONFIG,
2007-04-24) introduced the variable to be able to treat the special case
where prefix == /usr. It was never intended as a user-settable value.
In 49fa65a7 (Allow the built-in exec path to be relative to the command
invocation path, 2008-07-23), I added the comment above because at that
time, a relocatable build should be requested by setting ETC_GITCONFIG to
a relative path, but not by changing sysconfdir. (The comment sounds as if
the user can set sysconfdir, but I did not intend to say that.)
026fa0d5 (Move computation of absolute paths from Makefile to runtime (in
preparation for RUNTIME_PREFIX), 2009-01-18) practically obsoleted
sysconfdir. In particular, it removed one of the cases where the value of
sysconfdir mattered, leaving only the reference where it is guaranteed to
be set to /etc. This commit could have removed sysconfdir entirely.
6df42ab9 (Add global and system-wide gitattributes, 2010-09-01) added
another consumer of sysconfdir, but in the same spirit as ETC_GITCONFIG.
So, I don't think that sysconfdir must survive. It was always only a
helper variable to shorten the code.
-- Hannes