[Buildroot] [PATCH v2 0/3] Fix for top-level parallel make part 1

[Buildroot] [PATCH v2 0/3] Fix for top-level parallel make part 1

[Fabio Porcedda]

Hi all,
this is the first patch set for fixing top-level parallel make in buildroot,
the common problem scattered in buildroot's top-level makefile is that in the
rules it relies on the order of evaluation of the prerequisites,
to be able to use top-level parallel make instead of reling on the left to
right ordering of evaluation of the prerequisites we must add an explicit
rule to describe the dependency.

v2:
 - remove patch "package: add toolchain dependency to inner-generic-package"
   because was not working fine against recent toolchain changes.

Fabio Porcedda (3):
  package/Makefile.in: add a way to don't force jobs in sub-make
  package: fix generic extract target for top-level parallel make
  package: fix generic patch target for top-level parallel make

 package/Makefile.in    | 2 +-
 package/pkg-generic.mk | 7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)

-- 
1.8.1.4

[Buildroot] [PATCH v2 1/3] package/Makefile.in: add a way to don't force jobs in sub-make

[Fabio Porcedda]

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>
---
 package/Makefile.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/Makefile.in b/package/Makefile.in
index a597290..c36ee4c 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-linux-$(LIBC)$(ABI)
-- 
1.8.1.4

[Buildroot] [PATCH v2 1/3] package/Makefile.in: add a way to don't force jobs in sub-make

[Arnout Vandecappelle]

On 18/07/13 11:12, Fabio Porcedda wrote:
> 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>

  If only because it's a bad idea to start a linux build with 
-j<nothing>. I did that while testing the baseline for this patch and was 
caught in a swap storm...


  Regards,
  Arnout

> ---
>   package/Makefile.in | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/package/Makefile.in b/package/Makefile.in
> index a597290..c36ee4c 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-linux-$(LIBC)$(ABI)
>


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH v2 2/3] package: fix generic extract target for top-level parallel make

[Fabio Porcedda]

To be able to use top-level parallel make we must don't depend in a rule
on the order of evaluation of the prerequisites, so instead of reling on
the left to right ordering of evaluation of the prerequisites add
an explicit rule to describe the dependencies.

Add a rule to specify that the $(2)_TARGET_EXTRACT target depends
on $(2)_TARGET_SOURCE target.

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 package/pkg-generic.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 668f011..f29ea99 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -391,8 +391,8 @@ $(1)-configure:		$(1)-patch $(1)-depends \
 
 $(1)-patch:		$(1)-extract $$($(2)_TARGET_PATCH)
 
-$(1)-extract:		$(1)-source \
-			$$($(2)_TARGET_EXTRACT)
+$$($(2)_TARGET_EXTRACT):	$$($(2)_TARGET_SOURCE)
+$(1)-extract:			$$($(2)_TARGET_EXTRACT)
 
 $(1)-depends:		$$($(2)_DEPENDENCIES)
 
-- 
1.8.1.4

[Buildroot] [PATCH v2 2/3] package: fix generic extract target for top-level parallel make

[Arnout Vandecappelle]

On 18/07/13 11:12, Fabio Porcedda wrote:
> To be able to use top-level parallel make we must don't depend in a rule
> on the order of evaluation of the prerequisites, so instead of reling on
> the left to right ordering of evaluation of the prerequisites add
> an explicit rule to describe the dependencies.
>
> Add a rule to specify that the $(2)_TARGET_EXTRACT target depends
> on $(2)_TARGET_SOURCE target.
>
> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
> ---
>   package/pkg-generic.mk | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 668f011..f29ea99 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -391,8 +391,8 @@ $(1)-configure:		$(1)-patch $(1)-depends \
>
>   $(1)-patch:		$(1)-extract $$($(2)_TARGET_PATCH)
>
> -$(1)-extract:		$(1)-source \
> -			$$($(2)_TARGET_EXTRACT)
> +$$($(2)_TARGET_EXTRACT):	$$($(2)_TARGET_SOURCE)
> +$(1)-extract:			$$($(2)_TARGET_EXTRACT)
>
>   $(1)-depends:		$$($(2)_DEPENDENCIES)
>
>

  On second observation, I don't really like this change, because it 
makes the extract and patch parts asymmetrical with the others. I would 
prefer one patch that changes it for all the targets. But then, the 
behaviour does change, because rebuilding one package will also trigger a 
rebuild of the packages that depend on it. Which may be a good thing, of 
course...

  Also, I think it would be nicer / clearer to put these dependencies in 
the %-rules at the top of the file, rather than specifying them per package.

  Regards,
  Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH v2 2/3] package: fix generic extract target for top-level parallel make

[Fabio Porcedda]

On Wed, Aug 21, 2013 at 9:24 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 18/07/13 11:12, Fabio Porcedda wrote:
>>
>> To be able to use top-level parallel make we must don't depend in a rule
>> on the order of evaluation of the prerequisites, so instead of reling on
>> the left to right ordering of evaluation of the prerequisites add
>> an explicit rule to describe the dependencies.
>>
>> Add a rule to specify that the $(2)_TARGET_EXTRACT target depends
>> on $(2)_TARGET_SOURCE target.
>>
>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
>> ---
>>   package/pkg-generic.mk | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
>> index 668f011..f29ea99 100644
>> --- a/package/pkg-generic.mk
>> +++ b/package/pkg-generic.mk
>> @@ -391,8 +391,8 @@ $(1)-configure:             $(1)-patch $(1)-depends \
>>
>>   $(1)-patch:           $(1)-extract $$($(2)_TARGET_PATCH)
>>
>> -$(1)-extract:          $(1)-source \
>> -                       $$($(2)_TARGET_EXTRACT)
>> +$$($(2)_TARGET_EXTRACT):       $$($(2)_TARGET_SOURCE)
>> +$(1)-extract:                  $$($(2)_TARGET_EXTRACT)
>>
>>   $(1)-depends:         $$($(2)_DEPENDENCIES)
>>
>>
>
>  On second observation, I don't really like this change, because it makes
> the extract and patch parts asymmetrical with the others. I would prefer one
> patch that changes it for all the targets. But then, the behaviour does
> change, because rebuilding one package will also trigger a rebuild of the
> packages that depend on it. Which may be a good thing, of course...

Do you mean a single patch that changes all the targets? IMHO the
patch becomes too complex, but if is the preferred way i'm fine with
that.

To be able to change the others targets i need to add stamp file for
every target inside $$($(2)_DEPENDENCIES,
i need to do that because a file cannot depends on a non existing file.
If there is any chance that such modification is going to be accepted
i restart to work on the second part.

>  Also, I think it would be nicer / clearer to put these dependencies in the
> %-rules at the top of the file, rather than specifying them per package.

Do you mean to put together all those rules between the %-rules
section and inner-generic-package function?
or to scatter them in the %-rules section?

Best regards
-- 
Fabio Porcedda

[Buildroot] [PATCH v2 2/3] package: fix generic extract target for top-level parallel make

[Arnout Vandecappelle]

On 22/08/13 09:44, Fabio Porcedda wrote:
> On Wed, Aug 21, 2013 at 9:24 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
>> On 18/07/13 11:12, Fabio Porcedda wrote:
>>>
[snip]
>>>
>>>    $(1)-patch:           $(1)-extract $$($(2)_TARGET_PATCH)
>>>
>>> -$(1)-extract:          $(1)-source \
>>> -                       $$($(2)_TARGET_EXTRACT)
>>> +$$($(2)_TARGET_EXTRACT):       $$($(2)_TARGET_SOURCE)
>>> +$(1)-extract:                  $$($(2)_TARGET_EXTRACT)
>>>
>>>    $(1)-depends:         $$($(2)_DEPENDENCIES)
>>>
>>>
>>
>>   On second observation, I don't really like this change, because it makes
>> the extract and patch parts asymmetrical with the others. I would prefer one
>> patch that changes it for all the targets. But then, the behaviour does
>> change, because rebuilding one package will also trigger a rebuild of the
>> packages that depend on it. Which may be a good thing, of course...
>
> Do you mean a single patch that changes all the targets? IMHO the
> patch becomes too complex, but if is the preferred way i'm fine with
> that.

  Yes. I estimate it will modify about 50 lines, so I don't see a problem.

>
> To be able to change the others targets i need to add stamp file for
> every target inside $$($(2)_DEPENDENCIES,
> i need to do that because a file cannot depends on a non existing file.

  That's not true. Take the following Makefile:

----------------
%.source:
	touch $@

%.extract: %.source
	touch $@

%.config: %.extract
	touch $@

%.build: %.config
	touch $@

X_DEPS = y z

x.config: $(X_DEPS)
x: x.build

Y_DEPS = z

y.config: $(Y_DEPS)
y: y.build

z.config: $(Z_DEPS)
z: z.build

.PRECIOUS: %.source %.extract
----------------

  Type 'make x', and all the build, config, extract, source files will be 
touched in the right order.

  The .PRECIOUS line may not be needed in practice, I added it here 
because there are no explicit rules involving *.source and *.extract, 
therefore these files will be deleted after a successful build.


> If there is any chance that such modification is going to be accepted
> i restart to work on the second part.
>
>>   Also, I think it would be nicer / clearer to put these dependencies in the
>> %-rules at the top of the file, rather than specifying them per package.
>
> Do you mean to put together all those rules between the %-rules
> section and inner-generic-package function?
> or to scatter them in the %-rules section?

  No, I mean to do it like the example above: use the % rules to specify 
the dependencies between the stamp files, rather than an explicit rule 
for each package.

  Regards,
  Arnout


>
> Best regards
>


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH v2 2/3] package: fix generic extract target for top-level parallel make

[Fabio Porcedda]

On Thu, Aug 22, 2013 at 5:59 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 22/08/13 09:44, Fabio Porcedda wrote:
>>
>> On Wed, Aug 21, 2013 at 9:24 PM, Arnout Vandecappelle <arnout@mind.be>
>> wrote:
>>>
>>> On 18/07/13 11:12, Fabio Porcedda wrote:
>>>>
>>>>
> [snip]
>
>>>>
>>>>    $(1)-patch:           $(1)-extract $$($(2)_TARGET_PATCH)
>>>>
>>>> -$(1)-extract:          $(1)-source \
>>>> -                       $$($(2)_TARGET_EXTRACT)
>>>> +$$($(2)_TARGET_EXTRACT):       $$($(2)_TARGET_SOURCE)
>>>> +$(1)-extract:                  $$($(2)_TARGET_EXTRACT)
>>>>
>>>>    $(1)-depends:         $$($(2)_DEPENDENCIES)
>>>>
>>>>
>>>
>>>   On second observation, I don't really like this change, because it
>>> makes
>>> the extract and patch parts asymmetrical with the others. I would prefer
>>> one
>>> patch that changes it for all the targets. But then, the behaviour does
>>> change, because rebuilding one package will also trigger a rebuild of the
>>> packages that depend on it. Which may be a good thing, of course...
>>
>>
>> Do you mean a single patch that changes all the targets? IMHO the
>> patch becomes too complex, but if is the preferred way i'm fine with
>> that.
>
>
>  Yes. I estimate it will modify about 50 lines, so I don't see a problem.
>
>
>>
>> To be able to change the others targets i need to add stamp file for
>> every target inside $$($(2)_DEPENDENCIES,
>> i need to do that because a file cannot depends on a non existing file.
>
>
>  That's not true. Take the following Makefile:
>
> ----------------
> %.source:
>         touch $@
>
> %.extract: %.source
>         touch $@
>
> %.config: %.extract
>         touch $@
>
> %.build: %.config
>         touch $@
>
> X_DEPS = y z
>
> x.config: $(X_DEPS)
> x: x.build
>
> Y_DEPS = z
>
> y.config: $(Y_DEPS)
> y: y.build
>
> z.config: $(Z_DEPS)
> z: z.build
>
> .PRECIOUS: %.source %.extract
> ----------------
>
>  Type 'make x', and all the build, config, extract, source files will be
> touched in the right order.
>
>  The .PRECIOUS line may not be needed in practice, I added it here because
> there are no explicit rules involving *.source and *.extract, therefore
> these files will be deleted after a successful build.
>
>
>
>> If there is any chance that such modification is going to be accepted
>> i restart to work on the second part.
>>
>>>   Also, I think it would be nicer / clearer to put these dependencies in
>>> the
>>> %-rules at the top of the file, rather than specifying them per package.
>>
>>
>> Do you mean to put together all those rules between the %-rules
>> section and inner-generic-package function?
>> or to scatter them in the %-rules section?
>
>
>  No, I mean to do it like the example above: use the % rules to specify the
> dependencies between the stamp files, rather than an explicit rule for each
> package.
>

Thanks for the suggestions, I'll look into it.

Regards
-- 
Fabio Porcedda

[Buildroot] [PATCH v2 2/3] package: fix generic extract target for top-level parallel make

[Fabio Porcedda]

On Thu, Aug 22, 2013 at 5:59 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 22/08/13 09:44, Fabio Porcedda wrote:
>>
>> On Wed, Aug 21, 2013 at 9:24 PM, Arnout Vandecappelle <arnout@mind.be>
>> wrote:
>>>
>>> On 18/07/13 11:12, Fabio Porcedda wrote:
>>>>
>>>>
> [snip]
>
>>>>
>>>>    $(1)-patch:           $(1)-extract $$($(2)_TARGET_PATCH)
>>>>
>>>> -$(1)-extract:          $(1)-source \
>>>> -                       $$($(2)_TARGET_EXTRACT)
>>>> +$$($(2)_TARGET_EXTRACT):       $$($(2)_TARGET_SOURCE)
>>>> +$(1)-extract:                  $$($(2)_TARGET_EXTRACT)
>>>>
>>>>    $(1)-depends:         $$($(2)_DEPENDENCIES)
>>>>
>>>>
>>>
>>>   On second observation, I don't really like this change, because it
>>> makes
>>> the extract and patch parts asymmetrical with the others. I would prefer
>>> one
>>> patch that changes it for all the targets. But then, the behaviour does
>>> change, because rebuilding one package will also trigger a rebuild of the
>>> packages that depend on it. Which may be a good thing, of course...
>>
>>
>> Do you mean a single patch that changes all the targets? IMHO the
>> patch becomes too complex, but if is the preferred way i'm fine with
>> that.
>
>
>  Yes. I estimate it will modify about 50 lines, so I don't see a problem.
>
>
>>
>> To be able to change the others targets i need to add stamp file for
>> every target inside $$($(2)_DEPENDENCIES,
>> i need to do that because a file cannot depends on a non existing file.
>
>
>  That's not true. Take the following Makefile:
>
> ----------------
> %.source:
>         touch $@
>
> %.extract: %.source
>         touch $@
>
> %.config: %.extract
>         touch $@
>
> %.build: %.config
>         touch $@
>
> X_DEPS = y z
>
> x.config: $(X_DEPS)

The problem with this solution is that the "x.config" target is
reevaluated every time because it does not depends on a virtual
target, the only solution that i found is to add a stamp file for
every dependencies.
Using virtual target as dependency is fine only with virtual targets.

Regards
-- 
Fabio Porcedda

[Buildroot] [PATCH v2 2/3] package: fix generic extract target for top-level parallel make

[Arnout Vandecappelle]

On 08/26/13 10:29, Fabio Porcedda wrote:
> On Thu, Aug 22, 2013 at 5:59 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
>> On 22/08/13 09:44, Fabio Porcedda wrote:
[snip]
>>> To be able to change the others targets i need to add stamp file for
>>> every target inside $$($(2)_DEPENDENCIES,
>>> i need to do that because a file cannot depends on a non existing file.
>>
>>
>>   That's not true. Take the following Makefile:
>>
>> ----------------
>> %.source:
>>          touch $@
>>
>> %.extract: %.source
>>          touch $@
>>
>> %.config: %.extract
>>          touch $@
>>
>> %.build: %.config
>>          touch $@
>>
>> X_DEPS = y z
>>
>> x.config: $(X_DEPS)
>
> The problem with this solution is that the "x.config" target is
> reevaluated every time because it does not depends on a virtual
> target, the only solution that i found is to add a stamp file for
> every dependencies.
> Using virtual target as dependency is fine only with virtual targets.

  Ah yes, you're right.

  However, using the order-only rule that you mentioned in reply to 
Thomas solves the problem. So

x.config: | $(X_DEPS)


  Regards,
  Arnout


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH v2 2/3] package: fix generic extract target for top-level parallel make

[Fabio Porcedda]

On Tue, Aug 27, 2013 at 8:01 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 08/26/13 10:29, Fabio Porcedda wrote:
>>
>> On Thu, Aug 22, 2013 at 5:59 PM, Arnout Vandecappelle <arnout@mind.be>
>> wrote:
>>>
>>> On 22/08/13 09:44, Fabio Porcedda wrote:
>
> [snip]
>
>>>> To be able to change the others targets i need to add stamp file for
>>>> every target inside $$($(2)_DEPENDENCIES,
>>>> i need to do that because a file cannot depends on a non existing file.
>>>
>>>
>>>
>>>   That's not true. Take the following Makefile:
>>>
>>> ----------------
>>> %.source:
>>>          touch $@
>>>
>>> %.extract: %.source
>>>          touch $@
>>>
>>> %.config: %.extract
>>>          touch $@
>>>
>>> %.build: %.config
>>>          touch $@
>>>
>>> X_DEPS = y z
>>>
>>> x.config: $(X_DEPS)
>>
>>
>> The problem with this solution is that the "x.config" target is
>> reevaluated every time because it does not depends on a virtual
>> target, the only solution that i found is to add a stamp file for
>> every dependencies.
>> Using virtual target as dependency is fine only with virtual targets.
>
>
>  Ah yes, you're right.
>
>  However, using the order-only rule that you mentioned in reply to Thomas
> solves the problem. So
>
> x.config: | $(X_DEPS)

That's great, I didn't know that.
I'm working on the v3.

Regards
Fabio Porcedda

[Buildroot] [PATCH v2 2/3] package: fix generic extract target for top-level parallel make

[Thomas Petazzoni]

Dear Arnout Vandecappelle,

On Wed, 21 Aug 2013 21:24:19 +0200, Arnout Vandecappelle wrote:

>   On second observation, I don't really like this change, because it 
> makes the extract and patch parts asymmetrical with the others. I would 
> prefer one patch that changes it for all the targets. But then, the 
> behaviour does change, because rebuilding one package will also trigger a 
> rebuild of the packages that depend on it. Which may be a good thing, of 
> course...

When I did some experiments on top-level parallel build some years ago,
it is also one of the problem I had seen. Today, if you decide to
rebuild "libfoo", it rebuilds libfoo only and that's it. If libfoo is
used by something else, it's your responsibility to rebuild this
something else. Of course, this is all possible because the chain of
dependencies uses virtual targets and not stamp files.

If we use stamp files directly for the dependencies between the build
steps, then rebuilding a package will automatically rebuild all its
reverse dependencies. I believe this will be very very annoying for
people who just want to test a small change in a library, for example,
and make the whole OVERRIDE_SRCDIR thing used for development quite
useless.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH v2 2/3] package: fix generic extract target for top-level parallel make

[Fabio Porcedda]

On Fri, Aug 23, 2013 at 9:36 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Arnout Vandecappelle,
>
> On Wed, 21 Aug 2013 21:24:19 +0200, Arnout Vandecappelle wrote:
>
>>   On second observation, I don't really like this change, because it
>> makes the extract and patch parts asymmetrical with the others. I would
>> prefer one patch that changes it for all the targets. But then, the
>> behaviour does change, because rebuilding one package will also trigger a
>> rebuild of the packages that depend on it. Which may be a good thing, of
>> course...
>
> When I did some experiments on top-level parallel build some years ago,
> it is also one of the problem I had seen. Today, if you decide to
> rebuild "libfoo", it rebuilds libfoo only and that's it. If libfoo is
> used by something else, it's your responsibility to rebuild this
> something else. Of course, this is all possible because the chain of
> dependencies uses virtual targets and not stamp files.

For this problem there is a simple solution, to use a
"order-only-prerequisites", because
a target is not rebuild if is older than a "order-only-prerequisites".

http://www.gnu.org/software/make/manual/html_node/Prerequisite-Types.html#Prerequisite-Types

Best regards
-- 
Fabio Porcedda

[Buildroot] [PATCH v2 2/3] package: fix generic extract target for top-level parallel make

[Thomas Petazzoni]

Dear Fabio Porcedda,

On Fri, 23 Aug 2013 10:00:33 +0200, Fabio Porcedda wrote:

> > When I did some experiments on top-level parallel build some years ago,
> > it is also one of the problem I had seen. Today, if you decide to
> > rebuild "libfoo", it rebuilds libfoo only and that's it. If libfoo is
> > used by something else, it's your responsibility to rebuild this
> > something else. Of course, this is all possible because the chain of
> > dependencies uses virtual targets and not stamp files.
> 
> For this problem there is a simple solution, to use a
> "order-only-prerequisites", because
> a target is not rebuild if is older than a "order-only-prerequisites".
> 
> http://www.gnu.org/software/make/manual/html_node/Prerequisite-Types.html#Prerequisite-Types

Ah, yes, indeed!

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH v2 3/3] package: fix generic patch target for top-level parallel make

[Fabio Porcedda]

To be able to use top-level parallel make we must don't depend in a rule
on the order of evaluation of the prerequisites, so instead of reling on
the left to right ordering of evaluation of the prerequisites add
an explicit rule to describe the dependencies.

Add a rule to specify that the $(2)_TARGET_PATCH target depends
on $(2)_TARGET_EXTRACT target.

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 package/pkg-generic.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index f29ea99..bd03575 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -389,7 +389,8 @@ ifeq ($$($(2)_OVERRIDE_SRCDIR),)
 $(1)-configure:		$(1)-patch $(1)-depends \
 			$$($(2)_TARGET_CONFIGURE)
 
-$(1)-patch:		$(1)-extract $$($(2)_TARGET_PATCH)
+$$($(2)_TARGET_PATCH):		$$($(2)_TARGET_EXTRACT)
+$(1)-patch:			$$($(2)_TARGET_PATCH)
 
 $$($(2)_TARGET_EXTRACT):	$$($(2)_TARGET_SOURCE)
 $(1)-extract:			$$($(2)_TARGET_EXTRACT)
-- 
1.8.1.4

[Buildroot] [PATCH v2 0/3] Fix for top-level parallel make part 1

[Thomas Petazzoni]

Dear Fabio Porcedda,

Thanks for your patches!

On Thu, 18 Jul 2013 11:12:23 +0200, Fabio Porcedda wrote:

> this is the first patch set for fixing top-level parallel make in buildroot,
> the common problem scattered in buildroot's top-level makefile is that in the
> rules it relies on the order of evaluation of the prerequisites,
> to be able to use top-level parallel make instead of reling on the left to
> right ordering of evaluation of the prerequisites we must add an explicit
> rule to describe the dependency.

I don't remember if we had this discussion in the past, but before
going down the road of supporting top-level parallel make in Buildroot,
I'd like to understand what is your plan to solve the most important
problem that top-level parallel make creates: the need to create
per-package sysroot, instead of the global single sysroot we have
today. Have you thought about this problem already? What solution do
you have for it?

In case the problem is not clear, here is a quick summary. The global
sysroot we have today, in which all libraries and headers are
installed, works fine because we have a guarantee on the build order
that is consistent across rebuilds, because builds are non-parallel.

Now, let's introduce parallelization between builds of different
packages, and suppose package A has an optional dependency on package
B, but this dependency is not expressed in Buildroot Makefiles because
we haven't seen that package A could optionally use package B.

If you build things in parallel, then one build may lead to package A
having been built with support for package B, because package B was
built and installed in the sysroot prior to package A configure step.
The next build you do, package A may be built without support for
package B, because package A configure step is executed before package
B is installed into the sysroot.

So without doing *any* change in Buildroot or the configuration, two
consecutive builds would have different results, which is clearly not
nice.

The only solution to avoid this is to have a per-package sysroot, into
which only the dependencies explicitly listed in the package .mk files
are installed. It is probably not impossible to do with Buildroot, but
isn't completely simple either.

What are your plans in that respect?

Thanks!

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH v2 0/3] Fix for top-level parallel make part 1

[Fabio Porcedda]

On Thu, Jul 18, 2013 at 11:38 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Fabio Porcedda,
>
> Thanks for your patches!

Hi Thomas,

Thanks for reviewing the patches.

> On Thu, 18 Jul 2013 11:12:23 +0200, Fabio Porcedda wrote:
>
>> this is the first patch set for fixing top-level parallel make in buildroot,
>> the common problem scattered in buildroot's top-level makefile is that in the
>> rules it relies on the order of evaluation of the prerequisites,
>> to be able to use top-level parallel make instead of reling on the left to
>> right ordering of evaluation of the prerequisites we must add an explicit
>> rule to describe the dependency.
>
> I don't remember if we had this discussion in the past, but before
> going down the road of supporting top-level parallel make in Buildroot,
> I'd like to understand what is your plan to solve the most important
> problem that top-level parallel make creates: the need to create
> per-package sysroot, instead of the global single sysroot we have
> today. Have you thought about this problem already? What solution do
> you have for it?

I hadn't thought about that, thanks for pointing it out.

> In case the problem is not clear, here is a quick summary. The global
> sysroot we have today, in which all libraries and headers are
> installed, works fine because we have a guarantee on the build order
> that is consistent across rebuilds, because builds are non-parallel.
>
> Now, let's introduce parallelization between builds of different
> packages, and suppose package A has an optional dependency on package
> B, but this dependency is not expressed in Buildroot Makefiles because
> we haven't seen that package A could optionally use package B.
>
> If you build things in parallel, then one build may lead to package A
> having been built with support for package B, because package B was
> built and installed in the sysroot prior to package A configure step.
> The next build you do, package A may be built without support for
> package B, because package A configure step is executed before package
> B is installed into the sysroot.
>
> So without doing *any* change in Buildroot or the configuration, two
> consecutive builds would have different results, which is clearly not
> nice.
>
> The only solution to avoid this is to have a per-package sysroot, into
> which only the dependencies explicitly listed in the package .mk files
> are installed. It is probably not impossible to do with Buildroot, but
> isn't completely simple either.
>
> What are your plans in that respect?

So before to add top-level parallel make support we must add
per-package sysroot...
That's a bad news for me, i was already wokring on the second part :-(

I can try to work to add per-package sysroot, but i've some doubts about that:
 - Is per-package sysroot a desiderable feature regardless the
top-level parallel makefile support?
 - Is per-package an appropriate or overkill feature for buildroot?

Do you know if the other build system use per-package sysroot solution
to solve the same issue? like bitbake/os, debian, fedora, ...?

Thanks and best regards
--
Fabio Porcedda

[Buildroot] [PATCH v2 0/3] Fix for top-level parallel make part 1

[Thomas Petazzoni]

Dear Fabio Porcedda,

On Fri, 19 Jul 2013 17:41:42 +0200, Fabio Porcedda wrote:

> > I don't remember if we had this discussion in the past, but before
> > going down the road of supporting top-level parallel make in Buildroot,
> > I'd like to understand what is your plan to solve the most important
> > problem that top-level parallel make creates: the need to create
> > per-package sysroot, instead of the global single sysroot we have
> > today. Have you thought about this problem already? What solution do
> > you have for it?
> 
> I hadn't thought about that, thanks for pointing it out.

You're welcome. I believe that top-level parallel build is a bit like
the creation of binary packages: people initially believe that it's
fairly trivial to implement, but in reality, if you want to implement
it properly it's much more complex and maybe too complex for the KISS
principle of Buildroot.

> So before to add top-level parallel make support we must add
> per-package sysroot...
> That's a bad news for me, i was already wokring on the second part :-(
> 
> I can try to work to add per-package sysroot, but i've some doubts about that:
>  - Is per-package sysroot a desiderable feature regardless the
> top-level parallel makefile support?
>  - Is per-package an appropriate or overkill feature for buildroot?

Beyond top-level parallel support, per-package support is useful as it
ensures only the dependencies that are explicitly expressed in the
package .mk file are actually seen when building a given package. So
from a build correctness point of view, it's certainly a nice feature.

Now, whether it's overkill or not for Buildroot is hard to say. It
clearly would increase a bit the complexity of the package
infrastructure, but it's hard to guess whether this additional
complexity will be reasonable or not before doing some prototypes.

There is, however, another drawback than just complexity: build time.
With per-package sysroot, it means that for each package, you have to
create a completely new sysroot, which takes time.

> Do you know if the other build system use per-package sysroot solution
> to solve the same issue? like bitbake/os, debian, fedora, ...?

I know that Debian builds all its packages inside a chroot, in which
only the explicit Build-depends of that package are installed. So in
effect, it has per-package sysroot.

As per OpenEmbedded/Bitbake, I was told a few years ago that they were
not doing per-package sysroot, even though they are doing top-level
parallel builds, and that this was sometimes causing some spurious
failures or not completely reproducible builds. However, I haven't
verified this myself, and this statement was made some years ago, so it
may or may not longer be true.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH v2 0/3] Fix for top-level parallel make part 1

[Fabio Porcedda]

On Sat, Jul 27, 2013 at 1:18 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Fabio Porcedda,
>
> On Fri, 19 Jul 2013 17:41:42 +0200, Fabio Porcedda wrote:
>
>> > I don't remember if we had this discussion in the past, but before
>> > going down the road of supporting top-level parallel make in Buildroot,
>> > I'd like to understand what is your plan to solve the most important
>> > problem that top-level parallel make creates: the need to create
>> > per-package sysroot, instead of the global single sysroot we have
>> > today. Have you thought about this problem already? What solution do
>> > you have for it?
>>
>> I hadn't thought about that, thanks for pointing it out.
>
> You're welcome. I believe that top-level parallel build is a bit like
> the creation of binary packages: people initially believe that it's
> fairly trivial to implement, but in reality, if you want to implement
> it properly it's much more complex and maybe too complex for the KISS
> principle of Buildroot.
>
>> So before to add top-level parallel make support we must add
>> per-package sysroot...
>> That's a bad news for me, i was already wokring on the second part :-(
>>
>> I can try to work to add per-package sysroot, but i've some doubts about that:
>>  - Is per-package sysroot a desiderable feature regardless the
>> top-level parallel makefile support?
>>  - Is per-package an appropriate or overkill feature for buildroot?
>
> Beyond top-level parallel support, per-package support is useful as it
> ensures only the dependencies that are explicitly expressed in the
> package .mk file are actually seen when building a given package. So
> from a build correctness point of view, it's certainly a nice feature.
>
> Now, whether it's overkill or not for Buildroot is hard to say. It
> clearly would increase a bit the complexity of the package
> infrastructure, but it's hard to guess whether this additional
> complexity will be reasonable or not before doing some prototypes.
>
> There is, however, another drawback than just complexity: build time.
> With per-package sysroot, it means that for each package, you have to
> create a completely new sysroot, which takes time.
>
>> Do you know if the other build system use per-package sysroot solution
>> to solve the same issue? like bitbake/os, debian, fedora, ...?
>
> I know that Debian builds all its packages inside a chroot, in which
> only the explicit Build-depends of that package are installed. So in
> effect, it has per-package sysroot.
>
> As per OpenEmbedded/Bitbake, I was told a few years ago that they were
> not doing per-package sysroot, even though they are doing top-level
> parallel builds, and that this was sometimes causing some spurious
> failures or not completely reproducible builds. However, I haven't
> verified this myself, and this statement was made some years ago, so it
> may or may not longer be true.

Unfortunately right now i've not enough time to work on the
per-package sysroot feature, maybe in the feature, i don't know.

So are you going to accept this patch set even without per-package
sysroot feature?
IMHO top-level parallel make it's still a useful feature even without
per-package sysroot.
The per-package sysroot feature can be still added later.

Best regards
-- 
Fabio Porcedda

[Buildroot] [PATCH v2 0/3] Fix for top-level parallel make part 1

[Thomas Petazzoni]

Dear Fabio Porcedda,

On Tue, 30 Jul 2013 11:34:08 +0200, Fabio Porcedda wrote:

> > As per OpenEmbedded/Bitbake, I was told a few years ago that they were
> > not doing per-package sysroot, even though they are doing top-level
> > parallel builds, and that this was sometimes causing some spurious
> > failures or not completely reproducible builds. However, I haven't
> > verified this myself, and this statement was made some years ago, so it
> > may or may not longer be true.
> 
> Unfortunately right now i've not enough time to work on the
> per-package sysroot feature, maybe in the feature, i don't know.
> 
> So are you going to accept this patch set even without per-package
> sysroot feature?
> IMHO top-level parallel make it's still a useful feature even without
> per-package sysroot.
> The per-package sysroot feature can be still added later.

As we discussed in this thread, the problem is that the top-level
parallel make feature is broken if you don't have per-package sysroot:
builds become non-reproducible.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH v2 0/3] Fix for top-level parallel make part 1

[Fabio Porcedda]

On Tue, Jul 30, 2013 at 12:01 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Fabio Porcedda,
>
> On Tue, 30 Jul 2013 11:34:08 +0200, Fabio Porcedda wrote:
>
>> > As per OpenEmbedded/Bitbake, I was told a few years ago that they were
>> > not doing per-package sysroot, even though they are doing top-level
>> > parallel builds, and that this was sometimes causing some spurious
>> > failures or not completely reproducible builds. However, I haven't
>> > verified this myself, and this statement was made some years ago, so it
>> > may or may not longer be true.
>>
>> Unfortunately right now i've not enough time to work on the
>> per-package sysroot feature, maybe in the feature, i don't know.
>>
>> So are you going to accept this patch set even without per-package
>> sysroot feature?
>> IMHO top-level parallel make it's still a useful feature even without
>> per-package sysroot.
>> The per-package sysroot feature can be still added later.
>
> As we discussed in this thread, the problem is that the top-level
> parallel make feature is broken if you don't have per-package sysroot:
> builds become non-reproducible.

Just to understand better, this is true only for packages with
unexpressed dependencies, right?
So if a build has only packages with full expressed dependencies the
builds are reproducible?

Best regards and thanks
-- 
Fabio Porcedda

[Buildroot] [PATCH v2 0/3] Fix for top-level parallel make part 1

[Thomas Petazzoni]

Dear Fabio Porcedda,

On Tue, 30 Jul 2013 12:16:44 +0200, Fabio Porcedda wrote:

> > As we discussed in this thread, the problem is that the top-level
> > parallel make feature is broken if you don't have per-package sysroot:
> > builds become non-reproducible.
> 
> Just to understand better, this is true only for packages with
> unexpressed dependencies, right?
> So if a build has only packages with full expressed dependencies the
> builds are reproducible?

Yes, that's correct.

The problem is that it is very hard to be sure that /all/
possible dependencies are expressed in the Buildroot .mk file. When
someone bumps the version of a package, we rarely go through the
details of the changes in the configure.{ac,in} to see if additional
optional dependencies have been added by the upstream developers. We
could very easily overlook such changes, which would lead to
unreproducible results. I wouldn't be annoyed at all if the result was
a big fat build failure that is clearly visible. What annoys me is that
the issue is very hard to notice: from one build to another, you will
get different results. That's very hard to track down for Buildroot
developers and very confusing for users. Imagine the kind of support
requests we will get when users will face such spurious build
differences, and how complex it will be to handle them if we don't have
any guarantees on the reproducibility of the build.

I'd really like to see top-level parallel build in Buildroot, but I
don't think we should do it at the expense of build reproducibility and
increase of users support complexity.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH v2 0/3] Fix for top-level parallel make part 1

[Arnout Vandecappelle]

On 30/07/13 13:29, Thomas Petazzoni wrote:
> I'd really like to see top-level parallel build in Buildroot, but I
> don't think we should do it at the expense of build reproducibility and
> increase of users support complexity.

  That said, I would say that any patch that improves the top-level 
parallel build situation should be accepted, unless it really adds 
complexity. So 1/3 would definitely be acceptable IMHO.

  I also think that 2/3 and 3/3 clarify the generic infrastructure, so 
I'd accept them as well. Only, they also changes the behaviour when you 
touch a .stamp file, so it requires a bit more thinking.


  Regards,
  Arnout
-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH v2 0/3] Fix for top-level parallel make part 1

[Fabio Porcedda]

On Mon, Aug 12, 2013 at 6:48 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 30/07/13 13:29, Thomas Petazzoni wrote:
>>
>> I'd really like to see top-level parallel build in Buildroot, but I
>> don't think we should do it at the expense of build reproducibility and
>> increase of users support complexity.
>
>
>  That said, I would say that any patch that improves the top-level parallel
> build situation should be accepted, unless it really adds complexity. So 1/3
> would definitely be acceptable IMHO.
>
>  I also think that 2/3 and 3/3 clarify the generic infrastructure, so I'd
> accept them as well. Only, they also changes the behaviour when you touch a
> .stamp file, so it requires a bit more thinking.
>
>

Hi Arnout,
thanks for reviewing.

I obviously approve the idea of accepting patches that improve
top-level parallel make and does not add complexity, such as these
patches.

You are right that 2/3 and 3/3 change the behaviour when you touch
.stamp file, but i think that the touching of .stamp file for the
-extract and -patch targets does not happen using buildroot targets,
the touching of those .stamp files can happen only manually, so IMHO
it's not a real problem.

Best regards
-- 
Fabio Porcedda

[Buildroot] [PATCH v2 0/3] Fix for top-level parallel make part 1

[Arnout Vandecappelle]

On 20/08/13 14:14, Fabio Porcedda wrote:
> On Mon, Aug 12, 2013 at 6:48 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
>> On 30/07/13 13:29, Thomas Petazzoni wrote:
>>>
>>> I'd really like to see top-level parallel build in Buildroot, but I
>>> don't think we should do it at the expense of build reproducibility and
>>> increase of users support complexity.
>>
>>
>>   That said, I would say that any patch that improves the top-level parallel
>> build situation should be accepted, unless it really adds complexity. So 1/3
>> would definitely be acceptable IMHO.
>>
>>   I also think that 2/3 and 3/3 clarify the generic infrastructure, so I'd
>> accept them as well. Only, they also changes the behaviour when you touch a
>> .stamp file, so it requires a bit more thinking.
>>
>>
>
> Hi Arnout,
> thanks for reviewing.
>
> I obviously approve the idea of accepting patches that improve
> top-level parallel make and does not add complexity, such as these
> patches.
>
> You are right that 2/3 and 3/3 change the behaviour when you touch
> .stamp file, but i think that the touching of .stamp file for the
> -extract and -patch targets does not happen using buildroot targets,
> the touching of those .stamp files can happen only manually, so IMHO
> it's not a real problem.

  Ah that's right, the order changed a while ago and now the -depends 
come after -patch. It used to be different though.

  So I guess that 2/3 and 3/3 should be acceptable as well. After a bit 
of testing and a formal Ack, of course.

  Regards,
  Arnout



-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F