[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/2] configure: Check bzip2 is available
From: |
Thomas Huth |
Subject: |
Re: [PATCH v2 2/2] configure: Check bzip2 is available |
Date: |
Mon, 11 Nov 2019 14:38:33 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.0 |
On 08/11/2019 18.43, Laszlo Ersek wrote:
> On 11/08/19 12:48, Daniel P. Berrangé wrote:
>> On Fri, Nov 08, 2019 at 12:45:31PM +0100, Philippe Mathieu-Daudé wrote:
>>> The bzip2 tool is not included in default installations.
>>> On freshly installed systems, ./configure succeeds but 'make'
>>> might fail later:
>>>
>>> BUNZIP2 pc-bios/edk2-i386-secure-code.fd.bz2
>>> /bin/sh: bzip2: command not found
>>> make: *** [Makefile:305: pc-bios/edk2-i386-secure-code.fd] Error 127
>>> make: *** Deleting file 'pc-bios/edk2-i386-secure-code.fd'
>>> make: *** Waiting for unfinished jobs....
>>>
>>> Add a check in ./configure to warn the user if bzip2 is missing.
>>>
>>> Fixes: 536d2173b2b
>>> Reported-by: Thomas Huth <address@hidden>
>>> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
>>> ---
>>> v2: use better English (Daniel)
>>> (Not taking Daniel Reviewed-by because logic changed)
>>> ---
>>> configure | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>
>>> diff --git a/configure b/configure
>>> index 9b322284c3..2b419a8039 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -2147,6 +2147,7 @@ case " $target_list " in
>>> ;;
>>> esac
>>>
>>> +# Some firmware binaries are compressed with bzip2
>>
>> Squash into previous patch
>>
>>> for target in $target_list; do
>>> case "$target" in
>>> arm-softmmu | aarch64-softmmu | i386-softmmu | x86_64-softmmu)
>>> @@ -2154,6 +2155,9 @@ for target in $target_list; do
>>> ;;
>>> esac
>>> done
>>> +if test "$edk2_blobs" = "yes" && ! has bzip2; then
>>> + error_exit "The bzip2 program is required for building QEMU"
>>> +fi
>>>
>>> feature_not_found() {
>>> feature=$1
>>
>> Reviewed-by: Daniel P. Berrangé <address@hidden>
>
> I don't feel too strongly about this, but how about a different improvement:
>
> - keep the comment in this patch, but move it right before the "has
> bzip2" check
> - instead of "some firmware binaries", clearly state "edk2 blobs".
>
> Something like:
>
> +# Edk2 blobs are compressed with bzip2.
> +if test "$edk2_blobs" = "yes" && ! has bzip2; then
> + error_exit "The bzip2 program is required for building QEMU"
> +fi
>
> So... now we have three variants: the one posted by Phil (v2), the one
> recommended by Dan (as an update to v2), and the one I suggest above (as
> a different update to v2). For simplicity, I'm fine with any one of the
> three variants going in.
I'll add the two patches to my "qtest + misc" PULL request that I'm
planning to send out tomorrow - and use Laszlo's suggestion for moving
the comment. I hope that's fine for everybody, if not please complain.
Thomas
- [PATCH v2 1/2] configure: Only decompress EDK2 blobs for X86/ARM targets, (continued)
- [PATCH v2 1/2] configure: Only decompress EDK2 blobs for X86/ARM targets, Philippe Mathieu-Daudé, 2019/11/08
- Re: [PATCH v2 1/2] configure: Only decompress EDK2 blobs for X86/ARM targets, Daniel P . Berrangé, 2019/11/08
- Re: [PATCH v2 1/2] configure: Only decompress EDK2 blobs for X86/ARM targets, Philippe Mathieu-Daudé, 2019/11/08
- Re: [PATCH v2 1/2] configure: Only decompress EDK2 blobs for X86/ARM targets, Wainer dos Santos Moschetta, 2019/11/08
- Re: [PATCH v2 1/2] configure: Only decompress EDK2 blobs for X86/ARM targets, Luc Michel, 2019/11/08
- Re: [PATCH v2 1/2] configure: Only decompress EDK2 blobs for X86/ARM targets, Laszlo Ersek, 2019/11/08
- [PATCH v2 2/2] configure: Check bzip2 is available, Philippe Mathieu-Daudé, 2019/11/08
- Re: [PATCH v2 2/2] configure: Check bzip2 is available, Luc Michel, 2019/11/08