From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [PATCH v5 2/2] ci: Introduce travis builds for github repositories Date: Wed, 27 Feb 2019 16:23:00 +0100 Message-ID: <2641130.kfz5e2tDNj@xps> References: <20190206221308.22349-1-msantana@redhat.com> <2766343.Bn91GvS4A1@xps> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: dev@dpdk.org, Bruce Richardson , Honnappa Nagarahalli To: Aaron Conole , Michael Santana Return-path: Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by dpdk.org (Postfix) with ESMTP id 4AE295A6E for ; Wed, 27 Feb 2019 16:23:04 +0100 (CET) In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 27/02/2019 15:35, Aaron Conole: > Thomas Monjalon 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?