[PATCH 0/2] meson.build: Fix bzip2 program detection for EDK2 blobs installation

[PATCH 0/2] meson.build: Fix bzip2 program detection for EDK2 blobs installation

[Philippe Mathieu-Daudé]

Fix the following error when bzip2 program is not installed:

  ../pc-bios/meson.build:5:2: ERROR: Program 'bzip2' not found

(First patch easier to review using 'git-diff --ignore-all-space').

Philippe Mathieu-Daudé (2):
  meson.build: Declare global edk2_targets / install_edk2_blobs
    variables
  meson.build: Detect bzip2 program

 meson.build                     | 10 ++++++++++
 pc-bios/descriptors/meson.build | 30 ++++++++++++++++--------------
 pc-bios/meson.build             |  6 +-----
 3 files changed, 27 insertions(+), 19 deletions(-)

-- 
2.26.2

[PATCH 1/2] meson.build: Declare global edk2_targets / install_edk2_blobs variables

[Philippe Mathieu-Daudé]

Globally declare in the main meson.build:
- the list of EDK2 targets,
- whether the EDK2 blobs have to be installed.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Patch trivial to review using 'git-diff --ignore-all-space'
---
 meson.build                     |  8 ++++++++
 pc-bios/descriptors/meson.build | 30 ++++++++++++++++--------------
 pc-bios/meson.build             |  5 +----
 3 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/meson.build b/meson.build
index 3d889857a09..ecc45d04d6a 100644
--- a/meson.build
+++ b/meson.build
@@ -88,6 +88,14 @@
   }
 endif
 
+edk2_targets = [ 'arm-softmmu', 'aarch64-softmmu', 'i386-softmmu', 'x86_64-softmmu' ]
+install_edk2_blobs = false
+if get_option('install_blobs')
+  foreach target : target_dirs
+    install_edk2_blobs = install_edk2_blobs or target in edk2_targets
+  endforeach
+endif
+
 ##################
 # Compiler flags #
 ##################
diff --git a/pc-bios/descriptors/meson.build b/pc-bios/descriptors/meson.build
index 7040834573d..ac6ec66b007 100644
--- a/pc-bios/descriptors/meson.build
+++ b/pc-bios/descriptors/meson.build
@@ -1,14 +1,16 @@
-foreach f: [
-  '50-edk2-i386-secure.json',
-  '50-edk2-x86_64-secure.json',
-  '60-edk2-aarch64.json',
-  '60-edk2-arm.json',
-  '60-edk2-i386.json',
-  '60-edk2-x86_64.json'
-]
-  configure_file(input: files(f),
-                 output: f,
-                 configuration: {'DATADIR': qemu_datadir},
-                 install: get_option('install_blobs'),
-                 install_dir: qemu_datadir / 'firmware')
-endforeach
+if install_edk2_blobs
+  foreach f: [
+    '50-edk2-i386-secure.json',
+    '50-edk2-x86_64-secure.json',
+    '60-edk2-aarch64.json',
+    '60-edk2-arm.json',
+    '60-edk2-i386.json',
+    '60-edk2-x86_64.json'
+  ]
+    configure_file(input: files(f),
+                   output: f,
+                   configuration: {'DATADIR': qemu_datadir},
+                   install: get_option('install_blobs'),
+                   install_dir: qemu_datadir / 'firmware')
+  endforeach
+endif
diff --git a/pc-bios/meson.build b/pc-bios/meson.build
index fab323af84e..6a341b6cea0 100644
--- a/pc-bios/meson.build
+++ b/pc-bios/meson.build
@@ -1,7 +1,4 @@
-if 'arm-softmmu' in target_dirs or \
-    'aarch64-softmmu' in target_dirs or \
-    'i386-softmmu' in target_dirs or \
-    'x86_64-softmmu' in target_dirs
+if install_edk2_blobs
   bzip2 = find_program('bzip2', required: true)
   fds = [
     'edk2-aarch64-code.fd',
-- 
2.26.2

[PATCH 2/2] meson.build: Detect bzip2 program

[Philippe Mathieu-Daudé]

The --enable-bzip2/--disable-bzip2 configure arguments are
somehow misleading, they check for the bzip2 library, not
the bzip2 program.

We need the bzip2 program to install the EDK2 firmware blobs
(see commit 623ef637a2e "configure: Check bzip2 is available").

Check if the bzip2 program in the global meson.build to avoid
the configuration to succeed, but a later when trying to install
the firmware blobs:

    ../pc-bios/meson.build:5:2: ERROR: Program 'bzip2' not found

Reported-by: John Snow <jsnow@redhat.com>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Fixes: c8d5450bba3 ("configure: move install_blobs from configure to meson")
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 meson.build         | 2 ++
 pc-bios/meson.build | 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index ecc45d04d6a..954152c90fe 100644
--- a/meson.build
+++ b/meson.build
@@ -96,6 +96,8 @@
   endforeach
 endif
 
+bzip2 = find_program('bzip2', required: install_edk2_blobs)
+
 ##################
 # Compiler flags #
 ##################
diff --git a/pc-bios/meson.build b/pc-bios/meson.build
index 6a341b6cea0..af95c5d1f1d 100644
--- a/pc-bios/meson.build
+++ b/pc-bios/meson.build
@@ -1,5 +1,4 @@
 if install_edk2_blobs
-  bzip2 = find_program('bzip2', required: true)
   fds = [
     'edk2-aarch64-code.fd',
     'edk2-arm-code.fd',
-- 
2.26.2

Re: [PATCH 2/2] meson.build: Detect bzip2 program

[John Snow]

On 1/14/21 12:45 PM, Philippe Mathieu-Daudé wrote:
> The --enable-bzip2/--disable-bzip2 configure arguments are
> somehow misleading, they check for the bzip2 library, not
> the bzip2 program.
> 

Well. I was mislead, but they're fine, really.

> We need the bzip2 program to install the EDK2 firmware blobs
> (see commit 623ef637a2e "configure: Check bzip2 is available").
> 
> Check if the bzip2 program in the global meson.build to avoid
> the configuration to succeed, but a later when trying to install
> the firmware blobs:
> 
>      ../pc-bios/meson.build:5:2: ERROR: Program 'bzip2' not found
> 
> Reported-by: John Snow <jsnow@redhat.com>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Fixes: c8d5450bba3 ("configure: move install_blobs from configure to meson")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>   meson.build         | 2 ++
>   pc-bios/meson.build | 1 -
>   2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/meson.build b/meson.build
> index ecc45d04d6a..954152c90fe 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -96,6 +96,8 @@
>     endforeach
>   endif
>   
> +bzip2 = find_program('bzip2', required: install_edk2_blobs)
> +

Seems good though :)

>   ##################
>   # Compiler flags #
>   ##################
> diff --git a/pc-bios/meson.build b/pc-bios/meson.build
> index 6a341b6cea0..af95c5d1f1d 100644
> --- a/pc-bios/meson.build
> +++ b/pc-bios/meson.build
> @@ -1,5 +1,4 @@
>   if install_edk2_blobs
> -  bzip2 = find_program('bzip2', required: true)
>     fds = [
>       'edk2-aarch64-code.fd',
>       'edk2-arm-code.fd',
> 

Re: [PATCH 0/2] meson.build: Fix bzip2 program detection for EDK2 blobs installation

[Paolo Bonzini]

On 14/01/21 18:45, Philippe Mathieu-Daudé wrote:
> Fix the following error when bzip2 program is not installed:
> 
>    ../pc-bios/meson.build:5:2: ERROR: Program 'bzip2' not found
> 
> (First patch easier to review using 'git-diff --ignore-all-space').
> 
> Philippe Mathieu-Daudé (2):
>    meson.build: Declare global edk2_targets / install_edk2_blobs
>      variables
>    meson.build: Detect bzip2 program
> 
>   meson.build                     | 10 ++++++++++
>   pc-bios/descriptors/meson.build | 30 ++++++++++++++++--------------
>   pc-bios/meson.build             |  6 +-----
>   3 files changed, 27 insertions(+), 19 deletions(-)
> 

Queued, thanks.

Paolo

Re: [PATCH 1/2] meson.build: Declare global edk2_targets / install_edk2_blobs variables

[Laszlo Ersek]

On 01/14/21 18:45, Philippe Mathieu-Daudé wrote:
> Globally declare in the main meson.build:
> - the list of EDK2 targets,
> - whether the EDK2 blobs have to be installed.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Patch trivial to review using 'git-diff --ignore-all-space'
> ---
>  meson.build                     |  8 ++++++++
>  pc-bios/descriptors/meson.build | 30 ++++++++++++++++--------------
>  pc-bios/meson.build             |  5 +----
>  3 files changed, 25 insertions(+), 18 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 3d889857a09..ecc45d04d6a 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -88,6 +88,14 @@
>    }
>  endif
>  
> +edk2_targets = [ 'arm-softmmu', 'aarch64-softmmu', 'i386-softmmu', 'x86_64-softmmu' ]
> +install_edk2_blobs = false
> +if get_option('install_blobs')
> +  foreach target : target_dirs
> +    install_edk2_blobs = install_edk2_blobs or target in edk2_targets
> +  endforeach
> +endif
> +
>  ##################
>  # Compiler flags #
>  ##################
> diff --git a/pc-bios/descriptors/meson.build b/pc-bios/descriptors/meson.build
> index 7040834573d..ac6ec66b007 100644
> --- a/pc-bios/descriptors/meson.build
> +++ b/pc-bios/descriptors/meson.build
> @@ -1,14 +1,16 @@
> -foreach f: [
> -  '50-edk2-i386-secure.json',
> -  '50-edk2-x86_64-secure.json',
> -  '60-edk2-aarch64.json',
> -  '60-edk2-arm.json',
> -  '60-edk2-i386.json',
> -  '60-edk2-x86_64.json'
> -]
> -  configure_file(input: files(f),
> -                 output: f,
> -                 configuration: {'DATADIR': qemu_datadir},
> -                 install: get_option('install_blobs'),
> -                 install_dir: qemu_datadir / 'firmware')
> -endforeach
> +if install_edk2_blobs
> +  foreach f: [
> +    '50-edk2-i386-secure.json',
> +    '50-edk2-x86_64-secure.json',
> +    '60-edk2-aarch64.json',
> +    '60-edk2-arm.json',
> +    '60-edk2-i386.json',
> +    '60-edk2-x86_64.json'
> +  ]
> +    configure_file(input: files(f),
> +                   output: f,
> +                   configuration: {'DATADIR': qemu_datadir},
> +                   install: get_option('install_blobs'),
> +                   install_dir: qemu_datadir / 'firmware')
> +  endforeach
> +endif
> diff --git a/pc-bios/meson.build b/pc-bios/meson.build
> index fab323af84e..6a341b6cea0 100644
> --- a/pc-bios/meson.build
> +++ b/pc-bios/meson.build
> @@ -1,7 +1,4 @@
> -if 'arm-softmmu' in target_dirs or \
> -    'aarch64-softmmu' in target_dirs or \
> -    'i386-softmmu' in target_dirs or \
> -    'x86_64-softmmu' in target_dirs
> +if install_edk2_blobs
>    bzip2 = find_program('bzip2', required: true)
>    fds = [
>      'edk2-aarch64-code.fd',
> 

I vaguely understand what this patch does (I haven't followed the meson
conversion), but I'm unsure why it does that.

Is this patch useful in itself, or only in preparation for patch#2?

Thanks,
Laszlo

Re: [PATCH 2/2] meson.build: Detect bzip2 program

[Laszlo Ersek]

On 01/14/21 18:45, Philippe Mathieu-Daudé wrote:
> The --enable-bzip2/--disable-bzip2 configure arguments are
> somehow misleading, they check for the bzip2 library, not
> the bzip2 program.
> 
> We need the bzip2 program to install the EDK2 firmware blobs
> (see commit 623ef637a2e "configure: Check bzip2 is available").
> 
> Check if the bzip2 program in the global meson.build to avoid
> the configuration to succeed, but a later when trying to install
> the firmware blobs:
> 
>     ../pc-bios/meson.build:5:2: ERROR: Program 'bzip2' not found
> 
> Reported-by: John Snow <jsnow@redhat.com>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Fixes: c8d5450bba3 ("configure: move install_blobs from configure to meson")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  meson.build         | 2 ++
>  pc-bios/meson.build | 1 -
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/meson.build b/meson.build
> index ecc45d04d6a..954152c90fe 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -96,6 +96,8 @@
>    endforeach
>  endif
>  
> +bzip2 = find_program('bzip2', required: install_edk2_blobs)
> +
>  ##################
>  # Compiler flags #
>  ##################
> diff --git a/pc-bios/meson.build b/pc-bios/meson.build
> index 6a341b6cea0..af95c5d1f1d 100644
> --- a/pc-bios/meson.build
> +++ b/pc-bios/meson.build
> @@ -1,5 +1,4 @@
>  if install_edk2_blobs
> -  bzip2 = find_program('bzip2', required: true)
>    fds = [
>      'edk2-aarch64-code.fd',
>      'edk2-arm-code.fd',
> 

Right, this looks sensible; still it makes me think patch#1 is only prep
for this. (That's OK, but then patch#1 should say so.)

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

Thanks
laszlo

Re: [PATCH 1/2] meson.build: Declare global edk2_targets / install_edk2_blobs variables

[Philippe Mathieu-Daudé]

On 1/14/21 8:49 PM, Laszlo Ersek wrote:
> On 01/14/21 18:45, Philippe Mathieu-Daudé wrote:
>> Globally declare in the main meson.build:
>> - the list of EDK2 targets,
>> - whether the EDK2 blobs have to be installed.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> Patch trivial to review using 'git-diff --ignore-all-space'
>> ---
>>  meson.build                     |  8 ++++++++
>>  pc-bios/descriptors/meson.build | 30 ++++++++++++++++--------------
>>  pc-bios/meson.build             |  5 +----
>>  3 files changed, 25 insertions(+), 18 deletions(-)
>>
>> diff --git a/meson.build b/meson.build
>> index 3d889857a09..ecc45d04d6a 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -88,6 +88,14 @@
>>    }
>>  endif
>>  
>> +edk2_targets = [ 'arm-softmmu', 'aarch64-softmmu', 'i386-softmmu', 'x86_64-softmmu' ]
>> +install_edk2_blobs = false
>> +if get_option('install_blobs')
>> +  foreach target : target_dirs
>> +    install_edk2_blobs = install_edk2_blobs or target in edk2_targets
>> +  endforeach
>> +endif
>> +
>>  ##################
>>  # Compiler flags #
>>  ##################
>> diff --git a/pc-bios/descriptors/meson.build b/pc-bios/descriptors/meson.build
>> index 7040834573d..ac6ec66b007 100644
>> --- a/pc-bios/descriptors/meson.build
>> +++ b/pc-bios/descriptors/meson.build
>> @@ -1,14 +1,16 @@
>> -foreach f: [
>> -  '50-edk2-i386-secure.json',
>> -  '50-edk2-x86_64-secure.json',
>> -  '60-edk2-aarch64.json',
>> -  '60-edk2-arm.json',
>> -  '60-edk2-i386.json',
>> -  '60-edk2-x86_64.json'
>> -]
>> -  configure_file(input: files(f),
>> -                 output: f,
>> -                 configuration: {'DATADIR': qemu_datadir},
>> -                 install: get_option('install_blobs'),
>> -                 install_dir: qemu_datadir / 'firmware')
>> -endforeach
>> +if install_edk2_blobs
>> +  foreach f: [
>> +    '50-edk2-i386-secure.json',
>> +    '50-edk2-x86_64-secure.json',
>> +    '60-edk2-aarch64.json',
>> +    '60-edk2-arm.json',
>> +    '60-edk2-i386.json',
>> +    '60-edk2-x86_64.json'
>> +  ]
>> +    configure_file(input: files(f),
>> +                   output: f,
>> +                   configuration: {'DATADIR': qemu_datadir},
>> +                   install: get_option('install_blobs'),
>> +                   install_dir: qemu_datadir / 'firmware')
>> +  endforeach
>> +endif
>> diff --git a/pc-bios/meson.build b/pc-bios/meson.build
>> index fab323af84e..6a341b6cea0 100644
>> --- a/pc-bios/meson.build
>> +++ b/pc-bios/meson.build
>> @@ -1,7 +1,4 @@
>> -if 'arm-softmmu' in target_dirs or \
>> -    'aarch64-softmmu' in target_dirs or \
>> -    'i386-softmmu' in target_dirs or \
>> -    'x86_64-softmmu' in target_dirs
>> +if install_edk2_blobs
>>    bzip2 = find_program('bzip2', required: true)
>>    fds = [
>>      'edk2-aarch64-code.fd',
>>
> 
> I vaguely understand what this patch does (I haven't followed the meson
> conversion), but I'm unsure why it does that.
> 
> Is this patch useful in itself, or only in preparation for patch#2?

Well, something I forgot to mention is it disable the configure_file()
calls when arm/aarch64/i386/x86_64-softmmu targets are not selected.

(currently if you configure a restricted set of targets, such
--target-list=riscv64-softmmu,ppc-softmmu, it is called, and
the json descriptors files are installed, even if these targets
don't require them).

Re: [PATCH 1/2] meson.build: Declare global edk2_targets / install_edk2_blobs variables

[Laszlo Ersek]

On 01/14/21 20:57, Philippe Mathieu-Daudé wrote:
> On 1/14/21 8:49 PM, Laszlo Ersek wrote:
>> On 01/14/21 18:45, Philippe Mathieu-Daudé wrote:
>>> Globally declare in the main meson.build:
>>> - the list of EDK2 targets,
>>> - whether the EDK2 blobs have to be installed.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>> Patch trivial to review using 'git-diff --ignore-all-space'
>>> ---
>>>  meson.build                     |  8 ++++++++
>>>  pc-bios/descriptors/meson.build | 30 ++++++++++++++++--------------
>>>  pc-bios/meson.build             |  5 +----
>>>  3 files changed, 25 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/meson.build b/meson.build
>>> index 3d889857a09..ecc45d04d6a 100644
>>> --- a/meson.build
>>> +++ b/meson.build
>>> @@ -88,6 +88,14 @@
>>>    }
>>>  endif
>>>  
>>> +edk2_targets = [ 'arm-softmmu', 'aarch64-softmmu', 'i386-softmmu', 'x86_64-softmmu' ]
>>> +install_edk2_blobs = false
>>> +if get_option('install_blobs')
>>> +  foreach target : target_dirs
>>> +    install_edk2_blobs = install_edk2_blobs or target in edk2_targets
>>> +  endforeach
>>> +endif
>>> +
>>>  ##################
>>>  # Compiler flags #
>>>  ##################
>>> diff --git a/pc-bios/descriptors/meson.build b/pc-bios/descriptors/meson.build
>>> index 7040834573d..ac6ec66b007 100644
>>> --- a/pc-bios/descriptors/meson.build
>>> +++ b/pc-bios/descriptors/meson.build
>>> @@ -1,14 +1,16 @@
>>> -foreach f: [
>>> -  '50-edk2-i386-secure.json',
>>> -  '50-edk2-x86_64-secure.json',
>>> -  '60-edk2-aarch64.json',
>>> -  '60-edk2-arm.json',
>>> -  '60-edk2-i386.json',
>>> -  '60-edk2-x86_64.json'
>>> -]
>>> -  configure_file(input: files(f),
>>> -                 output: f,
>>> -                 configuration: {'DATADIR': qemu_datadir},
>>> -                 install: get_option('install_blobs'),
>>> -                 install_dir: qemu_datadir / 'firmware')
>>> -endforeach
>>> +if install_edk2_blobs
>>> +  foreach f: [
>>> +    '50-edk2-i386-secure.json',
>>> +    '50-edk2-x86_64-secure.json',
>>> +    '60-edk2-aarch64.json',
>>> +    '60-edk2-arm.json',
>>> +    '60-edk2-i386.json',
>>> +    '60-edk2-x86_64.json'
>>> +  ]
>>> +    configure_file(input: files(f),
>>> +                   output: f,
>>> +                   configuration: {'DATADIR': qemu_datadir},
>>> +                   install: get_option('install_blobs'),
>>> +                   install_dir: qemu_datadir / 'firmware')
>>> +  endforeach
>>> +endif
>>> diff --git a/pc-bios/meson.build b/pc-bios/meson.build
>>> index fab323af84e..6a341b6cea0 100644
>>> --- a/pc-bios/meson.build
>>> +++ b/pc-bios/meson.build
>>> @@ -1,7 +1,4 @@
>>> -if 'arm-softmmu' in target_dirs or \
>>> -    'aarch64-softmmu' in target_dirs or \
>>> -    'i386-softmmu' in target_dirs or \
>>> -    'x86_64-softmmu' in target_dirs
>>> +if install_edk2_blobs
>>>    bzip2 = find_program('bzip2', required: true)
>>>    fds = [
>>>      'edk2-aarch64-code.fd',
>>>
>>
>> I vaguely understand what this patch does (I haven't followed the meson
>> conversion), but I'm unsure why it does that.
>>
>> Is this patch useful in itself, or only in preparation for patch#2?
> 
> Well, something I forgot to mention is it disable the configure_file()
> calls when arm/aarch64/i386/x86_64-softmmu targets are not selected.
> 
> (currently if you configure a restricted set of targets, such
> --target-list=riscv64-softmmu,ppc-softmmu, it is called, and
> the json descriptors files are installed, even if these targets
> don't require them).
> 

Oof, sorry for missing that.

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

Laszlo