[Buildroot] [PATCH 0/7] Improvements to the download logic

[Buildroot] [PATCH 0/7] Improvements to the download logic

[Thomas Petazzoni]

Hello,

This set of 7 patches is some preparation work needed to fully bring
into the package infrastructure the responsability of features such as
"make source", "make external-deps" or "make legal-info".

In order to make this possible, we need to make sure that the package
infrastructure is aware of all downloads made by packages: calling the
"DOWNLOAD" macro directly is bypassing this. Currently, the linux and
perl packages are doing this, which means the package infrastructure
is not aware of certain things that the linux and perl packages are
downloading.

This series therefore:

 * Fixes the 'linux' package to use the package infra to download
   patches. No changes to the package infra are needed to make this
   possible.

 * Fixup the handling of the "Downloading" message.

 * Allow full URLs in <pkg>_EXTRA_DOWNLOADS, and not just a file name
   relative to <pkg>_SITE.

 * Fix the 'perl' package to use <pkg>_EXTRA_DOWNLOADS to download
   perl-cross.

I would like to see this small series merged, so that I can then
submit some additional patches later in the development cycle.

Thanks,

Thomas

Thomas Petazzoni (7):
  linux: use the package infrastructure to download patches
  pkg-generic: fix the logic showing the "Downloading" message
  pkg-generic: take into account <pkg>_EXTRA_DOWNLOADS to display
    "Downloading" message
  pkg-generic: refactor the "Downloading" message logic
  pkg-generic: allow full URLs for <pkg>_EXTRA_DOWNLOADS
  docs/manual: update documentation about <pkg>_SOURCE, <pkg>_PATCH and
    <pkg>_EXTRA_DOWNLOADS
  perl: use <pkg>_EXTRA_DOWNLOADS

 docs/manual/adding-packages-generic.txt | 32 ++++++++++++++++++--------------
 linux/linux.mk                          | 25 +++++++++----------------
 package/perl/perl.mk                    | 15 ++++++---------
 package/pkg-generic.mk                  | 23 ++++++++++++-----------
 4 files changed, 45 insertions(+), 50 deletions(-)

-- 
2.1.0

[Buildroot] [PATCH 1/7] linux: use the package infrastructure to download patches

[Thomas Petazzoni]

The linux package has a special handling of patches, with quite a bit
of legacy in it. A problem caused by this special handling is that the
linux package calls directly the DOWNLOAD_WGET macro, which means that
the package infrastructure isn't aware of which patches get
downloaded, and it prevents doing changes inside the package download
infrastructure.

This commit changes the handling of patches in the linux package in
the following way:

 * The LINUX_PATCHES variable is kept as is: it lists all the patches
   mentionned in the Config.in option BR2_LINUX_KERNEL_PATCH. This
   option can contain http://, ftp://, https:// URLs, path to local
   files or local directories.

   This variable is *not* used by the generic package infrastructure,
   so it is purely internal to the Linux package.

 * The LINUX_PATCH variable is now filled in with the list of patches
   that should be downloaded. It is derived from LINUX_PATCHES by
   filtering the patches that have http://, ftp:// or https:// in
   their path. Since <pkg>_PATCH is handled by the package
   infrastructure, it means that those patches are now automatically
   downloaded and applied by the package infrastructure.

 * The LINUX_APPLY_PATCHES hook is renamed to
   LINUX_APPLY_LOCAL_PATCHES, because it is now only responsible of
   applying local patches: remote patches are handled by
   LINUX_PATCH. The implementation of the hook is changed to filter
   out the patches that have already taken care of by LINUX_PATCH, so
   that we only iterate through the list of local patches or local
   patch directories.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 linux/linux.mk | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/linux/linux.mk b/linux/linux.mk
index fc90fc5..d591e1e 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -40,6 +40,11 @@ endif
 
 LINUX_PATCHES = $(call qstrip,$(BR2_LINUX_KERNEL_PATCH))
 
+# We really on the generic package infrastructure to download and
+# apply remote patches (downloaded from ftp, http or https). For local
+# patches, we have a custom post-patch hook below.
+LINUX_PATCH = $(filter ftp://% http://% https://%,$(LINUX_PATCHES))
+
 LINUX_INSTALL_IMAGES = YES
 LINUX_DEPENDENCIES += host-kmod host-lzop
 
@@ -144,20 +149,9 @@ else
 LINUX_IMAGE_PATH = $(KERNEL_ARCH_PATH)/boot/$(LINUX_IMAGE_NAME)
 endif # BR2_LINUX_KERNEL_VMLINUX
 
-define LINUX_DOWNLOAD_PATCHES
-	$(if $(LINUX_PATCHES),
-		@$(call MESSAGE,"Download additional patches"))
-	$(foreach patch,$(filter ftp://% http://% https://%,$(LINUX_PATCHES)),\
-		$(call DOWNLOAD_WGET,$(patch),$(notdir $(patch)))$(sep))
-endef
-
-LINUX_POST_DOWNLOAD_HOOKS += LINUX_DOWNLOAD_PATCHES
-
-define LINUX_APPLY_PATCHES
-	for p in $(LINUX_PATCHES) ; do \
-		if echo $$p | grep -q -E "^ftp://|^http://|^https://" ; then \
-			$(APPLY_PATCHES) $(@D) $(DL_DIR) `basename $$p` ; \
-		elif test -d $$p ; then \
+define LINUX_APPLY_LOCAL_PATCHES
+	for p in $(filter-out ftp://% http://% https://%,$(LINUX_PATCHES)) ; do \
+		if test -d $$p ; then \
 			$(APPLY_PATCHES) $(@D) $$p linux-\*.patch ; \
 		else \
 			$(APPLY_PATCHES) $(@D) `dirname $$p` `basename $$p` ; \
@@ -165,8 +159,7 @@ define LINUX_APPLY_PATCHES
 	done
 endef
 
-LINUX_POST_PATCH_HOOKS += LINUX_APPLY_PATCHES
-
+LINUX_POST_PATCH_HOOKS += LINUX_APPLY_LOCAL_PATCHES
 
 ifeq ($(BR2_LINUX_KERNEL_USE_DEFCONFIG),y)
 KERNEL_SOURCE_CONFIG = $(KERNEL_ARCH_PATH)/configs/$(call qstrip,$(BR2_LINUX_KERNEL_DEFCONFIG))_defconfig
-- 
2.1.0

[Buildroot] [PATCH 2/7] pkg-generic: fix the logic showing the "Downloading" message

[Thomas Petazzoni]

Even though the .stamp_downloaded target is executed for each package
being built, the pkg-generic tries to not display the "Downloading"
message when there is in fact nothing to download.

Unfortunately, the logic was incorrect for the patch download: it
forgot the fact that <pkg>_PATCH can contain either file names (in
which case we assume the patch should be downloaded from <pkg>_SITE),
or full URLs. The latter case was not properly handled, as we were
checking if $(DL_DIR)/<full URL> existed, while we should be testing
if $(DL_DIR)/`basename <full URL>` exists.

This patch fixes that, which makes sure the "Downloading" message is
displayed only when necessary.

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 1b09955..1482194 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -77,7 +77,7 @@ ifeq ($(DL_MODE),DOWNLOAD)
 		$(call MESSAGE,"Downloading") ; \
 	else \
 		for p in $($(PKG)_PATCH) ; do \
-			if test ! -e $(DL_DIR)/$$p ; then \
+			if test ! -e $(DL_DIR)/`basename $$p` ; then \
 				$(call MESSAGE,"Downloading") ; \
 				break ; \
 			fi ; \
-- 
2.1.0

[Buildroot] [PATCH 3/7] pkg-generic: take into account <pkg>_EXTRA_DOWNLOADS to display "Downloading" message

[Thomas Petazzoni]

The .stamp_downloaded target displays the "Downloading" message even
if there is really something to download. However, this logic only
checks for <pkg>_SOURCE and <pkg>_PATCH: it does not check if
something needs to be downloaded in <pkg>_EXTRA_DOWNLOADS.

This commit fixes that, which makes sure that the "Downloading"
message is displayed if one of the items in <pkg>_EXTRA_DOWNLOADS has
not yet been downloaded.

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 1482194..0098b29 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -76,7 +76,7 @@ ifeq ($(DL_MODE),DOWNLOAD)
 	$(Q)if test ! -e $(DL_DIR)/$($(PKG)_SOURCE); then \
 		$(call MESSAGE,"Downloading") ; \
 	else \
-		for p in $($(PKG)_PATCH) ; do \
+		for p in $($(PKG)_PATCH) $($(PKG)_EXTRA_DOWNLOADS) ; do \
 			if test ! -e $(DL_DIR)/`basename $$p` ; then \
 				$(call MESSAGE,"Downloading") ; \
 				break ; \
-- 
2.1.0

[Buildroot] [PATCH 4/7] pkg-generic: refactor the "Downloading" message logic

[Thomas Petazzoni]

In the .stamp_downloaded hook, the logic to decide whether or not to
display the "Downloading" message is treating the check of
<pkg>_SOURCE as a special case. But in fact, there is no real reason
to do so: the existing loop used for <pkg>_PATCH and
<pkg>_EXTRA_DOWNLOADS could work just as well.

This commit therefore refactors this piece of code, to have a single
loop checking <pkg>_SOURCE, <pkg>_PATCH and <pkg>_EXTRA_DOWNLOADS.

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 0098b29..7a9da43 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -73,16 +73,12 @@ $(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)if test ! -e $(DL_DIR)/$($(PKG)_SOURCE); then \
-		$(call MESSAGE,"Downloading") ; \
-	else \
-		for p in $($(PKG)_PATCH) $($(PKG)_EXTRA_DOWNLOADS) ; do \
-			if test ! -e $(DL_DIR)/`basename $$p` ; then \
-				$(call MESSAGE,"Downloading") ; \
-				break ; \
-			fi ; \
-		done ; \
-	fi
+	$(Q)for p in $($(PKG)_SOURCE) $($(PKG)_PATCH) $($(PKG)_EXTRA_DOWNLOADS) ; do \
+		if test ! -e $(DL_DIR)/`basename $$p` ; then \
+			$(call MESSAGE,"Downloading") ; \
+			break ; \
+		fi ; \
+	done
 endif
 	$(if $($(PKG)_SOURCE),$(call DOWNLOAD,$($(PKG)_SITE:/=)/$($(PKG)_SOURCE)))
 	$(foreach p,$($(PKG)_EXTRA_DOWNLOADS),$(call DOWNLOAD,$($(PKG)_SITE:/=)/$(p))$(sep))
-- 
2.1.0

[Buildroot] [PATCH 5/7] pkg-generic: allow full URLs for <pkg>_EXTRA_DOWNLOADS

[Thomas Petazzoni]

The current logic for <pkg>_EXTRA_DOWNLOADS assumes that it is a list
of files, all hosted at <pkg>_SITE. However, just like for
<pkg>_PATCH, it may be useful to specify <pkg>_EXTRA_DOWNLOADS entries
that are hosted on a different site than the package <pkg>_SITE.

This commit implements this, by re-using the same logic as the one
used for <pkg>_PATCH.

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 7a9da43..85b22de 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -81,7 +81,12 @@ ifeq ($(DL_MODE),DOWNLOAD)
 	done
 endif
 	$(if $($(PKG)_SOURCE),$(call DOWNLOAD,$($(PKG)_SITE:/=)/$($(PKG)_SOURCE)))
-	$(foreach p,$($(PKG)_EXTRA_DOWNLOADS),$(call DOWNLOAD,$($(PKG)_SITE:/=)/$(p))$(sep))
+	$(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)),\
-- 
2.1.0

[Buildroot] [PATCH 6/7] docs/manual: update documentation about <pkg>_SOURCE, <pkg>_PATCH and <pkg>_EXTRA_DOWNLOADS

[Thomas Petazzoni]

This commit updates the Buildroot manual for the variables used to
indicate where to download the source code from:

 - It updates the description of <pkg>_SOURCE to make it clear that
   Buildroot assume that the tarball is hosted at <pkg>_SITE.

 - It updates the description of <pkg>_PATCH to indicate that not only
   file names (assumed to be hosted at <pkg>_SITE) can be used, but
   also full URLs. This allows to match with what the current code is
   doing.

 - It updates the description of <pkg>_EXTRA_DOWNLOADS to also
   indicate that full URLs are now accepted, following the change made
   in the previous commit.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 docs/manual/adding-packages-generic.txt | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/docs/manual/adding-packages-generic.txt b/docs/manual/adding-packages-generic.txt
index e3029ed..2e46d55 100644
--- a/docs/manual/adding-packages-generic.txt
+++ b/docs/manual/adding-packages-generic.txt
@@ -204,18 +204,20 @@ information is (assuming the package name is +libfoo+) :
     +LIBFOO_VERSION = cb9d6aa9429e838f0e54faa3d455bcbab5eef057+ +
     +LIBFOO_VERSION = stable+
 
-* +LIBFOO_SOURCE+ may contain the name of the tarball of
-  the package. If +HOST_LIBFOO_SOURCE+ is not specified, it
-  defaults to +LIBFOO_SOURCE+. If none are specified, then
-  the value is assumed to be
-  +libfoo-$(LIBFOO_VERSION).tar.gz+. +
-  Example: +LIBFOO_SOURCE = foobar-$(LIBFOO_VERSION).tar.bz2+
+* +LIBFOO_SOURCE+ may contain the name of the tarball of the package,
+  which Buildroot will use to download the tarball from
+  +LIBFOO_SITE+. If +HOST_LIBFOO_SOURCE+ is not specified, it defaults
+  to +LIBFOO_SOURCE+. If none are specified, then the value is assumed
+  to be +libfoo-$(LIBFOO_VERSION).tar.gz+. + Example: +LIBFOO_SOURCE =
+  foobar-$(LIBFOO_VERSION).tar.bz2+
 
 * +LIBFOO_PATCH+ may contain a space-separated list of patch file
-  names, that will be downloaded from the same location as the tarball
-  indicated in +LIBFOO_SOURCE+, and then applied to the package source
-  code. If +HOST_LIBFOO_PATCH+ is not specified, it defaults to
-  +LIBFOO_PATCH+. Note that patches that are included in Buildroot
+  names, that Buildroot will download and apply to the package source
+  code. If an entry contains +://+, then Buildroot will assume it is a
+  full URL and download the patch from this location. Otherwise,
+  Buildroot will assume that the patch should be downloaded from
+  +LIBFOO_SITE+. If +HOST_LIBFOO_PATCH+ is not specified, it defaults
+  to +LIBFOO_PATCH+. Note that patches that are included in Buildroot
   itself use a different mechanism: all files of the form
   +<packagename>-*.patch+ present in the package directory inside
   Buildroot will be applied to the package after extraction (see
@@ -246,10 +248,12 @@ information is (assuming the package name is +libfoo+) :
     +LIBFOO_SITE=/opt/software/libfoo.tar.gz+ +
     +LIBFOO_SITE=$(TOPDIR)/../src/libfoo/+
 
-* +LIBFOO_EXTRA_DOWNLOADS+ lists a number of additional files that
-  Buildroot should download from +LIBFOO_SITE+ in addition to the main
-  +LIBFOO_SOURCE+ (which usually is a tarball). Buildroot will not do
-  anything with those additional files, except download files: it will
+* +LIBFOO_EXTRA_DOWNLOADS+ is a space-separated list of additional
+  files that Buildroot should download. If an entry contains +://+
+  then Buildroot will assume it is a complete URL and will download
+  the file using this URL. Otherwise, Buildroot will assume the file
+  to be downloaded is located at +LIBFOO_SITE+. Buildroot will not do
+  anything with those additional files, except download them: it will
   be up to the package recipe to use them from +$(BR2_DL_DIR)+.
 
 * +LIBFOO_SITE_METHOD+ determines the method used to fetch or copy the
-- 
2.1.0

[Buildroot] [PATCH 7/7] perl: use <pkg>_EXTRA_DOWNLOADS

[Thomas Petazzoni]

Instead of manually using the DOWNLOAD macro (which should remain an
internal macro), this commit converts the Perl package to use
<pkg>_EXTRA_DOWNLOADS, now that it has been extended to allow full
URLs.

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

diff --git a/package/perl/perl.mk b/package/perl/perl.mk
index 0636aba..cb99a98 100644
--- a/package/perl/perl.mk
+++ b/package/perl/perl.mk
@@ -12,6 +12,11 @@ PERL_LICENSE = Artistic or GPLv1+
 PERL_LICENSE_FILES = Artistic Copying README
 PERL_INSTALL_STAGING = YES
 
+# We use the perlcross hack to cross-compile perl. It should
+# be extracted over the perl sources, so we don't define that
+# as a separate package. Instead, it is downloaded and extracted
+# together with perl
+
 PERL_CROSS_VERSION = 0.9.4
 PERL_CROSS_BASE_VERSION = 5.$(PERL_VERSION_MAJOR).1
 # DO NOT refactor with the github helper (the result is not the same)
@@ -20,15 +25,7 @@ PERL_CROSS_SOURCE = perl-$(PERL_CROSS_BASE_VERSION)-cross-$(PERL_CROSS_VERSION).
 PERL_CROSS_OLD_POD = perl$(subst .,,$(PERL_CROSS_BASE_VERSION))delta.pod
 PERL_CROSS_NEW_POD = perl$(subst .,,$(PERL_VERSION))delta.pod
 
-# We use the perlcross hack to cross-compile perl. It should
-# be extracted over the perl sources, so we don't define that
-# as a separate package. Instead, it is downloaded and extracted
-# together with perl
-
-define PERL_CROSS_DOWNLOAD
-	$(call DOWNLOAD,$(PERL_CROSS_SITE:/=)/$(PERL_CROSS_SOURCE))
-endef
-PERL_POST_DOWNLOAD_HOOKS += PERL_CROSS_DOWNLOAD
+PERL_EXTRA_DOWNLOADS = $(PERL_CROSS_SITE)/$(PERL_CROSS_SOURCE)
 
 define PERL_CROSS_EXTRACT
 	$(call suitable-extractor,$(PERL_CROSS_SOURCE)) $(DL_DIR)/$(PERL_CROSS_SOURCE) | \
-- 
2.1.0

[Buildroot] [PATCH 1/7] linux: use the package infrastructure to download patches

[Baruch Siach]

Hi Thomas,

On Sat, Feb 28, 2015 at 07:15:07PM +0100, Thomas Petazzoni wrote:
> +# We really on the generic package infrastructure to download and

s/really/rely/

> +# apply remote patches (downloaded from ftp, http or https). For local
> +# patches, we have a custom post-patch hook below.
> +LINUX_PATCH = $(filter ftp://% http://% https://%,$(LINUX_PATCHES))

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

[Buildroot] [PATCH 1/7] linux: use the package infrastructure to download patches

[Romain Naour]

Hello Thomas,

[snip]
> +define LINUX_APPLY_LOCAL_PATCHES
> +	for p in $(filter-out ftp://% http://% https://%,$(LINUX_PATCHES)) ; do \
> +		if test -d $$p ; then \
>  			$(APPLY_PATCHES) $(@D) $$p linux-\*.patch ; \
[snip]

I know, it's not related to your patch but linux patches doesn't follow the new
naming convention here.

Do we want to keep the "linux-" prefix since linux is not really a package or
get rid of it ?

It would be convenient to use linux patches generated by git format-patch
command like for other packages.

Best regards,
Romain

[Buildroot] [PATCH 1/7] linux: use the package infrastructure to download patches

[Thomas Petazzoni]

Dear Romain Naour,

On Sun, 15 Mar 2015 15:59:07 +0100, Romain Naour wrote:

> I know, it's not related to your patch but linux patches doesn't follow the new
> naming convention here.
> 
> Do we want to keep the "linux-" prefix since linux is not really a package or
> get rid of it ?

linux is really a package.

> It would be convenient to use linux patches generated by git format-patch
> command like for other packages.

You in fact can already do that, if you use BR2_GLOBAL_PATCH_DIR.

Regarding whether we should switch from linux-*.patch to *.patch, I
don't know. It might break the build for people having various patches
in the directory point by BR2_LINUX_KERNEL_PATCH. But maybe we should
break the build for them at some point, and get to a situation where we
are consistent.

Best regards,

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

[Buildroot] [PATCH 1/7] linux: use the package infrastructure to download patches

[Romain Naour]

Hello Thomas,

Le 15/03/2015 16:14, Thomas Petazzoni a ?crit :
> Dear Romain Naour,
> 
> On Sun, 15 Mar 2015 15:59:07 +0100, Romain Naour wrote:
> 
>> I know, it's not related to your patch but linux patches doesn't follow the new
>> naming convention here.
>>
>> Do we want to keep the "linux-" prefix since linux is not really a package or
>> get rid of it ?
> 
> linux is really a package.

humm... yes (hide) :)

> 
>> It would be convenient to use linux patches generated by git format-patch
>> command like for other packages.
> 
> You in fact can already do that, if you use BR2_GLOBAL_PATCH_DIR.
> 
> Regarding whether we should switch from linux-*.patch to *.patch, I
> don't know. It might break the build for people having various patches
> in the directory point by BR2_LINUX_KERNEL_PATCH. But maybe we should
> break the build for them at some point, and get to a situation where we
> are consistent.

I use BR2_LINUX_KERNEL_PATCH which point to a directory that contains all
patches for the kernel build for my target at work.

I already made the mistake to forget to rename a new patch with the "linux-"
prefix, because I added it like I do for other packages.

But sure, it would break the build if we decide to fix this.

Best regards,
Romain

> 
> Best regards,
> 
> Thomas
> 

[Buildroot] [PATCH 1/7] linux: use the package infrastructure to download patches

[Romain Naour]

Hi Thomas,

Le 28/02/2015 19:15, Thomas Petazzoni a ?crit :
> The linux package has a special handling of patches, with quite a bit
> of legacy in it. A problem caused by this special handling is that the
> linux package calls directly the DOWNLOAD_WGET macro, which means that
> the package infrastructure isn't aware of which patches get
> downloaded, and it prevents doing changes inside the package download
> infrastructure.
> 
> This commit changes the handling of patches in the linux package in
> the following way:
> 
>  * The LINUX_PATCHES variable is kept as is: it lists all the patches
>    mentionned in the Config.in option BR2_LINUX_KERNEL_PATCH. This

mentionned -> mentioned

>    option can contain http://, ftp://, https:// URLs, path to local
>    files or local directories.
> 
>    This variable is *not* used by the generic package infrastructure,
>    so it is purely internal to the Linux package.
> 
>  * The LINUX_PATCH variable is now filled in with the list of patches
>    that should be downloaded. It is derived from LINUX_PATCHES by
>    filtering the patches that have http://, ftp:// or https:// in
>    their path. Since <pkg>_PATCH is handled by the package
>    infrastructure, it means that those patches are now automatically
>    downloaded and applied by the package infrastructure.
> 
>  * The LINUX_APPLY_PATCHES hook is renamed to
>    LINUX_APPLY_LOCAL_PATCHES, because it is now only responsible of
>    applying local patches: remote patches are handled by
>    LINUX_PATCH. The implementation of the hook is changed to filter
>    out the patches that have already taken care of by LINUX_PATCH, so
>    that we only iterate through the list of local patches or local
>    patch directories.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---

Reviewed-by: Romain Naour <romain.naour@openwide.fr>

Partially tested with BR2_LINUX_KERNEL_PATCH which point to a directory that
contains all patches for the kernel to be build.

>  linux/linux.mk | 25 +++++++++----------------
>  1 file changed, 9 insertions(+), 16 deletions(-)
> 
> diff --git a/linux/linux.mk b/linux/linux.mk
> index fc90fc5..d591e1e 100644
> --- a/linux/linux.mk
> +++ b/linux/linux.mk
> @@ -40,6 +40,11 @@ endif
>  
>  LINUX_PATCHES = $(call qstrip,$(BR2_LINUX_KERNEL_PATCH))
>  
> +# We really on the generic package infrastructure to download and

really -> rely (as noted by Baruch)

> +# apply remote patches (downloaded from ftp, http or https). For local
> +# patches, we have a custom post-patch hook below.
> +LINUX_PATCH = $(filter ftp://% http://% https://%,$(LINUX_PATCHES))
> +
>  LINUX_INSTALL_IMAGES = YES
>  LINUX_DEPENDENCIES += host-kmod host-lzop
>  
> @@ -144,20 +149,9 @@ else
>  LINUX_IMAGE_PATH = $(KERNEL_ARCH_PATH)/boot/$(LINUX_IMAGE_NAME)
>  endif # BR2_LINUX_KERNEL_VMLINUX
>  
> -define LINUX_DOWNLOAD_PATCHES
> -	$(if $(LINUX_PATCHES),
> -		@$(call MESSAGE,"Download additional patches"))
> -	$(foreach patch,$(filter ftp://% http://% https://%,$(LINUX_PATCHES)),\
> -		$(call DOWNLOAD_WGET,$(patch),$(notdir $(patch)))$(sep))
> -endef
> -
> -LINUX_POST_DOWNLOAD_HOOKS += LINUX_DOWNLOAD_PATCHES
> -
> -define LINUX_APPLY_PATCHES
> -	for p in $(LINUX_PATCHES) ; do \
> -		if echo $$p | grep -q -E "^ftp://|^http://|^https://" ; then \
> -			$(APPLY_PATCHES) $(@D) $(DL_DIR) `basename $$p` ; \
> -		elif test -d $$p ; then \
> +define LINUX_APPLY_LOCAL_PATCHES
> +	for p in $(filter-out ftp://% http://% https://%,$(LINUX_PATCHES)) ; do \
> +		if test -d $$p ; then \
>  			$(APPLY_PATCHES) $(@D) $$p linux-\*.patch ; \
>  		else \
>  			$(APPLY_PATCHES) $(@D) `dirname $$p` `basename $$p` ; \
> @@ -165,8 +159,7 @@ define LINUX_APPLY_PATCHES
>  	done
>  endef
>  
> -LINUX_POST_PATCH_HOOKS += LINUX_APPLY_PATCHES
> -
> +LINUX_POST_PATCH_HOOKS += LINUX_APPLY_LOCAL_PATCHES
>  
>  ifeq ($(BR2_LINUX_KERNEL_USE_DEFCONFIG),y)
>  KERNEL_SOURCE_CONFIG = $(KERNEL_ARCH_PATH)/configs/$(call qstrip,$(BR2_LINUX_KERNEL_DEFCONFIG))_defconfig
> 

[Buildroot] [PATCH 2/7] pkg-generic: fix the logic showing the "Downloading" message

[Romain Naour]

Hi Thomas,

Le 28/02/2015 19:15, Thomas Petazzoni a ?crit :
> Even though the .stamp_downloaded target is executed for each package
> being built, the pkg-generic tries to not display the "Downloading"
> message when there is in fact nothing to download.
> 
> Unfortunately, the logic was incorrect for the patch download: it
> forgot the fact that <pkg>_PATCH can contain either file names (in
> which case we assume the patch should be downloaded from <pkg>_SITE),
> or full URLs. The latter case was not properly handled, as we were
> checking if $(DL_DIR)/<full URL> existed, while we should be testing
> if $(DL_DIR)/`basename <full URL>` exists.
> 
> This patch fixes that, which makes sure the "Downloading" message is
> displayed only when necessary.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---

Reviewed-by: Romain Naour <romain.naour@openwide.fr>

>  package/pkg-generic.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 1b09955..1482194 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -77,7 +77,7 @@ ifeq ($(DL_MODE),DOWNLOAD)
>  		$(call MESSAGE,"Downloading") ; \
>  	else \
>  		for p in $($(PKG)_PATCH) ; do \
> -			if test ! -e $(DL_DIR)/$$p ; then \
> +			if test ! -e $(DL_DIR)/`basename $$p` ; then \
>  				$(call MESSAGE,"Downloading") ; \
>  				break ; \
>  			fi ; \
> 

[Buildroot] [PATCH 3/7] pkg-generic: take into account <pkg>_EXTRA_DOWNLOADS to display "Downloading" message

[Romain Naour]

Hi Thomas,

Le 28/02/2015 19:15, Thomas Petazzoni a ?crit :
> The .stamp_downloaded target displays the "Downloading" message even
> if there is really something to download. However, this logic only
> checks for <pkg>_SOURCE and <pkg>_PATCH: it does not check if
> something needs to be downloaded in <pkg>_EXTRA_DOWNLOADS.
> 
> This commit fixes that, which makes sure that the "Downloading"
> message is displayed if one of the items in <pkg>_EXTRA_DOWNLOADS has
> not yet been downloaded.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---

Reviewed-by: Romain Naour <romain.naour@openwide.fr>

>  package/pkg-generic.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 1482194..0098b29 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -76,7 +76,7 @@ ifeq ($(DL_MODE),DOWNLOAD)
>  	$(Q)if test ! -e $(DL_DIR)/$($(PKG)_SOURCE); then \
>  		$(call MESSAGE,"Downloading") ; \
>  	else \
> -		for p in $($(PKG)_PATCH) ; do \
> +		for p in $($(PKG)_PATCH) $($(PKG)_EXTRA_DOWNLOADS) ; do \
>  			if test ! -e $(DL_DIR)/`basename $$p` ; then \
>  				$(call MESSAGE,"Downloading") ; \
>  				break ; \
> 

[Buildroot] [PATCH 4/7] pkg-generic: refactor the "Downloading" message logic

[Romain Naour]

Hi Thomas,

Le 28/02/2015 19:15, Thomas Petazzoni a ?crit :
> In the .stamp_downloaded hook, the logic to decide whether or not to
> display the "Downloading" message is treating the check of
> <pkg>_SOURCE as a special case. But in fact, there is no real reason
> to do so: the existing loop used for <pkg>_PATCH and
> <pkg>_EXTRA_DOWNLOADS could work just as well.
> 
> This commit therefore refactors this piece of code, to have a single
> loop checking <pkg>_SOURCE, <pkg>_PATCH and <pkg>_EXTRA_DOWNLOADS.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---

Reviewed-by: Romain Naour <romain.naour@openwide.fr>

>  package/pkg-generic.mk | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 0098b29..7a9da43 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -73,16 +73,12 @@ $(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)if test ! -e $(DL_DIR)/$($(PKG)_SOURCE); then \
> -		$(call MESSAGE,"Downloading") ; \
> -	else \
> -		for p in $($(PKG)_PATCH) $($(PKG)_EXTRA_DOWNLOADS) ; do \
> -			if test ! -e $(DL_DIR)/`basename $$p` ; then \
> -				$(call MESSAGE,"Downloading") ; \
> -				break ; \
> -			fi ; \
> -		done ; \
> -	fi
> +	$(Q)for p in $($(PKG)_SOURCE) $($(PKG)_PATCH) $($(PKG)_EXTRA_DOWNLOADS) ; do \
> +		if test ! -e $(DL_DIR)/`basename $$p` ; then \
> +			$(call MESSAGE,"Downloading") ; \
> +			break ; \
> +		fi ; \
> +	done
>  endif
>  	$(if $($(PKG)_SOURCE),$(call DOWNLOAD,$($(PKG)_SITE:/=)/$($(PKG)_SOURCE)))
>  	$(foreach p,$($(PKG)_EXTRA_DOWNLOADS),$(call DOWNLOAD,$($(PKG)_SITE:/=)/$(p))$(sep))
> 

[Buildroot] [PATCH 5/7] pkg-generic: allow full URLs for <pkg>_EXTRA_DOWNLOADS

[Romain Naour]

Hi Thomas,

Le 28/02/2015 19:15, Thomas Petazzoni a ?crit :
> The current logic for <pkg>_EXTRA_DOWNLOADS assumes that it is a list
> of files, all hosted at <pkg>_SITE. However, just like for
> <pkg>_PATCH, it may be useful to specify <pkg>_EXTRA_DOWNLOADS entries
> that are hosted on a different site than the package <pkg>_SITE.
> 
> This commit implements this, by re-using the same logic as the one
> used for <pkg>_PATCH.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---

Reviewed-by: Romain Naour <romain.naour@openwide.fr>

>  package/pkg-generic.mk | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 7a9da43..85b22de 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -81,7 +81,12 @@ ifeq ($(DL_MODE),DOWNLOAD)
>  	done
>  endif
>  	$(if $($(PKG)_SOURCE),$(call DOWNLOAD,$($(PKG)_SITE:/=)/$($(PKG)_SOURCE)))
> -	$(foreach p,$($(PKG)_EXTRA_DOWNLOADS),$(call DOWNLOAD,$($(PKG)_SITE:/=)/$(p))$(sep))
> +	$(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)),\
> 

[Buildroot] [PATCH 6/7] docs/manual: update documentation about <pkg>_SOURCE, <pkg>_PATCH and <pkg>_EXTRA_DOWNLOADS

[Romain Naour]

Hi Thomas,

Le 28/02/2015 19:15, Thomas Petazzoni a ?crit :
> This commit updates the Buildroot manual for the variables used to
> indicate where to download the source code from:
> 
>  - It updates the description of <pkg>_SOURCE to make it clear that
>    Buildroot assume that the tarball is hosted at <pkg>_SITE.
> 
>  - It updates the description of <pkg>_PATCH to indicate that not only
>    file names (assumed to be hosted at <pkg>_SITE) can be used, but
>    also full URLs. This allows to match with what the current code is
>    doing.
> 
>  - It updates the description of <pkg>_EXTRA_DOWNLOADS to also
>    indicate that full URLs are now accepted, following the change made
>    in the previous commit.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---

Reviewed-by: Romain Naour <romain.naour@openwide.fr>

>  docs/manual/adding-packages-generic.txt | 32 ++++++++++++++++++--------------
>  1 file changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/docs/manual/adding-packages-generic.txt b/docs/manual/adding-packages-generic.txt
> index e3029ed..2e46d55 100644
> --- a/docs/manual/adding-packages-generic.txt
> +++ b/docs/manual/adding-packages-generic.txt
> @@ -204,18 +204,20 @@ information is (assuming the package name is +libfoo+) :
>      +LIBFOO_VERSION = cb9d6aa9429e838f0e54faa3d455bcbab5eef057+ +
>      +LIBFOO_VERSION = stable+
>  
> -* +LIBFOO_SOURCE+ may contain the name of the tarball of
> -  the package. If +HOST_LIBFOO_SOURCE+ is not specified, it
> -  defaults to +LIBFOO_SOURCE+. If none are specified, then
> -  the value is assumed to be
> -  +libfoo-$(LIBFOO_VERSION).tar.gz+. +
> -  Example: +LIBFOO_SOURCE = foobar-$(LIBFOO_VERSION).tar.bz2+
> +* +LIBFOO_SOURCE+ may contain the name of the tarball of the package,
> +  which Buildroot will use to download the tarball from
> +  +LIBFOO_SITE+. If +HOST_LIBFOO_SOURCE+ is not specified, it defaults
> +  to +LIBFOO_SOURCE+. If none are specified, then the value is assumed
> +  to be +libfoo-$(LIBFOO_VERSION).tar.gz+. + Example: +LIBFOO_SOURCE =
> +  foobar-$(LIBFOO_VERSION).tar.bz2+
>  
>  * +LIBFOO_PATCH+ may contain a space-separated list of patch file
> -  names, that will be downloaded from the same location as the tarball
> -  indicated in +LIBFOO_SOURCE+, and then applied to the package source
> -  code. If +HOST_LIBFOO_PATCH+ is not specified, it defaults to
> -  +LIBFOO_PATCH+. Note that patches that are included in Buildroot
> +  names, that Buildroot will download and apply to the package source
> +  code. If an entry contains +://+, then Buildroot will assume it is a
> +  full URL and download the patch from this location. Otherwise,
> +  Buildroot will assume that the patch should be downloaded from
> +  +LIBFOO_SITE+. If +HOST_LIBFOO_PATCH+ is not specified, it defaults
> +  to +LIBFOO_PATCH+. Note that patches that are included in Buildroot
>    itself use a different mechanism: all files of the form
>    +<packagename>-*.patch+ present in the package directory inside
>    Buildroot will be applied to the package after extraction (see
> @@ -246,10 +248,12 @@ information is (assuming the package name is +libfoo+) :
>      +LIBFOO_SITE=/opt/software/libfoo.tar.gz+ +
>      +LIBFOO_SITE=$(TOPDIR)/../src/libfoo/+
>  
> -* +LIBFOO_EXTRA_DOWNLOADS+ lists a number of additional files that
> -  Buildroot should download from +LIBFOO_SITE+ in addition to the main
> -  +LIBFOO_SOURCE+ (which usually is a tarball). Buildroot will not do
> -  anything with those additional files, except download files: it will
> +* +LIBFOO_EXTRA_DOWNLOADS+ is a space-separated list of additional
> +  files that Buildroot should download. If an entry contains +://+
> +  then Buildroot will assume it is a complete URL and will download
> +  the file using this URL. Otherwise, Buildroot will assume the file
> +  to be downloaded is located at +LIBFOO_SITE+. Buildroot will not do
> +  anything with those additional files, except download them: it will
>    be up to the package recipe to use them from +$(BR2_DL_DIR)+.
>  
>  * +LIBFOO_SITE_METHOD+ determines the method used to fetch or copy the
> 

[Buildroot] [PATCH 7/7] perl: use <pkg>_EXTRA_DOWNLOADS

[Romain Naour]

Hi Thomas,

Le 28/02/2015 19:15, Thomas Petazzoni a ?crit :
> Instead of manually using the DOWNLOAD macro (which should remain an
> internal macro), this commit converts the Perl package to use
> <pkg>_EXTRA_DOWNLOADS, now that it has been extended to allow full
> URLs.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---

This patch needs to be rebased on master since the perl package has been
updated, but the aim of this patch is the same.

Reviewed-by: Romain Naour <romain.naour@openwide.fr>

>  package/perl/perl.mk | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/package/perl/perl.mk b/package/perl/perl.mk
> index 0636aba..cb99a98 100644
> --- a/package/perl/perl.mk
> +++ b/package/perl/perl.mk
> @@ -12,6 +12,11 @@ PERL_LICENSE = Artistic or GPLv1+
>  PERL_LICENSE_FILES = Artistic Copying README
>  PERL_INSTALL_STAGING = YES
>  
> +# We use the perlcross hack to cross-compile perl. It should
> +# be extracted over the perl sources, so we don't define that
> +# as a separate package. Instead, it is downloaded and extracted
> +# together with perl
> +
>  PERL_CROSS_VERSION = 0.9.4
>  PERL_CROSS_BASE_VERSION = 5.$(PERL_VERSION_MAJOR).1
>  # DO NOT refactor with the github helper (the result is not the same)
> @@ -20,15 +25,7 @@ PERL_CROSS_SOURCE = perl-$(PERL_CROSS_BASE_VERSION)-cross-$(PERL_CROSS_VERSION).
>  PERL_CROSS_OLD_POD = perl$(subst .,,$(PERL_CROSS_BASE_VERSION))delta.pod
>  PERL_CROSS_NEW_POD = perl$(subst .,,$(PERL_VERSION))delta.pod
>  
> -# We use the perlcross hack to cross-compile perl. It should
> -# be extracted over the perl sources, so we don't define that
> -# as a separate package. Instead, it is downloaded and extracted
> -# together with perl
> -
> -define PERL_CROSS_DOWNLOAD
> -	$(call DOWNLOAD,$(PERL_CROSS_SITE:/=)/$(PERL_CROSS_SOURCE))
> -endef
> -PERL_POST_DOWNLOAD_HOOKS += PERL_CROSS_DOWNLOAD
> +PERL_EXTRA_DOWNLOADS = $(PERL_CROSS_SITE)/$(PERL_CROSS_SOURCE)
>  
>  define PERL_CROSS_EXTRACT
>  	$(call suitable-extractor,$(PERL_CROSS_SOURCE)) $(DL_DIR)/$(PERL_CROSS_SOURCE) | \
> 

[Buildroot] [PATCH 1/7] linux: use the package infrastructure to download patches

[Arnout Vandecappelle]

On 15/03/15 16:14, Thomas Petazzoni wrote:
>> > It would be convenient to use linux patches generated by git format-patch
>> > command like for other packages.
> You in fact can already do that, if you use BR2_GLOBAL_PATCH_DIR.
> 
> Regarding whether we should switch from linux-*.patch to *.patch, I
> don't know. It might break the build for people having various patches
> in the directory point by BR2_LINUX_KERNEL_PATCH. But maybe we should
> break the build for them at some point, and get to a situation where we
> are consistent.

 I would say, break the build. But in a separate patch of course :-)

 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] [PATCH 1/7] linux: use the package infrastructure to download patches

[Arnout Vandecappelle]

On 28/02/15 19:15, Thomas Petazzoni wrote:
> The linux package has a special handling of patches, with quite a bit
> of legacy in it. A problem caused by this special handling is that the
> linux package calls directly the DOWNLOAD_WGET macro, which means that
> the package infrastructure isn't aware of which patches get
> downloaded, and it prevents doing changes inside the package download
> infrastructure.
> 
> This commit changes the handling of patches in the linux package in
> the following way:
> 
>  * The LINUX_PATCHES variable is kept as is: it lists all the patches
>    mentionned in the Config.in option BR2_LINUX_KERNEL_PATCH. This
>    option can contain http://, ftp://, https:// URLs, path to local
>    files or local directories.
> 
>    This variable is *not* used by the generic package infrastructure,
>    so it is purely internal to the Linux package.
> 
>  * The LINUX_PATCH variable is now filled in with the list of patches
>    that should be downloaded. It is derived from LINUX_PATCHES by
>    filtering the patches that have http://, ftp:// or https:// in
>    their path. Since <pkg>_PATCH is handled by the package
>    infrastructure, it means that those patches are now automatically
>    downloaded and applied by the package infrastructure.
> 
>  * The LINUX_APPLY_PATCHES hook is renamed to
>    LINUX_APPLY_LOCAL_PATCHES, because it is now only responsible of
>    applying local patches: remote patches are handled by
>    LINUX_PATCH. The implementation of the hook is changed to filter
>    out the patches that have already taken care of by LINUX_PATCH, so
>    that we only iterate through the list of local patches or local
>    patch directories.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

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

 It's time for this one to be applied. There are of course the two spelling
corrections noted by Romain and Baruch, but I'm sure you can fix those while
applying.

 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] [PATCH 2/7] pkg-generic: fix the logic showing the "Downloading" message

[Arnout Vandecappelle]

On 28/02/15 19:15, Thomas Petazzoni wrote:
> Even though the .stamp_downloaded target is executed for each package
> being built, the pkg-generic tries to not display the "Downloading"
> message when there is in fact nothing to download.
> 
> Unfortunately, the logic was incorrect for the patch download: it
> forgot the fact that <pkg>_PATCH can contain either file names (in
> which case we assume the patch should be downloaded from <pkg>_SITE),
> or full URLs. The latter case was not properly handled, as we were
> checking if $(DL_DIR)/<full URL> existed, while we should be testing
> if $(DL_DIR)/`basename <full URL>` exists.
> 
> This patch fixes that, which makes sure the "Downloading" message is
> displayed only when necessary.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

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


 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] [PATCH 3/7] pkg-generic: take into account <pkg>_EXTRA_DOWNLOADS to display "Downloading" message

[Arnout Vandecappelle]

On 28/02/15 19:15, Thomas Petazzoni wrote:
> The .stamp_downloaded target displays the "Downloading" message even
> if there is really something to download. However, this logic only
> checks for <pkg>_SOURCE and <pkg>_PATCH: it does not check if
> something needs to be downloaded in <pkg>_EXTRA_DOWNLOADS.
> 
> This commit fixes that, which makes sure that the "Downloading"
> message is displayed if one of the items in <pkg>_EXTRA_DOWNLOADS has
> not yet been downloaded.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

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


 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] [PATCH 4/7] pkg-generic: refactor the "Downloading" message logic

[Arnout Vandecappelle]

On 28/02/15 19:15, Thomas Petazzoni wrote:
> In the .stamp_downloaded hook, the logic to decide whether or not to
> display the "Downloading" message is treating the check of
> <pkg>_SOURCE as a special case. But in fact, there is no real reason
> to do so: the existing loop used for <pkg>_PATCH and
> <pkg>_EXTRA_DOWNLOADS could work just as well.
> 
> This commit therefore refactors this piece of code, to have a single
> loop checking <pkg>_SOURCE, <pkg>_PATCH and <pkg>_EXTRA_DOWNLOADS.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

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

 I'm a bit worried though about URLs with special characters. But since things
weren't quoted before, they were already broken anyway...

 I also thought that maybe it could be done in pure make syntax, but then it
becomes horribly complicated.

 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] [PATCH 5/7] pkg-generic: allow full URLs for <pkg>_EXTRA_DOWNLOADS

[Arnout Vandecappelle]

On 28/02/15 19:15, Thomas Petazzoni wrote:
> The current logic for <pkg>_EXTRA_DOWNLOADS assumes that it is a list
> of files, all hosted at <pkg>_SITE. However, just like for
> <pkg>_PATCH, it may be useful to specify <pkg>_EXTRA_DOWNLOADS entries
> that are hosted on a different site than the package <pkg>_SITE.
> 
> This commit implements this, by re-using the same logic as the one
> used for <pkg>_PATCH.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  package/pkg-generic.mk | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 7a9da43..85b22de 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -81,7 +81,12 @@ ifeq ($(DL_MODE),DOWNLOAD)
>  	done
>  endif
>  	$(if $($(PKG)_SOURCE),$(call DOWNLOAD,$($(PKG)_SITE:/=)/$($(PKG)_SOURCE)))
> -	$(foreach p,$($(PKG)_EXTRA_DOWNLOADS),$(call DOWNLOAD,$($(PKG)_SITE:/=)/$(p))$(sep))
> +	$(foreach p,$($(PKG)_EXTRA_DOWNLOADS),\
> +		$(if $(findstring ://,$(p)),\
> +			$(call DOWNLOAD,$(p)),\
> +			$(call DOWNLOAD,$($(PKG)_SITE:/=)/$(p))\
> +		)\
> +	$(sep))
>  	$(foreach p,$($(PKG)_PATCH),\

 Why not

	$(foreach p,$($(PKG)_EXTRA_DOWNLOADS) $($(PKG)_PATCH),\

and save a few lines?


 Regards,
 Arnout

>  		$(if $(findstring ://,$(p)),\
>  			$(call DOWNLOAD,$(p)),\
> 


-- 
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] [PATCH 6/7] docs/manual: update documentation about <pkg>_SOURCE, <pkg>_PATCH and <pkg>_EXTRA_DOWNLOADS

[Arnout Vandecappelle]

On 28/02/15 19:15, Thomas Petazzoni wrote:
> This commit updates the Buildroot manual for the variables used to
> indicate where to download the source code from:
> 
>  - It updates the description of <pkg>_SOURCE to make it clear that
>    Buildroot assume that the tarball is hosted at <pkg>_SITE.
> 
>  - It updates the description of <pkg>_PATCH to indicate that not only
>    file names (assumed to be hosted at <pkg>_SITE) can be used, but
>    also full URLs. This allows to match with what the current code is
>    doing.
> 
>  - It updates the description of <pkg>_EXTRA_DOWNLOADS to also
>    indicate that full URLs are now accepted, following the change made
>    in the previous commit.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  docs/manual/adding-packages-generic.txt | 32 ++++++++++++++++++--------------
>  1 file changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/docs/manual/adding-packages-generic.txt b/docs/manual/adding-packages-generic.txt
> index e3029ed..2e46d55 100644
> --- a/docs/manual/adding-packages-generic.txt
> +++ b/docs/manual/adding-packages-generic.txt
> @@ -204,18 +204,20 @@ information is (assuming the package name is +libfoo+) :
>      +LIBFOO_VERSION = cb9d6aa9429e838f0e54faa3d455bcbab5eef057+ +
>      +LIBFOO_VERSION = stable+
>  
> -* +LIBFOO_SOURCE+ may contain the name of the tarball of
> -  the package. If +HOST_LIBFOO_SOURCE+ is not specified, it
> -  defaults to +LIBFOO_SOURCE+. If none are specified, then
> -  the value is assumed to be
> -  +libfoo-$(LIBFOO_VERSION).tar.gz+. +
> -  Example: +LIBFOO_SOURCE = foobar-$(LIBFOO_VERSION).tar.bz2+
> +* +LIBFOO_SOURCE+ may contain the name of the tarball of the package,
> +  which Buildroot will use to download the tarball from
> +  +LIBFOO_SITE+. If +HOST_LIBFOO_SOURCE+ is not specified, it defaults
> +  to +LIBFOO_SOURCE+. If none are specified, then the value is assumed
> +  to be +libfoo-$(LIBFOO_VERSION).tar.gz+. + Example: +LIBFOO_SOURCE =

 The + was there to mark a line split, so there should be a newline after the +.

 Otherwise, looks good to me.

 Regards,
 Arnout

> +  foobar-$(LIBFOO_VERSION).tar.bz2+
>  
>  * +LIBFOO_PATCH+ may contain a space-separated list of patch file
> -  names, that will be downloaded from the same location as the tarball
> -  indicated in +LIBFOO_SOURCE+, and then applied to the package source
> -  code. If +HOST_LIBFOO_PATCH+ is not specified, it defaults to
> -  +LIBFOO_PATCH+. Note that patches that are included in Buildroot
> +  names, that Buildroot will download and apply to the package source
> +  code. If an entry contains +://+, then Buildroot will assume it is a
> +  full URL and download the patch from this location. Otherwise,
> +  Buildroot will assume that the patch should be downloaded from
> +  +LIBFOO_SITE+. If +HOST_LIBFOO_PATCH+ is not specified, it defaults
> +  to +LIBFOO_PATCH+. Note that patches that are included in Buildroot
>    itself use a different mechanism: all files of the form
>    +<packagename>-*.patch+ present in the package directory inside
>    Buildroot will be applied to the package after extraction (see
> @@ -246,10 +248,12 @@ information is (assuming the package name is +libfoo+) :
>      +LIBFOO_SITE=/opt/software/libfoo.tar.gz+ +
>      +LIBFOO_SITE=$(TOPDIR)/../src/libfoo/+
>  
> -* +LIBFOO_EXTRA_DOWNLOADS+ lists a number of additional files that
> -  Buildroot should download from +LIBFOO_SITE+ in addition to the main
> -  +LIBFOO_SOURCE+ (which usually is a tarball). Buildroot will not do
> -  anything with those additional files, except download files: it will
> +* +LIBFOO_EXTRA_DOWNLOADS+ is a space-separated list of additional
> +  files that Buildroot should download. If an entry contains +://+
> +  then Buildroot will assume it is a complete URL and will download
> +  the file using this URL. Otherwise, Buildroot will assume the file
> +  to be downloaded is located at +LIBFOO_SITE+. Buildroot will not do
> +  anything with those additional files, except download them: it will
>    be up to the package recipe to use them from +$(BR2_DL_DIR)+.
>  
>  * +LIBFOO_SITE_METHOD+ determines the method used to fetch or copy the
> 


-- 
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] [PATCH 7/7] perl: use <pkg>_EXTRA_DOWNLOADS

[Arnout Vandecappelle]

On 28/02/15 19:15, Thomas Petazzoni wrote:
> Instead of manually using the DOWNLOAD macro (which should remain an
> internal macro), this commit converts the Perl package to use
> <pkg>_EXTRA_DOWNLOADS, now that it has been extended to allow full
> URLs.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  package/perl/perl.mk | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/package/perl/perl.mk b/package/perl/perl.mk
> index 0636aba..cb99a98 100644
> --- a/package/perl/perl.mk
> +++ b/package/perl/perl.mk
> @@ -12,6 +12,11 @@ PERL_LICENSE = Artistic or GPLv1+
>  PERL_LICENSE_FILES = Artistic Copying README
>  PERL_INSTALL_STAGING = YES
>  
> +# We use the perlcross hack to cross-compile perl. It should
> +# be extracted over the perl sources, so we don't define that
> +# as a separate package. Instead, it is downloaded and extracted
> +# together with perl
> +
>  PERL_CROSS_VERSION = 0.9.4
>  PERL_CROSS_BASE_VERSION = 5.$(PERL_VERSION_MAJOR).1
>  # DO NOT refactor with the github helper (the result is not the same)
> @@ -20,15 +25,7 @@ PERL_CROSS_SOURCE = perl-$(PERL_CROSS_BASE_VERSION)-cross-$(PERL_CROSS_VERSION).
>  PERL_CROSS_OLD_POD = perl$(subst .,,$(PERL_CROSS_BASE_VERSION))delta.pod
>  PERL_CROSS_NEW_POD = perl$(subst .,,$(PERL_VERSION))delta.pod
>  
> -# We use the perlcross hack to cross-compile perl. It should
> -# be extracted over the perl sources, so we don't define that
> -# as a separate package. Instead, it is downloaded and extracted
> -# together with perl
> -
> -define PERL_CROSS_DOWNLOAD
> -	$(call DOWNLOAD,$(PERL_CROSS_SITE:/=)/$(PERL_CROSS_SOURCE))
> -endef
> -PERL_POST_DOWNLOAD_HOOKS += PERL_CROSS_DOWNLOAD
> +PERL_EXTRA_DOWNLOADS = $(PERL_CROSS_SITE)/$(PERL_CROSS_SOURCE)

 It makes more sense to me to put this just below the definition of
PERL_CROSS_SOURCE, so before the _POD definitions.

 Otherwise, looks good to me.

 Regards,
 Arnout

>  
>  define PERL_CROSS_EXTRACT
>  	$(call suitable-extractor,$(PERL_CROSS_SOURCE)) $(DL_DIR)/$(PERL_CROSS_SOURCE) | \
> 


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