[PATCH 0/3] gitlab-ci: Add a job to build EDK2 firmware binaries

[PATCH 0/3] gitlab-ci: Add a job to build EDK2 firmware binaries

[Philippe Mathieu-Daudé]

We provide the EDK2 firmware binaries in pc-bios/. When we
update the roms/edk2/ submodule, we need to rebuild the
firmware binaries.
To avoid the burden on a single developer, this series add
a GitLab job to generate reproducible EDK2 firmware binaries.

The job is only on specific conditions:
- roms/edk2/ is updated
- the branch or tag start with 'edk2'
- 'edk2' appears in last commit description

Using ccache this job takes 32minutes on a GitLab free runner.

The proposed procedure to update the EDK2 submodule is:

- add a commit to update roms/edk2/ submodule
- push to gitlab
- download generated artifacts (only available if job succeed)
- unzip the firmware binaries from the artifacts archive
- test the binaries
- add a commit with the binaries, referencing the ci job url.

Example of job that built the edk2-stable201905 firmwares:
https://gitlab.com/philmd/qemu/-/jobs/395017298

The first patch is already reviewed, but is a prerequisite to
use the Ubuntu docker image to build, so I included it.

Regards,

Phil.

Philippe Mathieu-Daudé (3):
  roms/edk2-funcs: Force softfloat ARM toolchain prefix on Debian
  gitlab-ci.yml: Add a job to build EDK2 firmware binaries
  gitlab-ci-edk2.yml: Use ccache

 .gitlab-ci-edk2.yml | 48 +++++++++++++++++++++++++++++++++++++++++++++
 .gitlab-ci.yml      |  3 +++
 MAINTAINERS         |  3 ++-
 roms/edk2-funcs.sh  |  3 +++
 4 files changed, 56 insertions(+), 1 deletion(-)
 create mode 100644 .gitlab-ci-edk2.yml

-- 
2.21.1

[PATCH 1/3] roms/edk2-funcs: Force softfloat ARM toolchain prefix on Debian

[Philippe Mathieu-Daudé]

The Debian (based) distributions currently provides 2 ARM
toolchains, documented as [1]:

* The ARM EABI (armel) port targets a range of older 32-bit ARM
  devices, particularly those used in NAS hardware and a variety
  of *plug computers.
* The newer ARM hard-float (armhf) port supports newer, more
  powerful 32-bit devices using version 7 of the ARM architecture
  specification.

For various reasons documented in [2], the EDK2 project suggests
to use the softfloat toolchain (named 'armel' by Debian).

Force the softfloat cross toolchain prefix on Debian distributions.

[1] https://www.debian.org/ports/arm/#status
[2] https://github.com/tianocore/edk2/commit/41203b9a

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v3: fixed s/hard/float/ typo (Laszlo)
---
 roms/edk2-funcs.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/roms/edk2-funcs.sh b/roms/edk2-funcs.sh
index 3f4485b201..cd6e4f2c82 100644
--- a/roms/edk2-funcs.sh
+++ b/roms/edk2-funcs.sh
@@ -112,6 +112,9 @@ qemu_edk2_get_cross_prefix()
      ( [ "$gcc_arch" == i686 ] && [ "$host_arch" == x86_64 ] ); then
     # no cross-compiler needed
     :
+  elif ( [ -e /etc/debian_version ] && [ "$gcc_arch" == arm ] ); then
+    # force soft-float cross-compiler on Debian
+    printf 'arm-linux-gnueabi-'
   else
     printf '%s-linux-gnu-\n' "$gcc_arch"
   fi
-- 
2.21.1

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

[Philippe Mathieu-Daudé]

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

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é <philmd@redhat.com>
---
 .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'
+   when: always
+ - if: '$CI_COMMIT_MESSAGE =~ /edk2/i' # last commit description contains 'EDK2'
+   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
+ - git -C roms/edk2 submodule update --init
+ - 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)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index ebcef0ebe9..f799246047 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -1,3 +1,6 @@
+include:
+  - local: '/.gitlab-ci-edk2.yml'
+
 before_script:
  - apt-get update -qq
  - apt-get install -y -qq flex bison libglib2.0-dev libpixman-1-dev genisoimage
diff --git a/MAINTAINERS b/MAINTAINERS
index 8571327881..22a1fd5824 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2346,6 +2346,7 @@ F: roms/edk2
 F: roms/edk2-*
 F: tests/data/uefi-boot-images/
 F: tests/uefi-test-tools/
+F: .gitlab-ci-edk2.yml
 
 Usermode Emulation
 ------------------
@@ -2689,7 +2690,7 @@ W: https://cirrus-ci.com/github/qemu/qemu
 GitLab Continuous Integration
 M: Thomas Huth <thuth@redhat.com>
 S: Maintained
-F: .gitlab-ci.yml
+F: .gitlab-ci*.yml
 
 Guest Test Compilation Support
 M: Alex Bennée <alex.bennee@linaro.org>
-- 
2.21.1

[PATCH 3/3] gitlab-ci-edk2.yml: Use ccache

[Philippe Mathieu-Daudé]

By using ccache we reduce the job duration from
40 minutes 26 seconds to 32 minutes 6 seconds.

  Running after script...
  $ ccache --show-stats
  cache hit (direct)                  6604
  files in cache                     12090
  cache size                         335.5 MB

For now downloading this cache takes 16 seconds, archiving
it 44 seconds.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 .gitlab-ci-edk2.yml | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/.gitlab-ci-edk2.yml b/.gitlab-ci-edk2.yml
index abfaf52874..329ba24f20 100644
--- a/.gitlab-ci-edk2.yml
+++ b/.gitlab-ci-edk2.yml
@@ -14,12 +14,19 @@ build-edk2:
    - edk2-stdout.log
    - edk2-stderr.log
  image: ubuntu:16.04 # Use Ubuntu Xenial
+ variables:
+   CCACHE_DIR: ${CI_PROJECT_DIR}/.ccache
+ cache: # Use the same cache for all EDK2 jobs
+   key: ubuntu16.04-edk2-ccache
+   paths:
+   - ${CCACHE_DIR}
  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
+     ccache
      dos2unix
      gcc-aarch64-linux-gnu
      gcc-arm-linux-gnueabi
@@ -29,9 +36,13 @@ build-edk2:
      nasm
      python
      uuid-dev
+ - export PATH=/usr/lib/ccache:$PATH
+ - ccache --zero-stats
  script: # Clone the required submodules and build EDK2
  - git submodule update --init roms/edk2
  - git -C roms/edk2 submodule update --init
  - 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)
+ after_script:
+ - ccache --show-stats
-- 
2.21.1

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

[Philippe Mathieu-Daudé]

On 1/6/20 7:46 PM, 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'.
> 
> 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é <philmd@redhat.com>
> ---
>   .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'
> +   when: always
> + - if: '$CI_COMMIT_MESSAGE =~ /edk2/i' # last commit description contains 'EDK2'
> +   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

typo "required"

> + - 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
> + - git -C roms/edk2 submodule update --init
> + - 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)
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index ebcef0ebe9..f799246047 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -1,3 +1,6 @@
> +include:
> +  - local: '/.gitlab-ci-edk2.yml'
> +
>   before_script:
>    - apt-get update -qq
>    - apt-get install -y -qq flex bison libglib2.0-dev libpixman-1-dev genisoimage
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8571327881..22a1fd5824 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2346,6 +2346,7 @@ F: roms/edk2
>   F: roms/edk2-*
>   F: tests/data/uefi-boot-images/
>   F: tests/uefi-test-tools/
> +F: .gitlab-ci-edk2.yml
>   
>   Usermode Emulation
>   ------------------
> @@ -2689,7 +2690,7 @@ W: https://cirrus-ci.com/github/qemu/qemu
>   GitLab Continuous Integration
>   M: Thomas Huth <thuth@redhat.com>
>   S: Maintained
> -F: .gitlab-ci.yml
> +F: .gitlab-ci*.yml
>   
>   Guest Test Compilation Support
>   M: Alex Bennée <alex.bennee@linaro.org>
> 

Re: [PATCH 1/3] roms/edk2-funcs: Force softfloat ARM toolchain prefix on Debian

[Laszlo Ersek]

On 01/06/20 19:45, Philippe Mathieu-Daudé wrote:
> The Debian (based) distributions currently provides 2 ARM
> toolchains, documented as [1]:
> 
> * The ARM EABI (armel) port targets a range of older 32-bit ARM
>   devices, particularly those used in NAS hardware and a variety
>   of *plug computers.
> * The newer ARM hard-float (armhf) port supports newer, more
>   powerful 32-bit devices using version 7 of the ARM architecture
>   specification.
> 
> For various reasons documented in [2], the EDK2 project suggests
> to use the softfloat toolchain (named 'armel' by Debian).
> 
> Force the softfloat cross toolchain prefix on Debian distributions.
> 
> [1] https://www.debian.org/ports/arm/#status
> [2] https://github.com/tianocore/edk2/commit/41203b9a
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v3: fixed s/hard/float/ typo (Laszlo)

s/float/soft/, but OK otherwise :)

Thanks
Laszlo

> ---
>  roms/edk2-funcs.sh | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/roms/edk2-funcs.sh b/roms/edk2-funcs.sh
> index 3f4485b201..cd6e4f2c82 100644
> --- a/roms/edk2-funcs.sh
> +++ b/roms/edk2-funcs.sh
> @@ -112,6 +112,9 @@ qemu_edk2_get_cross_prefix()
>       ( [ "$gcc_arch" == i686 ] && [ "$host_arch" == x86_64 ] ); then
>      # no cross-compiler needed
>      :
> +  elif ( [ -e /etc/debian_version ] && [ "$gcc_arch" == arm ] ); then
> +    # force soft-float cross-compiler on Debian
> +    printf 'arm-linux-gnueabi-'
>    else
>      printf '%s-linux-gnu-\n' "$gcc_arch"
>    fi
> 

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

[Laszlo Ersek]

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é <philmd@redhat.com>
> ---
>  .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".

> + - 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 :)

Looks OK otherwise.

Thanks!
Laszlo

> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index ebcef0ebe9..f799246047 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -1,3 +1,6 @@
> +include:
> +  - local: '/.gitlab-ci-edk2.yml'
> +
>  before_script:
>   - apt-get update -qq
>   - apt-get install -y -qq flex bison libglib2.0-dev libpixman-1-dev genisoimage
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8571327881..22a1fd5824 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2346,6 +2346,7 @@ F: roms/edk2
>  F: roms/edk2-*
>  F: tests/data/uefi-boot-images/
>  F: tests/uefi-test-tools/
> +F: .gitlab-ci-edk2.yml
>  
>  Usermode Emulation
>  ------------------
> @@ -2689,7 +2690,7 @@ W: https://cirrus-ci.com/github/qemu/qemu
>  GitLab Continuous Integration
>  M: Thomas Huth <thuth@redhat.com>
>  S: Maintained
> -F: .gitlab-ci.yml
> +F: .gitlab-ci*.yml
>  
>  Guest Test Compilation Support
>  M: Alex Bennée <alex.bennee@linaro.org>
> 

Re: [PATCH 3/3] gitlab-ci-edk2.yml: Use ccache

[Laszlo Ersek]

On 01/06/20 19:46, Philippe Mathieu-Daudé wrote:
> By using ccache we reduce the job duration from
> 40 minutes 26 seconds to 32 minutes 6 seconds.
> 
>   Running after script...
>   $ ccache --show-stats
>   cache hit (direct)                  6604
>   files in cache                     12090
>   cache size                         335.5 MB
> 
> For now downloading this cache takes 16 seconds, archiving
> it 44 seconds.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  .gitlab-ci-edk2.yml | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/.gitlab-ci-edk2.yml b/.gitlab-ci-edk2.yml
> index abfaf52874..329ba24f20 100644
> --- a/.gitlab-ci-edk2.yml
> +++ b/.gitlab-ci-edk2.yml
> @@ -14,12 +14,19 @@ build-edk2:
>     - edk2-stdout.log
>     - edk2-stderr.log
>   image: ubuntu:16.04 # Use Ubuntu Xenial
> + variables:
> +   CCACHE_DIR: ${CI_PROJECT_DIR}/.ccache
> + cache: # Use the same cache for all EDK2 jobs
> +   key: ubuntu16.04-edk2-ccache
> +   paths:
> +   - ${CCACHE_DIR}
>   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
> +     ccache
>       dos2unix
>       gcc-aarch64-linux-gnu
>       gcc-arm-linux-gnueabi
> @@ -29,9 +36,13 @@ build-edk2:
>       nasm
>       python
>       uuid-dev
> + - export PATH=/usr/lib/ccache:$PATH
> + - ccache --zero-stats
>   script: # Clone the required submodules and build EDK2
>   - git submodule update --init roms/edk2
>   - git -C roms/edk2 submodule update --init
>   - 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)
> + after_script:
> + - ccache --show-stats
> 

I suggest dropping this patch. (In the first place: thank you for making
this a separate patch!)

I'm not a fan of ccache, to be honest. I've seen obscure failures with
it in the past. Also, the edk2 build system is a complicated beast in
itself; let's not compose that with another opaque thing. I'm especially
not fond of caching artifacts between multiple edk2 jobs.

For speeding up my builds, I used to use distcc instead; it worked
better than ccache (using multiple machines in my home). But I abandoned
even that, after a while.

I certainly don't intend to nack this patch -- if others really like
(and trust) ccache, they are welcome to ack. I'm just not a fan of it.

Thanks,
Laszlo

Re: [PATCH 3/3] gitlab-ci-edk2.yml: Use ccache

[Philippe Mathieu-Daudé]

On 1/7/20 11:19 AM, Laszlo Ersek wrote:
> On 01/06/20 19:46, Philippe Mathieu-Daudé wrote:
>> By using ccache we reduce the job duration from
>> 40 minutes 26 seconds to 32 minutes 6 seconds.
>>
>>    Running after script...
>>    $ ccache --show-stats
>>    cache hit (direct)                  6604
>>    files in cache                     12090
>>    cache size                         335.5 MB
>>
>> For now downloading this cache takes 16 seconds, archiving
>> it 44 seconds.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>   .gitlab-ci-edk2.yml | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/.gitlab-ci-edk2.yml b/.gitlab-ci-edk2.yml
>> index abfaf52874..329ba24f20 100644
>> --- a/.gitlab-ci-edk2.yml
>> +++ b/.gitlab-ci-edk2.yml
>> @@ -14,12 +14,19 @@ build-edk2:
>>      - edk2-stdout.log
>>      - edk2-stderr.log
>>    image: ubuntu:16.04 # Use Ubuntu Xenial
>> + variables:
>> +   CCACHE_DIR: ${CI_PROJECT_DIR}/.ccache
>> + cache: # Use the same cache for all EDK2 jobs
>> +   key: ubuntu16.04-edk2-ccache
>> +   paths:
>> +   - ${CCACHE_DIR}
>>    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
>> +     ccache
>>        dos2unix
>>        gcc-aarch64-linux-gnu
>>        gcc-arm-linux-gnueabi
>> @@ -29,9 +36,13 @@ build-edk2:
>>        nasm
>>        python
>>        uuid-dev
>> + - export PATH=/usr/lib/ccache:$PATH
>> + - ccache --zero-stats
>>    script: # Clone the required submodules and build EDK2
>>    - git submodule update --init roms/edk2
>>    - git -C roms/edk2 submodule update --init
>>    - 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)
>> + after_script:
>> + - ccache --show-stats
>>
> 
> I suggest dropping this patch. (In the first place: thank you for making
> this a separate patch!)
> 
> I'm not a fan of ccache, to be honest. I've seen obscure failures with
> it in the past. Also, the edk2 build system is a complicated beast in
> itself; let's not compose that with another opaque thing. I'm especially
> not fond of caching artifacts between multiple edk2 jobs.
> 
> For speeding up my builds, I used to use distcc instead; it worked
> better than ccache (using multiple machines in my home). But I abandoned
> even that, after a while.
> 
> I certainly don't intend to nack this patch -- if others really like
> (and trust) ccache, they are welcome to ack. I'm just not a fan of it.

I understand. I'll still include it in v2, tagged 'RFC' (or NOTFORMERGE) 
so Thomas can look at it, and it gets saved on the mailing list archive.

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é <philmd@redhat.com>
>> ---
>>   .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

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

[Laszlo Ersek]

On 01/07/20 12:35, Philippe Mathieu-Daudé wrote:
> 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é <philmd@redhat.com>
>>> ---
>>>   .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?

Huh, possibly a regression from commit f3e330e3c319
("roms/Makefile.edk2: don't pull in submodules when building from
tarball", 2019-10-07)?

Anyway, let me jump forward to your latest posting.

Thanks!
Laszlo

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