Re: [PATCH v2] Add travis-ci config file

[Daniel Kiper]

On Thu, Feb 14, 2019 at 02:12:28PM +0100, Alexander Graf wrote:
> There is a really convenient service for open source project from Travis
> CI: They allow for free CI testing using their infrastructure.
>
> Grub has had issues with broken builds for various targets for a long time
> already. The main reason is a lack of CI to just do smoke tests on whether
> all targets still at least compile.
>
> This patch adds a travis config file which builds (almost) all currently
> available targets.
>
> On top of that, this travis config also runs a small execution test on the
> x86_64-efi target.
>
> All of this config file can easily be extended further on. It probably makes
> sense to do something similar to the u-boot test infrastructure that
> communicates with the payload properly. Going forward, we also will want to
> do more qemu runtime checks for other targets.
>
> Currently, with this config alone, I already see about half of the available
> targets as broken. So it's definitely desperately needed :).
>
> Signed-off-by: Alexander Graf <address@hidden>
>
> ---
>
> v1 -> v2:
>
>   - Fix comment about toolchain variable
> ---
>  .travis.yml | 118 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 118 insertions(+)
>  create mode 100644 .travis.yml
>
> diff --git a/.travis.yml b/.travis.yml
> new file mode 100644
> index 000000000..8d6245ed4
> --- /dev/null
> +++ b/.travis.yml
> @@ -0,0 +1,118 @@
> +# SPDX-License-Identifier: GPL-3.0+
> +# Originally Copyright Roger Meier <address@hidden>
> +# Adapted for grub by Alexander Graf <address@hidden>

s/grub/GRUB/?

> +# build grub on Travis CI - https://travis-ci.org/

s/build grub/Build GRUB/?

> +dist: xenial
> +
> +language: c
> +
> +addons:
> +  apt:
> +    packages:
> +    - cppcheck
> +    - sloccount
> +    - sparse
> +    - bc
> +    - build-essential
> +    - libsdl1.2-dev
> +    - python
> +    - python-virtualenv
> +    - swig
> +    - libpython-dev
> +    - iasl
> +    - rpm2cpio
> +    - wget
> +    - device-tree-compiler
> +    - lzop
> +    - liblz4-tool
> +    - libisl15
> +    - qemu-system
> +    - ovmf
> +    - unifont

Do we really need all that stuff? sloccount? iasl? rpm2cpio? And maybe others?

> +env:
> +  global:
> +    - 
> PATH=/tmp/qemu-install/bin:/tmp/grub/bin:/usr/bin:/bin:/tmp/cross/gcc-8.1.0-nolibc/aarch64-linux/bin:/tmp/cross/gcc-8.1.0-nolibc/arm-linux-gnueabi/bin:/tmp/cross/gcc-8.1.0-nolibc/ia64-linux/bin:/tmp/cross/gcc-8.1.0-nolibc/mips64-linux/bin:/tmp/cross/gcc-8.1.0-nolibc/powerpc64-linux/bin:/tmp/cross/gcc-8.1.0-nolibc/riscv32-linux/bin:/tmp/cross/gcc-8.1.0-nolibc/riscv64-linux/bin:/tmp/cross/gcc-8.1.0-nolibc/sparc64-linux/bin

Ugh... Is it minimal PATH required to do our tests? Really? Could we
trim it down? If not I will ask you to say in the comment why we need
so big PATH.

> +before_script:
> +  # install necessary toolchains based on $CROSS_TARGETS variable

I know that there is a fashion to start everything with small latter but
I prefer proper full sentences with a full stop at the end.

> +  - mkdir /tmp/cross
> +  # results in binaries like 
> /tmp/cross/gcc-8.1.0-nolibc/ia64-linux/bin/ia64-linux-gcc
> +  - for i in $CROSS_TARGETS; do
> +        ( cd /tmp/cross; wget -O - 
> https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/8.1.0/x86_64-gcc-8.1.0-nolibc-$i.tar.xz
>  | xzcat | tar x );

Should not we add e.g. -t 3 to wget command line?

s/mirrors.edge.kernel.org/mirrors.kernel.org/?
s/xzcat | tar x/tar xJ/

And should not we verify signatures of
x86_64-gcc-8.1.0-nolibc-$i.tar.xz?

And why we need "(...)", brackets here?

> +    done
> +
> +script:
> +  # Comments must be outside the command strings below, or the Travis parser
> +  # will get confused.
> +  - ./autogen.sh
> +
> +  # Build all selected grub targets:
> +  - for target in $GRUB_TARGETS; do
> +      plat=${target#*-};
> +      arch=${target%-*};
> +      [ "$arch" = "arm64" ] && arch=aarch64-linux;
> +      [ "$arch" = "arm" ] && arch=arm-linux-gnueabi;
> +      [ "$arch" = "ia64" ] && arch=ia64-linux;
> +      [ "$arch" = "mipsel" ] && arch=mips64-linux;
> +      [ "$arch" = "powerpc" ] && arch=powerpc64-linux;
> +      [ "$arch" = "riscv32" ] && arch=riscv32-linux;
> +      [ "$arch" = "riscv64" ] && arch=riscv64-linux;
> +      [ "$arch" = "sparc64" ] && arch=sparc64-linux;
> +      echo "Building $target";
> +      mkdir obj-$target;
> +      ( cd obj-$target && ../configure --target=$arch --with-platform=$plat 
> --prefix=/tmp/grub && make -j4 && make -j4 install ) &> log || ( cat log; 
> false );

Are you sure that each build environment has 3 CPUs available? Could you
use "getconf _NPROCESSORS_ONLN" or nproc to get number of CPUs
available? I use something like that:

JOBS=`getconf _NPROCESSORS_ONLN 2> /dev/null || echo 1`
JOBS=`if [ "$JOBS" -gt 1 ]; then expr "$JOBS" + 1; else echo 1; fi`

The downside of using that is that build messages may not be aligned
with relevant commands. Do we care about that here? To some extent
probably yes.

> +    done
> +
> +  # Our test canary
> +  - echo -e "insmod echo\\ninsmod reboot\\necho hello world\\nreboot" > 
> grub.cfg
> +
> +  # Assemble images and possibly run them
> +  - for target in $GRUB_TARGETS; do grub-mkimage -c grub.cfg -p / -O $target 
> -o grub-$target echo reboot normal; done
> +
> +  # Run images we know how to run
> +  - if [[ "$GRUB_TARGETS" == *"x86_64-efi"* ]]; then qemu-system-x86_64 
> -bios /usr/share/ovmf/OVMF.fd -m 512 -no-reboot -nographic -net nic -net 
> user,tftp=.,bootfile=grub-x86_64-efi | tee grub.log && grep "hello world" 
> grub.log; fi
> +
> +matrix:
> +  include:
> +  # each env setting here is a dedicated build
> +    - name: "x86_64"
> +      env:
> +        - GRUB_TARGETS="x86_64-efi x86_64-xen"
> +    - name: "i386"
> +      env:
> +        - GRUB_TARGETS="i386-efi i386-xen i386-xen_pvh i386-pc 
> i386-multiboot i386-coreboot i386-ieee1275 i386-qemu"

May we have this sorted in alphabetic order?
And i386-emu seems missing to me.

> +    - name: "powerpc"
> +      env:
> +        - GRUB_TARGETS="powerpc-ieee1275"
> +        - CROSS_TARGETS=powerpc64-linux"
> +    - name: "sparc64"
> +      env:
> +        - GRUB_TARGETS="sparc64-ieee1275"
> +        - CROSS_TARGETS=sparc64-linux"
> +    - name: "ia64"
> +      env:
> +        - GRUB_TARGETS="ia64-efi"
> +        - CROSS_TARGETS=ia64-linux"
> +    - name: "mips"
> +      env:
> +        - GRUB_TARGETS="mips-qemu_mips mips-arc mipsel-arc mipsel-qemu_mips"

May we have this sorted in alphabetic order?

> +        - CROSS_TARGETS=mips64-linux"
> +    - name: "arm"
> +      env:
> +        - GRUB_TARGETS="arm-uboot arm-coreboot arm-efi"

Ditto.

Daniel