[v8,6/6] devtools: expand meson cross compiling coverage

  The default test script covers only default host cc compiler, either gcc or
clang, the fix is to cover both, gcc and clang. And also the build dirs are
changed to *-host-$c, indicating the difference of cc used.

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 | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
  

On Fri, Jun 15, 2018 at 04:01:20PM +0800, Gavin Hu wrote:
> The default test script covers only default host cc compiler, either gcc or
> clang, the fix is to cover both, gcc and clang. And also the build dirs are
> changed to *-host-$c, indicating the difference of cc used.
> 
> 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 | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
> index 8447c704b..f75ebbdb1 100755
> --- a/devtools/test-meson-builds.sh
> +++ b/devtools/test-meson-builds.sh
> @@ -50,5 +50,15 @@ for f in config/arm/arm*gcc ; do
>  	if ! command -v $c >/dev/null 2>&1 ; then
>  		continue
>  	fi
> -	build build-$(basename $f | tr '_' '-' | cut -d'-' -f-2) --cross-file $f
> +    export CC="ccache gcc"
> +    build build-$(basename $f | tr '_' '-' | cut -d'-' -f-2)-host-gcc \
> +        --cross-file $f
> +    unset CC
> +    # compile the general v8a also for clang to increase coverage
> +    if [ $f = config/arm/arm64_armv8_linuxapp_gcc ] ; then
> +        export CC="ccache clang"
> +        build build-$(basename $f | tr '_' '-' | cut -d'-' -f-2)-host-clang \
> +            --cross-file $f
> +        unset CC
> +    fi

Indentation is different in the new code, spaces vs tabs, perhaps. I'm also
not sure the "unset CC" is needed, because I think the build function
automatically unsets it when done.

One other style comment: rather than having an if condition in the loop and
putting the extra build there, it might be easier just to put the extra
call to build outside the main loop, since all values are essentially
hardcoded in that call anyway.
  

Hi Bruce,

Thanks for your review, new v9 patch set was submitted.
Any more comments are welcome!

Best Regards,
Gavin

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Friday, June 15, 2018 4:28 PM
> To: Gavin Hu <Gavin.Hu@arm.com>
> Cc: dev@dpdk.org; nd <nd@arm.com>; stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v8 6/6] devtools: expand meson cross
> compiling coverage
> 
> On Fri, Jun 15, 2018 at 04:01:20PM +0800, Gavin Hu wrote:
> > The default test script covers only default host cc compiler, either
> > gcc or clang, the fix is to cover both, gcc and clang. And also the
> > build dirs are changed to *-host-$c, indicating the difference of cc used.
> >
> > 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 | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/devtools/test-meson-builds.sh
> > b/devtools/test-meson-builds.sh index 8447c704b..f75ebbdb1 100755
> > --- a/devtools/test-meson-builds.sh
> > +++ b/devtools/test-meson-builds.sh
> > @@ -50,5 +50,15 @@ for f in config/arm/arm*gcc ; do
> >  	if ! command -v $c >/dev/null 2>&1 ; then
> >  		continue
> >  	fi
> > -	build build-$(basename $f | tr '_' '-' | cut -d'-' -f-2) --cross-file $f
> > +    export CC="ccache gcc"
> > +    build build-$(basename $f | tr '_' '-' | cut -d'-' -f-2)-host-gcc \
> > +        --cross-file $f
> > +    unset CC
> > +    # compile the general v8a also for clang to increase coverage
> > +    if [ $f = config/arm/arm64_armv8_linuxapp_gcc ] ; then
> > +        export CC="ccache clang"
> > +        build build-$(basename $f | tr '_' '-' | cut -d'-' -f-2)-host-clang \
> > +            --cross-file $f
> > +        unset CC
> > +    fi
> 
> Indentation is different in the new code, spaces vs tabs, perhaps. I'm also not
> sure the "unset CC" is needed, because I think the build function
> automatically unsets it when done.
> 
> One other style comment: rather than having an if condition in the loop and
> putting the extra build there, it might be easier just to put the extra call to
> build outside the main loop, since all values are essentially hardcoded in that
> call anyway.