Package based 'source', 'legal-info', 'source-check' and 'external-deps'

Just like the previous commit did for the pkg-generic infrastructure, this commit improves the pkg-kconfig infrastructure to declare its custom <pkg>-<something> targets as PHONY.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 package/pkg-kconfig.mk | 6 ++++++
 1 file changed, 6 insertions(+)

package/pkg-kconfig.mk +6 -0

index fd9f19d..8361064 100644
@@ -100,6 +100,12 @@ $(1)-update-defconfig: $(1)-savedefconfig
 
 endif # package enabled
 
+.PHONY: \
+	$(1)-update-config \
+	$(1)-update-defconfig \
+	$(1)-savedefconfig \
+	$$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS))
+
 endef # inner-kconfig-package
 
 ################################################################################

Signature · 6 lines

2.1.0

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

This commit improves the filesystem handling code to declare its various targets as PHONY when appropriate.

Signed-off-by Thomas Petazzoni <[email protected]>

fs/initramfs/initramfs.mk +0 -0

 ifeq ($(BR2_TARGET_ROOTFS_INITRAMFS),y)
 TARGETS_ROOTFS += rootfs-initramfs
 endif

fs/iso9660/iso9660.mk +2 -0

index 5b44ba4..4ccfce9 100644
@@ -48,6 +48,8 @@ rootfs-iso9660: $(BINARIES_DIR)/rootfs.iso9660
 rootfs-iso9660-show-depends:
 	@echo $(ROOTFS_ISO9660_DEPENDENCIES)
 
+.PHONY: rootfs-iso9660 rootfs-iso9660-show-depends
+
 ################################################################################
 #
 # Toplevel Makefile options

Signature · 6 lines

2.1.0

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

This made me think of the trick we use to force a kernel rebuild with this newly-created initramfs.

We're using a post-fs target to force the kerbnel rebuild:

    ROOTFS_INITRAMFS_POST_TARGETS += linux-rebuild-with-initramfs

But this rule is not PHONY, and your series does not make it PHONY. This could be done in a fallow-up patch, and is not a blocker.

The main Makefile was declaring a subset of the per-package targets as being PHONY, but not all of them. Now that the pkg-generic package infrastructure is taking care of that in a much more systematic fashion, this commit gets rid of the unneeded code from the main Makefile.

Signed-off-by Thomas Petazzoni <[email protected]>

Reviewed-by "Yann E. MORIN" <[email protected]>

---
 Makefile | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Makefile +0 -1

index f011918..fee2551 100644
@@ -390,7 +390,6 @@ include fs/common.mk
 include $(BR2_EXTERNAL)/external.mk
 
 TARGETS_SOURCE := $(patsubst %,%-source,$(TARGETS))
-TARGETS_DIRCLEAN := $(patsubst %,%-dirclean,$(TARGETS))
 
 # host-* dependencies have to be handled specially, as those aren't
 # visible in Kconfig and hence not added to a variable like TARGETS.

Since you first posted that series, the 'list-defconfig' was introduced by Arnout, and your patch removes it from being a PHONY target, which it should be...

Makefile world: target-post-image · line 428

Quoted 3 lines

Makefile +1 -2

-	$(TARGETS) $(TARGETS_ROOTFS) \
-	$(TARGETS_DIRCLEAN) $(TARGETS_SOURCE) $(TARGETS_LEGAL_INFO)
+	target-finalize target-post-image

... so should be kept in there.

When you fix that, you can keep my reviewd-by tag.

For clarity, this commit renames the TARGETS variable to the more meaningful PACKAGES variable. Indeed, only packages (handled by one of the package infrastructures) should be listed in this variable, and not other random non-package targets.

Signed-off-by Thomas Petazzoni <[email protected]>

package/matchbox/matchbox.mk +0 -0

 endif

package/pkg-generic.mk +1 -1

index 1d56f81..d1a1811 100644
@@ -746,7 +746,7 @@ $(eval $(call check-deprecated-variable,$(2)_BUILD_OPT,$(2)_BUILD_OPTS))
 $(eval $(call check-deprecated-variable,$(2)_GETTEXTIZE_OPT,$(2)_GETTEXTIZE_OPTS))
 $(eval $(call check-deprecated-variable,$(2)_KCONFIG_OPT,$(2)_KCONFIG_OPTS))
 
-TARGETS += $(1)
+PACKAGES += $(1)
 
 ifneq ($$($(2)_PERMISSIONS),)
 PACKAGES_PERMISSIONS_TABLE += $$($(2)_PERMISSIONS)$$(sep)

support/scripts/graph-depends +1 -1

index 08ecc5a..c26e1e0 100755
@@ -116,7 +116,7 @@ def get_version(pkgs):
     return version
 
 # Execute the "make show-targets" command to get the list of the main
-# Buildroot TARGETS and return it formatted as a Python list. This
+# Buildroot PACKAGES and return it formatted as a Python list. This
 # list is used as the starting point for full dependency graphs
 def get_targets():
     sys.stderr.write("Getting targets\n")

system/system.mk +1 -1

index 4a1eb4a..c95e436 100644
@@ -35,7 +35,7 @@ TARGET_FINALIZE_HOOKS += SYSTEM_ISSUE
 endif
 
 ifneq ($(TARGET_GENERIC_ROOT_PASSWD),)
-TARGETS += host-mkpasswd
+PACKAGES += host-mkpasswd
 endif
 
 define SET_NETWORK_LOCALHOST

Signature · 6 lines

2.1.0

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

package/matchbox/matchbox.mk line 3

Quoted 9 lines

package/matchbox/matchbox.mk +1 -1

index aec57c0..fe1a7db 100644
@@ -1,4 +1,4 @@
 ifeq ($(BR2_PACKAGE_MATCHBOX),y)
 include $(sort $(wildcard package/matchbox/*/*.mk))
-TARGETS += matchbox-lib matchbox-wm
+PACKAGES += matchbox-lib matchbox-wm

I understand we need to manually add them to the list of packages, because neither matchbox-lib nor matchbox-wm have an associated Kconfig option.

Yet, I believe this should be changed so that they behave like any other package, and that we use 'select' from the matchbox package to select the -lib and -wm dependenct packages.

If we are really concerned that we do not want to make those user-selectable, we could make them prompt-less packages, and so they'd be hidden.

Of course, this is not a blocker for this patch, as you're doing a mere renaming of an existing variable, and the fix can be done in a follow-up patch (OK, written on my TODO-list).

This commit changes the implementation of the global 'legal-info' target to use the newly introduced per-package <pkg>-all-legal-info target. This allows to avoid using the $(TARGET_HOST_DEPS) and $(HOST_DEPS) variables that we are trying to remove.

It is worth mentionning that this commit might change the output of 'make legal-info' by making it more correct than it was. With the existing implementations, we could be missing packages if they were host packages, or target packages not properly selected in terms of Config.in dependencies, and with a more than a two-level deep dependency from a target package properly selected at the Config.in level. This is because our previous logic was simply taking all packages in the "TARGETS" (now called "PACKAGES") variable, which are only the target packages explicitly selected in the .config file, and doing a two-level deep recursion in the dependencies.

With this commit, we switch legal-info to use proper make-based dependencies, so we no longer have the limitations we used to have. For this reason, the output of 'make legal-info' after this patch may contain *more* entries than before this patch, but it is really because it is now correct.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 Makefile | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Makefile +1 -4

index f4deec3..a22d99b 100644
@@ -410,9 +410,6 @@ HOST_DEPS = $(sort $(foreach dep,\
 		$($(dep))))
 HOST_SOURCE += $(addsuffix -source,$(sort $(TARGETS_HOST_DEPS) $(HOST_DEPS)))
 
-PACKAGES_LEGAL_INFO := $(patsubst %,%-legal-info,\
-		$(PACKAGES) $(TARGETS_HOST_DEPS) $(HOST_DEPS))
-
 dirs: $(BUILD_DIR) $(STAGING_DIR) $(TARGET_DIR) \
 	$(HOST_DIR) $(BINARIES_DIR)
 

@@ -631,7 +628,7 @@ legal-info-prepare: $(LEGAL_INFO_DIR)
 	@$(call legal-warning,the toolchain has not been saved)
 	@cp $(BR2_CONFIG) $(LEGAL_INFO_DIR)/buildroot.config
 
-legal-info: dirs legal-info-clean legal-info-prepare $(PACKAGES_LEGAL_INFO) \
+legal-info: dirs legal-info-clean legal-info-prepare $(foreach p,$(PACKAGES),$(p)-all-legal-info) \
 		$(REDIST_SOURCES_DIR_TARGET) $(REDIST_SOURCES_DIR_HOST)
 	@cat support/legal-info/README.header >>$(LEGAL_REPORT)
 	@if [ -r $(LEGAL_WARNINGS) ]; then \

Signature · 6 lines

2.1.0

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

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

Tested-by "Yann E. MORIN" <[email protected]>

Reviewed-by "Yann E. MORIN" <[email protected]>

---
 Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Makefile +0 -0

index e91c5e6..40ee2e2 100644

Makefile +0 -0

 legal-info-clean:
 	@rm -fr $(LEGAL_INFO_DIR)
 

Makefile target-post-image: $(TARGETS_ROOTFS) target-finalize · line 617

Quoted 4 lines

Makefile +2 -1

+_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).

Dear Arnout Vandecappelle,

Quoted 9 lines · Arnout Vandecappelle

Makefile target-post-image: $(TARGETS_ROOTFS) target-finalize · line 617

Quoted 4 lines

Makefile +2 -1

+_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

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.

Arnout,

Quoted 3 lines · Arnout Vandecappelle

Ah yes, silly me. I just _assumed_ that external deps would have been
implemented like:

Quoted 4 lines

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

Quoted 4 lines

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

Now that the external-deps implementation relies on the per-package <pkg>-all-external-deps and <pkg>-external-deps targets and no longer on the 'source' target with a custom DL_MODE, we can get rid of the support for the SHOW_EXTERNAL_DEPS DL_MODE value in the pkg-download logic.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 package/pkg-download.mk | 45 ++-------------------------------------------
 package/pkg-generic.mk  |  5 +----
 2 files changed, 3 insertions(+), 47 deletions(-)

package/pkg-download.mk +2 -43

index e274712..be3babb 100644
@@ -20,9 +20,8 @@ export LOCALFILES := $(call qstrip,$(BR2_LOCALFILES))
 
 DL_WRAPPER = support/download/dl-wrapper
 
-# Default spider mode is 'DOWNLOAD'. Other possible values are 'SOURCE_CHECK'
-# used by the _source-check target and 'SHOW_EXTERNAL_DEPS', used by the
-# external-deps target.
+# Default spider mode is 'DOWNLOAD'. Other possible value is
+# 'SOURCE_CHECK' used by the _source-check target.
 DL_MODE = DOWNLOAD
 
 # DL_DIR may have been set already from the environment

@@ -71,11 +70,6 @@ github = https://github.com/$(1)/$(2)/archive/$(3)
 # The SOURCE_CHECK_* helpers are in charge of simply checking that the source
 # is available for download. This can be used to make sure one will be able
 # to get all the sources needed for one's build configuration.
-#
-# The SHOW_EXTERNAL_DEPS_* helpers simply output to the console the names
-# of the files that will be downloaded, or path and revision of the
-# source repositories, producing a list of all the "external dependencies"
-# of a given build configuration.
 ################################################################################
 
 # Try a shallow clone - but that only works if the version is a ref (tag or

@@ -100,11 +94,6 @@ define SOURCE_CHECK_GIT
   $(GIT) ls-remote --heads $($(PKG)_SITE) > /dev/null
 endef
 
-define SHOW_EXTERNAL_DEPS_GIT
-	echo $($(PKG)_SOURCE)
-endef
-
-
 define DOWNLOAD_BZR
 	$(EXTRA_ENV) $(DL_WRAPPER) -b bzr \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \

@@ -119,10 +108,6 @@ define SOURCE_CHECK_BZR
 	$(BZR) ls --quiet $($(PKG)_SITE) > /dev/null
 endef
 
-define SHOW_EXTERNAL_DEPS_BZR
-	echo $($(PKG)_SOURCE)
-endef
-
 define DOWNLOAD_CVS
 	$(EXTRA_ENV) $(DL_WRAPPER) -b cvs \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \

@@ -139,10 +124,6 @@ define SOURCE_CHECK_CVS
 	$(CVS) -d:pserver:anonymous:@$(call stripurischeme,$(call qstrip,$($(PKG)_SITE))) login
 endef
 
-define SHOW_EXTERNAL_DEPS_CVS
-	echo $($(PKG)_SOURCE)
-endef
-
 define DOWNLOAD_SVN
 	$(EXTRA_ENV) $(DL_WRAPPER) -b svn \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \

@@ -157,10 +138,6 @@ define SOURCE_CHECK_SVN
   $(SVN) ls $($(PKG)_SITE)@$($(PKG)_DL_VERSION) > /dev/null
 endef
 
-define SHOW_EXTERNAL_DEPS_SVN
-  echo $($(PKG)_SOURCE)
-endef
-
 # SCP URIs should be of the form scp://[user@]host:filepath
 # Note that filepath is relative to the user's home directory, so you may want
 # to prepend the path with a slash: scp://[user@]host:/absolutepath

@@ -177,11 +154,6 @@ define SOURCE_CHECK_SCP
 	$(SSH) $(call domain,$(1),:) ls '$(call notdomain,$(1),:)' > /dev/null
 endef
 
-define SHOW_EXTERNAL_DEPS_SCP
-	echo $(2)
-endef
-
-
 define DOWNLOAD_HG
 	$(EXTRA_ENV) $(DL_WRAPPER) -b hg \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \

@@ -198,11 +170,6 @@ define SOURCE_CHECK_HG
   $(HG) incoming --force -l1 $($(PKG)_SITE) > /dev/null
 endef
 
-define SHOW_EXTERNAL_DEPS_HG
-  echo $($(PKG)_SOURCE)
-endef
-
-
 define DOWNLOAD_WGET
 	$(EXTRA_ENV) $(DL_WRAPPER) -b wget \
 		-o $(DL_DIR)/$(2) \

@@ -216,10 +183,6 @@ define SOURCE_CHECK_WGET
   $(WGET) --spider '$(call qstrip,$(1))'
 endef
 
-define SHOW_EXTERNAL_DEPS_WGET
-  echo $(2)
-endef
-
 define DOWNLOAD_LOCALFILES
 	$(EXTRA_ENV) $(DL_WRAPPER) -b cp \
 		-o $(DL_DIR)/$(2) \

@@ -233,10 +196,6 @@ define SOURCE_CHECK_LOCALFILES
   test -e $(call stripurischeme,$(call qstrip,$(1)))
 endef
 
-define SHOW_EXTERNAL_DEPS_LOCALFILES
-  echo $(2)
-endef
-
 ################################################################################
 # DOWNLOAD -- Download helper. Will try to download source from:
 # 1) BR2_PRIMARY_SITE if enabled

package/pkg-generic.mk +1 -4

index d1a1811..11edb34 100644
@@ -122,13 +122,10 @@ $(BUILD_DIR)/%/.stamp_rsynced:
 	$(foreach hook,$($(PKG)_POST_RSYNC_HOOKS),$(call $(hook))$(sep))
 	$(Q)touch $@
 
-# Handle the SOURCE_CHECK and SHOW_EXTERNAL_DEPS cases for rsynced
-# packages
+# Handle the SOURCE_CHECK case for rsynced packages
 $(BUILD_DIR)/%/.stamp_rsync_sourced:
 ifeq ($(DL_MODE),SOURCE_CHECK)
 	test -d $(SRCDIR)
-else ifeq ($(DL_MODE),SHOW_EXTERNAL_DEPS)
-	echo "file://$(SRCDIR)"
 else
 	@true # Nothing to do to source a local package
 endif

Signature · 6 lines

2.1.0

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

That's weird, the output looks like that:

    $ make external-deps
    make -C /home/ymorin/dev/buildroot/buildroot O=/home/ymorin/dev/buildroot/O/. external-deps
    [--SNIP--]
    gcc-4.8.4.tar.bz2
    gdbm-1.11.tar.gz
    genext2fs-1.4.1.tar.gz
      GEN     /home/ymorin/dev/buildroot/O/./Makefile
    genimage-7.tar.xz
    genpart-1.0.2.tar.bz2
    [--SNIP--]

Weird that the message about our Makefile wrapper is irght in the middle of that... :-/

Quoted 13 lines

That's weird, the output looks like that:

    $ make external-deps
    make -C /home/ymorin/dev/buildroot/buildroot O=/home/ymorin/dev/buildroot/O/. external-deps
    [--SNIP--]
    gcc-4.8.4.tar.bz2
    gdbm-1.11.tar.gz
    genext2fs-1.4.1.tar.gz
      GEN     /home/ymorin/dev/buildroot/O/./Makefile
    genimage-7.tar.xz
    genpart-1.0.2.tar.bz2
    [--SNIP--]

Quoted 2 lines

Weird that the message about our Makefile wrapper is irght in the middle of that... :-/

Weird, indeed. Could it be because external-deps is implement itself as a call to $(MAKE) _external-deps ?

Thomas, All,

On 2015-04-13 22:33 +0200, Thomas Petazzoni spake thusly:

Quoted 14 lines · Yann E. MORIN

Quoted 13 lines

That's weird, the output looks like that:

    $ make external-deps
    make -C /home/ymorin/dev/buildroot/buildroot O=/home/ymorin/dev/buildroot/O/. external-deps
    [--SNIP--]
    gcc-4.8.4.tar.bz2
    gdbm-1.11.tar.gz
    genext2fs-1.4.1.tar.gz
      GEN     /home/ymorin/dev/buildroot/O/./Makefile
    genimage-7.tar.xz
    genpart-1.0.2.tar.bz2
    [--SNIP--]

Quoted 5 lines

Quoted 2 lines

Weird that the message about our Makefile wrapper is irght in the middle of that... :-/

Weird, indeed. Could it be because external-deps is implement itself as a call to $(MAKE) _external-deps ?

Well, I'm no longer able to reproduce it... I'll try to get a reproducible setup and will report back...

Regards,
Yann E. MORIN.

Now that the external-deps implementation relies on the per-package <pkg>-all-external-deps and <pkg>-external-deps targets and no longer on the 'source' target with a custom DL_MODE, we can get rid of the support for the SHOW_EXTERNAL_DEPS DL_MODE value in the pkg-download logic.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 package/pkg-download.mk | 45 ++-------------------------------------------
 package/pkg-generic.mk  |  5 +----
 2 files changed, 3 insertions(+), 47 deletions(-)

package/pkg-download.mk +2 -43

index e274712..be3babb 100644
@@ -20,9 +20,8 @@ export LOCALFILES := $(call qstrip,$(BR2_LOCALFILES))
 
 DL_WRAPPER = support/download/dl-wrapper
 
-# Default spider mode is 'DOWNLOAD'. Other possible values are 'SOURCE_CHECK'
-# used by the _source-check target and 'SHOW_EXTERNAL_DEPS', used by the
-# external-deps target.
+# Default spider mode is 'DOWNLOAD'. Other possible value is
+# 'SOURCE_CHECK' used by the _source-check target.
 DL_MODE = DOWNLOAD
 
 # DL_DIR may have been set already from the environment

@@ -71,11 +70,6 @@ github = https://github.com/$(1)/$(2)/archive/$(3)
 # The SOURCE_CHECK_* helpers are in charge of simply checking that the source
 # is available for download. This can be used to make sure one will be able
 # to get all the sources needed for one's build configuration.
-#
-# The SHOW_EXTERNAL_DEPS_* helpers simply output to the console the names
-# of the files that will be downloaded, or path and revision of the
-# source repositories, producing a list of all the "external dependencies"
-# of a given build configuration.
 ################################################################################
 
 # Try a shallow clone - but that only works if the version is a ref (tag or

@@ -100,11 +94,6 @@ define SOURCE_CHECK_GIT
   $(GIT) ls-remote --heads $($(PKG)_SITE) > /dev/null
 endef
 
-define SHOW_EXTERNAL_DEPS_GIT
-	echo $($(PKG)_SOURCE)
-endef
-
-
 define DOWNLOAD_BZR
 	$(EXTRA_ENV) $(DL_WRAPPER) -b bzr \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \

@@ -119,10 +108,6 @@ define SOURCE_CHECK_BZR
 	$(BZR) ls --quiet $($(PKG)_SITE) > /dev/null
 endef
 
-define SHOW_EXTERNAL_DEPS_BZR
-	echo $($(PKG)_SOURCE)
-endef
-
 define DOWNLOAD_CVS
 	$(EXTRA_ENV) $(DL_WRAPPER) -b cvs \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \

@@ -139,10 +124,6 @@ define SOURCE_CHECK_CVS
 	$(CVS) -d:pserver:anonymous:@$(call stripurischeme,$(call qstrip,$($(PKG)_SITE))) login
 endef
 
-define SHOW_EXTERNAL_DEPS_CVS
-	echo $($(PKG)_SOURCE)
-endef
-
 define DOWNLOAD_SVN
 	$(EXTRA_ENV) $(DL_WRAPPER) -b svn \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \

@@ -157,10 +138,6 @@ define SOURCE_CHECK_SVN
   $(SVN) ls $($(PKG)_SITE)@$($(PKG)_DL_VERSION) > /dev/null
 endef
 
-define SHOW_EXTERNAL_DEPS_SVN
-  echo $($(PKG)_SOURCE)
-endef
-
 # SCP URIs should be of the form scp://[user@]host:filepath
 # Note that filepath is relative to the user's home directory, so you may want
 # to prepend the path with a slash: scp://[user@]host:/absolutepath

@@ -177,11 +154,6 @@ define SOURCE_CHECK_SCP
 	$(SSH) $(call domain,$(1),:) ls '$(call notdomain,$(1),:)' > /dev/null
 endef
 
-define SHOW_EXTERNAL_DEPS_SCP
-	echo $(2)
-endef
-
-
 define DOWNLOAD_HG
 	$(EXTRA_ENV) $(DL_WRAPPER) -b hg \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \

@@ -198,11 +170,6 @@ define SOURCE_CHECK_HG
   $(HG) incoming --force -l1 $($(PKG)_SITE) > /dev/null
 endef
 
-define SHOW_EXTERNAL_DEPS_HG
-  echo $($(PKG)_SOURCE)
-endef
-
-
 define DOWNLOAD_WGET
 	$(EXTRA_ENV) $(DL_WRAPPER) -b wget \
 		-o $(DL_DIR)/$(2) \

@@ -216,10 +183,6 @@ define SOURCE_CHECK_WGET
   $(WGET) --spider '$(call qstrip,$(1))'
 endef
 
-define SHOW_EXTERNAL_DEPS_WGET
-  echo $(2)
-endef
-
 define DOWNLOAD_LOCALFILES
 	$(EXTRA_ENV) $(DL_WRAPPER) -b cp \
 		-o $(DL_DIR)/$(2) \

@@ -233,10 +196,6 @@ define SOURCE_CHECK_LOCALFILES
   test -e $(call stripurischeme,$(call qstrip,$(1)))
 endef
 
-define SHOW_EXTERNAL_DEPS_LOCALFILES
-  echo $(2)
-endef
-
 ################################################################################
 # DOWNLOAD -- Download helper. Will try to download source from:
 # 1) BR2_PRIMARY_SITE if enabled

package/pkg-generic.mk +1 -4

index d1a1811..11edb34 100644
@@ -122,13 +122,10 @@ $(BUILD_DIR)/%/.stamp_rsynced:
 	$(foreach hook,$($(PKG)_POST_RSYNC_HOOKS),$(call $(hook))$(sep))
 	$(Q)touch $@
 
-# Handle the SOURCE_CHECK and SHOW_EXTERNAL_DEPS cases for rsynced
-# packages
+# Handle the SOURCE_CHECK case for rsynced packages
 $(BUILD_DIR)/%/.stamp_rsync_sourced:
 ifeq ($(DL_MODE),SOURCE_CHECK)
 	test -d $(SRCDIR)
-else ifeq ($(DL_MODE),SHOW_EXTERNAL_DEPS)
-	echo "file://$(SRCDIR)"
 else
 	@true # Nothing to do to source a local package
 endif

Signature · 6 lines

2.1.0

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Now that the external-deps implementation relies on the per-package <pkg>-all-external-deps and <pkg>-external-deps targets and no longer on the 'source' target with a custom DL_MODE, we can get rid of the support for the SHOW_EXTERNAL_DEPS DL_MODE value in the pkg-download logic.

Signed-off-by Thomas Petazzoni <[email protected]>

make source-check is here to check whether the remote sources for the current selection of packages are still available. So it cannot be a noconfig_targets, since it depends on a configuration being available. The very fact that 'source-check' is basically the same as 'source', and one is a noconfig_target and not the other is a clear indication that the current implementation is wrong.

So this commit moves source-check to no longer be a noconfig_target.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 Makefile | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Makefile +6 -6

index 40ee2e2..6937dd3 100644
@@ -75,7 +75,7 @@ export BR2_VERSION_FULL := $(BR2_VERSION)$(shell $(TOPDIR)/support/scripts/setlo
 noconfig_targets := menuconfig nconfig gconfig xconfig config oldconfig randconfig \
 	defconfig %_defconfig allyesconfig allnoconfig silentoldconfig release \
 	randpackageconfig allyespackageconfig allnopackageconfig \
-	source-check print-version olddefconfig
+	print-version olddefconfig
 
 # Strip quotes and then whitespaces
 qstrip = $(strip $(subst ",,$(1)))

@@ -422,7 +422,7 @@ world: target-post-image
 
 .PHONY: all world toolchain dirs clean distclean source outputmakefile \
 	legal-info legal-info-prepare legal-info-clean printvars help \
-	target-finalize target-post-image
+	target-finalize target-post-image source-check
 
 ################################################################################
 #

@@ -616,6 +616,10 @@ _external-deps: $(foreach p,$(PACKAGES),$(p)-all-external-deps)
 external-deps:
 	@$(MAKE1) -Bs $(EXTRAMAKEARGS) _external-deps | sort -u
 
+# check if download URLs are outdated
+source-check:
+	$(MAKE1) DL_MODE=SOURCE_CHECK $(EXTRAMAKEARGS) source
+
 legal-info-clean:
 	@rm -fr $(LEGAL_INFO_DIR)
 

@@ -794,10 +798,6 @@ savedefconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
 		--savedefconfig=$(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig) \
 		$(CONFIG_CONFIG_IN)
 
-# check if download URLs are outdated
-source-check:
-	$(MAKE1) DL_MODE=SOURCE_CHECK $(EXTRAMAKEARGS) source
-
 .PHONY: defconfig savedefconfig
 
 ################################################################################

Signature · 6 lines

2.1.0

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Reviewed-by "Yann E. MORIN" <[email protected]>

Tested-by "Yann E. MORIN" <[email protected]>

However, I noticed that source-check tries to go to the mirror. For example, cjson fails to download from svn for me here, and it falls back to looking on the mirror, and thus concludes it exists.

Shouldn't source-check be limited to looking at the upstream locations?

However, not a blocker, since it;s already the behaviour we have.

Quoted 11 lines

Reviewed-by "Yann E. MORIN" <[email protected]>

Tested-by "Yann E. MORIN" <[email protected]>

However, I noticed that source-check tries to go to the mirror. For example, cjson fails to download from svn for me here, and it falls back to looking on the mirror, and thus concludes it exists.

Shouldn't source-check be limited to looking at the upstream locations?

However, not a blocker, since it;s already the behaviour we have.

Yes, the behavior you're observing is indeed the current behavior as far as I know, so my patches are not changing this. Indeed, we could discuss whether source-check should check only primary site + upstream site, or primary site + upstream site + sources.b.o.

From a BR maintenance point of view, checking only primary site + upstream is probably better as it means we can get notified when an upstream has disappeared.

But from a BR user point of view, what's important is that the source code remains available *somewhere*, be it from upstream or sources.b.o.

So I'm not very decided on this. Opinions welcome.

Well, I have a slightly different opinion.

First, I agree that for Buildroot maintenance, we only care about upstream, not even primary or backup sites. We do not even have a primary.

Second, for a user that wants to be serious, the only thing that would really matter in the end is the existence of the package on the primary site.

Let me explain...

In an enterprise-grade project, one can not rely on external resources to be always available; one can only rely on internal resources. Thus, in that case, source-check should only look at the primary.

However, that primary has to filled in to begin with, and that is often done by just running "make source" and then copying those sources to the primary. If an upstream source is missing, it is the moment one wants to be notified. There's no reason to run a source-check onto upstream, even less so on the mirror.

So, in my opinion, source-check should behave as such:

- if primary is set, only check on primary
- if primary is not set, only check upstream
- never check on the mirror

make source-check is here to check whether the remote sources for the current selection of packages are still available. So it cannot be a noconfig_targets, since it depends on a configuration being available. The very fact that 'source-check' is basically the same as 'source', and one is a noconfig_target and not the other is a clear indication that the current implementation is wrong.

So this commit moves source-check to no longer be a noconfig_target.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 Makefile | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Makefile +6 -6

index 40ee2e2..6937dd3 100644
@@ -75,7 +75,7 @@ export BR2_VERSION_FULL := $(BR2_VERSION)$(shell $(TOPDIR)/support/scripts/setlo
 noconfig_targets := menuconfig nconfig gconfig xconfig config oldconfig randconfig \
 	defconfig %_defconfig allyesconfig allnoconfig silentoldconfig release \
 	randpackageconfig allyespackageconfig allnopackageconfig \
-	source-check print-version olddefconfig
+	print-version olddefconfig
 
 # Strip quotes and then whitespaces
 qstrip = $(strip $(subst ",,$(1)))

@@ -422,7 +422,7 @@ world: target-post-image
 
 .PHONY: all world toolchain dirs clean distclean source outputmakefile \
 	legal-info legal-info-prepare legal-info-clean printvars help \
-	target-finalize target-post-image
+	target-finalize target-post-image source-check
 
 ################################################################################
 #

@@ -616,6 +616,10 @@ _external-deps: $(foreach p,$(PACKAGES),$(p)-all-external-deps)
 external-deps:
 	@$(MAKE1) -Bs $(EXTRAMAKEARGS) _external-deps | sort -u
 
+# check if download URLs are outdated
+source-check:
+	$(MAKE1) DL_MODE=SOURCE_CHECK $(EXTRAMAKEARGS) source
+
 legal-info-clean:
 	@rm -fr $(LEGAL_INFO_DIR)
 

@@ -794,10 +798,6 @@ savedefconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
 		--savedefconfig=$(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig) \
 		$(CONFIG_CONFIG_IN)
 
-# check if download URLs are outdated
-source-check:
-	$(MAKE1) DL_MODE=SOURCE_CHECK $(EXTRAMAKEARGS) source
-
 .PHONY: defconfig savedefconfig
 
 ################################################################################

Quoted 6 lines

make source-check is here to check whether the remote sources for the current selection of packages are still available. So it cannot be a noconfig_targets, since it depends on a configuration being available. The very fact that 'source-check' is basically the same as 'source', and one is a noconfig_target and not the other is a clear indication that the current implementation is wrong.

Well, actually, source-check is a noconfig target because we don't need to read
the .config file to be able to run it.

Moving it out of noconfig almost doubles
the runtime, because now both the first level and the second level make have to parse all the makefiles.

So the real reason to do this is not because it was wrong (just a little weird
and missing explanation), but because you need it for three patches later.

Well, actually, source-check is a noconfig target because we don't need to read
the .config file to be able to run it. Moving it out of noconfig almost doubles the runtime, because now both the first level and the second level make have to parse all the makefiles.

So the real reason to do this is not because it was wrong (just a little weird
and missing explanation), but because you need it for three patches later.

That said, the change itself looks OK, so

Reviewed-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>

As part of moving to a package infrastructure based source-check implementation, we are going to move away from the global DL_MODE variable to select the behavior of the DOWNLOAD_INNER macro.

As a preparation to this, this commit makes the DOWNLOAD_INNER macro take a third argument, which is the action to be done: either DOWNLOAD or SOURCE_CHECK. For now, the DOWNLOAD macro passes $(DL_MODE) as this third argument, in order to keep the existing behavior.

In addition, a SOURCE_CHECK macro is added, which calls DOWNLOAD_INNER with the appropriate action. This macro will be used in the upcoming package infra based implementation of source-check.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 package/pkg-download.mk | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

package/pkg-download.mk +16 -12

index be3babb..829b54d 100644
@@ -209,14 +209,18 @@ endef
 ################################################################################
 
 define DOWNLOAD
-	$(call DOWNLOAD_INNER,$(1),$(notdir $(1)))
+	$(call DOWNLOAD_INNER,$(1),$(notdir $(1)),$(DL_MODE))
+endef
+
+define SOURCE_CHECK
+	$(call DOWNLOAD_INNER,$(1),$(notdir $(1)),SOURCE_CHECK)
 endef
 
 define DOWNLOAD_INNER
 	$(Q)if test -n "$(call qstrip,$(BR2_PRIMARY_SITE))" ; then \
 		case "$(call geturischeme,$(BR2_PRIMARY_SITE))" in \
-			scp) $(call $(DL_MODE)_SCP,$(BR2_PRIMARY_SITE)/$(2),$(2)) && exit ;; \
-			*) $(call $(DL_MODE)_WGET,$(BR2_PRIMARY_SITE)/$(2),$(2)) && exit ;; \
+			scp) $(call $(3)_SCP,$(BR2_PRIMARY_SITE)/$(2),$(2)) && exit ;; \
+			*) $(call $(3)_WGET,$(BR2_PRIMARY_SITE)/$(2),$(2)) && exit ;; \
 		esac ; \
 	fi ; \
 	if test "$(BR2_PRIMARY_SITE_ONLY)" = "y" ; then \

@@ -229,18 +233,18 @@ define DOWNLOAD_INNER
 			scheme="$($(PKG)_SITE_METHOD)" ; \
 		fi ; \
 		case "$$scheme" in \
-			git) $($(DL_MODE)_GIT) && exit ;; \
-			svn) $($(DL_MODE)_SVN) && exit ;; \
-			cvs) $($(DL_MODE)_CVS) && exit ;; \
-			bzr) $($(DL_MODE)_BZR) && exit ;; \
-			file) $($(DL_MODE)_LOCALFILES) && exit ;; \
-			scp) $($(DL_MODE)_SCP) && exit ;; \
-			hg) $($(DL_MODE)_HG) && exit ;; \
-			*) $(call $(DL_MODE)_WGET,$(1),$(2)) && exit ;; \
+			git) $($(3)_GIT) && exit ;; \
+			svn) $($(3)_SVN) && exit ;; \
+			cvs) $($(3)_CVS) && exit ;; \
+			bzr) $($(3)_BZR) && exit ;; \
+			file) $($(3)_LOCALFILES) && exit ;; \
+			scp) $($(3)_SCP) && exit ;; \
+			hg) $($(3)_HG) && exit ;; \
+			*) $(call $(3)_WGET,$(1),$(2)) && exit ;; \
 		esac ; \
 	fi ; \
 	if test -n "$(call qstrip,$(BR2_BACKUP_SITE))" ; then \
-		$(call $(DL_MODE)_WGET,$(BR2_BACKUP_SITE)/$(2),$(2)) && exit ; \
+		$(call $(3)_WGET,$(BR2_BACKUP_SITE)/$(2),$(2)) && exit ; \
 	fi ; \
 	exit 1
 endef

Signature · 6 lines

2.1.0

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Thomas, All,

On 2015-04-12 18:37 +0200, Thomas Petazzoni spake thusly:

Quoted 12 lines

As part of moving to a package infrastructure based source-check implementation, we are going to move away from the global DL_MODE variable to select the behavior of the DOWNLOAD_INNER macro.

As a preparation to this, this commit makes the DOWNLOAD_INNER macro take a third argument, which is the action to be done: either DOWNLOAD or SOURCE_CHECK. For now, the DOWNLOAD macro passes $(DL_MODE) as this third argument, in order to keep the existing behavior.

In addition, a SOURCE_CHECK macro is added, which calls DOWNLOAD_INNER with the appropriate action. This macro will be used in the upcoming package infra based implementation of source-check.

Quoted 2 lines

Signed-off-by Thomas Petazzoni <[email protected]>

Reviewed-by "Yann E. MORIN" <[email protected]>

Tested-by "Yann E. MORIN" <[email protected]>

[tested by doing a "make source" on a randpackageconfig]

Regards,
Yann E. MORIN.

Quoted 65 lines

---
 package/pkg-download.mk | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

package/pkg-download.mk +16 -12

index be3babb..829b54d 100644
@@ -209,14 +209,18 @@ endef
 ################################################################################
 
 define DOWNLOAD
-	$(call DOWNLOAD_INNER,$(1),$(notdir $(1)))
+	$(call DOWNLOAD_INNER,$(1),$(notdir $(1)),$(DL_MODE))
+endef
+
+define SOURCE_CHECK
+	$(call DOWNLOAD_INNER,$(1),$(notdir $(1)),SOURCE_CHECK)
 endef
 
 define DOWNLOAD_INNER
 	$(Q)if test -n "$(call qstrip,$(BR2_PRIMARY_SITE))" ; then \
 		case "$(call geturischeme,$(BR2_PRIMARY_SITE))" in \
-			scp) $(call $(DL_MODE)_SCP,$(BR2_PRIMARY_SITE)/$(2),$(2)) && exit ;; \
-			*) $(call $(DL_MODE)_WGET,$(BR2_PRIMARY_SITE)/$(2),$(2)) && exit ;; \
+			scp) $(call $(3)_SCP,$(BR2_PRIMARY_SITE)/$(2),$(2)) && exit ;; \
+			*) $(call $(3)_WGET,$(BR2_PRIMARY_SITE)/$(2),$(2)) && exit ;; \
 		esac ; \
 	fi ; \
 	if test "$(BR2_PRIMARY_SITE_ONLY)" = "y" ; then \

@@ -229,18 +233,18 @@ define DOWNLOAD_INNER
 			scheme="$($(PKG)_SITE_METHOD)" ; \
 		fi ; \
 		case "$$scheme" in \
-			git) $($(DL_MODE)_GIT) && exit ;; \
-			svn) $($(DL_MODE)_SVN) && exit ;; \
-			cvs) $($(DL_MODE)_CVS) && exit ;; \
-			bzr) $($(DL_MODE)_BZR) && exit ;; \
-			file) $($(DL_MODE)_LOCALFILES) && exit ;; \
-			scp) $($(DL_MODE)_SCP) && exit ;; \
-			hg) $($(DL_MODE)_HG) && exit ;; \
-			*) $(call $(DL_MODE)_WGET,$(1),$(2)) && exit ;; \
+			git) $($(3)_GIT) && exit ;; \
+			svn) $($(3)_SVN) && exit ;; \
+			cvs) $($(3)_CVS) && exit ;; \
+			bzr) $($(3)_BZR) && exit ;; \
+			file) $($(3)_LOCALFILES) && exit ;; \
+			scp) $($(3)_SCP) && exit ;; \
+			hg) $($(3)_HG) && exit ;; \
+			*) $(call $(3)_WGET,$(1),$(2)) && exit ;; \
 		esac ; \
 	fi ; \
 	if test -n "$(call qstrip,$(BR2_BACKUP_SITE))" ; then \
-		$(call $(DL_MODE)_WGET,$(BR2_BACKUP_SITE)/$(2),$(2)) && exit ; \
+		$(call $(3)_WGET,$(BR2_BACKUP_SITE)/$(2),$(2)) && exit ; \
 	fi ; \
 	exit 1
 endef

Signature · 6 lines

2.1.0

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Quoted 15 lines

Thomas, All,

On 2015-04-12 18:37 +0200, Thomas Petazzoni spake thusly:

Quoted 12 lines

As part of moving to a package infrastructure based source-check implementation, we are going to move away from the global DL_MODE variable to select the behavior of the DOWNLOAD_INNER macro.

As a preparation to this, this commit makes the DOWNLOAD_INNER macro take a third argument, which is the action to be done: either DOWNLOAD or SOURCE_CHECK. For now, the DOWNLOAD macro passes $(DL_MODE) as this third argument, in order to keep the existing behavior.

In addition, a SOURCE_CHECK macro is added, which calls DOWNLOAD_INNER with the appropriate action. This macro will be used in the upcoming package infra based implementation of source-check.

It seems a bit weird to do that in this commit, more logical to do it in the
next one or as a separate commit. But that's no major issue, so

Reviewed-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>

Quoted 1 line

Quoted 1 line

Quoted 5 lines

Quoted 1 line

Signed-off-by Thomas Petazzoni <[email protected]>

Reviewed-by "Yann E. MORIN" <[email protected]>

Tested-by "Yann E. MORIN" <[email protected]>

[tested by doing a "make source" on a randpackageconfig]

You should have tested make source-check.

In the ned, does source-check really makes sense?

Let me rephrase...

What's the purpose of source-check?

As far as I can see, there are two use-cases:

- for us Buildroot devs: check that an upstream still provides a

    resource;

- for a standard user to check if he can get the source of his set of packages.

But is that really necessary?

For a user, what can he do if the package can't be downloaded (i.e. not in upstream and not on our mirror)? Nothing...

For us, all we need to know is that something can't be downloaded so we can bump the package (or point it to our mirror). We have autobuilders to catch download failures.

So, does it make sense to keep source-check at all?

Quoted 23 lines · Yann E. MORIN

In the ned, does source-check really makes sense?

Let me rephrase...

What's the purpose of source-check?

As far as I can see, there are two use-cases:

- for us Buildroot devs: check that an upstream still provides a

    resource;

- for a standard user to check if he can get the source of his set of packages.

But is that really necessary?

For a user, what can he do if the package can't be downloaded (i.e. not in upstream and not on our mirror)? Nothing...

For us, all we need to know is that something can't be downloaded so we can bump the package (or point it to our mirror). We have autobuilders to catch download failures.

Quoted 1 line

So, does it make sense to keep source-check at all?

I was thinking the same thing, I can't really see a use case for it. Actually,
same thing for external-deps.

Quoted 1 line · Arnout Vandecappelle

On 15/04/15 00:25, Yann E. MORIN wrote:

Quoted 5 lines

Quoted 1 line

So, does it make sense to keep source-check at all?

I was thinking the same thing, I can't really see a use case for it. Actually,
same thing for external-deps.

external-deps is useful: We use it e.g. to clean a dl/ directory after bumping package versions, or to copy only relevant files in dl/ to an offline location.

Clearing the dl/ directory and running "make source" would end up with the same thing, but it would redownloads everything so it would be much slower.

This commits extends the common package infrastructure with the $(1)-source-check and $(1)-all-source-check targets.

The $(1)-source-check target simply calls the newly added SOURCE_CHECK macro on all items to be downloaded.

The $(1)-all-source-check target will depend on the $(1)-all-source-check targets of all dependent packages and the $(1)-source-check target of the current package, which allows to do a recursive source-check in the dependency tree.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 package/pkg-generic.mk | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

package/pkg-generic.mk +15 -1

index 11edb34..b45b86e 100644
@@ -577,6 +577,16 @@ endif
 $(1)-show-version:
 			@echo $$($(2)_VERSION)
 
+$(1)-source-check:
+ifeq ($$($(2)_OVERRIDE_SRCDIR),)
+	$$(foreach p,$$($(2)_SOURCE) $$($(2)_EXTRA_DOWNLOADS) $$($(2)_PATCH),\
+		$$(if $$(findstring ://,$$(p)),\
+			$$(call SOURCE_CHECK,$$(p)),\
+			$$(call SOURCE_CHECK,$$($(2)_SITE:/=)/$$(p))))
+else
+	test -d $$($(2)_OVERRIDE_SRCDIR)
+endif
+
 $(1)-show-depends:
 			@echo $$(sort $$($(2)_FINAL_DEPENDENCIES) $$($(2)_FINAL_PATCH_DEPENDENCIES))
 

@@ -589,6 +599,8 @@ $(1)-graph-depends: graph-depends-requirements
 
 $(1)-all-source:       $$(foreach p,$$($(2)_FINAL_DEPENDENCIES),$$(p)-all-source) $(1)-source
 
+$(1)-all-source-check:       $$(foreach p,$$($(2)_FINAL_DEPENDENCIES),$$(p)-all-source-check) $(1)-source-check
+
 $(1)-all-external-deps:        $$(foreach p,$$($(2)_FINAL_DEPENDENCIES),$$(p)-all-external-deps) $(1)-external-deps
 
 $(1)-all-legal-info:   $$(foreach p,$$($(2)_FINAL_DEPENDENCIES),$$(p)-all-legal-info) $(1)-legal-info

@@ -782,6 +794,7 @@ endif
 	$(1)-all-external-deps \
 	$(1)-all-legal-info \
 	$(1)-all-source \
+	$(1)-all-source-check \
 	$(1)-build \
 	$(1)-clean-for-rebuild \
 	$(1)-clean-for-reconfigure \

@@ -805,7 +818,8 @@ endif
 	$(1)-rsync \
 	$(1)-show-depends \
 	$(1)-show-version \
-	$(1)-source
+	$(1)-source \
+	$(1)-source-check
 
 endif # $(2)_KCONFIG_VAR
 endef # inner-generic-package

Signature · 6 lines

2.1.0

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

On 2015-04-12 18:37 +0200, Thomas Petazzoni spake thusly:

Quoted 11 lines

This commits extends the common package infrastructure with the $(1)-source-check and $(1)-all-source-check targets.

The $(1)-source-check target simply calls the newly added SOURCE_CHECK macro on all items to be downloaded.

The $(1)-all-source-check target will depend on the $(1)-all-source-check targets of all dependent packages and the $(1)-source-check target of the current package, which allows to do a recursive source-check in the dependency tree.

Quoted 1 line

Signed-off-by Thomas Petazzoni <[email protected]>

Reviewed-by "Yann E. MORIN" <[email protected]>

Tested-by "Yann E. MORIN" <[email protected]>

[manually tested with PKG-source-check and PKG-all-source-check, with
PKG being either ejabberd or xz, both enabled.]

Quoted 12 lines

---
 package/pkg-generic.mk | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

package/pkg-generic.mk +0 -0

index 11edb34..b45b86e 100644
@@ -577,6 +577,16 @@ endif
 $(1)-show-version:
 			@echo $$($(2)_VERSION)
 

package/pkg-generic.mk endif · line 585

Quoted 6 lines

package/pkg-generic.mk +6 -0

+$(1)-source-check:
+ifeq ($$($(2)_OVERRIDE_SRCDIR),)
+	$$(foreach p,$$($(2)_SOURCE) $$($(2)_EXTRA_DOWNLOADS) $$($(2)_PATCH),\
+		$$(if $$(findstring ://,$$(p)),\
+			$$(call SOURCE_CHECK,$$(p)),\
+			$$(call SOURCE_CHECK,$$($(2)_SITE:/=)/$$(p))))

Quoted 41 lines

package/pkg-generic.mk +9 -1

+else
+	test -d $$($(2)_OVERRIDE_SRCDIR)
+endif
+
 $(1)-show-depends:
 			@echo $$(sort $$($(2)_FINAL_DEPENDENCIES) $$($(2)_FINAL_PATCH_DEPENDENCIES))
 

@@ -589,6 +599,8 @@ $(1)-graph-depends: graph-depends-requirements
 
 $(1)-all-source:       $$(foreach p,$$($(2)_FINAL_DEPENDENCIES),$$(p)-all-source) $(1)-source
 
+$(1)-all-source-check:       $$(foreach p,$$($(2)_FINAL_DEPENDENCIES),$$(p)-all-source-check) $(1)-source-check
+
 $(1)-all-external-deps:        $$(foreach p,$$($(2)_FINAL_DEPENDENCIES),$$(p)-all-external-deps) $(1)-external-deps
 
 $(1)-all-legal-info:   $$(foreach p,$$($(2)_FINAL_DEPENDENCIES),$$(p)-all-legal-info) $(1)-legal-info

@@ -782,6 +794,7 @@ endif
 	$(1)-all-external-deps \
 	$(1)-all-legal-info \
 	$(1)-all-source \
+	$(1)-all-source-check \
 	$(1)-build \
 	$(1)-clean-for-rebuild \
 	$(1)-clean-for-reconfigure \

@@ -805,7 +818,8 @@ endif
 	$(1)-rsync \
 	$(1)-show-depends \
 	$(1)-show-version \
-	$(1)-source
+	$(1)-source \
+	$(1)-source-check
 
 endif # $(2)_KCONFIG_VAR
 endef # inner-generic-package

Signature · 6 lines

2.1.0

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Signature · 10 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

On 2015-04-13 23:05 +0200, Yann E. MORIN spake thusly:

Quoted 1 line

On 2015-04-12 18:37 +0200, Thomas Petazzoni spake thusly:

Quoted 6 lines

package/pkg-generic.mk endif · line 585

Quoted 6 lines

+$(1)-source-check:

package/pkg-generic.mk +5 -0

+ifeq ($$($(2)_OVERRIDE_SRCDIR),)
+	$$(foreach p,$$($(2)_SOURCE) $$($(2)_EXTRA_DOWNLOADS) $$($(2)_PATCH),\
+		$$(if $$(findstring ://,$$(p)),\
+			$$(call SOURCE_CHECK,$$(p)),\
+			$$(call SOURCE_CHECK,$$($(2)_SITE:/=)/$$(p))))

                                                            ^^

You're missing a $$(sep) before the last parenthesis, here ||

so that last line would be:

    $$(call SOURCE_CHECK,$$($(2)_SITE:/=)/$$(p)))$$(sep))

This commits extends the common package infrastructure with the $(1)-source-check and $(1)-all-source-check targets.

The $(1)-source-check target simply calls the newly added SOURCE_CHECK macro on all items to be downloaded.

The $(1)-all-source-check target will depend on the $(1)-all-source-check targets of all dependent packages and the $(1)-source-check target of the current package, which allows to do a recursive source-check in the dependency tree.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 package/pkg-generic.mk | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

package/pkg-generic.mk +10 -0

index 11edb34..b45b86e 100644
@@ -577,6 +577,16 @@ endif
 $(1)-show-version:
 			@echo $$($(2)_VERSION)
 
+$(1)-source-check:
+ifeq ($$($(2)_OVERRIDE_SRCDIR),)
+	$$(foreach p,$$($(2)_SOURCE) $$($(2)_EXTRA_DOWNLOADS) $$($(2)_PATCH),\
+		$$(if $$(findstring ://,$$(p)),\
+			$$(call SOURCE_CHECK,$$(p)),\
+			$$(call SOURCE_CHECK,$$($(2)_SITE:/=)/$$(p))))
+else
+	test -d $$($(2)_OVERRIDE_SRCDIR)
+endif
+
 $(1)-show-depends:
 			@echo $$(sort $$($(2)_FINAL_DEPENDENCIES) $$($(2)_FINAL_PATCH_DEPENDENCIES))
 

package/pkg-generic.mk +4 -1

+
 $(1)-all-external-deps:        $$(foreach p,$$($(2)_FINAL_DEPENDENCIES),$$(p)-all-external-deps) $(1)-external-deps
 
 $(1)-all-legal-info:   $$(foreach p,$$($(2)_FINAL_DEPENDENCIES),$$(p)-all-legal-info) $(1)-legal-info

@@ -782,6 +794,7 @@ endif
 	$(1)-all-external-deps \
 	$(1)-all-legal-info \
 	$(1)-all-source \
+	$(1)-all-source-check \
 	$(1)-build \
 	$(1)-clean-for-rebuild \
 	$(1)-clean-for-reconfigure \

@@ -805,7 +818,8 @@ endif
 	$(1)-rsync \
 	$(1)-show-depends \
 	$(1)-show-version \
-	$(1)-source
+	$(1)-source \
+	$(1)-source-check
 
 endif # $(2)_KCONFIG_VAR
 endef # inner-generic-package

This commit switches the implementation of the global source-check target to use a package infrastructure based mechanism, using the $(1)-all-source-check target added in the previous commit.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Makefile +1 -2

index 6937dd3..7d94ee8 100644
@@ -617,8 +617,7 @@ external-deps:
 	@$(MAKE1) -Bs $(EXTRAMAKEARGS) _external-deps | sort -u
 
 # check if download URLs are outdated
-source-check:
-	$(MAKE1) DL_MODE=SOURCE_CHECK $(EXTRAMAKEARGS) source
+source-check: $(foreach p,$(PACKAGES),$(p)-all-source-check)
 
 legal-info-clean:
 	@rm -fr $(LEGAL_INFO_DIR)

Signature · 6 lines

2.1.0

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

This commit switches the implementation of the global source-check target to use a package infrastructure based mechanism, using the $(1)-all-source-check target added in the previous commit.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Makefile +1 -2

index 6937dd3..7d94ee8 100644
@@ -617,8 +617,7 @@ external-deps:
 	@$(MAKE1) -Bs $(EXTRAMAKEARGS) _external-deps | sort -u
 
 # check if download URLs are outdated
-source-check:
-	$(MAKE1) DL_MODE=SOURCE_CHECK $(EXTRAMAKEARGS) source
+source-check: $(foreach p,$(PACKAGES),$(p)-all-source-check)
 
 legal-info-clean:
 	@rm -fr $(LEGAL_INFO_DIR)

The only reason for the .stamp_rsync_sourced fake stamp file target to exist was to handle the SOURCE_CHECK operation on packages using the OVERRIDE_SRCDIR mechanism. Now that this is handled directly inside $(1)-source-check, there is no longer any need for this part of the code.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 package/pkg-generic.mk | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

package/pkg-generic.mk +1 -12

index b45b86e..d432a2e 100644
@@ -122,14 +122,6 @@ $(BUILD_DIR)/%/.stamp_rsynced:
 	$(foreach hook,$($(PKG)_POST_RSYNC_HOOKS),$(call $(hook))$(sep))
 	$(Q)touch $@
 
-# Handle the SOURCE_CHECK case for rsynced packages
-$(BUILD_DIR)/%/.stamp_rsync_sourced:
-ifeq ($(DL_MODE),SOURCE_CHECK)
-	test -d $(SRCDIR)
-else
-	@true # Nothing to do to source a local package
-endif
-
 # Patch
 #
 # The RAWNAME variable is the lowercased package name, which allows to

@@ -439,7 +431,6 @@ $(2)_TARGET_INSTALL_HOST =      $$($(2)_DIR)/.stamp_host_installed
 $(2)_TARGET_BUILD =		$$($(2)_DIR)/.stamp_built
 $(2)_TARGET_CONFIGURE =		$$($(2)_DIR)/.stamp_configured
 $(2)_TARGET_RSYNC =	        $$($(2)_DIR)/.stamp_rsynced
-$(2)_TARGET_RSYNC_SOURCE =      $$($(2)_DIR)/.stamp_rsync_sourced
 $(2)_TARGET_PATCH =		$$($(2)_DIR)/.stamp_patched
 $(2)_TARGET_EXTRACT =		$$($(2)_DIR)/.stamp_extracted
 $(2)_TARGET_SOURCE =		$$($(2)_DIR)/.stamp_downloaded

@@ -568,7 +559,7 @@ $(1)-extract:		$(1)-rsync
 
 $(1)-rsync:		$$($(2)_TARGET_RSYNC)
 
-$(1)-source:		$$($(2)_TARGET_RSYNC_SOURCE)
+$(1)-source:
 
 $(1)-external-deps:
 	@echo "file://$$($(2)_OVERRIDE_SRCDIR)"

@@ -638,8 +629,6 @@ $$($(2)_TARGET_BUILD):			PKG=$(2)
 $$($(2)_TARGET_CONFIGURE):		PKG=$(2)
 $$($(2)_TARGET_RSYNC):                  SRCDIR=$$($(2)_OVERRIDE_SRCDIR)
 $$($(2)_TARGET_RSYNC):                  PKG=$(2)
-$$($(2)_TARGET_RSYNC_SOURCE):		SRCDIR=$$($(2)_OVERRIDE_SRCDIR)
-$$($(2)_TARGET_RSYNC_SOURCE):		PKG=$(2)
 $$($(2)_TARGET_PATCH):			PKG=$(2)
 $$($(2)_TARGET_PATCH):			RAWNAME=$$(patsubst host-%,%,$(1))
 $$($(2)_TARGET_PATCH):			PKGDIR=$(pkgdir)

The .stamp_downloaded target is now only being used to really download, and no longer for other activities like "source check" or "external deps". So the check on DL_MODE being equal to DOWNLOAD is no longer necessary.

Signed-off-by Thomas Petazzoni <[email protected]>

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

package/pkg-generic.mk +0 -4

index d432a2e..13a3843 100644
@@ -71,7 +71,6 @@ endif
 # Retrieve the archive
 $(BUILD_DIR)/%/.stamp_downloaded:
 	$(foreach hook,$($(PKG)_PRE_DOWNLOAD_HOOKS),$(call $(hook))$(sep))
-ifeq ($(DL_MODE),DOWNLOAD)
 # Only show the download message if it isn't already downloaded
 	$(Q)for p in $($(PKG)_SOURCE) $($(PKG)_PATCH) $($(PKG)_EXTRA_DOWNLOADS) ; do \
 		if test ! -e $(DL_DIR)/`basename $$p` ; then \

@@ -79,7 +78,6 @@ ifeq ($(DL_MODE),DOWNLOAD)
 			break ; \
 		fi ; \
 	done
-endif
 	$(if $($(PKG)_SOURCE),$(call DOWNLOAD,$($(PKG)_SITE:/=)/$($(PKG)_SOURCE)))
 	$(foreach p,$($(PKG)_EXTRA_DOWNLOADS),\
 		$(if $(findstring ://,$(p)),\

@@ -94,10 +92,8 @@ endif
 		)\
 	$(sep))
 	$(foreach hook,$($(PKG)_POST_DOWNLOAD_HOOKS),$(call $(hook))$(sep))
-ifeq ($(DL_MODE),DOWNLOAD)
 	$(Q)mkdir -p $(@D)
 	$(Q)touch $@
-endif
 
 # Unpack the archive
 $(BUILD_DIR)/%/.stamp_extracted:

The DL_MODE variable is now no longer used with any other value than "DOWNLOAD", so it no longer makes sense to have this variable at all. Therefore, this commit gets rid of it.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 package/pkg-download.mk | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

package/pkg-download.mk +1 -5

index 829b54d..8cd168a 100644
@@ -20,10 +20,6 @@ export LOCALFILES := $(call qstrip,$(BR2_LOCALFILES))
 
 DL_WRAPPER = support/download/dl-wrapper
 
-# Default spider mode is 'DOWNLOAD'. Other possible value is
-# 'SOURCE_CHECK' used by the _source-check target.
-DL_MODE = DOWNLOAD
-
 # DL_DIR may have been set already from the environment
 ifeq ($(origin DL_DIR),undefined)
 DL_DIR ?= $(call qstrip,$(BR2_DL_DIR))

@@ -209,7 +205,7 @@ endef
 ################################################################################
 
 define DOWNLOAD
-	$(call DOWNLOAD_INNER,$(1),$(notdir $(1)),$(DL_MODE))
+	$(call DOWNLOAD_INNER,$(1),$(notdir $(1)),DOWNLOAD)
 endef
 
 define SOURCE_CHECK

Some of the SOURCE_CHECK_* macros are using a non-standard two-spaces indentation. This commit switches them to use a single tab based indentation, like in the rest of Buildroot.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 package/pkg-download.mk | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

package/pkg-download.mk +5 -5

index 8cd168a..f35dc04 100644
@@ -87,7 +87,7 @@ endef
 # TODO: improve to check that the given PKG_DL_VERSION exists on the remote
 # repository
 define SOURCE_CHECK_GIT
-  $(GIT) ls-remote --heads $($(PKG)_SITE) > /dev/null
+	$(GIT) ls-remote --heads $($(PKG)_SITE) > /dev/null
 endef
 
 define DOWNLOAD_BZR

@@ -131,7 +131,7 @@ define DOWNLOAD_SVN
 endef
 
 define SOURCE_CHECK_SVN
-  $(SVN) ls $($(PKG)_SITE)@$($(PKG)_DL_VERSION) > /dev/null
+	$(SVN) ls $($(PKG)_SITE)@$($(PKG)_DL_VERSION) > /dev/null
 endef
 
 # SCP URIs should be of the form scp://[user@]host:filepath

@@ -163,7 +163,7 @@ endef
 # TODO: improve to check that the given PKG_DL_VERSION exists on the remote
 # repository
 define SOURCE_CHECK_HG
-  $(HG) incoming --force -l1 $($(PKG)_SITE) > /dev/null
+	$(HG) incoming --force -l1 $($(PKG)_SITE) > /dev/null
 endef
 
 define DOWNLOAD_WGET

@@ -176,7 +176,7 @@ define DOWNLOAD_WGET
 endef
 
 define SOURCE_CHECK_WGET
-  $(WGET) --spider '$(call qstrip,$(1))'
+	$(WGET) --spider '$(call qstrip,$(1))'
 endef
 
 define DOWNLOAD_LOCALFILES

@@ -189,7 +189,7 @@ define DOWNLOAD_LOCALFILES
 endef
 
 define SOURCE_CHECK_LOCALFILES
-  test -e $(call stripurischeme,$(call qstrip,$(1)))
+	test -e $(call stripurischeme,$(call qstrip,$(1)))
 endef
 
 ################################################################################

Just like <pkg>_PATCH, <pkg>_SOURCE or <pkg>_SITE, the <pkg>_EXTRA_DOWNLOADS variable of a host package should be automatically inferred from the <pkg>_EXTRA_DOWNLOADS value of the corresponding target package, unless an explicit value is being defined for the host package. This commit implements this for <pkg>_EXTRA_DOWNLOADS.

Signed-off-by Thomas Petazzoni <[email protected]>

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

package/pkg-generic.mk +6 -0

index 13a3843..d2ed605 100644
@@ -353,6 +353,12 @@ ifndef $(2)_PATCH
  endif
 endif
 
+ifndef $(2)_EXTRA_DOWNLOADS
+ ifdef $(3)_EXTRA_DOWNLOADS
+  $(2)_PATCH = $$($(3)_EXTRA_DOWNLOADS)
+ endif
+endif
+
 ifndef $(2)_SITE
  ifdef $(3)_SITE
   $(2)_SITE = $$($(3)_SITE)

Quoted 6 lines · Thomas Petazzoni

Just like <pkg>_PATCH, <pkg>_SOURCE or <pkg>_SITE, the <pkg>_EXTRA_DOWNLOADS variable of a host package should be automatically inferred from the <pkg>_EXTRA_DOWNLOADS value of the corresponding target package, unless an explicit value is being defined for the host package. This commit implements this for <pkg>_EXTRA_DOWNLOADS.

I'm not so sure about this one. _EXTRA_DOWNLOADS is currently used for the
blackfin toolchains and for perl. Toolchains are target packages so no issue there, but for perl, we really only need perl-cross for the target perl. At the moment, host-perl is only built as a dependency of perl so it doesn't hurt to download perl-cross for host-perl, but it is unnecessary.

Is there a hidden reason why you want this (i.e. is it required for some later
patch)?

Now, both the download and source-check code are iterating over <pkg>_SOURCE, <pkg>_PATCH and <pkg>_EXTRA_DOWNLOADS elements, figuring out whether they contain full URLs or not. Instead of doing this repeatdly, this patch introduces an internal <pkg>_ALL_DOWNLOADS variable, which contains the list of everything that needs to be downloaded, with URLs already expanded to take into account <pkg>_SITE if needed.

This allows to simplify quite significantly the .stamp_download and source-check implementation.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 package/pkg-generic.mk | 26 ++++++++------------------
 1 file changed, 8 insertions(+), 18 deletions(-)

package/pkg-generic.mk +8 -18

index d2ed605..7dc1cb8 100644
@@ -72,25 +72,13 @@ endif
 $(BUILD_DIR)/%/.stamp_downloaded:
 	$(foreach hook,$($(PKG)_PRE_DOWNLOAD_HOOKS),$(call $(hook))$(sep))
 # Only show the download message if it isn't already downloaded
-	$(Q)for p in $($(PKG)_SOURCE) $($(PKG)_PATCH) $($(PKG)_EXTRA_DOWNLOADS) ; do \
+	$(Q)for p in $($(PKG)_ALL_DOWNLOADS); do \
 		if test ! -e $(DL_DIR)/`basename $$p` ; then \
 			$(call MESSAGE,"Downloading") ; \
 			break ; \
 		fi ; \
 	done
-	$(if $($(PKG)_SOURCE),$(call DOWNLOAD,$($(PKG)_SITE:/=)/$($(PKG)_SOURCE)))
-	$(foreach p,$($(PKG)_EXTRA_DOWNLOADS),\
-		$(if $(findstring ://,$(p)),\
-			$(call DOWNLOAD,$(p)),\
-			$(call DOWNLOAD,$($(PKG)_SITE:/=)/$(p))\
-		)\
-	$(sep))
-	$(foreach p,$($(PKG)_PATCH),\
-		$(if $(findstring ://,$(p)),\
-			$(call DOWNLOAD,$(p)),\
-			$(call DOWNLOAD,$($(PKG)_SITE:/=)/$(p))\
-		)\
-	$(sep))
+	$(foreach p,$($(PKG)_ALL_DOWNLOADS),$(call DOWNLOAD,$(p))$(sep))
 	$(foreach hook,$($(PKG)_POST_DOWNLOAD_HOOKS),$(call $(hook))$(sep))
 	$(Q)mkdir -p $(@D)
 	$(Q)touch $@

@@ -359,6 +347,11 @@ ifndef $(2)_EXTRA_DOWNLOADS
  endif
 endif
 
+$(2)_ALL_DOWNLOADS = \
+	$$(foreach p,$$($(2)_SOURCE) $$($(2)_PATCH) $$($(2)_EXTRA_DOWNLOADS),\
+		$$(if $$(findstring ://,$$(p)),$$(p),\
+			$$($(2)_SITE:/=)/$$(p)))
+
 ifndef $(2)_SITE
  ifdef $(3)_SITE
   $(2)_SITE = $$($(3)_SITE)

@@ -572,10 +565,7 @@ $(1)-show-version:
 
 $(1)-source-check:
 ifeq ($$($(2)_OVERRIDE_SRCDIR),)
-	$$(foreach p,$$($(2)_SOURCE) $$($(2)_EXTRA_DOWNLOADS) $$($(2)_PATCH),\
-		$$(if $$(findstring ://,$$(p)),\
-			$$(call SOURCE_CHECK,$$(p)),\
-			$$(call SOURCE_CHECK,$$($(2)_SITE:/=)/$$(p))))
+	$$(foreach p,$$($(2)_ALL_DOWNLOADS),$$(call SOURCE_CHECK,$$(p))$$(sep))
 else
 	test -d $$($(2)_OVERRIDE_SRCDIR)
 endif

Now that all the bits are in place, switch the global 'source' target to use the package infrastructure logic.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Makefile +1 -1

index 7d94ee8..06a9f05 100644
@@ -610,7 +610,7 @@ target-post-image: $(TARGETS_ROOTFS) target-finalize
 		$(call MESSAGE,"Executing post-image script $(s)"); \
 		$(EXTRA_ENV) $(s) $(BINARIES_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
 
-source: $(PACKAGES_SOURCE) $(HOST_SOURCE)
+source: $(foreach p,$(PACKAGES),$(p)-all-source)
 
 _external-deps: $(foreach p,$(PACKAGES),$(p)-all-external-deps)
 external-deps:

Now that all the external-deps, source-check and source targets are properly implemented based on the package infrastructure, the PACKAGES_SOURCE, TARGET_HOST_DEPS, HOST_DEPS and HOST_SOURCE variables are no longer needed. This is a good thing since they were anyway incorrect, as they were only doing a two level recursion in the dependencies of host packages.

Signed-off-by Thomas Petazzoni <[email protected]>

Signed-off-by Thomas Petazzoni <[email protected]>

---
 Makefile | 21 ---------------------
 1 file changed, 21 deletions(-)

Makefile +0 -21

index 06a9f05..a598063 100644
@@ -389,27 +389,6 @@ include fs/common.mk
 
 include $(BR2_EXTERNAL)/external.mk
 
-PACKAGES_SOURCE := $(patsubst %,%-source,$(PACKAGES))
-
-# host-* dependencies have to be handled specially, as those aren't
-# visible in Kconfig and hence not added to a variable like PACKAGES.
-# instead, find all the host-* targets listed in each <PKG>_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,$(PACKAGES) $(TARGETS_ROOTFS))),\
-		$($(dep)))))
-# Host packages can in turn have their own dependencies. Likewise find
-# all the package names listed in the HOST_<PKG>_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)))
-
 dirs: $(BUILD_DIR) $(STAGING_DIR) $(TARGET_DIR) \
 	$(HOST_DIR) $(BINARIES_DIR)
 

This is a new iteration of the "Package based 'source', 'legal-info', 'source-check' and 'external-deps'" series.

This is a new iteration of the "Package based 'source', 'legal-info', 'source-check' and 'external-deps'" series.

Thomas Petazzoni (21):
pkg-kconfig: declare phony targets as such
fs: declare phony targets as such
Makefile: targets are now declared phony by the appropriate
infrastructures
Makefile: rename TARGETS to PACKAGES
fs: add rootfs dependencies to PACKAGES
Makefile: use <pkg>-all-legal-info to implement the legal-info target
Makefile: simplify show-targets

Makefile: use the package infra based external-deps