[Qemu-devel] [PATCH 0/2] Fix tarball builds of UEFI/EDK2 firmware

[Qemu-devel] [PATCH 0/2] Fix tarball builds of UEFI/EDK2 firmware

[Michael Roth]

Bruce noticed that we cannot build `make efi` target from the v4.1.0
tarball. This is due to a failure on the part of the make-release script
to pull in submodules nested under other submodules, as well as
Makefile.edk2's assumptions about being in a git tree.

Suggestions for more robust solutions are definitely welcome, but for
now this series takes what seems to be the most direct approach.

 roms/Makefile.edk2   | 7 ++++++-
 scripts/make-release | 8 ++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

[Qemu-devel] [PATCH 1/2] make-release: pull in edk2 submodules so we can build it from tarballs

[Michael Roth]

The `make efi` target added by 536d2173 is built from the roms/edk2
submodule, which in turn relies on additional submodules nested under
roms/edk2.

The make-release script currently only pulls in top-level submodules,
so these nested submodules are missing in the resulting tarball.

We could try to address this situation more generally by recursively
pulling in all submodules, but this doesn't necessarily ensure the
end-result will build properly (this case also required other changes).

Additionally, due to the nature of submodules, we may not always have
control over how these sorts of things are dealt with, so for now we
continue to handle it on a case-by-case in the make-release script.

Reported-by: Bruce Rogers <brogers@suse.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Bruce Rogers <brogers@suse.com>
Cc: qemu-stable@nongnu.org # v4.1.0
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 scripts/make-release | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/scripts/make-release b/scripts/make-release
index b4af9c9e52..a2a8cda33c 100755
--- a/scripts/make-release
+++ b/scripts/make-release
@@ -20,6 +20,14 @@ git checkout "v${version}"
 git submodule update --init
 (cd roms/seabios && git describe --tags --long --dirty > .version)
 (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)
 popd
 tar --exclude=.git -cjf ${destination}.tar.bz2 ${destination}
 rm -rf ${destination}
-- 
2.17.1

[Qemu-devel] [PATCH 2/2] roms/Makefile.edk2: don't pull in submodules when building from tarball

[Michael Roth]

Currently the `make efi` target pulls submodules nested under the
roms/edk2 submodule as dependencies. However, when we attempt to build
from a tarball this fails since we are no longer in a git tree.

A preceding patch will pre-populate these submodules in the tarball,
so assume this build dependency is only needed when building from a
git tree.

Reported-by: Bruce Rogers <brogers@suse.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Bruce Rogers <brogers@suse.com>
Cc: qemu-stable@nongnu.org # v4.1.0
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 roms/Makefile.edk2 | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/roms/Makefile.edk2 b/roms/Makefile.edk2
index c2f2ff59d5..33a074d3a4 100644
--- a/roms/Makefile.edk2
+++ b/roms/Makefile.edk2
@@ -46,8 +46,13 @@ all: $(foreach flashdev,$(flashdevs),../pc-bios/edk2-$(flashdev).fd.bz2) \
 # files.
 .INTERMEDIATE: $(foreach flashdev,$(flashdevs),../pc-bios/edk2-$(flashdev).fd)
 
+# Fetch edk2 submodule's submodules. If it is not in a git tree, assume
+# we're building from a tarball and that they've already been fetched by
+# make-release/tarball scripts.
 submodules:
-	cd edk2 && git submodule update --init --force
+	if test -d edk2/.git; then \
+		cd edk2 && git submodule update --init --force; \
+	fi
 
 # See notes on the ".NOTPARALLEL" target and the "+" indicator in
 # "tests/uefi-test-tools/Makefile".
-- 
2.17.1

Re: [Qemu-devel] [PATCH 2/2] roms/Makefile.edk2: don't pull in submodules when building from tarball

[Laszlo Ersek]

On 09/13/19 01:12, Michael Roth wrote:
> Currently the `make efi` target pulls submodules nested under the
> roms/edk2 submodule as dependencies. However, when we attempt to build
> from a tarball this fails since we are no longer in a git tree.
> 
> A preceding patch will pre-populate these submodules in the tarball,
> so assume this build dependency is only needed when building from a
> git tree.
> 
> Reported-by: Bruce Rogers <brogers@suse.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Bruce Rogers <brogers@suse.com>
> Cc: qemu-stable@nongnu.org # v4.1.0
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  roms/Makefile.edk2 | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/roms/Makefile.edk2 b/roms/Makefile.edk2
> index c2f2ff59d5..33a074d3a4 100644
> --- a/roms/Makefile.edk2
> +++ b/roms/Makefile.edk2
> @@ -46,8 +46,13 @@ all: $(foreach flashdev,$(flashdevs),../pc-bios/edk2-$(flashdev).fd.bz2) \
>  # files.
>  .INTERMEDIATE: $(foreach flashdev,$(flashdevs),../pc-bios/edk2-$(flashdev).fd)
>  
> +# Fetch edk2 submodule's submodules. If it is not in a git tree, assume
> +# we're building from a tarball and that they've already been fetched by
> +# make-release/tarball scripts.
>  submodules:
> -	cd edk2 && git submodule update --init --force
> +	if test -d edk2/.git; then \
> +		cd edk2 && git submodule update --init --force; \
> +	fi
>  
>  # See notes on the ".NOTPARALLEL" target and the "+" indicator in
>  # "tests/uefi-test-tools/Makefile".
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thank you,
Laszlo

Re: [Qemu-devel] [PATCH 0/2] Fix tarball builds of UEFI/EDK2 firmware

[Philippe Mathieu-Daudé]

On 9/13/19 1:12 AM, Michael Roth wrote:
> Bruce noticed that we cannot build `make efi` target from the v4.1.0
> tarball. This is due to a failure on the part of the make-release script
> to pull in submodules nested under other submodules, as well as
> Makefile.edk2's assumptions about being in a git tree.

Hmm I'd expect distributions interested in building EDK2, build it
properly tuned, outside of QEMU. I consider what is here as default
config useful for testing, not for production use. I might be wrong.

Meanwhile, it is true 'make efi' should not fail.

> Suggestions for more robust solutions are definitely welcome, but for
> now this series takes what seems to be the most direct approach.

This is fine:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

I'll queue these patches in my edk2-next branch since I'm preparing a
pull request.

Thanks,

Phil.

Re: [Qemu-devel] [PATCH 2/2] roms/Makefile.edk2: don't pull in submodules when building from tarball

[Philippe Mathieu-Daudé]

On 9/13/19 1:12 AM, Michael Roth wrote:
> Currently the `make efi` target pulls submodules nested under the
> roms/edk2 submodule as dependencies. However, when we attempt to build
> from a tarball this fails since we are no longer in a git tree.
> 
> A preceding patch will pre-populate these submodules in the tarball,
> so assume this build dependency is only needed when building from a
> git tree.
> 
> Reported-by: Bruce Rogers <brogers@suse.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Bruce Rogers <brogers@suse.com>
> Cc: qemu-stable@nongnu.org # v4.1.0
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  roms/Makefile.edk2 | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/roms/Makefile.edk2 b/roms/Makefile.edk2
> index c2f2ff59d5..33a074d3a4 100644
> --- a/roms/Makefile.edk2
> +++ b/roms/Makefile.edk2
> @@ -46,8 +46,13 @@ all: $(foreach flashdev,$(flashdevs),../pc-bios/edk2-$(flashdev).fd.bz2) \
>  # files.
>  .INTERMEDIATE: $(foreach flashdev,$(flashdevs),../pc-bios/edk2-$(flashdev).fd)
>  
> +# Fetch edk2 submodule's submodules. If it is not in a git tree, assume
> +# we're building from a tarball and that they've already been fetched by
> +# make-release/tarball scripts.

Annoying, without using the make-release tool in a fresh clone, I get
qemu/roms/edk2/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf(-1):
error 000E: File/directory not found in workspace

I vaguely remember there was a thread about not using 'git submodule
update --init --recursive' in the root directory but can't find it, any
idea?

>  submodules:
> -	cd edk2 && git submodule update --init --force
> +	if test -d edk2/.git; then \
> +		cd edk2 && git submodule update --init --force; \
> +	fi
>  
>  # See notes on the ".NOTPARALLEL" target and the "+" indicator in
>  # "tests/uefi-test-tools/Makefile".
> 

Re: [Qemu-devel] [PATCH 2/2] roms/Makefile.edk2: don't pull in submodules when building from tarball

[Laszlo Ersek]

On 09/20/19 21:50, Philippe Mathieu-Daudé wrote:
> On 9/13/19 1:12 AM, Michael Roth wrote:
>> Currently the `make efi` target pulls submodules nested under the
>> roms/edk2 submodule as dependencies. However, when we attempt to build
>> from a tarball this fails since we are no longer in a git tree.
>>
>> A preceding patch will pre-populate these submodules in the tarball,
>> so assume this build dependency is only needed when building from a
>> git tree.
>>
>> Reported-by: Bruce Rogers <brogers@suse.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Bruce Rogers <brogers@suse.com>
>> Cc: qemu-stable@nongnu.org # v4.1.0
>> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
>> ---
>>  roms/Makefile.edk2 | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/roms/Makefile.edk2 b/roms/Makefile.edk2
>> index c2f2ff59d5..33a074d3a4 100644
>> --- a/roms/Makefile.edk2
>> +++ b/roms/Makefile.edk2
>> @@ -46,8 +46,13 @@ all: $(foreach flashdev,$(flashdevs),../pc-bios/edk2-$(flashdev).fd.bz2) \
>>  # files.
>>  .INTERMEDIATE: $(foreach flashdev,$(flashdevs),../pc-bios/edk2-$(flashdev).fd)
>>  
>> +# Fetch edk2 submodule's submodules. If it is not in a git tree, assume
>> +# we're building from a tarball and that they've already been fetched by
>> +# make-release/tarball scripts.
> 
> Annoying, without using the make-release tool in a fresh clone, I get
> qemu/roms/edk2/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf(-1):
> error 000E: File/directory not found in workspace
> 
> I vaguely remember there was a thread about not using 'git submodule
> update --init --recursive' in the root directory but can't find it, any
> idea?

I think that was a point made in the first patch of this series -- the
"--recursive" flag is unbounded, and it might pull in several unneeded
submodules.

For example, openssl itself appears to have three submodules (boringssl,
krb5, pyca-cryptography), and none of those is needed for building
openssl the way edk2 consumes it.

I seem to remember that patch#1 in this series stated: we'd be handling
the submodules on a case-by-case basis.

Thanks
Laszlo

> 
>>  submodules:
>> -	cd edk2 && git submodule update --init --force
>> +	if test -d edk2/.git; then \
>> +		cd edk2 && git submodule update --init --force; \
>> +	fi
>>  
>>  # See notes on the ".NOTPARALLEL" target and the "+" indicator in
>>  # "tests/uefi-test-tools/Makefile".
>>