Re: [Qemu-devel] [PATCH 06/10] roms/Makefile: replace the $(EFIROM) target with "edk2-basetools"

[Laszlo Ersek]

On 03/10/19 16:16, Philippe Mathieu-Daudé wrote:
> On 3/9/19 2:53 AM, Philippe Mathieu-Daudé wrote:
>> On 3/9/19 2:32 AM, Philippe Mathieu-Daudé wrote:
>>> Hi Laszlo,
>>>
>>> On 3/9/19 1:48 AM, Laszlo Ersek wrote:
>>>> We don't (can't) have a recipe for building just $(EFIROM); therefore,
>>>> while we call the target $(EFIROM), we actually build all of the edk2
>>>> BaseTools. Rename the target to edk2-basetools, and update the iPXE
>>>> prerequisite accordingly. This will let other targets depend on
>>>> "edk2-basetools", where an $(EFIROM) pre-requisite would be misleading.
>>>>
>>>> Signed-off-by: Laszlo Ersek <address@hidden>
>>>> ---
>>>>  roms/Makefile | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/roms/Makefile b/roms/Makefile
>>>> index 78d5dd18c301..2e83ececa25a 100644
>>>> --- a/roms/Makefile
>>>> +++ b/roms/Makefile
>>>> @@ -102,7 +102,7 @@ pxe-rom-%: build-pxe-roms
>>>>  
>>>>  efirom: $(patsubst %,efi-rom-%,$(pxerom_variants))
>>>>  
>>>> -efi-rom-%: build-pxe-roms build-efi-roms $(EFIROM)
>>>> +efi-rom-%: build-pxe-roms build-efi-roms edk2-basetools
>>>>    $(EFIROM) -f "0x$(VID)" -i "0x$(DID)" -l 0x02 \
>>>>            -b ipxe/src/bin/$(VID)$(DID).rom \
>>>>            -ec ipxe/src/bin-i386-efi/$(VID)$(DID).efidrv \
>>>> @@ -120,7 +120,7 @@ build-efi-roms: build-pxe-roms
>>>>            $(patsubst %,bin-i386-efi/%.efidrv,$(pxerom_targets)) \
>>>>            $(patsubst %,bin-x86_64-efi/%.efidrv,$(pxerom_targets))
>>>>  
>>>> -$(EFIROM):
>>>> +edk2-basetools:
>>>
>>> Should we add:
>>>
>>>         $(MAKE) -f Makefile.edk2 submodules
>>>
>>> from your next patch?
>>>
>>> There might be a circular dependency else, if the user doesn't init the
>>> submodules manually.
>>>
>>> The next patch add a make dependency 'efi' -> 'edk2-basetools', but
>>> 'edk2-basetools' build the BaseTools within edk2/.
>>>
>>
>> OK I mixed, the Makefile.edk2's submodules rules is for the edk2
>> repository submodules, while by 'the user submodules' I mean the QEMU
>> submodules...
>>
>> So the user has to initialize the roms/edk2 submodule previous to run
>> both efi/edk2-basetools rules of this Makefile.
> 
> Maybe we can 'test -e edk2/.git' and display an error "edk2 submodule
> not initialized.", what do you think?

The point was to simply stick with the current workflow for maintainers
that are already used to rebuilding ROM files from roms/, for pc-bios/.
Those maintainers are used to initializing / updating submodules
directly under roms/, but not used to initing / updating submodules
recursively (because edk2's OpenSSL submodule is the first such
submodule). This is the idea that's captured in the patches.

> Regardless:
> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
> Tested-by: Philippe Mathieu-Daudé <address@hidden>

Thanks!
Laszlo

> 
>>>>    $(MAKE) -C edk2/BaseTools
>>>>  
>>>>  slof:
>>>>