| Message ID | 20190309004826.9027-7-lersek@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | bundle edk2 platform firmware with QEMU | expand |
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 <lersek@redhat.com> > --- > 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/. > $(MAKE) -C edk2/BaseTools > > slof: >
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 <lersek@redhat.com> >> --- >> 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. >> $(MAKE) -C edk2/BaseTools >> >> slof: >>
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 <lersek@redhat.com> >>> --- >>> 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? Regardless: Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com> >>> $(MAKE) -C edk2/BaseTools >>> >>> slof: >>>
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 <lersek@redhat.com> >>>> --- >>>> 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é <philmd@redhat.com> > Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com> Thanks! Laszlo > >>>> $(MAKE) -C edk2/BaseTools >>>> >>>> slof: >>>>
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: $(MAKE) -C edk2/BaseTools slof:
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 <lersek@redhat.com> --- roms/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)