Re: [Qemu-devel] [PATCH 4/5] tests/uefi-test-tools: add build scripts

[Laszlo Ersek]

On 01/22/19 13:02, Laszlo Ersek wrote:
> On 01/21/19 20:30, Peter Maydell wrote:
>> On Mon, 21 Jan 2019 at 19:09, Laszlo Ersek <address@hidden> wrote:
>>> It wasn't clear to me whether and how multi-threaded builds were
>>> supposed to be used by maintainers, whenever they'd update
>>> "tests/data/uefi-boot-images/*".
>>>
>>> I saw that "make" was invoked everywhere as $(MAKE), but that didn't
>>> clarify any intent around "-j". So I didn't test "-j" at all, and in
>>> fact I wouldn't have expected it to work:
>>
>> The usual assumption with make is that "-jN" should work
>> and at least (if the thing being built can't actually
>> be parallelized usefully) be no worse than if you'd not
>> specified a -j option. We have occasionally had problems
>> with -jN in 'make check' (usually because several test cases
>> were trying to use the same temp filename or similar) but
>> we've treated them as bugs and squashed them.
>>
>>> The "build" base tool in edk2 implements part of the job with generated
>>> makefiles and invoking "make" itself, thus, despite .NOTPARALLEL, it
>>> likely inherits the outermost -j<N> setting -- and it doesn't expect such.
>>>
>>> So the best I can offer here is to check $MAKEFLAGS in "build.sh", and
>>> exit with an early, explicit error if $MAKEFLAGS contains "-j", "-l", or
>>> their long variants (--jobs, --load-average).
>>
>> Could you sanitize MAKEFLAGS in build.sh instead to remove the
>> parallelization options?
> 
> I've looked into MAKEFLAGS in a bit more depth now; both the
> documentation and some debug prints. Manipulating MAKEFLAGS looks
> somewhat brittle.
> 
> So, I was about to suggest that I use the .NOTPARALLEL special target
> recommended by Phil, for the .efi binaries (which would ensure that no
> two instances of the "build" base tool run at the same time), *plus*
> that I submit a patch to edk2 so that the makefiles generated by the
> "build" tool also contain .NOTPARALLEL.
> 
> (Because, to quote the make docs again, "If .NOTPARALLEL is mentioned as
> a target, then this invocation of make will be run serially, even if the
> ‘-j’ option is given. Any recursively invoked make command will still
> run recipes in parallel (unless its makefile also contains this target).")
> 
> However: when I wanted to see the actual error from using .NOTPARALLEL
> *only* in "tests/uefi-test-tools/Makefile", and not in the
> build-generated makefiles, I failed to get any error. All the output
> images were built just fine.
> 
> Phil: when you wrote that "The following patch didn't help" -- referring
> to .NOTPARALLEL, added only to "tests/uefi-test-tools/Makefile"-- , did
> you clean your tree first (with "make clean" or "git clean -ffdx")?
> 
> Because now I'm thinking that the *individual* makefiles generated by
> edk2's "build" base tool might actually compatible with "-j", and your
> testing of .NOTPARALLEL failed only because your build tree (for example
> the Conf/ subdir -- list it with "-A") was in a messy state from your
> previous -j4 attempt (where you didn't use .NOTPARALLEL at all).

FWIW, I've asked

- both on edk2-devel, about adding .NOTPARALLEL to the generated inner
makefiles:

  "parallelism in the module-level, generated GNUmakefile's"
  https://lists.01.org/pipermail/edk2-devel/2019-January/035463.html

- and on help-make, about safely filtering the job-related options from
MAKEFLAGS, between the outer and inner "make" processes:

  "filtering job options from MAKEFLAGS, manually"
  http://lists.gnu.org/archive/html/help-make/2019-01/msg00003.html

Thanks
Laszlo