From mboxrd@z Thu Jan  1 00:00:00 1970
From: Bruce Richardson <bruce.richardson@intel.com>
Subject: Re: [PATCH v8 6/6] devtools: expand meson cross
 compiling coverage
Date: Fri, 15 Jun 2018 09:28:07 +0100
Message-ID: <20180615082807.GB23520@bricha3-MOBL.ger.corp.intel.com>
References: <20180615070811.18686-1-gavin.hu@arm.com>
 <20180615080120.29253-1-gavin.hu@arm.com>
 <20180615080120.29253-7-gavin.hu@arm.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: dev@dpdk.org, nd@arm.com, stable@dpdk.org
To: Gavin Hu <gavin.hu@arm.com>
Return-path: <dev-bounces@dpdk.org>
Content-Disposition: inline
In-Reply-To: <20180615080120.29253-7-gavin.hu@arm.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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.