[RFC/PATCH] build: avoid possible confusion between GNU/XPG4 make on Solaris

[RFC/PATCH] build: avoid possible confusion between GNU/XPG4 make on Solaris

[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

Re: [RFC/PATCH] build: avoid possible confusion between GNU/XPG4 make on Solaris

[Thomas Rast]

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

Re: [RFC/PATCH] build: avoid possible confusion between GNU/XPG4 make on Solaris

[Stefano Lattarini]

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

[PATCH v2] build: avoid possible confusion between GNU/XPG4 make on Solaris

[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.

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

Re: [RFC/PATCH] build: avoid possible confusion between GNU/XPG4 make on Solaris

[Junio C Hamano]

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

Re: [RFC/PATCH] build: avoid possible confusion between GNU/XPG4 make on Solaris

[Stefano Lattarini]

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

Re: [RFC/PATCH] build: avoid possible confusion between GNU/XPG4 make on Solaris

[Junio C Hamano]

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.

[PATCH] configure: allow user to prevent $PATH "sanitization" on Solaris

[Stefano Lattarini]

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

Re: [PATCH] configure: allow user to prevent $PATH "sanitization" on Solaris

[Junio C Hamano]

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.

Re: [PATCH] configure: allow user to prevent $PATH "sanitization" on Solaris

[Junio C Hamano]

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?

Re: [PATCH] configure: allow user to prevent $PATH "sanitization" on Solaris

[Stefano Lattarini]

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

Re: [PATCH] configure: allow user to prevent $PATH "sanitization" on Solaris

[Junio C Hamano]

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.