Matt Kraai [off-list ref] writes:
The top-level Makefile currently contains
quoted
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.
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 <redacted>
---
On Mon, Oct 19, 2009 at 11:36:26PM -0700, Junio C Hamano wrote:
> Matt Kraai [off-list ref] 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