Patchwork [v4,3/3] package/Makefile.in: add a way to don't force jobs in sub-make

login
register
mail settings
Submitter Fabio Porcedda
Date Sept. 17, 2013, 7:59 a.m.
Message ID <1379404753-3471-4-git-send-email-fabio.porcedda@gmail.com>
Download mbox | patch
Permalink /patch/275394/
State Superseded
Headers show

Comments

Fabio Porcedda - Sept. 17, 2013, 7:59 a.m.
When the "BR2_JLEVEL" variable is empty use "make"
without the "-j" option.
To be able to use top-level parallel make we must don't force
the number of jobs in sub-make.

Example:
	make BR2_JLEVEL= -j8

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 package/Makefile.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Thomas Petazzoni - Sept. 17, 2013, 6:17 p.m.
Dear Fabio Porcedda,

That's a minor nit, but "add a way to don't force jobs in sub-make"
isn't really correct english I blieve. "add away to not force the
number of jobs in sub-make" or something like that would be more
appropriate.

Thomas
Ciarán Rehill - Sept. 17, 2013, 6:24 p.m.
Maybe "add a way to prevent forcing of [parallel?] jobs in sub-make"?
On Sep 17, 2013 7:18 PM, "Thomas Petazzoni" <
thomas.petazzoni@free-electrons.com> wrote:

> Dear Fabio Porcedda,
>
> That's a minor nit, but "add a way to don't force jobs in sub-make"
> isn't really correct english I blieve. "add away to not force the
> number of jobs in sub-make" or something like that would be more
> appropriate.
>
> Thomas
> --
> Thomas Petazzoni, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
>
Fabio Porcedda - Sept. 19, 2013, 6:35 a.m.
Hi Thomas and Claràn,
thanks for reviewing.

On Tue, Sep 17, 2013 at 8:24 PM, Ciarán Rehill <cir.vfi@gmail.com> wrote:
> Maybe "add a way to prevent forcing of [parallel?] jobs in sub-make"?
>
> On Sep 17, 2013 7:18 PM, "Thomas Petazzoni"
> <thomas.petazzoni@free-electrons.com> wrote:
>>
>> Dear Fabio Porcedda,
>>
>> That's a minor nit, but "add a way to don't force jobs in sub-make"
>> isn't really correct english I blieve. "add away to not force the
>> number of jobs in sub-make" or something like that would be more
>> appropriate.

What about a mix of both?
"add a way to prevent forcing the number of jobs in sub-make"

Thanks
Thomas De Schampheleire - Sept. 19, 2013, 6:44 a.m.
Op 19-sep.-2013 08:35 schreef "Fabio Porcedda" <fabio.porcedda@gmail.com>
het volgende:
>
> Hi Thomas and Claràn,
> thanks for reviewing.
>
> On Tue, Sep 17, 2013 at 8:24 PM, Ciarán Rehill <cir.vfi@gmail.com> wrote:
> > Maybe "add a way to prevent forcing of [parallel?] jobs in sub-make"?
> >
> > On Sep 17, 2013 7:18 PM, "Thomas Petazzoni"
> > <thomas.petazzoni@free-electrons.com> wrote:
> >>
> >> Dear Fabio Porcedda,
> >>
> >> That's a minor nit, but "add a way to don't force jobs in sub-make"
> >> isn't really correct english I blieve. "add away to not force the
> >> number of jobs in sub-make" or something like that would be more
> >> appropriate.
>
> What about a mix of both?
> "add a way to prevent forcing the number of jobs in sub-make"
>

In fact, we don't add a way, since the user cannot choose to force or not.
So I'd suggest something like:
Don't force the number of jobs...
Or if you prefer:
Prevent forcing...

Best regards,
Thomas
Fabio Porcedda - Sept. 19, 2013, 6:59 a.m.
Hi Thomas,
thanks for reviewing.

On Thu, Sep 19, 2013 at 8:44 AM, Thomas De Schampheleire
<patrickdepinguin@gmail.com> wrote:
>
> Op 19-sep.-2013 08:35 schreef "Fabio Porcedda" <fabio.porcedda@gmail.com>
> het volgende:
>
>
>>
>> Hi Thomas and Claràn,
>> thanks for reviewing.
>>
>> On Tue, Sep 17, 2013 at 8:24 PM, Ciarán Rehill <cir.vfi@gmail.com> wrote:
>> > Maybe "add a way to prevent forcing of [parallel?] jobs in sub-make"?
>> >
>> > On Sep 17, 2013 7:18 PM, "Thomas Petazzoni"
>> > <thomas.petazzoni@free-electrons.com> wrote:
>> >>
>> >> Dear Fabio Porcedda,
>> >>
>> >> That's a minor nit, but "add a way to don't force jobs in sub-make"
>> >> isn't really correct english I blieve. "add away to not force the
>> >> number of jobs in sub-make" or something like that would be more
>> >> appropriate.
>>
>> What about a mix of both?
>> "add a way to prevent forcing the number of jobs in sub-make"
>>
>
> In fact, we don't add a way, since the user cannot choose to force or not.

IMHO the user can choose  by means of the "BR2_JLEVEL" variable, if
that variable contains a value buildroot will force the number of
jobs, if that variable is empty buildroot will not force the number of
jobs.

> So I'd suggest something like:
> Don't force the number of jobs...
> Or if you prefer:
> Prevent forcing...

Thanks and best regards
Thomas De Schampheleire - Sept. 19, 2013, 7:11 a.m.
Hi Fabio,

On Thu, Sep 19, 2013 at 8:59 AM, Fabio Porcedda
<fabio.porcedda@gmail.com> wrote:
[..]
>> In fact, we don't add a way, since the user cannot choose to force or not.
>
> IMHO the user can choose  by means of the "BR2_JLEVEL" variable, if
> that variable contains a value buildroot will force the number of
> jobs, if that variable is empty buildroot will not force the number of
> jobs.

True, but BR2_JLEVEL was already there before your patch. What your
patch fixes is the situation where BR2_JLEVEL is empty, not 0, 1, 2,
3, ... as the original code expected. So, maybe a better title is:

package/Makefile.in: don't force parallel building when BR2_JLEVEL is empty

Best regards,
Thomas
Fabio Porcedda - Sept. 20, 2013, 2:57 p.m.
On Thu, Sep 19, 2013 at 9:11 AM, Thomas De Schampheleire
<patrickdepinguin@gmail.com> wrote:
> Hi Fabio,
>
> On Thu, Sep 19, 2013 at 8:59 AM, Fabio Porcedda
> <fabio.porcedda@gmail.com> wrote:
> [..]
>>> In fact, we don't add a way, since the user cannot choose to force or not.
>>
>> IMHO the user can choose  by means of the "BR2_JLEVEL" variable, if
>> that variable contains a value buildroot will force the number of
>> jobs, if that variable is empty buildroot will not force the number of
>> jobs.
>
> True, but BR2_JLEVEL was already there before your patch. What your
> patch fixes is the situation where BR2_JLEVEL is empty, not 0, 1, 2,
> 3, ... as the original code expected. So, maybe a better title is:
>
> package/Makefile.in: don't force parallel building when BR2_JLEVEL is empty

Ok it's fine for me.

Thanks and best regards
Fabio Porcedda - Sept. 23, 2013, 8 a.m.
On Fri, Sep 20, 2013 at 4:57 PM, Fabio Porcedda
<fabio.porcedda@gmail.com> wrote:
> On Thu, Sep 19, 2013 at 9:11 AM, Thomas De Schampheleire
> <patrickdepinguin@gmail.com> wrote:
>> Hi Fabio,
>>
>> On Thu, Sep 19, 2013 at 8:59 AM, Fabio Porcedda
>> <fabio.porcedda@gmail.com> wrote:
>> [..]
>>>> In fact, we don't add a way, since the user cannot choose to force or not.
>>>
>>> IMHO the user can choose  by means of the "BR2_JLEVEL" variable, if
>>> that variable contains a value buildroot will force the number of
>>> jobs, if that variable is empty buildroot will not force the number of
>>> jobs.
>>
>> True, but BR2_JLEVEL was already there before your patch. What your
>> patch fixes is the situation where BR2_JLEVEL is empty, not 0, 1, 2,
>> 3, ... as the original code expected. So, maybe a better title is:
>>
>> package/Makefile.in: don't force parallel building when BR2_JLEVEL is empty
>
> Ok it's fine for me.

Well the line it's too long (75 character), i will use:

package/Makefile.in: don't force jobs when BR2_JLEVEL is empty

Best regards

Patch

diff --git a/package/Makefile.in b/package/Makefile.in
index 170ad78..d406901 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -18,7 +18,7 @@  PARALLEL_JOBS:=$(BR2_JLEVEL)
 endif
 
 MAKE1:=$(HOSTMAKE) -j1
-MAKE:=$(HOSTMAKE) -j$(PARALLEL_JOBS)
+MAKE:=$(HOSTMAKE) $(if $(PARALLEL_JOBS),-j$(PARALLEL_JOBS))
 
 # Compute GNU_TARGET_NAME
 GNU_TARGET_NAME=$(ARCH)-buildroot-$(TARGET_OS)-$(LIBC)$(ABI)