[Buildroot] [PATCH 1/1] package/libnss: fix parallel build failure

[Giulio Benetti]

Or…

> Il giorno 19 ott 2021, alle ore 19:40, Giulio Benetti <giulio.benetti at benettiengineering.com> ha scritto:
> 
> Hi Arnout,
> 
>>> Il giorno 6 ott 2021, alle ore 21:20, Arnout Vandecappelle <arnout at mind.be> ha scritto:
>>> 
>> 
>> 
>>> On 22/09/2021 01:59, Giulio Benetti wrote:
>>> As explained in the patch itself(pending here[1]), I couldn't reproduce
>>> the failure but it seems to be right. Basically I'm making not parallel
>>> every 'nsinstall -R' command that is where it fails.
>>> [1]: https://bugzilla.mozilla.org/show_bug.cgi?id=1731911
>>> Fixes:
>>> http://autobuild.buildroot.net/results/a895ed7aa48edc5cd9ac8bbbd689433a98c9c057/
>>> Signed-off-by: Giulio Benetti <giulio.benetti at benettiengineering.com>
>>> ---
>>>  ...31911-Fix-nsinstall-parallel-failure.patch | 34 +++++++++++++++++++
>>>  1 file changed, 34 insertions(+)
>>>  create mode 100644 package/libnss/0001-Bug-1731911-Fix-nsinstall-parallel-failure.patch
>>> diff --git a/package/libnss/0001-Bug-1731911-Fix-nsinstall-parallel-failure.patch b/package/libnss/0001-Bug-1731911-Fix-nsinstall-parallel-failure.patch
>>> new file mode 100644
>>> index 0000000000..7ac340f1ca
>>> --- /dev/null
>>> +++ b/package/libnss/0001-Bug-1731911-Fix-nsinstall-parallel-failure.patch
>>> @@ -0,0 +1,34 @@
>>> +From 7ba4d627212507f876dc112e89c68e82e1c50188 Mon Sep 17 00:00:00 2001
>>> +From: Giulio Benetti <giulio.benetti at benettiengineering.com>
>>> +Date: Wed, 22 Sep 2021 01:40:31 +0200
>>> +Subject: [PATCH] Bug 1731911 - Fix nsinstall parallel failure
>>> +
>>> +Fix nsinstall race condition when building using Makefile with parallel
>>> +options on. I've end up making $(1) recipe in copy_varlist_into_dir_RULE
>>> +.NOTPARALLEL. I can't test it that much since it's very very rare, but
>>> +it seems to be the correct way. This way every copy_varlist_into_dir_RULE
>>> +called should occur not parallel and I think that this is acceptable
>>> +since it only create symlinks that takes very few. Further improving is
>>> +possible of course.
>>> +
>>> +Signed-off-by: Giulio Benetti <giulio.benetti at benettiengineering.com>
>>> +---
>>> + nss/coreconf/rules.mk | 2 ++
>>> + 1 file changed, 2 insertions(+)
>>> +
>>> +diff --git a/nss/coreconf/rules.mk b/nss/coreconf/rules.mk
>>> +index dc72e9cef..a341b8385 100644
>>> +--- a/nss/coreconf/rules.mk
>>> ++++ b/nss/coreconf/rules.mk
>>> +@@ -377,6 +377,8 @@ $(3)/%: %
>>> +    $$(INSTALL) -m 444 $$^ $(3)
>>> +
>>> + $(1): $$(addprefix $(3)/,$$($(2))) | $(3)/d
>>> ++
>>> ++.NOTPARALLEL: $(1)
>> 
>> This doesn't make a whole lot of sense: .NOTPARALLEL is something that applies to the Makefile as a whole, not to one specific target. I haven't looked in any depth in the libnss build system, but unless it builds recursively, adding this is exactly equivalent to adding
>> 
>> .NOTPARALLEL:
> 
> As I’ve tested it’s not like this.
> 
> I read this from [1]:
> ‘’’
> .NOTPARALLEL
> 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). Any prerequisites on this target are ignored.
> 
> ‘’’
> 
> They talk about target and it is what I experience.
> It’s a bit ambiguous honestly. In the past I’ve also
> experienced it on a big Makefile where I had .bmp files to be converted with xxd and that gave parallel problems. So I’ve tagged it like:
> 
> .NOTPARALLEL: %.bmp
> 
> and that solved the problem.

…it is because in both cases they are called recursively.

Giulio

> 
> Also they have just upstreamed the patch [2], so
> we can test if it works committing this patch or
> by bumping next version.
> 
> Then it’s up to you :-)
> 
> Best regards
> Giulio Benetti
> Benetti Engineering sas
> 
> [1]: https://www.gnu.org/software/make/manual/html_node/Special-Targets.html
> [2]: https://bugzilla.mozilla.org/show_bug.cgi?id=1731911
> 
>> 
>> anywhere in the Makefiles. This is BTW what we do in Buildroot.
>> 
>> And in the end, that's the same as calling $(MAKE1) from Buildroot, so patching is not really needed.
>> 
>> If you want to be a little smarter, you will have to manually "unroll" the dependency, like:
>> 
>> $(1): | $(3)/d
>>    $$(foreach tgt,$(2),$(MAKE) $(3)/$$($(tgt)))
>> 
>> (chances are I made some major mistake in this thing).
>> 
>> Or if it does build recursively, you can probably conditionally add .NOTPARALLEL in some smart way.
>> 
>> Anyway, I've marked this patch as Changed Requested.
>> 
>> Regards,
>> Arnout
>> 
>> 
>>> + endif
>>> + else
>>> + $(1):
>>> +--
>>> +2.25.1
>>> +
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.buildroot.org/pipermail/buildroot/attachments/20211019/e1a2d55f/attachment.html>