Re: [PATCH 2/3] gitlab-ci.yml: Add a job to build EDK2 firmware binaries

[Philippe Mathieu-Daudé]

On 1/7/20 11:12 AM, Laszlo Ersek wrote:
> On 01/06/20 19:46, Philippe Mathieu-Daudé wrote:
> > Add a GitLab job to build the EDK2 firmware binaries.
> > This job is only built when the roms/edk2/ submodule is updated,
> > when a git-ref starts with 'edk2' or when the last commit contains
> > 'EDK2'.
>
> keyword "or"; okay.
>
> > GitLab CI generates an artifacts.zip file containing the firmware
> > binaries.
> >
> > With edk2-stable201905, the job took 40 minutes 26 seconds,
> > the artifacts.zip takes 10MiB.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> > ---
> >   .gitlab-ci-edk2.yml | 37 +++++++++++++++++++++++++++++++++++++
> >   .gitlab-ci.yml      |  3 +++
> >   MAINTAINERS         |  3 ++-
> >   3 files changed, 42 insertions(+), 1 deletion(-)
> >   create mode 100644 .gitlab-ci-edk2.yml
> >
> > diff --git a/.gitlab-ci-edk2.yml b/.gitlab-ci-edk2.yml
> > new file mode 100644
> > index 0000000000..abfaf52874
> > --- /dev/null
> > +++ b/.gitlab-ci-edk2.yml
> > @@ -0,0 +1,37 @@
> > +build-edk2:
> > + rules: # Only run this job when ...
> > + - changes: # ... roms/edk2/ is modified (submodule updated)
> > +   - roms/edk2/*
> > +   when: always
> > + - if: '$CI_COMMIT_REF_NAME =~ /^edk2/' # ... the branch/tag starts with 'edk2'
>
> (1) can you add "or" in the comment here?
>
> > +   when: always
> > + - if: '$CI_COMMIT_MESSAGE =~ /edk2/i' # last commit description contains 
> > 'EDK2'
>
> (2) ditto
>
> > +   when: always
> > + artifacts:
> > +   paths: # 'artifacts.zip' will contains the following files:
> > +   - pc-bios/edk2*bz2
> > +   - pc-bios/edk2-licenses.txt
> > +   - edk2-stdout.log
> > +   - edk2-stderr.log
> > + image: ubuntu:16.04 # Use Ubuntu Xenial
> > + before_script: # Install packages requiered to build EDK2
> > + - apt-get update --quiet --quiet
> > + - DEBIAN_FRONTEND=noninteractive
> > +   apt-get install --assume-yes --no-install-recommends --quiet --quiet
> > +     build-essential
> > +     ca-certificates
> > +     dos2unix
> > +     gcc-aarch64-linux-gnu
> > +     gcc-arm-linux-gnueabi
> > +     git
> > +     iasl
> > +     make
> > +     nasm
> > +     python
> > +     uuid-dev
> > + script: # Clone the required submodules and build EDK2
> > + - git submodule update --init roms/edk2
>
> yes, this is needed; qemu users are used to updating top-level
> submodules (which is why we didn't try to automate that away in the edk2
> build stuff)
>
> > + - git -C roms/edk2 submodule update --init
>
> (3) but this should not be necessary. See the "submodules" target in
> "roms/Makefile.edk2".

Hmm build fails without it:
https://gitlab.com/philmd/qemu/-/jobs/395644357#L436

The 'test -d edk2/.git' might be not enough?

> > + - export JOBS=$(($(getconf _NPROCESSORS_ONLN) + 1))
> > + - echo "=== Using ${JOBS} simultaneous jobs ==="
> > + - make -j${JOBS} -C roms efi 1>edk2-stdout.log 2> >(tee -a edk2-stderr.log 
> > >&2)
>
> Process substitution is a nifty feature, but perhaps we can do without
> it, for simplicity. (I realize this is bash-only; I just like to
> minimize the use of non-portable features if there is a portable
> replacement that is also simple.)
>
> Redirections are processed in the order they appear on the command line
> [1], *after* stdout/stdin is redirected for pipelining [2]:
>
> [1]
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_07
>
> "If more than one redirection operator is specified with a command, the
> order of evaluation is from beginning to end."
>
> [2]
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_02
>
> "The standard input, standard output, or both of a command shall be
> considered to be assigned by the pipeline before any redirection
> specified by redirection operators that are part of the command"
>
>
> (4) Therefore, the following should work:
>
>    make -j${JOBS} -C roms efi 2>&1 1>edk2-stdout.log \
>    | tee -a edk2-stderr.log >&2
>
> Untested, of course :)

This works like charm :>

> Looks OK otherwise.

Thanks for the review!

> Thanks!
> Laszlo