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
