threads / discuss / 21288
Subject: Extra checks for PERL_PATH and SHELL_PATH?
Hi,
The top-level Makefile currently contains
> ifndef SHELL_PATH > SHELL_PATH = /bin/sh > endif > ifndef PERL_PATH > PERL_PATH = /usr/bin/perl > endif
The checks are only necessary if these variables need to be overridden by environment variables, not just via the make command line. Is this the case?
-- Matt Kraai http://ftbfs.org/
Junio C HamanoRe: Extra checks for PERL_PATH and SHELL_PATH?
Matt Kraai <kraai@ftbfs.org> writes:
> The top-level Makefile currently contains > >> ifndef SHELL_PATH >> SHELL_PATH = /bin/sh >> endif >> ifndef PERL_PATH >> PERL_PATH = /usr/bin/perl >> endif > > The checks are only necessary if these variables need to be overridden > by environment variables, not just via the make command line. Is this > the case?
It may not have been the original intention, but the above would mean that some people may have learned to run "SHELL_PATH=/bin/ksh make" and changing it would break things for them, no?
I do not think changing them is bad per-se, but we would need to add extra warnings in the release note to explain this change, that's all. This would only affect people who build from the source (including distro people) so it is not really a big deal.
[PATCH] Makefile: set PERL_PATH and SHELL_PATH unconditionally
Do not check whether PERL_PATH and SHELL_PATH are undefined before setting their default values. This prevents them from being set via environment variables.
Signed-off-by: Matt Kraai <kraai@ftbfs.org> --- On Mon, Oct 19, 2009 at 11:36:26PM -0700, Junio C Hamano wrote: > Matt Kraai <kraai@ftbfs.org> writes: > > > The top-level Makefile currently contains > > > >> ifndef SHELL_PATH > >> SHELL_PATH = /bin/sh > >> endif > >> ifndef PERL_PATH > >> PERL_PATH = /usr/bin/perl > >> endif > > > > The checks are only necessary if these variables need to be overridden > > by environment variables, not just via the make command line. Is this > > the case? > > It may not have been the original intention, but the above would mean that > some people may have learned to run "SHELL_PATH=/bin/ksh make" and > changing it would break things for them, no?
Yes, that's what I was concerned about. This appears to be possible for PERL_PATH on all platforms and for SHELL_PATH on platforms other than SCO UnixWare, SunOS, and IRIX.
> I do not think changing them is bad per-se, but we would need to add extra > warnings in the release note to explain this change, that's all. This > would only affect people who build from the source (including distro > people) so it is not really a big deal.
I hope this patch is OK.
Documentation/RelNotes-1.6.6.txt | 3 +++ Makefile | 8 ++------ 2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/Documentation/RelNotes-1.6.6.txt b/Documentation/RelNotes-1.6.6.txt index 5f1fecb..bfda14c 100644 --- a/Documentation/RelNotes-1.6.6.txt +++ b/Documentation/RelNotes-1.6.6.txt @@ -58,3 +58,6 @@ release, unless otherwise noted. whitespace attribute). The 'trailing-space' whitespace error class has become a short-hand to cover both of these and there is no behaviour change for existing set-ups. + + * PERL_PATH and SHELL_PATH may not be overridden using environment + variables during the build. diff --git a/Makefile b/Makefile index 42b7d60..5bac305 100644 --- a/Makefile +++ b/Makefile @@ -392,12 +392,8 @@ ALL_PROGRAMS = $(PROGRAMS) $(SCRIPTS) OTHER_PROGRAMS = git$X # Set paths to tools early so that they can be used for version tests. -ifndef SHELL_PATH - SHELL_PATH = /bin/sh -endif -ifndef PERL_PATH - PERL_PATH = /usr/bin/perl -endif +SHELL_PATH = /bin/sh +PERL_PATH = /usr/bin/perl export PERL_PATH
-- 1.6.5
Junio C HamanoRe: [PATCH] Makefile: set PERL_PATH and SHELL_PATH unconditionally
Matt Kraai <kraai@ftbfs.org> writes:
> Do not check whether PERL_PATH and SHELL_PATH are undefined before > setting their default values. This prevents them from being set via > environment variables.
Is there an upside of "preventing them from getting set", by the way?
Re: [PATCH] Makefile: set PERL_PATH and SHELL_PATH unconditionally
On Tue, Oct 20, 2009 at 09:18:29AM -0700, Junio C Hamano wrote:
> Matt Kraai <kraai@ftbfs.org> writes: > > > Do not check whether PERL_PATH and SHELL_PATH are undefined before > > setting their default values. This prevents them from being set via > > environment variables. > > Is there an upside of "preventing them from getting set", by the way?
Not that I know of.
I originally thought that the checks were superfluous, but now I just believe they're inconsistent and confusing to people like me who think they understand Makefiles but don't. :)
-- Matt Kraai http://ftbfs.org/