threads / rfc / 29810

RFC patchbuild: avoid possible confusion between GNU/XPG4 make on Solaris

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

## tl;dr

12 messages between Mar 2, 2012 and Mar 9, 2012. Diffs are folded; open one to read it.

replies: 11people: 3as markdown or json

Stefano Lattarini· Mar 2, 2012, 09:13 UTC · lore

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(-)
Show changes to Makefile +13 −0
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
Thomas Rast· Mar 2, 2012, 09:34 UTC · re: Stefano Lattarini · lore

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

Stefano Lattarini <stefano.lattarini@gmail.com> writes:
Show 5 quoted lines
> +# 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
Stefano Lattarini· Mar 2, 2012, 09:41 UTC · re: Thomas Rast · lore

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

On 03/02/2012 10:34 AM, Thomas Rast wrote:
Show 27 quoted lines
> 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
Stefano Lattarini· Mar 2, 2012, 10:18 UTC · re: Stefano Lattarini · lore

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

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(-)
Show changes to Makefile +15 −0
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
Junio C Hamano· Mar 2, 2012, 18:35 UTC · re: Stefano Lattarini · lore

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

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?

Show 16 quoted lines
> +# 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
Stefano Lattarini· Mar 2, 2012, 19:00 UTC · re: Junio C Hamano · lore

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

On 03/02/2012 07:35 PM, Junio C Hamano wrote:
Show 25 quoted lines
> 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
Junio C Hamano· Mar 2, 2012, 19:46 UTC · re: Stefano Lattarini · lore

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

Stefano Lattarini <stefano.lattarini@gmail.com> writes:
Show 7 quoted lines
> 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.

Stefano Lattarini· Mar 9, 2012, 12:43 UTC · re: Junio C Hamano · lore

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

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(-)
Show changes to configure.ac +17 −0
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
Junio C Hamano· Mar 9, 2012, 16:29 UTC · re: Stefano Lattarini · lore

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

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.

Junio C Hamano· Mar 9, 2012, 19:46 UTC · re: Stefano Lattarini · lore

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

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?
Stefano Lattarini· Mar 9, 2012, 21:46 UTC · re: Junio C Hamano · lore

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

On 03/09/2012 08:46 PM, Junio C Hamano wrote:
Show 11 quoted lines
> 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
Junio C Hamano· Mar 9, 2012, 21:59 UTC · re: Stefano Lattarini · lore

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

Stefano Lattarini <stefano.lattarini@gmail.com> writes:
Show 14 quoted lines
> 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.

← back to recent threads