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

[Aaron Conole <aconole@redhat.com>]

Bruce Richardson <bruce.richardson@intel.com> writes:

> On Wed, Jan 30, 2019 at 05:16:02PM -0500, Michael Santana wrote:
>> GitHub is a service used by developers to store repositories.  GitHub
>> provides service integrations that allow 3rd party services to access
>> developer repositories and perform actions.  One of these services is
>> Travis-CI, a simple continuous integration platform.
>> 
>> This is a simple initial implementation of a travis build for the DPDK
>> project.  It doesn't require any changes from individual developers to
>> enable, but will allow those developers who opt-in to GitHub and the
>> travis service to get automatic builds for every push they make.
>> 
>> Additionally, the travis service will send an email to the test-report
>> list informing anyone interested in the automated build (including a
>> result).
>> 
>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>> Signed-off-by: Michael Santana <msantana@redhat.com>
>> ---
>
> A few comments inline below.
>
>> v2:
>>   - Added aarch64 build
>>   - Added multiple meson options for builds
>>   - Added multiple make/config options for builds
>> 
>>  .ci/linux-build.sh                  |  88 +++++++++++++++
>>  .ci/linux-setup.sh                  |  31 ++++++
>>  .travis.yml                         | 159 ++++++++++++++++++++++++++++
>>  MAINTAINERS                         |   7 ++
>>  doc/guides/contributing/patches.rst |   4 +
>>  meson_cross_aarch64_gcc.txt         |  12 +++
>>  6 files changed, 301 insertions(+)
>>  create mode 100755 .ci/linux-build.sh
>>  create mode 100755 .ci/linux-setup.sh
>>  create mode 100644 .travis.yml
>>  create mode 100644 meson_cross_aarch64_gcc.txt
>>
>  
> <snip>
>
>> +
>> +if [ "${NINJABUILD}" == "1" ]; then
>> +    OPTS=""
>> +
>> +    DEF_LIB="static"
>> +    if [ "${SHARED}" == "1" ]; then
>> +        DEF_LIB="shared"
>> +    fi
>> +
>> +    if [ "${KERNEL}" == "1" ]; then
>> +        OPTS="-Denable_kmods=false"
>> +    fi
>
> Is this condition correct? If kernel is set to "1" you want to disable
> module builds? I think it should be the other way around.

For meson, it's defaulted on (IIUC):

option('enable_kmods', type: 'boolean', value: true,
	description: 'build kernel modules')

So this will shut it off.  Maybe the test should have been for KERNEL !=
"1", though.  Okay, will adjust.

>> +
>> +    if [ "${ARM64}" == "1" ]; then
>> +        OPTS="${OPTS} --cross-file meson_cross_aarch64_${CC}.txt"
>> +    fi
>> +
>
> <snip>
>
>> diff --git a/.travis.yml b/.travis.yml
>> new file mode 100644
>> index 000000000..f296d6914
>> --- /dev/null
>> +++ b/.travis.yml
>> @@ -0,0 +1,159 @@
>> +language: c
>> +compiler:
>> +  - gcc
>> +  - clang
>> +
>> +os:
>> +  - linux
>> +
>> +addons:
>> +  apt:
>> +    sources:
>> +      - deadsnakes #source for python 3.5
>
> Do we really need python sources? Are the binaries not enough?

The sources section is for package repository configurations.  I agree,
I'm not sure why this is needed, but probably it has to do with some
travis container limitation (I don't know if the right python version is
available by default).

>> +      - sourceline: 'ppa:mstipicevic/ninja-build-1-7-2'
>> +    packages:
>> +      - [libnuma-dev, linux-headers-$(uname -r), python3.5, python3-pip, ninja-build]
>> +
>> +before_install: ./.ci/${TRAVIS_OS_NAME}-setup.sh
>> +
>
> <snip>
>
>> diff --git a/meson_cross_aarch64_gcc.txt b/meson_cross_aarch64_gcc.txt
>> new file mode 100644
>> index 000000000..aee167d13
>> --- /dev/null
>> +++ b/meson_cross_aarch64_gcc.txt
>> @@ -0,0 +1,12 @@
>> +[binaries]
>> +c = 'aarch64-linux-gnu-gcc'
>> +cpp = 'aarch64-linux-gnu-g++'
>> +ar = 'aarch64-linux-gnu-gcc-ar'
>> +strip = 'aarch64-linux-gnu-strip'
>> +pkgconfig = 'aarch64-linux-gnu-pkg-config'
>> +
>> +[host_machine]
>> +system = 'linux'
>> +cpu_family = 'aarch64'
>> +cpu = 'aarch64'
>> +endian = 'little'
>> -- 
>
> This looks very similar to the "config/arm/arm64_armv8_linuxapp_gcc" file.
> I suggest that file be used instead of adding a new one.

Agreed, we can drop this.  I completely missed the configurations in
that directory.

Thanks again, Bruce!

> /Bruce