* [RFC/PATCH] build: avoid possible confusion between GNU/XPG4 make on Solaris
@ 2012-03-02 9:13 Stefano Lattarini
2012-03-02 9:34 ` Thomas Rast
2012-03-02 18:35 ` [RFC/PATCH] " Junio C Hamano
0 siblings, 2 replies; 12+ messages in thread
From: Stefano Lattarini @ 2012-03-02 9:13 UTC (permalink / raw
To: git; +Cc: Stefano Lattarini
On a Solaris 10 system with Solaris XPG4 make installed as /usr/xpg4/bin/make,
GNU make installed as /usr/local/bin/make, and with /usr/local/bin appearing
in $PATH *before* /usr/xpg4/bin, I was seeing errors like this upon invoking
"make all":
SUBDIR perl
make: Warning: Ignoring DistributedMake -o option
Usage : make [ -f makefile ][ -K statefile ]...
make: Fatal error: Unknown option `-C'
This happens because the Git's Makefiles, when running on Solaris, sanitize
$PATH by prepending /usr/xpg6/bin and /usr/xpg4/bin to it, but in the setup
described above such a behaviour has the unintended consequence of forcing
the use of Solaris make in recursive make invocations, even if the $(MAKE)
macro is being correctly used in them; this happens because, in that setup,
the original GNU make process was invoked simply as "make".
To avoid such an issue, we instruct our Makefile to redefine $(MAKE) to
point to the absolute path of the originally-invoked make program.
---
The implementation is still rough and not completely portable, but before
investing more time in refining it I'd like to know if you gitsters think
the idea behind it is sound.
Regards,
Stefano
Makefile | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/Makefile b/Makefile
index e4f8e0e..8cba6e8 100644
--- a/Makefile
+++ b/Makefile
@@ -303,6 +303,19 @@ ifdef MSVC
uname_O := Windows
endif
+# This Makefile will possibly sanitize PATH by prepending system-specific
+# directories to it (e.g., /usr/xpg4/bin on Solaris). This can become
+# problematic for recursive make invocations, if one of those directories
+# contains a "make" program and the user has called GNU make by simply
+# invoking "make" (this can happen e.g. when GNU make has been installed
+# as /usr/local/bin/make). To avoid such issues, we redefine $(MAKE) to
+# point to the absolute path of the originally-invoked make program.
+# FIXME: this is ugly, and which(1) is quite unportable. Find a better
+# way to obtain the same effect.
+MAKE := $(shell set $(MAKE); m1=$$1; shift; \
+ m2=`which $$m1 2>/dev/null` && test -n "$$m2" || m2=$$m1; \
+ echo "$$m2 $$*")
+
# CFLAGS and LDFLAGS are for the users to override from the command line.
CFLAGS = -g -O2 -Wall
--
1.7.9
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RFC/PATCH] build: avoid possible confusion between GNU/XPG4 make on Solaris
2012-03-02 9:13 [RFC/PATCH] build: avoid possible confusion between GNU/XPG4 make on Solaris Stefano Lattarini
@ 2012-03-02 9:34 ` Thomas Rast
2012-03-02 9:41 ` Stefano Lattarini
2012-03-02 18:35 ` [RFC/PATCH] " Junio C Hamano
1 sibling, 1 reply; 12+ messages in thread
From: Thomas Rast @ 2012-03-02 9:34 UTC (permalink / raw
To: Stefano Lattarini; +Cc: git
Stefano Lattarini <stefano.lattarini@gmail.com> writes:
> +# FIXME: this is ugly, and which(1) is quite unportable. Find a better
> +# way to obtain the same effect.
> +MAKE := $(shell set $(MAKE); m1=$$1; shift; \
> + m2=`which $$m1 2>/dev/null` && test -n "$$m2" || m2=$$m1; \
> + echo "$$m2 $$*")
There's 'command -v make'. 'man 1p command' on my system (opensuse
installs a bunch of POSIX reference material) says
-v (On systems supporting the User Portability Utilities
option.) Write a string to standard output that indicates
the pathname or command that will be used by the shell, in
the current shell execution environment (see Shell Execu-
tion Environment ), to invoke command_name, but do not
invoke command_name.
* Utilities, regular built-in utilities, command_names
including a slash character, and any implementation-
defined functions that are found using the PATH vari-
able (as described in Command Search and Execution ),
shall be written as absolute pathnames.
So perhaps enough systems including Solaris "support the User
Portability Utilities option", and you can use this?
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC/PATCH] build: avoid possible confusion between GNU/XPG4 make on Solaris
2012-03-02 9:34 ` Thomas Rast
@ 2012-03-02 9:41 ` Stefano Lattarini
2012-03-02 10:18 ` [PATCH v2] " Stefano Lattarini
0 siblings, 1 reply; 12+ messages in thread
From: Stefano Lattarini @ 2012-03-02 9:41 UTC (permalink / raw
To: Thomas Rast; +Cc: git
On 03/02/2012 10:34 AM, Thomas Rast wrote:
> Stefano Lattarini <stefano.lattarini@gmail.com> writes:
>
>> +# FIXME: this is ugly, and which(1) is quite unportable. Find a better
>> +# way to obtain the same effect.
>> +MAKE := $(shell set $(MAKE); m1=$$1; shift; \
>> + m2=`which $$m1 2>/dev/null` && test -n "$$m2" || m2=$$m1; \
>> + echo "$$m2 $$*")
>
> There's 'command -v make'. 'man 1p command' on my system (opensuse
> installs a bunch of POSIX reference material) says
>
> -v (On systems supporting the User Portability Utilities
> option.) Write a string to standard output that indicates
> the pathname or command that will be used by the shell, in
> the current shell execution environment (see Shell Execu-
> tion Environment ), to invoke command_name, but do not
> invoke command_name.
>
> * Utilities, regular built-in utilities, command_names
> including a slash character, and any implementation-
> defined functions that are found using the PATH vari-
> able (as described in Command Search and Execution ),
> shall be written as absolute pathnames.
>
> So perhaps enough systems including Solaris "support the User
> Portability Utilities option", and you can use this?
>
Thanks, I had completely forgotten about this "trick". It works correctly
with both /bin/sh and /bin/ksh on all of NetBSD 5.1, OpenBSD 5.0 and
Solaris 10, as well as with bash (4.1.5) and dash (0.5.5.1) on my Debian
unstable. I will post an updated patch later today (or this evening).
Regards,
Stefano
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] build: avoid possible confusion between GNU/XPG4 make on Solaris
2012-03-02 9:41 ` Stefano Lattarini
@ 2012-03-02 10:18 ` Stefano Lattarini
0 siblings, 0 replies; 12+ messages in thread
From: Stefano Lattarini @ 2012-03-02 10:18 UTC (permalink / raw
To: git; +Cc: gitster
On a Solaris 10 system with Solaris XPG4 make installed as /usr/xpg4/bin/make,
GNU make installed as /usr/local/bin/make, and with /usr/local/bin appearing
in $PATH *before* /usr/xpg4/bin, I was seeing errors like this upon invoking
"make all":
SUBDIR perl
make: Warning: Ignoring DistributedMake -o option
Usage : make [ -f makefile ][ -K statefile ]...
make: Fatal error: Unknown option `-C'
This happens because the Git's Makefiles, when running on Solaris, sanitize
$PATH by prepending /usr/xpg6/bin and /usr/xpg4/bin to it, but in the setup
described above such a behaviour has the unintended consequence of forcing
the use of Solaris make in recursive make invocations, even if the $(MAKE)
macro is being correctly used in them; this happens because, in that setup,
the original GNU make process was invoked simply as "make".
To avoid such an issue, we instruct our Makefile to redefine $(MAKE) to
point to the absolute path of the originally-invoked make program.
Helped-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Stefano Lattarini <stefano.lattarini@gmail.com>
---
Makefile | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/Makefile b/Makefile
index e4f8e0e..e71d688 100644
--- a/Makefile
+++ b/Makefile
@@ -303,6 +303,21 @@ ifdef MSVC
uname_O := Windows
endif
+# This Makefile will possibly sanitize PATH by prepending system-specific
+# directories to it (e.g., /usr/xpg4/bin on Solaris). This can become
+# problematic for recursive make invocations, if one of those directories
+# contains a "make" program and the user has called GNU make by simply
+# invoking "make" (this can happen e.g. when GNU make has been installed
+# as /usr/local/bin/make). To avoid such issues, we redefine $(MAKE) to
+# point to the absolute path of the originally-invoked make program.
+original_MAKE := $(MAKE)
+MAKE := $(shell command -v $(firstword $(original_MAKE)) 2>/dev/null)
+ifeq ($(MAKE),)
+ MAKE := $(original_MAKE)
+else
+ MAKE += $(wordlist 2,$(words $(original_MAKE)),$(original_MAKE))
+endif
+
# CFLAGS and LDFLAGS are for the users to override from the command line.
CFLAGS = -g -O2 -Wall
--
1.7.9
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RFC/PATCH] build: avoid possible confusion between GNU/XPG4 make on Solaris
2012-03-02 9:13 [RFC/PATCH] build: avoid possible confusion between GNU/XPG4 make on Solaris Stefano Lattarini
2012-03-02 9:34 ` Thomas Rast
@ 2012-03-02 18:35 ` Junio C Hamano
2012-03-02 19:00 ` Stefano Lattarini
2012-03-09 12:43 ` [PATCH] configure: allow user to prevent $PATH "sanitization" " Stefano Lattarini
1 sibling, 2 replies; 12+ messages in thread
From: Junio C Hamano @ 2012-03-02 18:35 UTC (permalink / raw
To: Stefano Lattarini; +Cc: git
Stefano Lattarini <stefano.lattarini@gmail.com> writes:
> On a Solaris 10 system with Solaris XPG4 make installed as /usr/xpg4/bin/make,
> GNU make installed as /usr/local/bin/make, and with /usr/local/bin appearing
> in $PATH *before* /usr/xpg4/bin, I was seeing errors like this upon invoking
> "make all":
After reading this explanation, my first reaction is that the prefixing of
path _is_ what is wrong. The prefixing is done to help a subset of
Solaris users who are unaware of /usr/xpg4/bin that are more POSIX than
what they have in /usr/bin, and that is what is hurting people like you
who know what you are doing and have suitable tools in other places, like
you do in /usr/local/bin.
And the real fix for your problem is _not_ an ugly override of $(MAKE)
like you do in this patch, I think. After all, somebody else who have a
tool in /usr/local/bin that is saner than what is in /usr/xpg4/bin may
suffer from the same issue for commands other than "make".
So the real solution would probably be to let you override how the
BROKEN_PATH_FIX works, no?
Ah... and I think we already have such a solution in our Makefile. Can't
you override SANE_TOOL_PATH in your config.mak instead?
> +# This Makefile will possibly sanitize PATH by prepending system-specific
> +# directories to it (e.g., /usr/xpg4/bin on Solaris). This can become
> +# problematic for recursive make invocations, if one of those directories
> +# contains a "make" program and the user has called GNU make by simply
> +# invoking "make" (this can happen e.g. when GNU make has been installed
> +# as /usr/local/bin/make). To avoid such issues, we redefine $(MAKE) to
> +# point to the absolute path of the originally-invoked make program.
> +# FIXME: this is ugly, and which(1) is quite unportable. Find a better
> +# way to obtain the same effect.
> +MAKE := $(shell set $(MAKE); m1=$$1; shift; \
> + m2=`which $$m1 2>/dev/null` && test -n "$$m2" || m2=$$m1; \
> + echo "$$m2 $$*")
> +
> # CFLAGS and LDFLAGS are for the users to override from the command line.
>
> CFLAGS = -g -O2 -Wall
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC/PATCH] build: avoid possible confusion between GNU/XPG4 make on Solaris
2012-03-02 18:35 ` [RFC/PATCH] " Junio C Hamano
@ 2012-03-02 19:00 ` Stefano Lattarini
2012-03-02 19:46 ` Junio C Hamano
2012-03-09 12:43 ` [PATCH] configure: allow user to prevent $PATH "sanitization" " Stefano Lattarini
1 sibling, 1 reply; 12+ messages in thread
From: Stefano Lattarini @ 2012-03-02 19:00 UTC (permalink / raw
To: Junio C Hamano; +Cc: git
On 03/02/2012 07:35 PM, Junio C Hamano wrote:
> Stefano Lattarini <stefano.lattarini@gmail.com> writes:
>
>> On a Solaris 10 system with Solaris XPG4 make installed as /usr/xpg4/bin/make,
>> GNU make installed as /usr/local/bin/make, and with /usr/local/bin appearing
>> in $PATH *before* /usr/xpg4/bin, I was seeing errors like this upon invoking
>> "make all":
>
> After reading this explanation, my first reaction is that the prefixing of
> path _is_ what is wrong. The prefixing is done to help a subset of
> Solaris users who are unaware of /usr/xpg4/bin that are more POSIX than
> what they have in /usr/bin, and that is what is hurting people like you
> who know what you are doing and have suitable tools in other places, like
> you do in /usr/local/bin.
>
> And the real fix for your problem is _not_ an ugly override of $(MAKE)
> like you do in this patch, I think. After all, somebody else who have a
> tool in /usr/local/bin that is saner than what is in /usr/xpg4/bin may
> suffer from the same issue for commands other than "make".
>
> So the real solution would probably be to let you override how the
> BROKEN_PATH_FIX works, no?
>
> Ah... and I think we already have such a solution in our Makefile. Can't
> you override SANE_TOOL_PATH in your config.mak instead?
>
Yes and no. While in hindsight I agree that using this already-provided
solution is much better than my ugly automatic munging of $(MAKE) (so yes,
let's scrap the present patch), I also think that if one is setting up the
configuration of the Git tree using the the autoconf-generated configure
script (as I do), he wants configure to Do The Right Thing automatically
in this regard too -- thus recognizing that /usr/xpg4/bin is already in
$PATH before /bin and /usr/bin, and setting up an appropriate default for
SANE_TOOL_PATH in 'config.make.autogen' accordingly.
What would you think of a patch in this direction?
Thanks,
Stefano
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC/PATCH] build: avoid possible confusion between GNU/XPG4 make on Solaris
2012-03-02 19:00 ` Stefano Lattarini
@ 2012-03-02 19:46 ` Junio C Hamano
0 siblings, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2012-03-02 19:46 UTC (permalink / raw
To: Stefano Lattarini; +Cc: git
Stefano Lattarini <stefano.lattarini@gmail.com> writes:
> configuration of the Git tree using the the autoconf-generated configure
> script (as I do), he wants configure to Do The Right Thing automatically
> in this regard too
> $PATH before /bin and /usr/bin, and setting up an appropriate default for
> SANE_TOOL_PATH in 'config.make.autogen' accordingly.
>
> What would you think of a patch in this direction?
Yeah, that was exactly what I was suggesting, but it may not have been
obvious.
As I consider autoconf merely a secondary mechanism to set config.mak,
whenever I say "that is solved by local tweak in config.mak", letting
./configure to do that tweak automatically is already covered by that
suggestion.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] configure: allow user to prevent $PATH "sanitization" on Solaris
2012-03-02 18:35 ` [RFC/PATCH] " Junio C Hamano
2012-03-02 19:00 ` Stefano Lattarini
@ 2012-03-09 12:43 ` Stefano Lattarini
2012-03-09 16:29 ` Junio C Hamano
2012-03-09 19:46 ` Junio C Hamano
1 sibling, 2 replies; 12+ messages in thread
From: Stefano Lattarini @ 2012-03-09 12:43 UTC (permalink / raw
To: git; +Cc: gitster
On a Solaris 10 system with Solaris make installed as '/usr/xpg4/bin/make',
GNU make installed as '/usr/local/bin/make', and with '/usr/local/bin'
appearing in $PATH *before* '/usr/xpg4/bin', I was seeing errors like this
upon invoking "make all":
SUBDIR perl
make: Warning: Ignoring DistributedMake -o option
Usage : make [ -f makefile ][ -K statefile ]...
make: Fatal error: Unknown option `-C'
This happened because the Git's Makefile, when running on Solaris,
automatically "sanitizes" $PATH by prepending '/usr/xpg6/bin' and
'/usr/xpg4/bin' to it, and in the setup described above such a behaviour
has the unintended consequence of forcing the use of Solaris make
in recursive make invocations -- even if the $(MAKE) macro is being
correctly used in them!
For developers that don't use the autoconf machinery, the best and easier
fix in such a situation is to properly override $(SANE_TOOL_PATH) in
config.mak. But a developer using the autoconf-generated configure script
to set up its Git source tree's configuration wouldn't expect to also have
to provide such an override *by hand* after having run configure; he would
either expect:
1) that the issue is automatically detected and transparently worked
around by configure; or at least
2) that a configure-time override telling how (and if) PATH is to be
sanitized is provided.
This change implements the second approach, which is less-ambitious but
also much less fragile. Note that, even if we should decide to implement
the first approach in the future, the code added by the present change
still be useful, allowing the user the possibility to override the
configure's choices in case they turn out wrong (in true autotools
spirit).
---
Hi Junio, sorry for the delay. Here is a simpler approach that worked
out well enough for me; the commit message should explain the rationales
and motivations for it in detail.
Regards,
Stefano
configure.ac | 17 +++++++++++++++++
1 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/configure.ac b/configure.ac
index 8bb0f44..72f7958 100644
--- a/configure.ac
+++ b/configure.ac
@@ -137,6 +137,23 @@ if test -n "$1"; then
fi
])
+# Directories holding "saner" versions of common or POSIX binaries.
+AC_ARG_WITH([sane-tool-path],
+ [AS_HELP_STRING(
+ [--with-sane-tool-path=DIR-1[[:DIR-2...:DIR-n]]],
+ [Directories to prepend to PATH in build system and generated scripts])],
+ [if test "$withval" = "no"; then
+ withval=''
+ else
+ AC_MSG_NOTICE([Setting SANE_TOOL_PATH to '$withval'])
+ fi
+ GIT_CONF_APPEND_LINE([SANE_TOOL_PATH=$withval])],
+ [# If the "--with-sane-tool-path" option was not given, don't touch
+ # SANE_TOOL_PATH here, but let defaults in Makefile take care of it.
+ # This should minimize spurious differences in the behaviour of the
+ # Git build system when configure is used w.r.t. when it is not.
+ :])
+
## Site configuration related to programs (before tests)
## --with-PACKAGE[=ARG] and --without-PACKAGE
#
--
1.7.9
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] configure: allow user to prevent $PATH "sanitization" on Solaris
2012-03-09 12:43 ` [PATCH] configure: allow user to prevent $PATH "sanitization" " Stefano Lattarini
@ 2012-03-09 16:29 ` Junio C Hamano
2012-03-09 19:46 ` Junio C Hamano
1 sibling, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2012-03-09 16:29 UTC (permalink / raw
To: Stefano Lattarini; +Cc: git
Stefano Lattarini <stefano.lattarini@gmail.com> writes:
> Hi Junio, sorry for the delay.
Thanks for not forgetting.
The contributors are used as "tracking system" around here, and they
won't get "what happened to your stalled patch? Any progress?" from
anybody (unless there are others who are deeply interested in the
topic). Delay is not a problem.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] configure: allow user to prevent $PATH "sanitization" on Solaris
2012-03-09 12:43 ` [PATCH] configure: allow user to prevent $PATH "sanitization" " Stefano Lattarini
2012-03-09 16:29 ` Junio C Hamano
@ 2012-03-09 19:46 ` Junio C Hamano
2012-03-09 21:46 ` Stefano Lattarini
1 sibling, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2012-03-09 19:46 UTC (permalink / raw
To: Stefano Lattarini; +Cc: git
Stefano Lattarini <stefano.lattarini@gmail.com> writes:
> This change implements the second approach, which is less-ambitious but
> also much less fragile.
I personally think your second one is the only sane option.
> ---
You forgot to sign-off the patch, perhaps?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] configure: allow user to prevent $PATH "sanitization" on Solaris
2012-03-09 19:46 ` Junio C Hamano
@ 2012-03-09 21:46 ` Stefano Lattarini
2012-03-09 21:59 ` Junio C Hamano
0 siblings, 1 reply; 12+ messages in thread
From: Stefano Lattarini @ 2012-03-09 21:46 UTC (permalink / raw
To: Junio C Hamano; +Cc: git
On 03/09/2012 08:46 PM, Junio C Hamano wrote:
> Stefano Lattarini <stefano.lattarini@gmail.com> writes:
>
>> This change implements the second approach, which is less-ambitious but
>> also much less fragile.
>
> I personally think your second one is the only sane option.
>
>> ---
>
> You forgot to sign-off the patch, perhaps?
>
Indeed (but I've now fixed my git aliases to avoid similar issues in
the future). Will you fix the issue locally, or should I re-roll?
Thanks, and sorry for the noise,
Stefano
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] configure: allow user to prevent $PATH "sanitization" on Solaris
2012-03-09 21:46 ` Stefano Lattarini
@ 2012-03-09 21:59 ` Junio C Hamano
0 siblings, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2012-03-09 21:59 UTC (permalink / raw
To: Stefano Lattarini; +Cc: git
Stefano Lattarini <stefano.lattarini@gmail.com> writes:
> On 03/09/2012 08:46 PM, Junio C Hamano wrote:
>> Stefano Lattarini <stefano.lattarini@gmail.com> writes:
>>
>>> This change implements the second approach, which is less-ambitious but
>>> also much less fragile.
>>
>> I personally think your second one is the only sane option.
>>
>>> ---
>>
>> You forgot to sign-off the patch, perhaps?
>>
> Indeed (but I've now fixed my git aliases to avoid similar issues in
> the future). Will you fix the issue locally, or should I re-roll?
Sure, I'll fix it up. Thanks.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2012-03-09 21:59 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-02 9:13 [RFC/PATCH] build: avoid possible confusion between GNU/XPG4 make on Solaris Stefano Lattarini
2012-03-02 9:34 ` Thomas Rast
2012-03-02 9:41 ` Stefano Lattarini
2012-03-02 10:18 ` [PATCH v2] " Stefano Lattarini
2012-03-02 18:35 ` [RFC/PATCH] " Junio C Hamano
2012-03-02 19:00 ` Stefano Lattarini
2012-03-02 19:46 ` Junio C Hamano
2012-03-09 12:43 ` [PATCH] configure: allow user to prevent $PATH "sanitization" " Stefano Lattarini
2012-03-09 16:29 ` Junio C Hamano
2012-03-09 19:46 ` Junio C Hamano
2012-03-09 21:46 ` Stefano Lattarini
2012-03-09 21:59 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).