[v6,5/7] devtools: fix the missing ninja command error on CentOS

Message ID 20180614095127.16245-6-gavin.hu@arm.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series *** fix the cross compile errors *** |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Gavin Hu June 14, 2018, 9:51 a.m. UTC
  On CentOS, the ninja executable has a different name:
ninja-build, this patch is to fix the missing command error
on CentOS as follows:
./devtools/test-meson-builds.sh: line 24: ninja: command not found

Fixes: a55277a788 ("devtools: add test script for meson builds")
Cc: stable@dpdk.org

Signed-off-by: Gavin Hu <gavin.hu@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Song Zhu <song.zhu@arm.com>
---
 devtools/test-meson-builds.sh | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
  

Comments

Bruce Richardson June 14, 2018, 10:40 a.m. UTC | #1
On Thu, Jun 14, 2018 at 05:51:25PM +0800, Gavin Hu wrote:
> On CentOS, the ninja executable has a different name:
> ninja-build, this patch is to fix the missing command error
> on CentOS as follows:
> ./devtools/test-meson-builds.sh: line 24: ninja: command not found
> 
> Fixes: a55277a788 ("devtools: add test script for meson builds")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>
> Reviewed-by: Song Zhu <song.zhu@arm.com>
> ---
>  devtools/test-meson-builds.sh | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
> index 6bce3df7f..4afac76dd 100755
> --- a/devtools/test-meson-builds.sh
> +++ b/devtools/test-meson-builds.sh
> @@ -21,7 +21,11 @@ build () # <directory> <meson options>
>  		$MESON $options $srcdir $builddir
>  	fi
>  	echo "ninja -C $builddir"
> -	ninja -C $builddir
> +	if [ "$(lsb_release -d | grep -c 'CentOS')" != "0" ] ; then
> +		ninja-build -C $builddir
> +	else
> +		ninja -C $builddir
> +	fi
>  }

Rather than tying this to CentOS explicitly, would it be better at the
start of the script to test e.g "which ninja" and "which ninja-build" and
use that to work out the command to use. It's possible to have ninja
installed directly from tarball on CentOS as "ninja" (as I have in my test
VM), and the binary might be called ninja-build on other systems too e.g.
RHEL, perhaps.

/Bruce
  
Gavin Hu June 15, 2018, 8:08 a.m. UTC | #2
> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Thursday, June 14, 2018 6:41 PM
> To: Gavin Hu <Gavin.Hu@arm.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v6 5/7] devtools: fix the missing ninja
> command error on CentOS
>
> On Thu, Jun 14, 2018 at 05:51:25PM +0800, Gavin Hu wrote:
> > On CentOS, the ninja executable has a different name:
> > ninja-build, this patch is to fix the missing command error on CentOS
> > as follows:
> > ./devtools/test-meson-builds.sh: line 24: ninja: command not found
> >
> > Fixes: a55277a788 ("devtools: add test script for meson builds")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> > Reviewed-by: Phil Yang <phil.yang@arm.com>
> > Reviewed-by: Song Zhu <song.zhu@arm.com>
> > ---
> >  devtools/test-meson-builds.sh | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/devtools/test-meson-builds.sh
> > b/devtools/test-meson-builds.sh index 6bce3df7f..4afac76dd 100755
> > --- a/devtools/test-meson-builds.sh
> > +++ b/devtools/test-meson-builds.sh
> > @@ -21,7 +21,11 @@ build () # <directory> <meson options>
> >  $MESON $options $srcdir $builddir
> >  fi
> >  echo "ninja -C $builddir"
> > -ninja -C $builddir
> > +if [ "$(lsb_release -d | grep -c 'CentOS')" != "0" ] ; then
> > +ninja-build -C $builddir
> > +else
> > +ninja -C $builddir
> > +fi
> >  }
>
> Rather than tying this to CentOS explicitly, would it be better at the start of
> the script to test e.g "which ninja" and "which ninja-build" and use that to
> work out the command to use. It's possible to have ninja installed directly
> from tarball on CentOS as "ninja" (as I have in my test VM), and the binary
> might be called ninja-build on other systems too e.g.
> RHEL, perhaps.
>
> /Bruce
[Gavin Hu] I submitted new v8 patch for this, please help review it.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
  

Patch

diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
index 6bce3df7f..4afac76dd 100755
--- a/devtools/test-meson-builds.sh
+++ b/devtools/test-meson-builds.sh
@@ -21,7 +21,11 @@  build () # <directory> <meson options>
 		$MESON $options $srcdir $builddir
 	fi
 	echo "ninja -C $builddir"
-	ninja -C $builddir
+	if [ "$(lsb_release -d | grep -c 'CentOS')" != "0" ] ; then
+		ninja-build -C $builddir
+	else
+		ninja -C $builddir
+	fi
 }
 
 # shared and static linked builds with gcc and clang