| Message ID | 20191108114531.21518-3-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | configure: Only decompress EDK2 blobs and check for bzip2 for X86/ARM | expand |
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 <thuth@redhat.com> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > 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é <berrange@redhat.com> Regards, Daniel
On 11/8/19 9:48 AM, 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 <thuth@redhat.com> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> 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 Ditto. > >> 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é <berrange@redhat.com> Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com> - Wainer > > > Regards, > Daniel
On 11/8/19 12:45 PM, 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 <thuth@redhat.com> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > 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 With this comment squashed in previous commit: Reviewed-by: Luc Michel <luc.michel@greensocs.com> > 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 >
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 <thuth@redhat.com> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> 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é <berrange@redhat.com> 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. Reviewed-by: Laszlo Ersek <lersek@redhat.com> Thanks! Laszlo
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 <thuth@redhat.com> >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >>> --- >>> 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é <berrange@redhat.com> > > 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
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 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
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 <thuth@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- v2: use better English (Daniel) (Not taking Daniel Reviewed-by because logic changed) --- configure | 4 ++++ 1 file changed, 4 insertions(+)