From mboxrd@z Thu Jan 1 00:00:00 1970 From: Arnout Vandecappelle Date: Thu, 10 Oct 2013 19:24:12 +0200 Subject: [Buildroot] [PATCHv3 19/20] package: package-based implementation of source, external-deps and legal-info In-Reply-To: <1381256237-27948-20-git-send-email-thomas.petazzoni@free-electrons.com> References: <1381256237-27948-1-git-send-email-thomas.petazzoni@free-electrons.com> <1381256237-27948-20-git-send-email-thomas.petazzoni@free-electrons.com> Message-ID: <5256E2BC.8020503@mind.be> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: buildroot@busybox.net On 08/10/13 20:17, Thomas Petazzoni wrote: > Until now, the make source, make external-deps and make legal-info > where relying on enumerating the packages by looking at $(TARGETS) > which contains only the target packages and not the host > packages. Thanks to the TARGET_HOST_DEPS and HOST_DEPS variables, it > was doing a two-level resolution of host package dependencies, but it > was not entirely correct since the dependency chain might be deeper > than that: some packages could be missed. > >>>From an idea of Arnout, this patch introduces in each package > additional targets -all-source, -all-legal-info and > -all-external-deps that executes the 'source', 'legal-info' and > 'external-deps' targets for this package and all its dependencies, be > they target or host dependencies. > > This provides a much cleaner implementation of this mechanism, which > is also more robust. > > In order to achieve this, this patch also separates the "package" > targets from other targets: instead of mixing them both in the global > TARGETS variable, the new PACKAGES variable contains the name of all > packages that are enabled in the configuration, while TARGETS is only > used for additional things to be done (target-finalize, etc.). This > separation is needed so that we don't try to use targets (such as > -all-external-deps) on things that are not packages. Some further > cleanups in this direction are possible, this commit takes a > relatively minimal approach for now. > > Signed-off-by: Thomas Petazzoni > --- > Makefile | 53 +++++++++++++------------------------------------- > package/pkg-generic.mk | 16 ++++++++++++++- > 2 files changed, 28 insertions(+), 41 deletions(-) > > diff --git a/Makefile b/Makefile > index f266e2d..702e7d4 100644 > --- a/Makefile > +++ b/Makefile > @@ -238,8 +238,6 @@ GNU_HOST_NAME:=$(shell support/gnuconfig/config.guess) > > BASE_TARGETS = toolchain > > -TARGETS:= > - > # silent mode requested? > QUIET:=$(if $(findstring s,$(MAKEFLAGS)),-q) > > @@ -310,9 +308,6 @@ endif > include package/Makefile.in > include support/dependencies/dependencies.mk > You could remove this empty line as well. > -# We also need the various per-package makefiles, which also add > -# each selected package to TARGETS if that package was selected > -# in the .config file. > include toolchain/helpers.mk > include toolchain/*/*.mk > > @@ -349,34 +344,11 @@ include fs/common.mk > > TARGETS+=target-post-image > > -TARGETS_CLEAN:=$(patsubst %,%-clean,$(TARGETS)) > -TARGETS_SOURCE:=$(patsubst %,%-source,$(TARGETS) $(BASE_TARGETS)) > -TARGETS_DIRCLEAN:=$(patsubst %,%-dirclean,$(TARGETS)) > -TARGETS_ALL:=$(patsubst %,__real_tgt_%,$(TARGETS)) > - > -# host-* dependencies have to be handled specially, as those aren't > -# visible in Kconfig and hence not added to a variable like TARGETS. > -# instead, find all the host-* targets listed in each _DEPENDENCIES > -# variable for each enabled target. > -# Notice: this only works for newstyle gentargets/autotargets packages > -TARGETS_HOST_DEPS = $(sort $(filter host-%,$(foreach dep,\ > - $(addsuffix _DEPENDENCIES,$(call UPPERCASE,$(TARGETS))),\ > - $($(dep))))) > -# Host packages can in turn have their own dependencies. Likewise find > -# all the package names listed in the HOST__DEPENDENCIES for each > -# host package found above. Ideally this should be done recursively until > -# no more packages are found, but that's hard to do in make, so limit to > -# 1 level for now. > -HOST_DEPS = $(sort $(foreach dep,\ > - $(addsuffix _DEPENDENCIES,$(call UPPERCASE,$(TARGETS_HOST_DEPS))),\ > - $($(dep)))) > -HOST_SOURCE += $(addsuffix -source,$(sort $(TARGETS_HOST_DEPS) $(HOST_DEPS))) > - > -TARGETS_LEGAL_INFO:=$(patsubst %,%-legal-info,\ > - $(TARGETS) $(BASE_TARGETS) $(TARGETS_HOST_DEPS) $(HOST_DEPS)))) > - > -# all targets depend on the crosscompiler and it's prerequisites > -$(TARGETS_ALL): __real_tgt_%: $(BASE_TARGETS) % > +PACKAGES_CLEAN:=$(patsubst %,%-clean,$(PACKAGES)) Can you make these match the coding style? PACKAGES_CLEAN = ... > +PACKAGES_SOURCE:=$(patsubst %,%-all-source,$(PACKAGES) $(BASE_TARGETS)) BASE_TARGETS is just "toolchain", right? And that is already part of the dependencies of PACKAGES, right? So why is that still needed here? > +PACKAGES_EXTERNAL_DEPS:=$(patsubst %,%-all-external-deps,$(PACKAGES) $(BASE_TARGETS)) > +PACKAGES_DIRCLEAN:=$(patsubst %,%-dirclean,$(PACKAGES)) The PACKAGES_DIRCLEAN is really redundant, it is only used to define them as .PHONY but that is not done for e.g. -build. > +PACKAGES_LEGAL_INFO:=$(patsubst %,%-all-legal-info,$(PACKAGES) $(BASE_TARGETS)) > > dirs: $(BUILD_DIR) $(STAGING_DIR) $(TARGET_DIR) \ > $(HOST_DIR) $(BINARIES_DIR) $(STAMP_DIR) > @@ -386,12 +358,13 @@ $(BUILD_DIR)/buildroot-config/auto.conf: $(BUILDROOT_CONFIG) > > prepare: $(BUILD_DIR)/buildroot-config/auto.conf > > -world: $(BASE_TARGETS) $(TARGETS_ALL) > +world: $(BASE_TARGETS) $(PACKAGES) $(TARGETS) > > .PHONY: all world toolchain dirs clean distclean source outputmakefile \ > legal-info legal-info-prepare legal-info-clean printvars \ > - $(BASE_TARGETS) $(TARGETS) $(TARGETS_ALL) \ > - $(TARGETS_CLEAN) $(TARGETS_DIRCLEAN) $(TARGETS_SOURCE) $(TARGETS_LEGAL_INFO) \ > + $(BASE_TARGETS) $(PACKAGES) $(TARGETS) \ > + $(PACKAGES_CLEAN) $(PACKAGES_DIRCLEAN) $(PACKAGES_SOURCE) $(PACKAGES_LEGAL_INFO) \ > + $(PACKAGES_EXTERNAL_DEPS) \ > $(BUILD_DIR) $(STAGING_DIR) $(TARGET_DIR) \ > $(HOST_DIR) $(BINARIES_DIR) $(STAMP_DIR) > > @@ -562,10 +535,10 @@ target-post-image: > toolchain-eclipse-register: > ./support/scripts/eclipse-register-toolchain `readlink -f $(O)` $(notdir $(TARGET_CROSS)) $(BR2_ARCH) > > -source: dirs $(TARGETS_SOURCE) $(HOST_SOURCE) > +source: dirs $(PACKAGES_SOURCE) > > external-deps: > - @$(MAKE) -Bs DL_MODE=SHOW_EXTERNAL_DEPS $(EXTRAMAKEARGS) source | sort -u > + @$(MAKE) -s $(EXTRAMAKEARGS) $(PACKAGES_EXTERNAL_DEPS) | sort -u > > legal-info-clean: > @rm -fr $(LEGAL_INFO_DIR) > @@ -580,7 +553,7 @@ legal-info-prepare: $(LEGAL_INFO_DIR) > @cp $(BUILDROOT_CONFIG) $(LEGAL_INFO_DIR)/buildroot.config > > legal-info: dirs legal-info-clean legal-info-prepare $(REDIST_SOURCES_DIR) \ > - $(TARGETS_LEGAL_INFO) > + $(PACKAGES_LEGAL_INFO) > @cat support/legal-info/README.header >>$(LEGAL_REPORT) > @if [ -r $(LEGAL_WARNINGS) ]; then \ > cat support/legal-info/README.warnings-header \ > @@ -590,7 +563,7 @@ legal-info: dirs legal-info-clean legal-info-prepare $(REDIST_SOURCES_DIR) \ > @rm -f $(LEGAL_WARNINGS) > > show-targets: > - @echo $(TARGETS) > + @echo $(BASE_TARGETS) $(PACKAGES) $(TARGETS) > > else # ifeq ($(BR2_HAVE_DOT_CONFIG),y) > > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk > index a46457c..bd6169c 100644 > --- a/package/pkg-generic.mk > +++ b/package/pkg-generic.mk > @@ -424,6 +424,20 @@ endif > $(1)-show-depends: > @echo $$($(2)_DEPENDENCIES) > > +$(1)-all-source: $(foreach p,$($(2)_DEPENDENCIES),$(p)-all-source) $(1)-source > + > +$(1)-external-deps: > +ifneq ($$($(2)_SOURCE),) > + @echo $$($(2)_SOURCE) > +endif > +ifneq ($$($(2)_EXTRA_DOWNLOADS),y) > + @echo $$($(2)_EXTRA_DOWNLOADS) > +endif While you're at it, you could add $(2)_PATCH. Are the double dollars really needed? They're not used in -all-source so why would you use them here... > + > +$(1)-all-external-deps: $(foreach p,$($(2)_DEPENDENCIES),$(p)-all-external-deps) $(1)-external-deps > + > +$(1)-all-legal-info: $(foreach p,$($(2)_DEPENDENCIES),$(p)-all-legal-info) $(1)-legal-info > + > $(1)-uninstall: $(1)-configure $$($(2)_TARGET_UNINSTALL) > > $(1)-clean: $(1)-uninstall \ > @@ -528,7 +542,7 @@ endif # ifneq ($(call qstrip,$$($(2)_SOURCE)),) > # configuration > ifeq ($$($$($(2)_KCONFIG_VAR)),y) > > -TARGETS += $(1) > +PACKAGES += $(1) > PACKAGES_PERMISSIONS_TABLE += $$($(2)_PERMISSIONS)$$(sep) > PACKAGES_DEVICES_TABLE += $$($(2)_DEVICES)$$(sep) > PACKAGES_USERS += $$($(2)_USERS)$$(sep) Shouldn't you do something similar in fs/ ? Otherwise 'make source' will not download e.g. mtd. 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