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

From: Laszlo Ersek <lersek@redhat.com>
To: "Philippe Mathieu-Daudé" <philmd@redhat.com>, qemu-devel@nongnu.org
Cc: Paolo Bonzini <pbonzini@redhat.com>, John Snow <jsnow@redhat.com>
Subject: Re: [PATCH 1/2] meson.build: Declare global edk2_targets / install_edk2_blobs variables
Date: Thu, 14 Jan 2021 21:05:36 +0100	[thread overview]
Message-ID: <174f3a18-22b2-eb15-f91c-99b690e4fbc1@redhat.com> (raw)
In-Reply-To: <dec726cf-5247-dcbe-914a-a54b0d4b2b03@redhat.com>

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