Do not hard-code the command name 'make' in upper-level tools

PATCH Superseded Masahiro Yamada 4 participants 10 messages Jul 17, 2014 – Jul 22, 2014

Share & tools

Masahiro Yamada

author

2014-07-17 09:17 UTC #0 lore raw

cover Do not hard-code the command name 'make' in upper-level tools

Masahiro Yamada (3):
scripts: add scripts/gnu_make to not hard-code make command
MAKEALL: make sure to invoke GNU Make
buildman: make sure to invoke GNU Make

 MAKEALL                   |  8 +++++---
 scripts/gnu_make          | 30 ++++++++++++++++++++++++++++++
 tools/buildman/builder.py |  3 ++-
 3 files changed, 37 insertions(+), 4 deletions(-)
 create mode 100755 scripts/gnu_make

Signature · 1 line

1.9.1

Masahiro Yamada

author

2014-07-17 09:18 UTC #1 lore raw

PATCH 1/3 scripts: add scripts/gnu_make to not hard-code make command

U-Boot is expected to be built on various platforms but makefiles are written for GNU Make.

We should keep in mind that the command 'make' is not always GNU Make.

For example, on Linux, people generally do:

make <board>_config; make

But FreeBSD folks do

gmake <board>_config; gmake

(The command 'make' on FreeBSD is BSD Make, not GNU Make)

It is not a good idea to hard-code the command name 'make' in MAKEALL or buildman.

Instead, they should call the make command via this helper script to make sure it is GNU Make.

Signed-off-by Masahiro Yamada <[email protected]>

---

 scripts/gnu_make | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100755 scripts/gnu_make

scripts/gnu_make new file +30 -0

new file mode 100755
index 0000000..2eeafbb
@@ -0,0 +1,30 @@
+#!/bin/sh
+#
+# Call GNU Make
+#
+# U-Boot is supposed to be built on various platforms.
+# One problem is that the command 'make' is not always GNU Make.
+# (For ex. the command name for GNU Make on FreeBSD is usually 'gmake'.)
+# It is not a good idea to hard-code the command name 'make'
+# in scripts where where GNU Make is expected.
+# In that case, call this helper script to make sure to invoke GNU Make.
+#
+# SPDX-License-Identifier:	GPL-2.0+
+#
+
+gnu_make=
+
+for m in make gmake
+do
+	if $m --version 2>/dev/null | grep -q GNU; then
+		gnu_make=$m
+		break
+	fi
+done
+
+if [ -z "$gnu_make" ]; then
+	echo "GNU Make not found" >&2
+	exit 1
+fi
+
+$gnu_make "$@"

Signature · 1 line

1.9.1

Jeroen Hofstee

2014-07-18 18:19 UTC #2 lore raw

PATCH 1/3 scripts: add scripts/gnu_make to not hard-code make command

Hello Masahiro,

Quoted 9 lines · Masahiro Yamada

U-Boot is expected to be built on various platforms but makefiles are written for GNU Make.
We should keep in mind that the command 'make' is not always GNU Make.
For example, on Linux, people generally do:
make <board>_config; make

Quoted 8 lines

But FreeBSD folks do
gmake <board>_config; gmake
(The command 'make' on FreeBSD is BSD Make, not GNU Make)
It is not a good idea to hard-code the command name 'make' in MAKEALL or buildman.

indeed, not a good idea.

Quoted 2 lines

Instead, they should call the make command via this helper script to make sure it is GNU Make.

yup, or the name of the executable could be displayed, so it only needs to be checked once at startup (and error out if no GNU make is found at all).

Quoted 1 line

+++ b/scripts/gnu_make

nitpicking, most script use a dash instead of an underscore.

Quoted 22 lines

scripts/gnu_make new file +20 -0

@@ -0,0 +1,30 @@
+#!/bin/sh
+#
+# Call GNU Make
+#
+# U-Boot is supposed to be built on various platforms.
+# One problem is that the command 'make' is not always GNU Make.
+# (For ex. the command name for GNU Make on FreeBSD is usually 'gmake'.)
+# It is not a good idea to hard-code the command name 'make'
+# in scripts where where GNU Make is expected.
+# In that case, call this helper script to make sure to invoke GNU Make.
+#
+# SPDX-License-Identifier:	GPL-2.0+
+#
+
+gnu_make=
+
+for m in make gmake
+do
+	if $m --version 2>/dev/null | grep -q GNU; then
+		gnu_make=$m

scripts/gnu_make line 30

Quoted 10 lines

scripts/gnu_make new file +10 -0

+		break
+	fi
+done
+
+if [ -z "$gnu_make" ]; then
+	echo "GNU Make not found" >&2
+	exit 1
+fi
+
+$gnu_make "$@"

I guess, it could use exec here, since the script itself is no longer needed.

Regards,
Jeroen

Masahiro Yamada

author

2014-07-22 03:54 UTC #3 lore raw

PATCH 1/3 scripts: add scripts/gnu_make to not hard-code make command

Hi Jeroen,

Quoted 6 lines · On Fri, 18 Jul 2014 20:19:52 +0200 Jeroen Hofstee <[email protected]> wrote:

Quoted 2 lines
Instead, they should call the make command via this helper script to make sure it is GNU Make.
yup, or the name of the executable could be displayed, so it only needs to be checked once at startup (and error out if no GNU make is found at all).

Good idea!

MAKEALL and buildman generally invoke make for a lot of boards. Searching it once at startup seems better.

Quoted 3 lines

Quoted 1 line
+++ b/scripts/gnu_make
nitpicking, most script use a dash instead of an underscore.

Renamed in v2.

Best Regards
Masahiro Yamada

Masahiro Yamada

author

2014-07-17 09:18 UTC #4 lore raw

PATCH 2/3 MAKEALL: make sure to invoke GNU Make

In reply to Masahiro Yamada

Since the command name 'make' is not GNU Make on some platforms such as FreeBSD, MAKEALL should call the make via scripts/gnu_make.

Signed-off-by Masahiro Yamada <[email protected]>

---

 MAKEALL | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

MAKEALL +5 -3

index 37ef71e..02c2f9f 100755
@@ -630,14 +630,16 @@ build_target() {
 		output_dir="${OUTPUT_PREFIX}"
 	fi
 
+	GNU_MAKE=scripts/gnu_make
+
 	target_arch=$(get_target_arch ${target})
 	eval cross_toolchain=\$CROSS_COMPILE_`echo $target_arch | tr '[:lower:]' '[:upper:]'`
 	if [ "${cross_toolchain}" ] ; then
-	    MAKE="make CROSS_COMPILE=${cross_toolchain}"
+	    MAKE="$GNU_MAKE CROSS_COMPILE=${cross_toolchain}"
 	elif [ "${CROSS_COMPILE}" ] ; then
-	    MAKE="make CROSS_COMPILE=${CROSS_COMPILE}"
+	    MAKE="$GNU_MAKE CROSS_COMPILE=${CROSS_COMPILE}"
 	else
-	    MAKE=make
+	    MAKE=$GNU_MAKE
 	fi
 
 	if [  "${output_dir}" != "." ] ; then

Signature · 1 line

1.9.1

Jeroen Hofstee

2014-07-18 18:29 UTC #5 lore raw

PATCH 2/3 MAKEALL: make sure to invoke GNU Make

Hello Masahiro,

Quoted 23 lines · Masahiro Yamada

Since the command name 'make' is not GNU Make on some platforms such as FreeBSD, MAKEALL should call the make via scripts/gnu_make.

Signed-off-by Masahiro Yamada <[email protected]>

---

  MAKEALL | 8 +++++---
  1 file changed, 5 insertions(+), 3 deletions(-)

MAKEALL +2 -1

index 37ef71e..02c2f9f 100755
@@ -630,14 +630,16 @@ build_target() {
  		output_dir="${OUTPUT_PREFIX}"
  	fi
 
+	GNU_MAKE=scripts/gnu_make
+
  	target_arch=$(get_target_arch ${target})
  	eval cross_toolchain=\$CROSS_COMPILE_`echo $target_arch | tr '[:lower:]' '[:upper:]'`
  	if [ "${cross_toolchain}" ] ; then
-	    MAKE="make CROSS_COMPILE=${cross_toolchain}"

MAKEALL build_target() { · line 645

Quoted 10 lines

MAKEALL +3 -2

+	    MAKE="$GNU_MAKE CROSS_COMPILE=${cross_toolchain}"
  	elif [ "${CROSS_COMPILE}" ] ; then
-	    MAKE="make CROSS_COMPILE=${CROSS_COMPILE}"
+	    MAKE="$GNU_MAKE CROSS_COMPILE=${CROSS_COMPILE}"
  	else
-	    MAKE=make
+	    MAKE=$GNU_MAKE
  	fi
 
  	if [  "${output_dir}" != "." ] ; then

while by itself this might be correct, opt is not a POSIX command and MAKEALL will not work at all on a BSD host. Perhaps it is time I have a look at what buildman is....

Regards,
Jeroen

Simon Glass

2014-07-19 04:41 UTC #6 lore raw

PATCH 2/3 MAKEALL: make sure to invoke GNU Make

Hi Jeroen,

Quoted 34 lines · Jeroen Hofstee

Hello Masahiro,

Quoted 30 lines · Masahiro Yamada

Since the command name 'make' is not GNU Make on some platforms such as FreeBSD, MAKEALL should call the make via scripts/gnu_make.

Signed-off-by Masahiro Yamada <[email protected]>

---

  MAKEALL | 8 +++++---
  1 file changed, 5 insertions(+), 3 deletions(-)

MAKEALL +1 -0

index 37ef71e..02c2f9f 100755
@@ -630,14 +630,16 @@ build_target() {
                output_dir="${OUTPUT_PREFIX}"
        fi
  +     GNU_MAKE=scripts/gnu_make
+
        target_arch=$(get_target_arch ${target})
        eval cross_toolchain=\$CROSS_COMPILE_`echo $target_arch | tr

'[:lower:]' '[:upper:]'`

        if [ "${cross_toolchain}" ] ; then

MAKEALL +2 -3

-           MAKE="make CROSS_COMPILE=${cross_toolchain}"
+           MAKE="$GNU_MAKE CROSS_COMPILE=${cross_toolchain}"
        elif [ "${CROSS_COMPILE}" ] ; then
-           MAKE="make CROSS_COMPILE=${CROSS_COMPILE}"
+           MAKE="$GNU_MAKE CROSS_COMPILE=${CROSS_COMPILE}"
        else
-           MAKE=make

Quoted 8 lines

Quoted 3 lines
+ MAKE=$GNU_MAKE
        fi
        if [  "${output_dir}" != "." ] ; then
while by itself this might be correct, opt is not a POSIX command and MAKEALL will not work at all on a BSD host. Perhaps it is time I have a look at what buildman is....

http://patchwork.ozlabs.org/patch/369788/

:-)

Regards,
Simon

Masahiro Yamada

author

2014-07-17 09:18 UTC #7 lore raw

PATCH 3/3 buildman: make sure to invoke GNU Make

In reply to Masahiro Yamada

Since the command name 'make' is not GNU Make on some platforms such as FreeBSD, MAKEALL should call the make via scripts/gnu_make.

Signed-off-by Masahiro Yamada <[email protected]>

---

 tools/buildman/builder.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

tools/buildman/builder.py +2 -1

index 4a2d753..c68cc8c 100644
@@ -667,7 +667,8 @@ class Builder:
             args: Arguments to pass to make
             kwargs: Arguments to pass to command.RunPipe()
         """
-        cmd = ['make'] + list(args)
+        gnu_make = os.path.join(self.git_dir, '../scripts/gnu_make')
+        cmd = [gnu_make] + list(args)
         result = command.RunPipe([cmd], capture=True, capture_stderr=True,
                 cwd=cwd, raise_on_error=False, **kwargs)
         return result

Signature · 1 line

1.9.1

Simon Glass

2014-07-19 04:40 UTC #8 lore raw

PATCH 3/3 buildman: make sure to invoke GNU Make

Quoted 4 lines · Masahiro Yamada

Since the command name 'make' is not GNU Make on some platforms such as FreeBSD, MAKEALL should call the make via scripts/gnu_make.
Signed-off-by Masahiro Yamada <[email protected]>

Maybe you will rename the script, but the code looks good.

Acked-by Simon Glass <[email protected]>

Jeroen Hofstee

2014-07-20 18:13 UTC #9 lore raw

PATCH 3/3 buildman: make sure to invoke GNU Make

In reply to Masahiro Yamada

Hello Masahiro,

Quoted 14 lines · Masahiro Yamada

Since the command name 'make' is not GNU Make on some platforms such as FreeBSD, MAKEALL should call the make via scripts/gnu_make.
Signed-off-by Masahiro Yamada <[email protected]>
---
  tools/buildman/builder.py | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
tools/buildman/builder.py +0 -0
index 4a2d753..c68cc8c 100644

tools/buildman/builder.py class Builder: · line 674

Quoted 9 lines

tools/buildman/builder.py +2 -1

@@ -667,7 +667,8 @@ class Builder:
              args: Arguments to pass to make
              kwargs: Arguments to pass to command.RunPipe()
          """
-        cmd = ['make'] + list(args)
+        gnu_make = os.path.join(self.git_dir, '../scripts/gnu_make')
+        cmd = [gnu_make] + list(args)
          result = command.RunPipe([cmd], capture=True, capture_stderr=True,
                  cwd=cwd, raise_on_error=False, **kwargs)
          return result

with a trick to let buildman call clang, this works fine on FreeBSD.

Tested-by Jeroen Hofstee <[email protected]>

Regards,
Jeroen