[Buildroot] [PATCHv2 00/21] Package based 'source', 'legal-info', 'source-check' and 'external-deps'

[Buildroot] [PATCHv2 00/21] Package based 'source', 'legal-info', 'source-check' and 'external-deps'

[Thomas Petazzoni]

Hello,

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

Changes since v1:
=================

 - Remove all the patches that have been applied to master, and rebase
   on top of the latest master, fixing the conflicts along the way.

 - Fix a bug in the implementation of $(1)-source-check: the code was
   using $(p) to reference the loop iterator, while it should have
   been using $$(p) instead.

 - Add missing phony targets in pkg-kconfig: $(1)-savedefconfig and
   $(1)-update-defconfig. Noticed by Yann E. Morin.

 - Mark rootfs-ext2-symlink as a phony target. Noticed by Yann
   E. Morin.

 - Add Reviewed-by from Yann on "Makefile: targets are now declared
   phony by the appropriate infrastructures" since the problem he
   pointed out in the review has been fixed in previous patches.

 - In patch "Makefile: rename TARGETS to PACKAGES", fix typo in commit
   log (should be *listed*), and also rename TARGETS to PACKAGES in
   package/matchbox/matchbox.mk as well as
   support/scripts/graph-depends, as mentionned by Yann E. Morin.

 - Added Acked-by from Yann on "fs: add rootfs dependencies to
   PACKAGES".

 - Extend the commit log of "Makefile: use <pkg>-all-legal-info to
   implement the legal-info target" to explain why the 'make
   legal-info' output before and after this patch may be different,
   and why it is actually more correct after the patch. Suggested by
   Yann.

 - Add Reviewed-by from Yann on patch "Makefile: simplify
   show-targets".

 - Added Tested-by and Reviewed-by from Yann on patch "Makefile: use
   the package infra based external-deps".

Original introduction of the series
===================================

(Note: this is the original introduction of the series, matching the
v1. Since some patches have been merged, the patch numbers below do
not match this current version of the series.)

The initial goal of this series was to change the way we recursively
go through host dependencies for the implementation of the 'source',
'external-deps', 'legal-info' and 'source-check' targets.

The implementation of such targets currently do not rely on the
package infrastructure, and the main Makefile tries to iterate through
the dependencies of all packages using the TARGETS, TARGETS_HOST_DEPS
and HOST_DEPS variable, but it does so with only a two-level
recursion, which does not guarantee that all dependencies will have
been taken into account. It is also not at all in the spirit of the
rest of the package infrastructure.

So our final goal is simply that all those targets are implemented
using per-package make targets that recursively go through their own
dependencies.

Along the way, a number of other related changes or cleanups have been
made, and we try to describe below the overall logic of the patch
series:

 - Currently, some packages do directly use the 'DOWNLOAD' macro to
   download files. This has the unfortunate effect that the package
   infrastructure is not aware that those files are being downloaded,
   which is bad.

   To fix this, patches 1 to 7 fixes the two problematic packages: the
   'linux' and 'perl' packages. In the case of the 'perl' package, an
   addition was needed to the package infrastructure: supporting full
   URLs in <pkg>_EXTRA_DOWNLOADS. Some related cleanups and
   improvements are done as part of these changes.

   Compared to the v1 of those patches: I've fixed the typos found by
   Baruch and Romain, added the Reviewed-by tags that were given,
   fixed the manual line-break issue reported by Arnout, and adjusted
   the order of the variables in perl.mk as suggested by Arnout.

 - As part of the review of the first version of this series (which
   only included patches 1 to 7), a comment was made that packages
   like Linux were only applying patches named linux-*.patch, while we
   have moved for all other packages to a convention that consists in
   applying *.patch. Therefore, patches 8 and 9 make the Linux,
   U-Boot, Barebox, AT91Bootstrap and AT91Bootstrap3 follow this
   convention, and fix the user manual accordingly.

 - Patches 10 and 11 start extending the package infrastructure by
   adding the <pkg>-external-deps, <pkg>-all-external-deps,
   <pkg>-all-source and <pkg>-all-legal-info targets.

 - Patch 12 is doing some silly cleanup of the main Makefile.

 - Patches 13 to 17 clarify and fix the way we declare the PHONY make
   targets, by making the package infrastructure responsible for doing
   that for package targets.

 - Patches 18 and 19 make some not very important cleanups (variable
   renaming, etc.)

 - Patch 20 switches the 'legal-info' target to fully use the package
   infrastructure, thanks to the <pkg>-all-legal-info target added
   previously.

 - Patch 21 simplifies show-targets a bit.

 - Patch 22, like patch 20, switches the global 'external-deps' target
   to fully use the package infrastructure, thanks to the
   <pkg>-all-external-deps target added previously.

 - Patch 23 cleans up the download infra by removing the support for
   the SHOW_EXTERNAL_DEPS DL_MODE, unneeded now that the package infra
   is in charge of the 'external-deps' feature.

 - Patches 24, 25, 26, 27, 28, 29 and 30 take care of the global
   'source-check' target, also moved to use the package infrastructure
   through newly introduced <pkg>-source-check and
   <pkg>-all-source-check targets.

 - Patch 31 does some indentation cleanup.

 - Patch 32 and 33 make some improvements to the package
   infrastructure in terms of downloading source, patches and extra
   downloads.

 - Patch 34 implements the global 'source' target using the package
   infrastructrure, thanks to <pkg>-source and <pkg>-all-source.

 - Patch 35 removes the now unneeded variables from the main Makefile,
   which was our original goal.

Thomas

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
  pkg-download: remove support for the SHOW_EXTERNAL_DEPS DL_MODE
  Makefile: move source-check outside of noconfig_targets
  pkg-download: extend DOWNLOAD_INNER, add a SOURCE_CHECK macro
  pkg-generic: implement source-check targets
  Makefile: implement a package based source-check target
  pkg-generic: remove the .stamp_rsync_sourced fake stamp file
  pkg-generic: don't use DL_MODE in .stamp_downloaded
  pkg-download: get rid of DL_MODE
  pkg-download: fix indentation for SOURCE_CHECK_* macros
  pkg-generic: propagate <pkg>_EXTRA_DOWNLOADS from target to host
    package
  pkg-generic: introduce a <pkg>_ALL_DOWNLOADS variable and factorize
    code
  Makefile: implement the 'source' target using the package
    infrastructure
  Makefile: remove unneeded variables

 Makefile                      | 53 +++++++--------------------
 fs/common.mk                  |  3 ++
 fs/ext2/ext2.mk               |  2 ++
 fs/initramfs/initramfs.mk     |  2 ++
 fs/iso9660/iso9660.mk         |  2 ++
 package/matchbox/matchbox.mk  |  2 +-
 package/pkg-download.mk       | 83 +++++++++++--------------------------------
 package/pkg-generic.mk        | 62 ++++++++++++++------------------
 package/pkg-kconfig.mk        |  6 ++++
 support/scripts/graph-depends |  2 +-
 system/system.mk              |  2 +-
 11 files changed, 79 insertions(+), 140 deletions(-)

-- 
2.1.0

[Buildroot] [PATCHv2 01/21] pkg-kconfig: declare phony targets as such

[Thomas Petazzoni]

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 <thomas.petazzoni@free-electrons.com>
---
 package/pkg-kconfig.mk | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
index fd9f19d..8361064 100644
--- a/package/pkg-kconfig.mk
+++ b/package/pkg-kconfig.mk
@@ -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
 
 ################################################################################
-- 
2.1.0

[Buildroot] [PATCHv2 01/21] pkg-kconfig: declare phony targets as such

[Yann E. MORIN]

Thomas, All,

On 2015-04-12 18:37 +0200, Thomas Petazzoni spake thusly:
> 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 <thomas.petazzoni@free-electrons.com>

Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.

> ---
>  package/pkg-kconfig.mk | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
> index fd9f19d..8361064 100644
> --- a/package/pkg-kconfig.mk
> +++ b/package/pkg-kconfig.mk
> @@ -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
>  
>  ################################################################################
> -- 
> 2.1.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCHv2 02/21] fs: declare phony targets as such

[Thomas Petazzoni]

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

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 fs/common.mk              | 2 ++
 fs/ext2/ext2.mk           | 2 ++
 fs/initramfs/initramfs.mk | 2 ++
 fs/iso9660/iso9660.mk     | 2 ++
 4 files changed, 8 insertions(+)

diff --git a/fs/common.mk b/fs/common.mk
index 5d07f00..cac127f 100644
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -102,6 +102,8 @@ rootfs-$(1)-show-depends:
 
 rootfs-$(1): $$(BINARIES_DIR)/rootfs.$(1) $$(ROOTFS_$(2)_POST_TARGETS)
 
+.PHONY: rootfs-$(1) rootfs-$(1)-show-depends
+
 ifeq ($$(BR2_TARGET_ROOTFS_$(2)),y)
 TARGETS_ROOTFS += rootfs-$(1)
 endif
diff --git a/fs/ext2/ext2.mk b/fs/ext2/ext2.mk
index 1cac72e..cab66a5 100644
--- a/fs/ext2/ext2.mk
+++ b/fs/ext2/ext2.mk
@@ -35,6 +35,8 @@ endef
 rootfs-ext2-symlink:
 	ln -sf rootfs.ext2$(ROOTFS_EXT2_COMPRESS_EXT) $(BINARIES_DIR)/rootfs.ext$(BR2_TARGET_ROOTFS_EXT2_GEN)$(ROOTFS_EXT2_COMPRESS_EXT)
 
+.PHONY: rootfs-ext2-symlink
+
 ifneq ($(BR2_TARGET_ROOTFS_EXT2_GEN),2)
 ROOTFS_EXT2_POST_TARGETS += rootfs-ext2-symlink
 endif
diff --git a/fs/initramfs/initramfs.mk b/fs/initramfs/initramfs.mk
index 308924d..db50812 100644
--- a/fs/initramfs/initramfs.mk
+++ b/fs/initramfs/initramfs.mk
@@ -17,6 +17,8 @@ rootfs-initramfs: $(ROOTFS_INITRAMFS_DEPENDENCIES) $(ROOTFS_INITRAMFS_POST_TARGE
 rootfs-initramfs-show-depends:
 	@echo $(ROOTFS_INITRAMFS_DEPENDENCIES)
 
+.PHONY: rootfs-initramfs rootfs-initramfs-show-depends
+
 ifeq ($(BR2_TARGET_ROOTFS_INITRAMFS),y)
 TARGETS_ROOTFS += rootfs-initramfs
 endif
diff --git a/fs/iso9660/iso9660.mk b/fs/iso9660/iso9660.mk
index 5b44ba4..4ccfce9 100644
--- a/fs/iso9660/iso9660.mk
+++ b/fs/iso9660/iso9660.mk
@@ -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
-- 
2.1.0

[Buildroot] [PATCHv2 02/21] fs: declare phony targets as such

[Yann E. MORIN]

Thomas, All,

On 2015-04-12 18:37 +0200, Thomas Petazzoni spake thusly:
> This commit improves the filesystem handling code to declare its
> various targets as PHONY when appropriate.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Just a commnet, below...

> diff --git a/fs/initramfs/initramfs.mk b/fs/initramfs/initramfs.mk
> index 308924d..db50812 100644
> --- a/fs/initramfs/initramfs.mk
> +++ b/fs/initramfs/initramfs.mk
> @@ -17,6 +17,8 @@ rootfs-initramfs: $(ROOTFS_INITRAMFS_DEPENDENCIES) $(ROOTFS_INITRAMFS_POST_TARGE
>  rootfs-initramfs-show-depends:
>  	@echo $(ROOTFS_INITRAMFS_DEPENDENCIES)
>  
> +.PHONY: rootfs-initramfs rootfs-initramfs-show-depends

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.

Regards,
Yann E. MORIN.

>  ifeq ($(BR2_TARGET_ROOTFS_INITRAMFS),y)
>  TARGETS_ROOTFS += rootfs-initramfs
>  endif
> diff --git a/fs/iso9660/iso9660.mk b/fs/iso9660/iso9660.mk
> index 5b44ba4..4ccfce9 100644
> --- a/fs/iso9660/iso9660.mk
> +++ b/fs/iso9660/iso9660.mk
> @@ -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
> -- 
> 2.1.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCHv2 02/21] fs: declare phony targets as such

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Mon, 13 Apr 2015 21:37:07 +0200, Yann E. MORIN wrote:

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

Correct, this could be a patch to linux/linux.mk making this target
a .PHONY target.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCHv2 03/21] Makefile: targets are now declared phony by the appropriate infrastructures

[Thomas Petazzoni]

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 <thomas.petazzoni@free-electrons.com>
Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 Makefile | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index f011918..fee2551 100644
--- a/Makefile
+++ b/Makefile
@@ -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.
@@ -426,9 +425,7 @@ world: target-post-image
 
 .PHONY: all world toolchain dirs clean distclean source outputmakefile \
 	legal-info legal-info-prepare legal-info-clean printvars help \
-	list-defconfigs target-finalize target-post-image \
-	$(TARGETS) $(TARGETS_ROOTFS) \
-	$(TARGETS_DIRCLEAN) $(TARGETS_SOURCE) $(TARGETS_LEGAL_INFO)
+	target-finalize target-post-image
 
 ################################################################################
 #
-- 
2.1.0

[Buildroot] [PATCHv2 03/21] Makefile: targets are now declared phony by the appropriate infrastructures

[Yann E. MORIN]

Thomas, All,

On 2015-04-12 18:37 +0200, Thomas Petazzoni spake thusly:
> 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 <thomas.petazzoni@free-electrons.com>
> Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

I'll have to withdraw that tag of mine, see below...

> ---
>  Makefile | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index f011918..fee2551 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -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.
> @@ -426,9 +425,7 @@ world: target-post-image
>  
>  .PHONY: all world toolchain dirs clean distclean source outputmakefile \
>  	legal-info legal-info-prepare legal-info-clean printvars help \
> -	list-defconfigs target-finalize target-post-image \

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

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

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCHv2 03/21] Makefile: targets are now declared phony by the appropriate infrastructures

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Mon, 13 Apr 2015 21:44:33 +0200, Yann E. MORIN wrote:

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

Wow, great, thanks for spotting! Obviously an incorrect rebase conflict
resolution on my side. I've fixed that up before applying the patch.
Thanks again for the thorough review!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCHv2 04/21] Makefile: rename TARGETS to PACKAGES

[Thomas Petazzoni]

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 <thomas.petazzoni@free-electrons.com>
---
 Makefile                      | 22 +++++++++++-----------
 package/matchbox/matchbox.mk  |  2 +-
 package/pkg-generic.mk        |  2 +-
 support/scripts/graph-depends |  2 +-
 system/system.mk              |  2 +-
 5 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/Makefile b/Makefile
index fee2551..f4deec3 100644
--- a/Makefile
+++ b/Makefile
@@ -297,7 +297,7 @@ unexport MACHINE
 
 GNU_HOST_NAME := $(shell support/gnuconfig/config.guess)
 
-TARGETS :=
+PACKAGES :=
 
 # silent mode requested?
 QUIET := $(if $(findstring s,$(filter-out --%,$(MAKEFLAGS))),-q)
@@ -389,16 +389,16 @@ include fs/common.mk
 
 include $(BR2_EXTERNAL)/external.mk
 
-TARGETS_SOURCE := $(patsubst %,%-source,$(TARGETS))
+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 TARGETS.
+# 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,$(TARGETS) $(TARGETS_ROOTFS))),\
+			$(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
@@ -410,8 +410,8 @@ HOST_DEPS = $(sort $(foreach dep,\
 		$($(dep))))
 HOST_SOURCE += $(addsuffix -source,$(sort $(TARGETS_HOST_DEPS) $(HOST_DEPS)))
 
-TARGETS_LEGAL_INFO := $(patsubst %,%-legal-info,\
-		$(TARGETS) $(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)
@@ -505,7 +505,7 @@ endif
 ifeq ($(BR2_TOOLCHAIN_USES_GLIBC),y)
 GLIBC_GENERATE_LOCALES = $(call qstrip,$(BR2_GENERATE_LOCALE))
 ifneq ($(GLIBC_GENERATE_LOCALES),)
-TARGETS += host-localedef
+PACKAGES += host-localedef
 
 define GENERATE_GLIBC_LOCALES
 	$(Q)mkdir -p $(TARGET_DIR)/usr/lib/locale/
@@ -549,7 +549,7 @@ endif
 
 $(TARGETS_ROOTFS): target-finalize
 
-target-finalize: $(TARGETS)
+target-finalize: $(PACKAGES)
 	@$(call MESSAGE,"Finalizing target directory")
 	$(foreach hook,$(TARGET_FINALIZE_HOOKS),$($(hook))$(sep))
 	rm -rf $(TARGET_DIR)/usr/include $(TARGET_DIR)/usr/share/aclocal \
@@ -613,7 +613,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: $(TARGETS_SOURCE) $(HOST_SOURCE)
+source: $(PACKAGES_SOURCE) $(HOST_SOURCE)
 
 external-deps:
 	@$(MAKE1) -Bs DL_MODE=SHOW_EXTERNAL_DEPS $(EXTRAMAKEARGS) source | sort -u
@@ -631,7 +631,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 $(TARGETS_LEGAL_INFO) \
+legal-info: dirs legal-info-clean legal-info-prepare $(PACKAGES_LEGAL_INFO) \
 		$(REDIST_SOURCES_DIR_TARGET) $(REDIST_SOURCES_DIR_HOST)
 	@cat support/legal-info/README.header >>$(LEGAL_REPORT)
 	@if [ -r $(LEGAL_WARNINGS) ]; then \
@@ -642,7 +642,7 @@ legal-info: dirs legal-info-clean legal-info-prepare $(TARGETS_LEGAL_INFO) \
 	@rm -f $(LEGAL_WARNINGS)
 
 show-targets:
-	@echo $(HOST_DEPS) $(TARGETS_HOST_DEPS) $(TARGETS) $(TARGETS_ROOTFS)
+	@echo $(HOST_DEPS) $(TARGETS_HOST_DEPS) $(PACKAGES) $(TARGETS_ROOTFS)
 
 graph-build: $(O)/build/build-time.log
 	@install -d $(GRAPHS_DIR)
diff --git a/package/matchbox/matchbox.mk b/package/matchbox/matchbox.mk
index aec57c0..fe1a7db 100644
--- a/package/matchbox/matchbox.mk
+++ b/package/matchbox/matchbox.mk
@@ -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
 endif
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 1d56f81..d1a1811 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -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)
diff --git a/support/scripts/graph-depends b/support/scripts/graph-depends
index 08ecc5a..c26e1e0 100755
--- a/support/scripts/graph-depends
+++ b/support/scripts/graph-depends
@@ -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")
diff --git a/system/system.mk b/system/system.mk
index 4a1eb4a..c95e436 100644
--- a/system/system.mk
+++ b/system/system.mk
@@ -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
-- 
2.1.0

[Buildroot] [PATCHv2 04/21] Makefile: rename TARGETS to PACKAGES

[Yann E. MORIN]

Thomas, All,

On 2015-04-12 18:37 +0200, Thomas Petazzoni spake thusly:
> 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 <thomas.petazzoni@free-electrons.com>

Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Small comment however, see below...

> diff --git a/package/matchbox/matchbox.mk b/package/matchbox/matchbox.mk
> index aec57c0..fe1a7db 100644
> --- a/package/matchbox/matchbox.mk
> +++ b/package/matchbox/matchbox.mk
> @@ -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).

Regards,
Yann E. MORIN.

>  endif
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 1d56f81..d1a1811 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -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)
> diff --git a/support/scripts/graph-depends b/support/scripts/graph-depends
> index 08ecc5a..c26e1e0 100755
> --- a/support/scripts/graph-depends
> +++ b/support/scripts/graph-depends
> @@ -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")
> diff --git a/system/system.mk b/system/system.mk
> index 4a1eb4a..c95e436 100644
> --- a/system/system.mk
> +++ b/system/system.mk
> @@ -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
> -- 
> 2.1.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCHv2 04/21] Makefile: rename TARGETS to PACKAGES

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Mon, 13 Apr 2015 22:01:15 +0200, Yann E. MORIN wrote:

> > diff --git a/package/matchbox/matchbox.mk b/package/matchbox/matchbox.mk
> > index aec57c0..fe1a7db 100644
> > --- a/package/matchbox/matchbox.mk
> > +++ b/package/matchbox/matchbox.mk
> > @@ -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).

Fully agreed, this custom matchbox stuff is weird, and should be
changed to better match what we normally do with other packages.
Looking forward to your patches on this, once you reach this point in
your ever-growing TODO-list!

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCHv2 05/21] fs: add rootfs dependencies to PACKAGES

[Thomas Petazzoni]

The logic for creating most of the filesystem images requires a
certain number of host packages to be built. However, those packages
are not currently listed in the global PACKAGES variables, and they
are not dependencies of any other package listed in the PACKAGES
variable.

While it does not have any practical implications, it makes sense to
have those packages listed in the global PACKAGES variable, which this
commit implements.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 fs/common.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/common.mk b/fs/common.mk
index cac127f..41ee86d 100644
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -106,6 +106,7 @@ rootfs-$(1): $$(BINARIES_DIR)/rootfs.$(1) $$(ROOTFS_$(2)_POST_TARGETS)
 
 ifeq ($$(BR2_TARGET_ROOTFS_$(2)),y)
 TARGETS_ROOTFS += rootfs-$(1)
+PACKAGES += $$(ROOTFS_$(2)_DEPENDENCIES)
 endif
 endef
 
-- 
2.1.0

[Buildroot] [PATCHv2 06/21] Makefile: use <pkg>-all-legal-info to implement the legal-info target

[Thomas Petazzoni]

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 <thomas.petazzoni@free-electrons.com>
---
 Makefile | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index f4deec3..a22d99b 100644
--- a/Makefile
+++ b/Makefile
@@ -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 \
-- 
2.1.0

[Buildroot] [PATCHv2 06/21] Makefile: use <pkg>-all-legal-info to implement the legal-info target

[Yann E. MORIN]

Thomas, All,

On 2015-04-12 18:37 +0200, Thomas Petazzoni spake thusly:
> 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 <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>

(which you forgot to carry from iteration #1, or maybe on-purpose
because you changed the commit log. However, I've re-tested it.)

Regards,
Yann E. MORIN.

> ---
>  Makefile | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index f4deec3..a22d99b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -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 \
> -- 
> 2.1.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCHv2 07/21] Makefile: simplify show-targets

[Thomas Petazzoni]

show-targets is only used currently by the graph-depends script, which
already recurses into the dependencies of the selected packages to
build the dependency graph. Therefore, dumping the contents of
$(PACKAGES) and $(ROOTFS_TARGETS) is sufficient: $(HOST_DEPS) and
$(TARGET_HOST_DEPS) will contain packages that are dependencies of
packages already listed in $(PACKAGES), which graph-depends will
discover by itself.

This allows to remove one more usage of $(HOST_DEPS) and
$(TARGET_HOST_DEPS), which is one more step towards their removal.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index a22d99b..e91c5e6 100644
--- a/Makefile
+++ b/Makefile
@@ -639,7 +639,7 @@ legal-info: dirs legal-info-clean legal-info-prepare $(foreach p,$(PACKAGES),$(p
 	@rm -f $(LEGAL_WARNINGS)
 
 show-targets:
-	@echo $(HOST_DEPS) $(TARGETS_HOST_DEPS) $(PACKAGES) $(TARGETS_ROOTFS)
+	@echo $(PACKAGES) $(TARGETS_ROOTFS)
 
 graph-build: $(O)/build/build-time.log
 	@install -d $(GRAPHS_DIR)
-- 
2.1.0

[Buildroot] [PATCHv2 08/21] Makefile: use the package infra based external-deps

[Thomas Petazzoni]

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
 
 legal-info-clean:
 	@rm -fr $(LEGAL_INFO_DIR)
-- 
2.1.0

[Buildroot] [PATCHv2 08/21] Makefile: use the package infra based external-deps

[Arnout Vandecappelle]

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


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

[Buildroot] [PATCHv2 08/21] Makefile: use the package infra based external-deps

[Thomas Petazzoni]

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCHv2 08/21] Makefile: use the package infra based external-deps

[Arnout Vandecappelle]

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

[Buildroot] [PATCHv2 08/21] Makefile: use the package infra based external-deps

[Thomas Petazzoni]

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCHv2 08/21] Makefile: use the package infra based external-deps

[Arnout Vandecappelle]

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
> 


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

[Buildroot] [PATCHv2 09/21] pkg-download: remove support for the SHOW_EXTERNAL_DEPS DL_MODE

[Thomas Petazzoni]

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 <thomas.petazzoni@free-electrons.com>
---
 package/pkg-download.mk | 45 ++-------------------------------------------
 package/pkg-generic.mk  |  5 +----
 2 files changed, 3 insertions(+), 47 deletions(-)

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index e274712..be3babb 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -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
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index d1a1811..11edb34 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -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
-- 
2.1.0

[Buildroot] [PATCHv2 09/21] pkg-download: remove support for the SHOW_EXTERNAL_DEPS DL_MODE

[Yann E. MORIN]

Thomas, All,

On 2015-04-12 18:37 +0200, Thomas Petazzoni spake thusly:
> 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 <thomas.petazzoni@free-electrons.com>

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

Regards,
Yann E. MORIN.

> ---
>  package/pkg-download.mk | 45 ++-------------------------------------------
>  package/pkg-generic.mk  |  5 +----
>  2 files changed, 3 insertions(+), 47 deletions(-)
> 
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index e274712..be3babb 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -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
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index d1a1811..11edb34 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -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
> -- 
> 2.1.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCHv2 09/21] pkg-download: remove support for the SHOW_EXTERNAL_DEPS DL_MODE

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Mon, 13 Apr 2015 22:31:15 +0200, Yann E. MORIN wrote:

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

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

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCHv2 09/21] pkg-download: remove support for the SHOW_EXTERNAL_DEPS DL_MODE

[Yann E. MORIN]

Thomas, All,

On 2015-04-13 22:33 +0200, Thomas Petazzoni spake thusly:
> On Mon, 13 Apr 2015 22:31:15 +0200, Yann E. MORIN wrote:
> > 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... :-/
> 
> 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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCHv2 09/21] pkg-download: remove support for the SHOW_EXTERNAL_DEPS DL_MODE

[Arnout Vandecappelle]

On 13/04/15 22:40, Yann E. MORIN wrote:
> Thomas, All,
> 
> On 2015-04-13 22:33 +0200, Thomas Petazzoni spake thusly:
>> On Mon, 13 Apr 2015 22:31:15 +0200, Yann E. MORIN wrote:
>>> 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... :-/
>>
>> 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...

 Try 'touch O/.config; make external-deps'.

 You probably did something like:

make O=O foo_defconfig
make O=O external-deps

 After foo_defconfig, .config is more recent than auto.conf so silentoldconfig
is run, and that one depends on the dependency-less outputmakefile target.
Really silly inheritance from the kernel's kconfig infra.


 Regards,
 Arnout

> 
> Regards,
> Yann E. MORIN.
> 


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

[Buildroot] [PATCHv2 09/21] pkg-download: remove support for the SHOW_EXTERNAL_DEPS DL_MODE

[Yann E. MORIN]

Thomas, All,

On 2015-04-12 18:37 +0200, Thomas Petazzoni spake thusly:
> 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 <thomas.petazzoni@free-electrons.com>

Of course, since my previous comments are not related to this patch,
this one gets my:

Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.

> ---
>  package/pkg-download.mk | 45 ++-------------------------------------------
>  package/pkg-generic.mk  |  5 +----
>  2 files changed, 3 insertions(+), 47 deletions(-)
> 
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index e274712..be3babb 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -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
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index d1a1811..11edb34 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -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
> -- 
> 2.1.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCHv2 09/21] pkg-download: remove support for the SHOW_EXTERNAL_DEPS DL_MODE

[Arnout Vandecappelle]

On 12/04/15 18:37, Thomas Petazzoni wrote:
> 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 <thomas.petazzoni@free-electrons.com>

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 I also wanted to add my Tested-by, but it currently fails due to the
rootfs-ubifs dependency.

 Regards,
 Arnout

[snip]


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

[Buildroot] [PATCHv2 10/21] Makefile: move source-check outside of noconfig_targets

[Thomas Petazzoni]

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 <thomas.petazzoni@free-electrons.com>
---
 Makefile | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 40ee2e2..6937dd3 100644
--- a/Makefile
+++ b/Makefile
@@ -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
 
 ################################################################################
-- 
2.1.0

[Buildroot] [PATCHv2 10/21] Makefile: move source-check outside of noconfig_targets

[Yann E. MORIN]

Thomas, All,

On 2015-04-12 18:37 +0200, Thomas Petazzoni spake thusly:
> 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 <thomas.petazzoni@free-electrons.com>

Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Tested-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

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.

Regards,
Yann E. MORIN.

> ---
>  Makefile | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 40ee2e2..6937dd3 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -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
>  
>  ################################################################################
> -- 
> 2.1.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCHv2 10/21] Makefile: move source-check outside of noconfig_targets

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Mon, 13 Apr 2015 22:49:16 +0200, Yann E. MORIN wrote:

> Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Tested-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> 
> 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.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCHv2 10/21] Makefile: move source-check outside of noconfig_targets

[Yann E. MORIN]

Thomas, All,

On 2015-04-13 23:06 +0200, Thomas Petazzoni spake thusly:
> On Mon, 13 Apr 2015 22:49:16 +0200, Yann E. MORIN wrote:
> 
> > Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > Tested-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > 
> > 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

That would cover the use-cases above.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCHv2 10/21] Makefile: move source-check outside of noconfig_targets

[Ryan Barnett]

Yann,

On Mon, Apr 13, 2015 at 4:58 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:

[...]

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

I agree with this statement and would also like to add that they are
probably also sitting being a firewall so having the primary site be
the only source of downloads helps avoid errors and confusion.

> 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

The only reason I could see someone in large enterprise needing to be
able to reach the mirror is if there are behind a firewall where
getting an opening to the mirror (sources.b.o) is easier for them to
get access to than others. However, I as I typed the above sentence, I
thought it would then make more sense for them to just mirror
sources.b.o themselves outside of buildroot for a them to create
primary site internally.

Anyway that is my two-cents on your comments and I don't really have a
preference whatever direction this goes.

Thanks,
-Ryan

-- 
Ryan Barnett / Sr Software Engineer
Airborne Information Systems / Security Systems and Software
MS 131-100, C Ave NE, Cedar Rapids, IA, 52498, USA
ryan.barnett at rockwellcollins.com
www.rockwellcollins.com

[Buildroot] [PATCHv2 10/21] Makefile: move source-check outside of noconfig_targets

[Arnout Vandecappelle]

On 12/04/15 18:37, Thomas Petazzoni wrote:
> 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.

 That said, the change itself looks OK, so

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

> 
> So this commit moves source-check to no longer be a noconfig_target.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  Makefile | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 40ee2e2..6937dd3 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -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
>  
>  ################################################################################
> 


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

[Buildroot] [PATCHv2 10/21] Makefile: move source-check outside of noconfig_targets

[Yann E. MORIN]

Arnout, All,

On 2015-04-14 21:42 +0200, Arnout Vandecappelle spake thusly:
> On 12/04/15 18:37, Thomas Petazzoni wrote:
> > 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.

Maybe I missed something, but don't we need .config to get the list of
enabled packages?

Hmm.. Let me think.. Ah, I see what you mean. The first-level 'make'
does not need it, because its only rule for now is to call itself with
the 'source' goal and a variable set.

I get it now.

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

Yes, this change all by itself is meaningless, but we need it later,
when source-check is eventually part of the package infra. It seemed
obvious during my review, given the goal of the series.

For clarification, that could be added to the commit log, indeed.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCHv2 10/21] Makefile: move source-check outside of noconfig_targets

[Thomas Petazzoni]

Dear Arnout Vandecappelle,

On Tue, 14 Apr 2015 21:42:19 +0200, Arnout Vandecappelle wrote:

>  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) <arnout@mind.be>

Aaah, yes, good catch. make source-check recursively calls into make by
calling make source. So the make invocation executing 'source-check'
does not need the config file, so a noconfig target is OK. However, the
sub-make invocation that executes the 'source' target does need the
config file.

I think the most correct option here is to squash this change with the
change actually modifying the source-check implementation to use the
package infrastructure, because as you say it's really why we need
source-check to be outside of the noconfig target thing.

Which makes me think that 'external-deps' could also be a noconfig
target, while _external-deps would need to remain a normal target. But
I'm not sure it's really worth the effort and additional "weirdness" of
the code.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCHv2 11/21] pkg-download: extend DOWNLOAD_INNER, add a SOURCE_CHECK macro

[Thomas Petazzoni]

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 <thomas.petazzoni@free-electrons.com>
---
 package/pkg-download.mk | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index be3babb..829b54d 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -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
-- 
2.1.0

[Buildroot] [PATCHv2 11/21] pkg-download: extend DOWNLOAD_INNER, add a SOURCE_CHECK macro

[Yann E. MORIN]

Thomas, All,

On 2015-04-12 18:37 +0200, Thomas Petazzoni spake thusly:
> 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 <thomas.petazzoni@free-electrons.com>

Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Tested-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
[tested by doing a "make source" on a randpackageconfig]

Regards,
Yann E. MORIN.

> ---
>  package/pkg-download.mk | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index be3babb..829b54d 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -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
> -- 
> 2.1.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCHv2 11/21] pkg-download: extend DOWNLOAD_INNER, add a SOURCE_CHECK macro

[Arnout Vandecappelle]

On 13/04/15 23:00, Yann E. MORIN wrote:
> Thomas, All,
> 
> On 2015-04-12 18:37 +0200, Thomas Petazzoni spake thusly:
>> 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) <arnout@mind.be>

>>
>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> 
> Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Tested-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> [tested by doing a "make source" on a randpackageconfig]

 You should have tested make source-check.

 I tested it but didn't finish because of a failure in github (it seems that the
AWS servers don't accept the HEAD request, and after a couple of retries I ran
into github's bandwidth limitation, so eventually I gave up).


 Regards,
 Arnout

> 
> Regards,
> Yann E. MORIN.
> 
>> ---
>>  package/pkg-download.mk | 28 ++++++++++++++++------------
>>  1 file changed, 16 insertions(+), 12 deletions(-)
>>
>> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
>> index be3babb..829b54d 100644
>> --- a/package/pkg-download.mk
>> +++ b/package/pkg-download.mk
>> @@ -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
>> -- 
>> 2.1.0
>>
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
> 


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

[Buildroot] [PATCHv2 11/21] pkg-download: extend DOWNLOAD_INNER, add a SOURCE_CHECK macro

[Yann E. MORIN]

Arnout, All,

On 2015-04-14 22:06 +0200, Arnout Vandecappelle spake thusly:
> On 13/04/15 23:00, Yann E. MORIN wrote:
> > Thomas, All,
> > 
> > On 2015-04-12 18:37 +0200, Thomas Petazzoni spake thusly:
> >> 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) <arnout@mind.be>
> 
> >>
> >> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > 
> > Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > Tested-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > [tested by doing a "make source" on a randpackageconfig]
> 
>  You should have tested make source-check.

Well, I did run source-check and source; just forgot to state it.

However, I did not notice a failure, because buildroot will fallback to
our mirror when it does not find something. Because source-check takes
quite some time to run, I just ran it and went off doing smething else;
when I came back, there was no error of a failing package, because
every missing pieces were retrieved from the mirror. And in the default
config, there is a mirror (our).

And then, it means we can't reliably run sourcce-check to check that
upstream is still available, just becasue some upstream do not allow us
to do so (HEAD not being supported).

An alternative would be to use a GET instead of a HEAD, and just ask for
zero byte (or probably just a few bytes in case some servers would
refuse to serve zero byte). But there's no way to do so with wget. curl
is a little bit more flexible and it is possible ask for a range, but
not all server will abide to that range.

So, I can see a few blockign points baring source-check from being
completely useful:

  - no or limited VCS support: git does not always support remote-listing
    a sha1 so we only check the remote exists, our bzr, cvs and hg checks
    are no better, svn is OK though;

  - scp only works if the user has a shell (which is not mandatory, scp
    can work even if the user can't get a shell);

  - some HTTP upstreams do not accept a HEAD request;

  - as far as I could see all out FTP-hosted packages are on behaving
    servers.

Bizarelly enough, what works best are the legacy (or oldish) methods:
svn and FTP. Yeah for grumpy old guys... :-(

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?

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCHv2 11/21] pkg-download: extend DOWNLOAD_INNER, add a SOURCE_CHECK macro

[Arnout Vandecappelle]

On 15/04/15 00:25, Yann E. MORIN wrote:
> 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?


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

 We could rename source-check to deprecated-source-check and see who complains.

 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

[Buildroot] [PATCHv2 11/21] pkg-download: extend DOWNLOAD_INNER, add a SOURCE_CHECK macro

[Nicolas Cavallari]

On 15/04/2015 00:41, Arnout Vandecappelle wrote:
> On 15/04/15 00:25, Yann E. MORIN wrote:
>> 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?
> 
> 
>  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.

[Buildroot] [PATCHv2 11/21] pkg-download: extend DOWNLOAD_INNER, add a SOURCE_CHECK macro

[Arnout Vandecappelle]

On 17/04/15 12:38, Nicolas Cavallari wrote:
> On 15/04/2015 00:41, Arnout Vandecappelle wrote:
>> On 15/04/15 00:25, Yann E. MORIN wrote:

[snip]

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

 Care to write up some documentation for this use case?

 source-check remains useless, right?

 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

[Buildroot] [PATCHv2 12/21] pkg-generic: implement source-check targets

[Thomas Petazzoni]

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 <thomas.petazzoni@free-electrons.com>
---
 package/pkg-generic.mk | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 11edb34..b45b86e 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -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
-- 
2.1.0

[Buildroot] [PATCHv2 12/21] pkg-generic: implement source-check targets

[Yann E. MORIN]

Thomas, All,

On 2015-04-12 18:37 +0200, Thomas Petazzoni spake thusly:
> 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 <thomas.petazzoni@free-electrons.com>

Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Tested-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
[manually tested with PKG-source-check and PKG-all-source-check, with
 PKG being either ejabberd or xz, both enabled.]

Regards,
Yann E. MORIN.

> ---
>  package/pkg-generic.mk | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 11edb34..b45b86e 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -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
> -- 
> 2.1.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCHv2 12/21] pkg-generic: implement source-check targets

[Yann E. MORIN]

Thomas, All,

On 2015-04-13 23:05 +0200, Yann E. MORIN spake thusly:
> On 2015-04-12 18:37 +0200, Thomas Petazzoni spake thusly:
> > 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 <thomas.petazzoni@free-electrons.com>
> 
> Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Tested-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> [manually tested with PKG-source-check and PKG-all-source-check, with
>  PKG being either ejabberd or xz, both enabled.]

So, after another round of randpackageconfig a bit later, I found an
issue with this patch, see below...

> > ---
> >  package/pkg-generic.mk | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> > 
> > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> > index 11edb34..b45b86e 100644
> > --- a/package/pkg-generic.mk
> > +++ b/package/pkg-generic.mk
> > @@ -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))))
                                                            ^^
You're missing a $$(sep) before the last parenthesis, here  ||
so that last line would be:

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

That's pretty easy to test: enable hwdata, and run hwdata-source-check.
hwdata is nive in that it has a _PATCH to be downloaded. Any other
package with either or both of a _PATCH or _EXTRA_DOWNLOAD would have
the same issue.

Regards,
Yann E. MORIN.

> > +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
> > -- 
> > 2.1.0
> > 
> > _______________________________________________
> > buildroot mailing list
> > buildroot at busybox.net
> > http://lists.busybox.net/mailman/listinfo/buildroot
> 
> -- 
> .-----------------.--------------------.------------------.--------------------.
> |  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

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCHv2 12/21] pkg-generic: implement source-check targets

[Yann E. MORIN]

Thomas, All,

On 2015-04-13 23:25 +0200, Yann E. MORIN spake thusly:
> On 2015-04-13 23:05 +0200, Yann E. MORIN spake thusly:
> > On 2015-04-12 18:37 +0200, Thomas Petazzoni spake thusly:
[--SNIP--]
> > > +$(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))))
>                                                             ^^
> You're missing a $$(sep) before the last parenthesis, here  ||

And because my tabstop=4, I missed the right position. Regardless, the
following is correct:

> so that last line would be:
> 
>     $$(call SOURCE_CHECK,$$($(2)_SITE:/=)/$$(p)))$$(sep))

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCHv2 12/21] pkg-generic: implement source-check targets

[Arnout Vandecappelle]

On 12/04/15 18:37, Thomas Petazzoni wrote:
> 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 <thomas.petazzoni@free-electrons.com>

Tested-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
by breaking bzip2 package (change version to .69) and observing that
bzip-source-check and python-all-source-check fail.

 However...

> ---
>  package/pkg-generic.mk | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 11edb34..b45b86e 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -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

 As mentioned before, I believe _FINAL_PATCH_DEPENDENCIES is missing here, which
is currently not observable because only linux extensions use it and the linux
package will be reached through PACKAGES anyway.


 Regards,
 Arnout

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


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

[Buildroot] [PATCHv2 13/21] Makefile: implement a package based source-check target

[Thomas Petazzoni]

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 <thomas.petazzoni@free-electrons.com>
---
 Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 6937dd3..7d94ee8 100644
--- a/Makefile
+++ b/Makefile
@@ -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)
-- 
2.1.0

[Buildroot] [PATCHv2 13/21] Makefile: implement a package based source-check target

[Yann E. MORIN]

Thomas, All,

On 2015-04-12 18:37 +0200, Thomas Petazzoni spake thusly:
> 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 <thomas.petazzoni@free-electrons.com>

Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Tested-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
[tested against a randpackageconfig]

Regards,
Yann E. MORIN.

> ---
>  Makefile | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 6937dd3..7d94ee8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -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)
> -- 
> 2.1.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCHv2 13/21] Makefile: implement a package based source-check target

[Arnout Vandecappelle]

On 12/04/15 18:37, Thomas Petazzoni wrote:
> 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 <thomas.petazzoni@free-electrons.com>

 There is something still missing to complete the conversion: some packages
(at91bootstrap, at91bootstrap3, barebox, uboot, xloader, linux, sunxi-boards)
check that the configuration files specified in .config really exist, _unless_
make source is called (presumably this exception is to support the 'make
allpackageyesconfig; make source' way of refreshing the buildroot mirror).
Previously (actually, before you removed source-check from noconfig_targets),
this would also apply to source-check, but now it doesn't anymore.

 So I'd update all those filter expressions with source-check as well.

 But that doesn't affect this patch, so

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>


 Regards,
 Arnout

> ---
>  Makefile | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 6937dd3..7d94ee8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -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)
> 


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

[Buildroot] [PATCHv2 14/21] pkg-generic: remove the .stamp_rsync_sourced fake stamp file

[Thomas Petazzoni]

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 <thomas.petazzoni@free-electrons.com>
---
 package/pkg-generic.mk | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index b45b86e..d432a2e 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -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)
-- 
2.1.0

[Buildroot] [PATCHv2 14/21] pkg-generic: remove the .stamp_rsync_sourced fake stamp file

[Arnout Vandecappelle]

On 12/04/15 18:37, Thomas Petazzoni wrote:
> 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 <thomas.petazzoni@free-electrons.com>

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Tested-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

by running python-source-check, python-source and python-all-source-check with
PYTHON_OVERRIDE_SRCDIR set. python-all-source-check actually fails (which seems
to be a recurring theme in this series) because host-python-source{,-check} does
_not_ have OVERRIDE_SRCDIR set, but it inherits the PKG_VERSION=custom from the
target package. But since this already occurs on current master, I don't
consider this a failure of this patch.

 Regards,
 Arnout

> ---
>  package/pkg-generic.mk | 13 +------------
>  1 file changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index b45b86e..d432a2e 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -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)
> 


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

[Buildroot] [PATCHv2 15/21] pkg-generic: don't use DL_MODE in .stamp_downloaded

[Thomas Petazzoni]

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 <thomas.petazzoni@free-electrons.com>
---
 package/pkg-generic.mk | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index d432a2e..13a3843 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -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:
-- 
2.1.0

[Buildroot] [PATCHv2 15/21] pkg-generic: don't use DL_MODE in .stamp_downloaded

[Arnout Vandecappelle]

On 12/04/15 18:37, Thomas Petazzoni wrote:
> 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 <thomas.petazzoni@free-electrons.com>

Tested-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>


 Regards,
 Arnout

> ---
>  package/pkg-generic.mk | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index d432a2e..13a3843 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -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:
> 


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

[Buildroot] [PATCHv2 16/21] pkg-download: get rid of DL_MODE

[Thomas Petazzoni]

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 <thomas.petazzoni@free-electrons.com>
---
 package/pkg-download.mk | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 829b54d..8cd168a 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -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
-- 
2.1.0

[Buildroot] [PATCHv2 16/21] pkg-download: get rid of DL_MODE

[Arnout Vandecappelle]

On 12/04/15 18:38, Thomas Petazzoni wrote:
> 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 <thomas.petazzoni@free-electrons.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Tested-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

By doing 'make source'.

 Regards,
 Arnout

> ---
>  package/pkg-download.mk | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index 829b54d..8cd168a 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -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
> 


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

[Buildroot] [PATCHv2 17/21] pkg-download: fix indentation for SOURCE_CHECK_* macros

[Thomas Petazzoni]

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 <thomas.petazzoni@free-electrons.com>
---
 package/pkg-download.mk | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 8cd168a..f35dc04 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -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
 
 ################################################################################
-- 
2.1.0

[Buildroot] [PATCHv2 17/21] pkg-download: fix indentation for SOURCE_CHECK_* macros

[Arnout Vandecappelle]

On 12/04/15 18:38, Thomas Petazzoni wrote:
> 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 <thomas.petazzoni@free-electrons.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Tested-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
with git show -w

 Regards,
 Arnout

> ---
>  package/pkg-download.mk | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index 8cd168a..f35dc04 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -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
>  
>  ################################################################################
> 


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

[Buildroot] [PATCHv2 18/21] pkg-generic: propagate <pkg>_EXTRA_DOWNLOADS from target to host package

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

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 package/pkg-generic.mk | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 13a3843..d2ed605 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -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)
-- 
2.1.0

[Buildroot] [PATCHv2 18/21] pkg-generic: propagate <pkg>_EXTRA_DOWNLOADS from target to host package

[Arnout Vandecappelle]

On 12/04/15 18:38, Thomas Petazzoni wrote:
> 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)?

 Regards,
 Arnout
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  package/pkg-generic.mk | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 13a3843..d2ed605 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -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)
> 


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

[Buildroot] [PATCHv2 18/21] pkg-generic: propagate <pkg>_EXTRA_DOWNLOADS from target to host package

[Thomas Petazzoni]

Dear Arnout Vandecappelle,

On Tue, 14 Apr 2015 23:50:59 +0200, Arnout Vandecappelle wrote:
> On 12/04/15 18:38, Thomas Petazzoni wrote:
> > 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)?

Not that I remember of. The only reason was pure consistency: since
source, site, version and patch are all inherited from the target
variant to the host variant, is seemed to make sense as well for extra
downloads.

I can get rid of this aspect if you think it's not really appropriate,
since I indeed don't really have a use case for it, except a pure
consistency feeling.

Thanks for the review!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCHv2 19/21] pkg-generic: introduce a <pkg>_ALL_DOWNLOADS variable and factorize code

[Thomas Petazzoni]

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 <thomas.petazzoni@free-electrons.com>
---
 package/pkg-generic.mk | 26 ++++++++------------------
 1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index d2ed605..7dc1cb8 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -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
-- 
2.1.0

[Buildroot] [PATCHv2 19/21] pkg-generic: introduce a <pkg>_ALL_DOWNLOADS variable and factorize code

[Arnout Vandecappelle]

On 12/04/15 18:38, Thomas Petazzoni wrote:
> 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.

 Nice!

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Tested-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

with make perl

 Regards,
 Arnout

> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  package/pkg-generic.mk | 26 ++++++++------------------
>  1 file changed, 8 insertions(+), 18 deletions(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index d2ed605..7dc1cb8 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -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
> 


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

[Buildroot] [PATCHv2 20/21] Makefile: implement the 'source' target using the package infrastructure

[Thomas Petazzoni]

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

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 7d94ee8..06a9f05 100644
--- a/Makefile
+++ b/Makefile
@@ -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:
-- 
2.1.0

[Buildroot] [PATCHv2 20/21] Makefile: implement the 'source' target using the package infrastructure

[Arnout Vandecappelle]

On 12/04/15 18:38, Thomas Petazzoni wrote:
> Now that all the bits are in place, switch the global 'source' target
> to use the package infrastructure logic.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Tested-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

with 'make source' (actually together with the next patch).

 Regards,
 Arnout

> ---
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 7d94ee8..06a9f05 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -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:
> 


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

[Buildroot] [PATCHv2 21/21] Makefile: remove unneeded variables

[Thomas Petazzoni]

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 <thomas.petazzoni@free-electrons.com>
---
 Makefile | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/Makefile b/Makefile
index 06a9f05..a598063 100644
--- a/Makefile
+++ b/Makefile
@@ -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)
 
-- 
2.1.0

[Buildroot] [PATCHv2 21/21] Makefile: remove unneeded variables

[Arnout Vandecappelle]

On 12/04/15 18:38, Thomas Petazzoni wrote:
> 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 <thomas.petazzoni@free-electrons.com>

> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Tested-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

with 'make source' (actually together with the next patch).

 Regards,
 Arnout

> ---
>  Makefile | 21 ---------------------
>  1 file changed, 21 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 06a9f05..a598063 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -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)
>  
> 


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

[Buildroot] [PATCHv2 00/21] Package based 'source', 'legal-info', 'source-check' and 'external-deps'

[Thomas Petazzoni]

Hello,

On Sun, 12 Apr 2015 18:37:44 +0200, Thomas Petazzoni wrote:

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

In case someone would like to test this series, I've pushed it to the
following repo/branch:

  http://git.free-electrons.com/users/thomas-petazzoni/buildroot/log/?h=pkg-based-deps-v2

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCHv2 00/21] Package based 'source', 'legal-info', 'source-check' and 'external-deps'

[Yann E. MORIN]

Thomas, All,

On 2015-04-12 18:37 +0200, Thomas Petazzoni spake thusly:
> This is a new iteration of the "Package based 'source', 'legal-info',
> 'source-check' and 'external-deps'" series.

OK, I'm bailing out for tonight. More review tomorow (hopefully on a v3
of the series! ;-p ).

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] [PATCHv2 00/21] Package based 'source', 'legal-info', 'source-check' and 'external-deps'

[Thomas Petazzoni]

Hello,

On Sun, 12 Apr 2015 18:37:44 +0200, Thomas Petazzoni wrote:

> 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

Applied up to this point.

>   Makefile: use the package infra based external-deps

I've stopped here, because Arnout raised a concern, and I'd like the
concern to be resolved before moving forward with applying the rest of
the series. I've already replied to Arnout's concern, so hopefully it
will be resolved soon.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com