roms: Rename the EFIROM variable and let it be overridable

[Qemu-devel] [PATCH for-4.0 v2 0/2] roms: Rename the EFIROM variable and let it be overridable

Posted by Philippe Mathieu-Daudé 6 years, 10 months ago

Hi,

Two trivial fixes to avoid the latest EDK2 testing series to
cause trouble to downstream distributions (in particular if
they have PIE enforced).

Regards,

Phil.

Philippe Mathieu-Daudé (2):
  roms: Rename the EFIROM variable to avoid clashing with iPXE
  roms: Allow the EDK2_EFIROM variable to be overridden

 roms/Makefile | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

-- 
2.20.1

Re: [Qemu-devel] [PATCH for-4.0 v2 0/2] roms: Rename the EFIROM variable and let it be overridable

Posted by Laszlo Ersek 6 years, 10 months ago

On 04/05/19 17:33, Philippe Mathieu-Daudé wrote:
> Hi,
>
> Two trivial fixes to avoid the latest EDK2 testing series to
> cause trouble to downstream distributions (in particular if
> they have PIE enforced).

I disgree with this.

(1) In the first commit message, you say,

"The iPXE project already uses the EFIROM for a tool named 'efirom'
which is not the Intel EfiRom used by the EDK2 project".

That's wrong. For building the combined (UEFI+BIOS) iPXE oprom binaries,
the efirom tool that is invoked is *most definitely* the tool from edk2.

What changes is that we now build efirom directly from the edk2
submodule, rather than using a binary that could possibly be found on a
GNU/Linux system from another package.

This is entirely aligned with the addition of the edk2 submodule. The
source for the efirom tool is now directly available, so in a
*maintainer* build -- i.e., when you decide to rebuild iPXE binaries --
we should certainly prefer to build everything from source.

Again, this is a *maintainer* build (which also covers downstream
package builds), not end-user build. If you decide to rebuild artifacts
that are otherwise offered in binary form to end-users, you commit to
building everything from source that goes into (or is necessary for)
producing those artifacts.

In the thread "edk2 fails to compile in v4.0.0-rc2", Olaf wrote,

"It is in ovmf-tools.rpm, which comes from ovmf."

That only proves my point.

(2) For a while now, it has been possible for downstream build scripts
to inject their preferred build flags into the BaseTools build recipes
(makefiles) themselves. Please see
<https://bugzilla.redhat.com/show_bug.cgi?id=1540244>. This is the
relevant upstream commit list:

  1  67983484a443 BaseTools/footer.makefile: expand BUILD_CFLAGS last
                  for C files too
  2  03252ae287c4 BaseTools/header.makefile: remove "-c" from
                  BUILD_CFLAGS
  3  b8a661702643 BaseTools/Source/C: split "-O2" to BUILD_OPTFLAGS
  4  b0ca5dae78ff BaseTools/Source/C: take EXTRA_OPTFLAGS from the
                  caller
  5  81502cee20ac BaseTools/Source/C: take EXTRA_LDFLAGS from the caller
  6  aa4e0df1f0c7 BaseTools/VfrCompile: honor EXTRA_LDFLAGS

Build BaseTools as follows:

  make -C "$EDK_TOOLS_PATH" EXTRA_OPTFLAGS="..."  EXTRA_LDFLAGS="..."

If you need to inject PIC/PIE-related flags into the BaseTools
compilation/linking, please use the above facility.

I think it's pretty usual that new upstream releases (of any open source
project) bring some changes for downstream packaging scripts.

Thanks
Laszlo

Re: [Qemu-devel] [PATCH for-4.0 v2 0/2] roms: Rename the EFIROM variable and let it be overridable

Posted by Laszlo Ersek 6 years, 10 months ago

On 04/08/19 11:02, Laszlo Ersek wrote:
> On 04/05/19 17:33, Philippe Mathieu-Daudé wrote:
>> Hi,
>>
>> Two trivial fixes to avoid the latest EDK2 testing series to
>> cause trouble to downstream distributions (in particular if
>> they have PIE enforced).
> 
> I disgree with this.
> 
> (1) In the first commit message, you say,
> 
> "The iPXE project already uses the EFIROM for a tool named 'efirom'
> which is not the Intel EfiRom used by the EDK2 project".
> 
> That's wrong. For building the combined (UEFI+BIOS) iPXE oprom binaries,
> the efirom tool that is invoked is *most definitely* the tool from edk2.
> 
> What changes is that we now build efirom directly from the edk2
> submodule, rather than using a binary that could possibly be found on a
> GNU/Linux system from another package.
> 
> This is entirely aligned with the addition of the edk2 submodule. The
> source for the efirom tool is now directly available, so in a
> *maintainer* build -- i.e., when you decide to rebuild iPXE binaries --
> we should certainly prefer to build everything from source.
> 
> Again, this is a *maintainer* build (which also covers downstream
> package builds), not end-user build. If you decide to rebuild artifacts
> that are otherwise offered in binary form to end-users, you commit to
> building everything from source that goes into (or is necessary for)
> producing those artifacts.
> 
> In the thread "edk2 fails to compile in v4.0.0-rc2", Olaf wrote,
> 
> "It is in ovmf-tools.rpm, which comes from ovmf."
> 
> That only proves my point.

Of course, ipxe.git itself has a macro called EFIROM:

src/Makefile:EFIROM             := ./util/efirom

but that has *zero* relevance for QEMU commit f590a812c210 ("roms: build
the EfiRom utility from the roms/edk2 submodule", 2019-02-21).

Because, the "efirom" utility from inside ipxe.git is not used for
anything at all in the *QEMU recipe* for building the combined iPXE
option ROMs.

Look at the diff in f590a812c210:

-EFIROM ?= $(shell which EfiRom 2>/dev/null)
+EFIROM = edk2/BaseTools/Source/C/bin/EfiRom

If the way we build the combined iPXE oproms, from QEMU's ipxe.git
submodule, using "roms/Makefile", had *anything* to do with iPXE's own
"efirom", then the pre-patch state would have been wrong *already*.

Laszlo

> (2) For a while now, it has been possible for downstream build scripts
> to inject their preferred build flags into the BaseTools build recipes
> (makefiles) themselves. Please see
> <https://bugzilla.redhat.com/show_bug.cgi?id=1540244>. This is the
> relevant upstream commit list:
> 
>   1  67983484a443 BaseTools/footer.makefile: expand BUILD_CFLAGS last
>                   for C files too
>   2  03252ae287c4 BaseTools/header.makefile: remove "-c" from
>                   BUILD_CFLAGS
>   3  b8a661702643 BaseTools/Source/C: split "-O2" to BUILD_OPTFLAGS
>   4  b0ca5dae78ff BaseTools/Source/C: take EXTRA_OPTFLAGS from the
>                   caller
>   5  81502cee20ac BaseTools/Source/C: take EXTRA_LDFLAGS from the caller
>   6  aa4e0df1f0c7 BaseTools/VfrCompile: honor EXTRA_LDFLAGS
> 
> Build BaseTools as follows:
> 
>   make -C "$EDK_TOOLS_PATH" EXTRA_OPTFLAGS="..."  EXTRA_LDFLAGS="..."
> 
> If you need to inject PIC/PIE-related flags into the BaseTools
> compilation/linking, please use the above facility.
> 
> I think it's pretty usual that new upstream releases (of any open source
> project) bring some changes for downstream packaging scripts.
> 
> Thanks
> Laszlo
>