[PATCH] scripts/make-release: Go back to cloning all the EDK2 submodules

[PATCH] scripts/make-release: Go back to cloning all the EDK2 submodules

[Peter Maydell]

In commit bd0da3a3d4f we changed make-release so that instead of
cloning every git submodule of EDK2 we only cloned a fixed list.
The original motivation for this was that one of the submodules:
 * was from a non-github repo
 * that repo had a "SSL certificate expired" failure
 * wasn't actually needed for the set of EDK2 binaries we build
and at the time we were trying to build the EDK2 binaries in one of
our CI jobs.

Unfortunately this change meant that we were exposed to bugs where
EDK2 adds a new submodule and the sources we ship in the release
tarball won't build any more.  In particular, in EDK2 commit
c6bb7d54beb05 the MipiSysTLib submodule was added, causing failure of
the ROM build in our tarball starting from QEMU release 8.2.0:

/tmp/qemu-10.0.0/roms/edk2/MdePkg/MdePkg.dec(32): error 000E: File/directory not found in workspace
        Library/MipiSysTLib/mipisyst/library/include is not found in packages path:
        /tmp/qemu-10.0.0/roms/.
        /tmp/qemu-10.0.0/roms/edk2

(Building from a QEMU git checkout works fine.)

In the intervening time EDK2 moved the submodule that had a problem
to be one they mirrored themselves (and at time of writing all their
submodules are hosted on github), and we stopped trying to build
EDK2 binaries in our own CI jobs with commit 690ceb71936f9037f6.

Go back to cloning every EDK2 submodule, so we don't have an
untested explicit list of submodules which will break without
our noticing it.

This increases the size of the QEMU tarball .tar.xz file from
133M to 139M in my testing.

Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3041
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I think ideally we would not ship these rom blobs in our source
tarball but instead provide them separately. Then we would definitely
have no reason to ship a huge amount of EDK2 sourcecode that
99.9% of the users of the tarball don't want. But that's an
idea that was floated at least three or four years ago now
and we don't in practice have time or effort to try to do that.
So the simple thing is to at least ship the right source...

NB: for me this can build the x86 blobs now, but it then tries
to build the Arm blobs with the x86 GCC, which obviously doesn't
work. Perhaps something needs to be set in the environment ?

---
 scripts/make-release | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/scripts/make-release b/scripts/make-release
index 4509a9fabf5..87f563ef5f7 100755
--- a/scripts/make-release
+++ b/scripts/make-release
@@ -62,17 +62,15 @@ meson subprojects download $SUBPROJECTS
 (cd roms/skiboot && ./make_version.sh > .version)
 # Fetch edk2 submodule's submodules, since it won't have access to them via
 # the tarball later.
-#
-# A more uniform way to handle this sort of situation would be nice, but we
-# don't necessarily have much control over how a submodule handles its
-# submodule dependencies, so we continue to handle these on a case-by-case
-# basis for now.
-(cd roms/edk2 && \
-    git submodule update --init --depth 1 -- \
-        ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3 \
-        BaseTools/Source/C/BrotliCompress/brotli \
-        CryptoPkg/Library/OpensslLib/openssl \
-        MdeModulePkg/Library/BrotliCustomDecompressLib/brotli)
+
+# As recommended by the EDK2 readme, we don't use --recursive here.
+# EDK2 won't use any code or feature from a submodule of a submodule,
+# so we don't need to add them to the tarball.
+# Although we don't necessarily need all of the submodules that EDK2
+# has, we clone them all, to avoid running into problems where EDK2
+# adds a new submodule or changes its use of an existing one and
+# the sources we ship in the tarball then fail to build.
+(cd roms/edk2 && git submodule update --init --depth 1)
 popd
 
 exclude=(--exclude=.git)
-- 
2.43.0

Re: [PATCH] scripts/make-release: Go back to cloning all the EDK2 submodules

[Peter Maydell]

On Mon, 21 Jul 2025 at 16:33, Peter Maydell <peter.maydell@linaro.org> wrote:
> I think ideally we would not ship these rom blobs in our source
> tarball but instead provide them separately. Then we would definitely
> have no reason to ship a huge amount of EDK2 sourcecode that
> 99.9% of the users of the tarball don't want. But that's an
> idea that was floated at least three or four years ago now
> and we don't in practice have time or effort to try to do that.
> So the simple thing is to at least ship the right source...

To quantify "huge amount": a 10.0.0 tarball uncompresses to
911MB; of that, 445MB is roms/edk2. Another 156MB is roms/u-boot,
and 48MB is roms/u-boot-sam460ex. roms/ overall is 743MB.

So 80% of our source tarball release is ROM blob source, and
48% of our tarball is EDK2...

-- PMM

Re: [PATCH] scripts/make-release: Go back to cloning all the EDK2 submodules

[Michael Tokarev]

On 21.07.2025 18:41, Peter Maydell wrote:

> To quantify "huge amount": a 10.0.0 tarball uncompresses to
> 911MB; of that, 445MB is roms/edk2. Another 156MB is roms/u-boot,
> and 48MB is roms/u-boot-sam460ex. roms/ overall is 743MB.
> 
> So 80% of our source tarball release is ROM blob source, and
> 48% of our tarball is EDK2...

Is it time to switch to having a separate qemu-firmware
source tarball, or something like that? :)

I'm not sure about pc-bios/s390-cww though (which uses SLOF sources).

Thanks,

/mjt

Re: [PATCH] scripts/make-release: Go back to cloning all the EDK2 submodules

[Peter Maydell]

Ping for code review on this one? Mike Roth: especially input from
from you on this would be good since you're the one who runs
the make-release script...

thanks
-- PMM

On Mon, 21 Jul 2025 at 16:33, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> In commit bd0da3a3d4f we changed make-release so that instead of
> cloning every git submodule of EDK2 we only cloned a fixed list.
> The original motivation for this was that one of the submodules:
>  * was from a non-github repo
>  * that repo had a "SSL certificate expired" failure
>  * wasn't actually needed for the set of EDK2 binaries we build
> and at the time we were trying to build the EDK2 binaries in one of
> our CI jobs.
>
> Unfortunately this change meant that we were exposed to bugs where
> EDK2 adds a new submodule and the sources we ship in the release
> tarball won't build any more.  In particular, in EDK2 commit
> c6bb7d54beb05 the MipiSysTLib submodule was added, causing failure of
> the ROM build in our tarball starting from QEMU release 8.2.0:
>
> /tmp/qemu-10.0.0/roms/edk2/MdePkg/MdePkg.dec(32): error 000E: File/directory not found in workspace
>         Library/MipiSysTLib/mipisyst/library/include is not found in packages path:
>         /tmp/qemu-10.0.0/roms/.
>         /tmp/qemu-10.0.0/roms/edk2
>
> (Building from a QEMU git checkout works fine.)
>
> In the intervening time EDK2 moved the submodule that had a problem
> to be one they mirrored themselves (and at time of writing all their
> submodules are hosted on github), and we stopped trying to build
> EDK2 binaries in our own CI jobs with commit 690ceb71936f9037f6.
>
> Go back to cloning every EDK2 submodule, so we don't have an
> untested explicit list of submodules which will break without
> our noticing it.
>
> This increases the size of the QEMU tarball .tar.xz file from
> 133M to 139M in my testing.
>
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3041
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I think ideally we would not ship these rom blobs in our source
> tarball but instead provide them separately. Then we would definitely
> have no reason to ship a huge amount of EDK2 sourcecode that
> 99.9% of the users of the tarball don't want. But that's an
> idea that was floated at least three or four years ago now
> and we don't in practice have time or effort to try to do that.
> So the simple thing is to at least ship the right source...
>
> NB: for me this can build the x86 blobs now, but it then tries
> to build the Arm blobs with the x86 GCC, which obviously doesn't
> work. Perhaps something needs to be set in the environment ?
>
> ---
>  scripts/make-release | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/scripts/make-release b/scripts/make-release
> index 4509a9fabf5..87f563ef5f7 100755
> --- a/scripts/make-release
> +++ b/scripts/make-release
> @@ -62,17 +62,15 @@ meson subprojects download $SUBPROJECTS
>  (cd roms/skiboot && ./make_version.sh > .version)
>  # Fetch edk2 submodule's submodules, since it won't have access to them via
>  # the tarball later.
> -#
> -# A more uniform way to handle this sort of situation would be nice, but we
> -# don't necessarily have much control over how a submodule handles its
> -# submodule dependencies, so we continue to handle these on a case-by-case
> -# basis for now.
> -(cd roms/edk2 && \
> -    git submodule update --init --depth 1 -- \
> -        ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3 \
> -        BaseTools/Source/C/BrotliCompress/brotli \
> -        CryptoPkg/Library/OpensslLib/openssl \
> -        MdeModulePkg/Library/BrotliCustomDecompressLib/brotli)
> +
> +# As recommended by the EDK2 readme, we don't use --recursive here.
> +# EDK2 won't use any code or feature from a submodule of a submodule,
> +# so we don't need to add them to the tarball.
> +# Although we don't necessarily need all of the submodules that EDK2
> +# has, we clone them all, to avoid running into problems where EDK2
> +# adds a new submodule or changes its use of an existing one and
> +# the sources we ship in the tarball then fail to build.
> +(cd roms/edk2 && git submodule update --init --depth 1)
>  popd
>
>  exclude=(--exclude=.git)
> --

Re: [PATCH] scripts/make-release: Go back to cloning all the EDK2 submodules

[Michael Tokarev]

On 21.07.2025 18:33, Peter Maydell wrote:
> In commit bd0da3a3d4f we changed make-release so that instead of
> cloning every git submodule of EDK2 we only cloned a fixed list.
> The original motivation for this was that one of the submodules:
>   * was from a non-github repo
>   * that repo had a "SSL certificate expired" failure
>   * wasn't actually needed for the set of EDK2 binaries we build
> and at the time we were trying to build the EDK2 binaries in one of
> our CI jobs.
> 
> Unfortunately this change meant that we were exposed to bugs where
> EDK2 adds a new submodule and the sources we ship in the release
> tarball won't build any more.  In particular, in EDK2 commit
> c6bb7d54beb05 the MipiSysTLib submodule was added, causing failure of
> the ROM build in our tarball starting from QEMU release 8.2.0:
> 
> /tmp/qemu-10.0.0/roms/edk2/MdePkg/MdePkg.dec(32): error 000E: File/directory not found in workspace
>          Library/MipiSysTLib/mipisyst/library/include is not found in packages path:
>          /tmp/qemu-10.0.0/roms/.
>          /tmp/qemu-10.0.0/roms/edk2
> 
> (Building from a QEMU git checkout works fine.)
> 
> In the intervening time EDK2 moved the submodule that had a problem
> to be one they mirrored themselves (and at time of writing all their
> submodules are hosted on github), and we stopped trying to build
> EDK2 binaries in our own CI jobs with commit 690ceb71936f9037f6.
> 
> Go back to cloning every EDK2 submodule, so we don't have an
> untested explicit list of submodules which will break without
> our noticing it.
> 
> This increases the size of the QEMU tarball .tar.xz file from
> 133M to 139M in my testing.
> 
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3041
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>

Let's fix this for 10.1, and think about splitting roms/ into
its own tarball in 10.2.

Thanks,

/mjt

Re: [PATCH] scripts/make-release: Go back to cloning all the EDK2 submodules

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> In commit bd0da3a3d4f we changed make-release so that instead of
> cloning every git submodule of EDK2 we only cloned a fixed list.
> The original motivation for this was that one of the submodules:
>  * was from a non-github repo
>  * that repo had a "SSL certificate expired" failure
>  * wasn't actually needed for the set of EDK2 binaries we build
> and at the time we were trying to build the EDK2 binaries in one of
> our CI jobs.
>
<snip>

Queued to maintainer/for-10.1-rc2, thanks.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro