| Message ID | 1374138746-23279-3-git-send-email-fabio.porcedda@gmail.com |
|---|---|
| State | RFC |
| Headers | show |
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
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
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 >
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
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
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
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
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
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
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
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)
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(-)