Fix for top-level parallel make part 1

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 <[email protected]>

---
  package/Makefile.in | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

package/Makefile.in +1 -1

index a597290..c36ee4c 100644
@@ -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)

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 <[email protected]>

---
  package/pkg-generic.mk | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

package/pkg-generic.mk +0 -0

index 668f011..f29ea99 100644

Quoted 27 lines · 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 <[email protected]>

---
  package/pkg-generic.mk | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

package/pkg-generic.mk +2 -2

index 668f011..f29ea99 100644
@@ -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)
 

package/pkg-generic.mk $(1)-configure: $(1)-patch $(1)-depends \ · line 398

Quoted 3 lines

package/pkg-generic.mk +0 -0

  $(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.

Quoted 2 lines · Arnout Vandecappelle

Quoted 1 line · Fabio Porcedda

Quoted 12 lines

Quoted 11 lines

   $(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)

Quoted 5 lines

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.

Quoted 2 lines

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

Quoted 7 lines · Fabio Porcedda

Quoted 4 lines · Arnout Vandecappelle

Quoted 2 lines · Fabio Porcedda

[snip]

Quoted 21 lines

Quoted 12 lines

Quoted 11 lines

   $(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)

Quoted 7 lines

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...

Quoted 3 lines

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.

Quoted 4 lines

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.

Quoted 8 lines

If there is any chance that such modification is going to be accepted i restart to work on the second part.

Quoted 3 lines

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.

Quoted 3 lines

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.

Quoted 7 lines · Fabio Porcedda

Quoted 4 lines · Arnout Vandecappelle

Quoted 2 lines · Fabio Porcedda

[snip]

Quoted 21 lines

Quoted 12 lines

Quoted 11 lines

   $(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)

Quoted 7 lines

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...

Quoted 3 lines

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.

Quoted 4 lines

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)

Quoted 1 line · Arnout Vandecappelle

On 22/08/13 09:44, Fabio Porcedda wrote:

Quoted 22 lines

Quoted 3 lines

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

Quoted 1 line

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.

Quoted 5 lines · Fabio Porcedda

Quoted 2 lines · Arnout Vandecappelle

On 22/08/13 09:44, Fabio Porcedda wrote:

[snip]

Quoted 23 lines

Quoted 23 lines

Quoted 3 lines

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

Quoted 8 lines

Quoted 1 line

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)

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...

Dear Arnout Vandecappelle,

Quoted 6 lines · Arnout Vandecappelle

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.

Quoted 6 lines

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

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.

Dear Fabio Porcedda,

Thanks for your patches!

Quoted 7 lines

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?

Quoted 7 lines

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.

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, ...?

Dear Fabio Porcedda,

Quoted 9 lines · Fabio Porcedda

Quoted 7 lines

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.

Quoted 8 lines

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.

Quoted 2 lines

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.

Quoted 6 lines

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.

Dear Fabio Porcedda,

Quoted 7 lines · Fabio Porcedda

Quoted 6 lines

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.

Quoted 8 lines

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.

Quoted 3 lines

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?

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.

Quoted 4 lines · Thomas Petazzoni

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.

Quoted 7 lines · Arnout Vandecappelle

Quoted 4 lines · Thomas Petazzoni

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.

Quoted 9 lines

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.