Re: [dpdk-dev] [PATCH v6 1/1] ci: Introduce travis builds for github repositories

[Thomas Monjalon <thomas@monjalon.net>]

Hi,

04/03/2019 17:12, Michael Santana:
>  .ci/linux-build.sh                  | 21 +++++++++
>  .ci/linux-setup.sh                  |  3 ++
>  .travis.yml                         | 73 +++++++++++++++++++++++++++++

Please, could you explain somewhere what is the relationship
between these files?
What is specific to Travis?
What is specific to GitHub?

May we add "travis-" as filename prefix of the scripts?
Or rename .ci to .travis?

> +++ b/.ci/linux-build.sh
> @@ -0,0 +1,21 @@
> +#!/bin/bash -xe

If possible, I would prefer a simple /bin/sh.

> +function on_error() {
> +    FILES_TO_PRINT=( "build/meson-logs/testlog.txt" "build/.ninja_log" "build/meson-logs/meson-log.txt")
> +
> +    for pr_file in "${FILES_TO_PRINT[@]}"; do

You can make FILES_TO_PRINT as a simple word list,
and so avoid bashism.

[...]
> +if [ "${AARCH64}" == "1" ]; then

Please explain in the comment where this variable comes from.
I suggest renaming it to ARMV8 as this is what it is translated to:

> +    # convert the arch specifier
> +    OPTS="${OPTS} -DRTE_ARCH_64=1 --cross-file config/arm/arm64_armv8_linuxapp_gcc"

I think -DRTE_ARCH_64=1 is useless.

> +fi
> +
> +OPTS="$OPTS --default-library=$DEF_LIB"
> +meson build --werror -Dexamples=all ${OPTS}
> +ninja -C build
[...]
> --- /dev/null
> +++ b/.travis.yml
> @@ -0,0 +1,73 @@
> +language: c
> +compiler:
> +  - gcc
> +  - clang
> +
> +dist: xenial

Are we going to update the distribution frequently?
Why not adding more distros?

> +os:
> +  - linux

Is it possible to run on FreeBSD?

> +addons:
> +  apt:
> +    update: true
> +    packages:
> +      - [libnuma-dev, linux-headers-$(uname -r), python3-setuptools, python3-wheel, python3-pip, ninja-build]
> +
> +before_install: ./.ci/${TRAVIS_OS_NAME}-setup.sh
> +
> +sudo: false
> +
> +env:
> +  - DEF_LIB="static"
> +  - DEF_LIB="shared"
> +  - DEF_LIB="static" OPTS="-Denable_kmods=false"
> +  - DEF_LIB="shared" OPTS="-Denable_kmods=false"

How is it different of the matrix below?
Why testing disabling kmods?

> +
> +matrix:
> +  include:
> +  - env: DEF_LIB="static" OPTS="-Denable_kmods=false" AARCH64=1
> +    compiler: gcc
> +    addons:
> +      apt:
> +        packages:
> +          - [libnuma-dev, linux-headers-$(uname -r), python3-setuptools, python3-wheel, python3-pip, ninja-build]
> +          - [gcc-aarch64-linux-gnu, libc6-dev-arm64-cross]

Why packages are repeated here again?
(sorry, I don't know Travis and I want to understand)

> +  - env: DEF_LIB="shared" OPTS="-Denable_kmods=false" AARCH64=1
> +    compiler: gcc
> +    addons:
> +      apt:
> +        packages:
> +          - [libnuma-dev, linux-headers-$(uname -r), python3-setuptools, python3-wheel, python3-pip, ninja-build]
> +          - [gcc-aarch64-linux-gnu, libc6-dev-arm64-cross]
> +  - env: DEF_LIB="static"
> +    compiler: gcc
> +    addons:
> +      apt:
> +        packages:
> +          - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4]
> +          - [libnuma-dev, linux-headers-$(uname -r), python3-setuptools, python3-wheel, python3-pip, ninja-build]
> +  - env: DEF_LIB="shared"
> +    compiler: gcc
> +    addons:
> +      apt:
> +        packages:
> +          - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4]
> +          - [libnuma-dev, linux-headers-$(uname -r), python3-setuptools, python3-wheel, python3-pip, ninja-build]
> +  - env: DEF_LIB="static" OPTS="-Denable_kmods=false"
> +    compiler: gcc
> +    addons:
> +      apt:
> +        packages:
> +          - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4]
> +          - [libnuma-dev, linux-headers-$(uname -r), python3-setuptools, python3-wheel, python3-pip, ninja-build]
> +  - env: DEF_LIB="shared" OPTS="-Denable_kmods=false"
> +    compiler: gcc
> +    addons:
> +      apt:
> +        packages:
> +          - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4]
> +          - [libnuma-dev, linux-headers-$(uname -r), python3-setuptools, python3-wheel, python3-pip, ninja-build]

It seems clang is not in the matrix. Why?

Thanks for this v6.
I will be available to follow more closely in next days,
so we can merge this feature soon this week.