| Message ID | 1428856685-4403-9-git-send-email-thomas.petazzoni@free-electrons.com |
|---|---|
| State | Superseded |
| Headers | show |
On 12/04/15 18:37, Thomas Petazzoni wrote: > This commit changes the global 'external-deps' target to use the newly > introduced per-package <pkg>-all-external-deps, instead of relying on > the 'source' target with a custom DL_MODE. > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > Tested-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > --- > Makefile | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index e91c5e6..40ee2e2 100644 > --- a/Makefile > +++ b/Makefile > @@ -612,8 +612,9 @@ target-post-image: $(TARGETS_ROOTFS) target-finalize > > source: $(PACKAGES_SOURCE) $(HOST_SOURCE) > > +_external-deps: $(foreach p,$(PACKAGES),$(p)-all-external-deps) > external-deps: > - @$(MAKE1) -Bs DL_MODE=SHOW_EXTERNAL_DEPS $(EXTRAMAKEARGS) source | sort -u > + @$(MAKE1) -Bs $(EXTRAMAKEARGS) _external-deps | sort -u If we also remove the sort -u (which removes duplicates, but there should be no duplicates to begin with), then there's no need for a recursive make and we avoid the GEN /home/ymorin/dev/buildroot/O/./Makefile that Yann ran into (which BTW was not caused by this patch but already existed before). The original implementation required recursive make to be able to pass the DL_MODE override. Regards, Arnout > > legal-info-clean: > @rm -fr $(LEGAL_INFO_DIR) >
Dear Arnout Vandecappelle, On Tue, 14 Apr 2015 02:10:03 +0200, Arnout Vandecappelle wrote: > > +_external-deps: $(foreach p,$(PACKAGES),$(p)-all-external-deps) > > external-deps: > > - @$(MAKE1) -Bs DL_MODE=SHOW_EXTERNAL_DEPS $(EXTRAMAKEARGS) source | sort -u > > + @$(MAKE1) -Bs $(EXTRAMAKEARGS) _external-deps | sort -u > > If we also remove the sort -u (which removes duplicates, but there should be no > duplicates to begin with), then there's no need for a recursive make and we > avoid the GEN /home/ymorin/dev/buildroot/O/./Makefile that Yann ran into > (which BTW was not caused by this patch but already existed before). Unless I missed something there will definitely be duplicates, if two packages depend on the same dependency, this dependency will be listed twice. I just did a "make randpackageconfig", followed by "make _external-deps | sort", and here is what I get: am335x-pru-package-506e074859891a2b350eb4f5fcb451c4961410ea.tar.gz am335x-pru-package-506e074859891a2b350eb4f5fcb451c4961410ea.tar.gz ... dbus-1.8.16.tar.gz dbus-1.8.16.tar.gz dbus-glib-0.104.tar.gz dbus-glib-0.104.tar.gz ... e2fsprogs-1.42.12.tar.xz e2fsprogs-1.42.12.tar.xz ... expat-2.1.0.tar.gz expat-2.1.0.tar.gz ... file-5.22.tar.gz file-5.22.tar.gz ... Want more of it? :-) Thomas
On 14/04/15 09:52, Thomas Petazzoni wrote: > Dear Arnout Vandecappelle, > > On Tue, 14 Apr 2015 02:10:03 +0200, Arnout Vandecappelle wrote: > >>> +_external-deps: $(foreach p,$(PACKAGES),$(p)-all-external-deps) >>> external-deps: >>> - @$(MAKE1) -Bs DL_MODE=SHOW_EXTERNAL_DEPS $(EXTRAMAKEARGS) source | sort -u >>> + @$(MAKE1) -Bs $(EXTRAMAKEARGS) _external-deps | sort -u >> >> If we also remove the sort -u (which removes duplicates, but there should be no >> duplicates to begin with), then there's no need for a recursive make and we >> avoid the GEN /home/ymorin/dev/buildroot/O/./Makefile that Yann ran into >> (which BTW was not caused by this patch but already existed before). > > Unless I missed something there will definitely be duplicates, if two > packages depend on the same dependency, this dependency will be listed > twice. I just did a "make randpackageconfig", followed by "make > _external-deps | sort", and here is what I get: > > am335x-pru-package-506e074859891a2b350eb4f5fcb451c4961410ea.tar.gz > am335x-pru-package-506e074859891a2b350eb4f5fcb451c4961410ea.tar.gz [snip] Ah yes, silly me. I just _assumed_ that external deps would have been implemented like: $(1)-external-deps: $$(foreach p,$$($(2)_FINAL_DEPENDENCIES),$$(p)-external-deps) instead of passing through -all-external-deps. I just tested that the above gives exactly the same result as the -all-external-deps version. The other reason why there are duplicates is because host packages and target packages (usually) have the same sources, so they will be listed twice. That's more difficult to get around. Regards, Arnout
Arnout, On Tue, 14 Apr 2015 13:22:09 +0200, Arnout Vandecappelle wrote: > Ah yes, silly me. I just _assumed_ that external deps would have been > implemented like: > > $(1)-external-deps: $$(foreach p,$$($(2)_FINAL_DEPENDENCIES),$$(p)-external-deps) > > instead of passing through -all-external-deps. I just tested that the > above gives exactly the same result as the -all-external-deps version. I might be missing something here, but I don't see why your version would be any different than the -all-external-deps version I've proposed. Except that $(1)-external-deps would not behave as I would expect it to behave: give only the list of external deps for $(1), not recursively for all dependencies of $(1). But except that I don't really see the difference. > The other reason why there are duplicates is because host packages and > target packages (usually) have the same sources, so they will be listed > twice. That's more difficult to get around. I believe this is more likely to be the reason: if you look back at my examples, I never got more than twice the same package. So we're probably hitting the target vs. host package thing. Is there anything that we can do about it besides keeping this | sort -u ? Thanks, Thomas
On 14/04/15 14:05, Thomas Petazzoni wrote: > Arnout, > > On Tue, 14 Apr 2015 13:22:09 +0200, Arnout Vandecappelle wrote: > >> Ah yes, silly me. I just _assumed_ that external deps would have been >> implemented like: >> >> $(1)-external-deps: $$(foreach p,$$($(2)_FINAL_DEPENDENCIES),$$(p)-external-deps) >> >> instead of passing through -all-external-deps. I just tested that the >> above gives exactly the same result as the -all-external-deps version. > > I might be missing something here, but I don't see why your version > would be any different than the -all-external-deps version I've > proposed. That's because I shouldn't be doing two things at once :-) > Except that $(1)-external-deps would not behave as I would > expect it to behave: give only the list of external deps for $(1), not > recursively for all dependencies of $(1). But except that I don't > really see the difference. Ack. I had interpreted pkg-external-deps as give me all the sources I need to be able to build package, but your interpretation makes more sense. > >> The other reason why there are duplicates is because host packages and >> target packages (usually) have the same sources, so they will be listed >> twice. That's more difficult to get around. > > I believe this is more likely to be the reason: if you look back at my > examples, I never got more than twice the same package. So we're > probably hitting the target vs. host package thing. > > Is there anything that we can do about it besides keeping this | sort > -u ? Not really I think. We'd have to make host-pkg-external-deps depend on pkg-external-deps instead of defining it directly. But that means that the host-generic-package macro would have to add the plain pkg-external-deps if it hasn't been defined yet (for host-only packages). Regards, Arnout > > Thanks, > > Thomas >
diff --git a/Makefile b/Makefile index e91c5e6..40ee2e2 100644 --- a/Makefile +++ b/Makefile @@ -612,8 +612,9 @@ target-post-image: $(TARGETS_ROOTFS) target-finalize source: $(PACKAGES_SOURCE) $(HOST_SOURCE) +_external-deps: $(foreach p,$(PACKAGES),$(p)-all-external-deps) external-deps: - @$(MAKE1) -Bs DL_MODE=SHOW_EXTERNAL_DEPS $(EXTRAMAKEARGS) source | sort -u + @$(MAKE1) -Bs $(EXTRAMAKEARGS) _external-deps | sort -u legal-info-clean: @rm -fr $(LEGAL_INFO_DIR)