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

[Aaron Conole <aconole@redhat.com>]

Thomas Monjalon <thomas@monjalon.net> writes:

> 27/02/2019 15:35, Aaron Conole:
>> Thomas Monjalon <thomas@monjalon.net> writes:
>> > 07/02/2019 23:01, Michael Santana:
>> >> +# Just used for the 'classic' configuration system (ie: make)
>> >
>> > I am not sure about supporting the legacy system in a new CI.
>> 
>> For now, documentation all says that the legacy system is the supported
>> system.  I think it's appropriate to continue to support it until such
>> time as it is eliminated.  Otherwise, when an end user builds we don't
>> know what to say about support - IE: if they have problems with the
>> classic system for whatever reason, do we tell them "sorry, we cannot
>> help"?
>
> It is tested by others and me.

I test it, too. :)

>> The patches.rst mentions that all patches must pass with the Makefile
>> system, and the contributing/documentation.rst calls it the "standard
>> DPDK build system."  If you want to change those things to reflect
>> something different please do, and we can drop all of the stuff related
>> to it, but until that time we won't.
>
> If it is forcing to have a translation of options, I think it is better
> to skip the legacy system in this CI.

We can do that.

>> >> +BUILD_ARCH="x86_64-native-linuxapp-"
>> >
>> > We could have some native Arm compilation.
>> 
>> Sure.  Is this just commentary?  Do you suggest a change here?  This is
>> a default, and will be adjusted later by other parameters.
>
> OK, I missed it is the default. Maybe a comment would help.

Okay.

>> >> +if [ "${AARCH64}" == "1" ]; then
>> >> +    # convert the arch specifier
>> >> +    BUILD_ARCH="arm64-armv8a-linuxapp-"
>> >> +    AARCH64_TOOL="linaro-arm-tool"
>> >
>> > What is this directory? It looks really specific to Travis.
>> 
>> It's specific to the AARCH64 toolchain that was pulled in as part of
>> linux-prepare.sh - do you think something should change?
>
> I think it should be a parameter somewhere else. In the .yml file?
>
>> >> +    DEF_LIB="static"
>> >> +    if [ "${SHARED}" == "1" ]; then
>> >> +        DEF_LIB="shared"
>> >> +    fi
>> >> +
>> >> +    if [ "${DISABLE_KERNEL_MODULES}" == "1" ]; then
>> >> +        OPTS="-Denable_kmods=false"
>> >> +    fi
>> >
>> > Isn't it possible to directly provide the meson options in travis.yml
>> > instead of doing a translation with new option names?
>> 
>> Yes and no.  It would be possible, but we try to support both build
>> systems and they have different options afaict.  So we need something
>> common.  Maybe we missed something?
>
> It was my understanding.
> That's why I think we should drop the legacy support
> and focus on a simpler meson support.

Okay.

>> >> +    set_conf build CONFIG_RTE_KNI_KMOD n
>> >> +    set_conf build CONFIG_RTE_EAL_IGB_UIO n
>> >
>> > Why these options are fixed?
>> 
>> There was a problem with the Travis system when trying to build some of
>> the kernel modules, but I don't remember the details.  Maybe Michael does.
>
> OK, please add a comment if kept.

I think the new plan is to drop it.

>> >> +python3.5 -m pip install --upgrade meson --user
>> >
>> > Which distributions have python3.5?
>> > It looks very specific.
>> 
>> I agree, could probably just be python3 we need to check and see if we
>> can just use that.
>> 
>> But, we did need the upgrade.
>> 
>> Travis environment comes up with ubuntu 14.04, which includes python3.4,
>> and the requisite version of meson needs python3.5 or higher.
>
> It could be a python --version check then?

Well, I don't know what the purpose of the version check would be.  We
can tailor the environments.  Just need to pass the right information,
that's all.  I think we'll make it look nicer, though.  We can do any
environment specific fix ups in the prepare scripts, which I think makes
sense.  Then the build script can be a bit more generic.

>> >> +if [ "${AARCH64}" == "1" ]; then
>> >> +    # need to build & install libnuma
>> >
>> > Why is it needed? linbnuma is optional.
>> > I think this file can be dropped or renamed to
>> > something like install-libnuma-for-cross-arm.sh
>> 
>> We needed this because it broke the meson build when cross compiling,
>> IIRC.  I will investigate it further to be sure.
>
> OK thanks
>
>> >> +notifications:
>> >> +  email:
>> >> +    recipients:
>> >> +      - test-report@dpdk.org
>> >
>> > The idea of this mailing list is to receive reports about
>> > the upstream development. When doing a private development,
>> > reports should not be sent. How can it be disabled?
>> 
>> There are a few variables related to controlling this that we can
>> investigate so that we only alert on the main repository, if you want.
>> 
>> We could also just remove it.  Not a problem either way.
>
> I think it should be disabled by default,
> and documented how to enable it.
>
> Then we may need a way to filter which travis report should be allowed
> in the mailing list, in order to avoid flooding it.
> I think we cannot filter by sender. Do we need procmail?

Maybe it would make sense to have a build list that we can use?  Then we
just let that absorb all the various build from different CI services.
Just thinking out loud.  That way, anyone interested in the build status
can look there, and it keeps the test-report clean.  Again, just a
half-baked idea.