[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 07/10] roms: build edk2 firmware binaries and va
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCH 07/10] roms: build edk2 firmware binaries and variable store templates |
Date: |
Tue, 12 Mar 2019 16:15:35 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
On 03/11/19 13:09, Eric Blake wrote:
> On 3/10/19 10:10 AM, Philippe Mathieu-Daudé wrote:
>> Hi Laszlo,
>>
>> On 3/9/19 5:48 PM, Philippe Mathieu-Daudé wrote:
>>> On 3/9/19 1:48 AM, Laszlo Ersek wrote:
>>>> Add the "efi" target to "Makefile".
>>>>
>
>>>> +
>>>> +toolchain = $(shell source ./edk2-funcs.sh && qemu_edk2_get_toolchain
>>>> $(1))
>>
>> Well I finally figured out why building on Ubuntu fails. It default
>> shell is dash, and 'source' is a bash builtin command. The portable
>> equivalent is '.' (dot).
Yes, I'm aware:
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_18
also,
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_01_01
If the command name matches the name of a utility listed in the
following table, the results are unspecified [...] /source/
>> The fix is:
>>
>> -- >8 --
>> -toolchain = $(shell source ./edk2-funcs.sh && qemu_edk2_get_toolchain $(1))
>> +toolchain = $(shell . ./edk2-funcs.sh && qemu_edk2_get_toolchain $(1))
>
> Ouch - this changes my analysis in 1/10, where I argued that since the
> file was only ever sourced by a bash script, its use of 'local' was
> okay. Now that you are also sourcing it from /bin/sh via Makefile, you
> HAVE to make edk2-funcs.sh portable to POSIX shell, by eliminating use
> of 'local'.
>
I'm acutely aware of portable vs. non-portable shell scripts. While
working on the code, I specifically checked that "local" was not
specified in SUSv4. The point is, I didn't care, because I didn't target
a POSIX system, but a GNU system.
We already require GNU Make, to my knowledge. Perhaps not by decree, but
through the feature set that we use.
(
For example, the extended description of the portable "Makefile"
language, at
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/make.html#tag_20_76_13
doesn't even mention $(shell), or other parametrizable macros.
)
I really want to keep "local", as it keeps the shell variable namespace
clean. Regarding the functions that I did put in the "global namespace",
I was careful to prefix them suitably.
IMO, it's perfectly fine to require the shell to be bash here, given
that this feature is meant for a subset of maintainers (and not for
end-users), and that building edk2 already requires a quite sizeable set
of packages installed (such as nasm, iasl, ...) So this feature is not
meant for any random POSIX system.
What I did miss in fact was that "GNU Make" didn't imply "/bin/bash":
https://www.gnu.org/software/make/manual/make.html#Choosing-the-Shell
The program used as the shell is taken from the variable SHELL. If
this variable is not set in your makefile, the program /bin/sh is
used as the shell.
Thus, the real fix here is to be explicit about the bash requirement. I
should add
SHELL=/bin/bash
near the top of "Makefile.edk2".
(The GNU make documentation also provides examples with
SHELL=/usr/bin/perl, so I think SHELL=/bin/bash should be safe,
generally speaking.)
Phil: can you please test whether SHELL=/bin/bash works for you on
Ubuntu? (After you install "bash", of course.)
Thanks!
Laszlo
- Re: [Qemu-devel] [PATCH 05/10] roms/edk2-funcs.sh: add the qemu_edk2_get_thread_count() function, (continued)
[Qemu-devel] [PATCH 07/10] roms: build edk2 firmware binaries and variable store templates, Laszlo Ersek, 2019/03/08
Re: [Qemu-devel] [PATCH 07/10] roms: build edk2 firmware binaries and variable store templates, Philippe Mathieu-Daudé, 2019/03/10
[Qemu-devel] [PATCH 08/10] pc-bios: add edk2 firmware binaries and variable store templates, Laszlo Ersek, 2019/03/08
[Qemu-devel] [PATCH 09/10] pc-bios: document the edk2 firmware images; add firmware descriptors, Laszlo Ersek, 2019/03/08
[Qemu-devel] [PATCH 10/10] Makefile: install the edk2 firmware images and their descriptors, Laszlo Ersek, 2019/03/08
Re: [Qemu-devel] [PATCH 00/10] bundle edk2 platform firmware with QEMU, Philippe Mathieu-Daudé, 2019/03/08