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

[Thomas Monjalon <thomas@monjalon.net>]

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.

> 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.

> >> +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.

> >> +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.

> >> +    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.

> >> +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?

> >> +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?