From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 2C537A00E6 for ; Wed, 20 Mar 2019 20:28:56 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 0AB241B20B; Wed, 20 Mar 2019 20:28:55 +0100 (CET) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id D86161B1F5 for ; Wed, 20 Mar 2019 20:28:53 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1AC7781E1A; Wed, 20 Mar 2019 19:28:53 +0000 (UTC) Received: from localhost.localdomain (unknown [10.18.25.8]) by smtp.corp.redhat.com (Postfix) with ESMTP id 402995D71C; Wed, 20 Mar 2019 19:28:52 +0000 (UTC) To: Thomas Monjalon , Aaron Conole Cc: dev@dpdk.org, Bruce Richardson , Honnappa Nagarahalli References: <20190207220114.8020-1-msantana@redhat.com> <20190304161232.5670-1-msantana@redhat.com> <20190304161232.5670-2-msantana@redhat.com> <9486436.3HrUK5duha@xps> From: Michael Santana Francisco Organization: Red Hat Message-ID: <8cc57d73-bb0e-7f42-aa45-ffc5ab6abbf7@redhat.com> Date: Wed, 20 Mar 2019 15:28:50 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: <9486436.3HrUK5duha@xps> Content-Type: text/plain; charset="UTF-8"; format="flowed" Content-Transfer-Encoding: 7bit Content-Language: en-US X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Wed, 20 Mar 2019 19:28:53 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH v6 1/1] ci: Introduce travis builds for github repositories X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list Reply-To: msantana@redhat.com List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190320192850.0VbQ-F_Cb-5fej1djWqstGFZ7B6yXOVJc5gYmQiNscI@z> Thank you for taking at look at this On 3/20/19 12:01 PM, Thomas Monjalon wrote: > 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? Only the .travis.yml is specific to travis. The other two files are used by travis, but are independent of travis. This allows us for in the future to change travis as CI and use something else instead, or add another CI in addition to travis if we wanted to. Other CI's would just run these scripts just like travis does. With that said, I would not change the names, but if you really think they should then it's not a problem >> +++ 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. Will look into it > > [...] >> +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: The variable comes from travis. If you look at the matrix in the travis.yml you will see lines containing environment variables like AARCH64=1. These lines tell the travis build job to explicitly export these variables so that they can be used by the CI scripts like this one. As for ARMV8 someone had asked specifically to be named AARCH64 > >> + # 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? The only Linux distribution travis supports is Ubuntu, and the latest Ubuntu version they support is xenial which is code name for Ubuntu 16.04 > >> +os: >> + - linux > Is it possible to run on FreeBSD? Not that I am aware. Travis only supports Ubuntu, Windows, and Mac > >> +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? This list inherits the packages from the package list above. The main difference is that this list does not contain extra packages such as libbsd-dev, libpcap-dev, etc, where as the matrix below does. This is so that we have a series of builds with minimal libraries installed (Minimal build) and another series of builds with many libraries installed (Full build). We want to mix and match all the possible build scenarios and see if a new changes breaks any of the build cases. The more builds we have with the many different configurations we can have the wider the net we can cast to ensure everything is working as it should > >> + >> +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) Yeah, we don't want to repeat ourselves either but we have no choice. This is due to a limitation in travis. This matrix does not inherit any packages from the main package list way above, which means we have to list them out manually here. In addition to the required packages we also want to install full builds with libraries like libbsd-dev, libpcap-dev, etc. We could of just put those libraries in the main package list above and put all the builds in the env: list because then the libraries would be inherited. The problem with that is that is that travis would not keep minimal builds and full builds separate. We could not have minimal builds because the minimal builds will also inherit the additional libraries; Meson will then automatically detect those additional libraries and builds with them. What we would like to have is a way to tell meson which libraries we want to use and which we dont, instead of being auto-detected. This would help us to get rid of this matrix. If someone knows a better way to do this we would greatly take in your ideas, but so far this is the best we could come up with > >> + - 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? It looks like my mistake, will look into it > > Thanks for this v6. > I will be available to follow more closely in next days, > so we can merge this feature soon this week. > >