Skip to contentSkip to conversation

Improvements to the download logic

PATCH New Thomas Petazzoni 4 participants 27 messages Feb 28, 2015 – Mar 22, 2015

lore
Share & tools
lore

Thomas Petazzoni

author
#0 lore raw

cover Improvements to the download logic

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(-)
Signature · 1 line
2.1.0
author
#1 lore raw

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

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 <[email protected]>
---
 linux/linux.mk | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)
linux/linux.mk +9 -16
index fc90fc5..d591e1e 100644
@@ -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
Signature · 1 line
2.1.0
#2 lore raw

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

Hi Thomas,

Quoted 1 line · Thomas Petazzoni

+# We really on the generic package infrastructure to download and

s/really/rely/

linux/linux.mk endif · line 46

Quoted 3 lines
linux/linux.mk +3 -0
+# 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

Signature · 3 lines
     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 -
#3 lore raw

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

In reply to Thomas Petazzoni

Hello Thomas,

[snip]

linux/linux.mk else · line 155

Quoted 4 lines
linux/linux.mk +3 -0
+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

author
#4 lore raw

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

Dear Romain Naour,

Quoted 5 lines · Romain Naour

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.

Quoted 2 lines

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

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#5 lore raw

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

Hello Thomas,

Le 15/03/2015 16:14, Thomas Petazzoni a ?crit :

Quoted 11 lines

Dear Romain Naour,

Quoted 5 lines · Romain Naour

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

Quoted 11 lines
Quoted 3 lines

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

Quoted 5 lines

Best regards,

Thomas

#6 lore raw

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

In reply to Thomas Petazzoni
Quoted 9 lines · Thomas Petazzoni
Quoted 2 lines
Quoted 2 lines

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

Signature · 6 lines
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
#7 lore raw

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

In reply to Thomas Petazzoni

Hi Thomas,

Le 28/02/2015 19:15, Thomas Petazzoni a ?crit :

Quoted 7 lines

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.

Quoted 5 lines

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

Quoted 21 lines

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.

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

---

Reviewed-by Romain Naour <[email protected]>

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 endif · line 43

Quoted 12 lines
 linux/linux.mk | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)
linux/linux.mk +1 -0
index fc90fc5..d591e1e 100644
@@ -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)

Quoted 42 lines
linux/linux.mk +8 -16
+# 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
#8 lore raw

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

In reply to Thomas Petazzoni
Quoted 33 lines · 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.

Quoted 1 line
Signed-off-by Thomas Petazzoni <[email protected]>
Acked-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>

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

Signature · 6 lines
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
author
#9 lore raw

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

In reply to 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 <[email protected]>
---
 package/pkg-generic.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
package/pkg-generic.mk +1 -1
index 1b09955..1482194 100644
@@ -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 ; \
Signature · 1 line
2.1.0
#10 lore raw

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

Hi Thomas,

Le 28/02/2015 19:15, Thomas Petazzoni a ?crit :

Quoted 11 lines

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.

Quoted 5 lines

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

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

---

Reviewed-by Romain Naour <[email protected]>
Quoted 17 lines
 package/pkg-generic.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
package/pkg-generic.mk +1 -1
index 1b09955..1482194 100644
@@ -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 ; \
#11 lore raw

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

In reply to Thomas Petazzoni
Quoted 11 lines · 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.

Quoted 4 lines

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

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

Regards,
Arnout

[snip]

Signature · 6 lines
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
author
#12 lore raw

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

In reply to 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 <[email protected]>
---
 package/pkg-generic.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
package/pkg-generic.mk +1 -1
index 1482194..0098b29 100644
@@ -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 ; \
Signature · 1 line
2.1.0
#13 lore raw

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

Hi Thomas,

Le 28/02/2015 19:15, Thomas Petazzoni a ?crit :

Quoted 11 lines

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

---

Reviewed-by Romain Naour <[email protected]>
Quoted 17 lines
 package/pkg-generic.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
package/pkg-generic.mk +1 -1
index 1482194..0098b29 100644
@@ -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 ; \
#14 lore raw

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

In reply to Thomas Petazzoni
Quoted 10 lines · 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 <[email protected]>
Reviewed-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>

Regards,
Arnout
[snip[

Signature · 6 lines
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
author
#15 lore raw

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

In reply to 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 <[email protected]>
---
 package/pkg-generic.mk | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)
package/pkg-generic.mk +6 -10
index 0098b29..7a9da43 100644
@@ -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))
Signature · 1 line
2.1.0
#16 lore raw

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

Hi Thomas,

Le 28/02/2015 19:15, Thomas Petazzoni a ?crit :

Quoted 11 lines

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

---

Reviewed-by Romain Naour <[email protected]>
Quoted 31 lines
 package/pkg-generic.mk | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)
package/pkg-generic.mk +6 -10
index 0098b29..7a9da43 100644
@@ -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))
#17 lore raw

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

In reply to Thomas Petazzoni
Quoted 10 lines · 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 <[email protected]>
Reviewed-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>

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]

Signature · 6 lines
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
author
#18 lore raw

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

In reply to 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 <[email protected]>
---
 package/pkg-generic.mk | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
package/pkg-generic.mk +6 -1
index 7a9da43..85b22de 100644
@@ -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)),\
Signature · 1 line
2.1.0
#19 lore raw

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

Hi Thomas,

Le 28/02/2015 19:15, Thomas Petazzoni a ?crit :

Quoted 10 lines

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

---

Reviewed-by Romain Naour <[email protected]>
Quoted 22 lines
 package/pkg-generic.mk | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
package/pkg-generic.mk +6 -1
index 7a9da43..85b22de 100644
@@ -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)),\
#20 lore raw

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

In reply to Thomas Petazzoni
Quoted 19 lines · 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 <[email protected]>
---
 package/pkg-generic.mk | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
package/pkg-generic.mk +0 -0
index 7a9da43..85b22de 100644
@@ -81,7 +81,12 @@ ifeq ($(DL_MODE),DOWNLOAD)
 	done

package/pkg-generic.mk ifeq ($(DL_MODE),DOWNLOAD) · line 90

Quoted 10 lines
package/pkg-generic.mk +6 -1
 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

Quoted 3 lines
package/pkg-generic.mk +0 -0
 		$(if $(findstring ://,$(p)),\
 			$(call DOWNLOAD,$(p)),\
Signature · 6 lines
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
author
#21 lore raw

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

In reply to 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 <[email protected]>
---
 docs/manual/adding-packages-generic.txt | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)
docs/manual/adding-packages-generic.txt +18 -14
index e3029ed..2e46d55 100644
@@ -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
Signature · 1 line
2.1.0
#22 lore raw

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

Hi Thomas,

Le 28/02/2015 19:15, Thomas Petazzoni a ?crit :

Quoted 11 lines

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.

Quoted 6 lines

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

---

Reviewed-by Romain Naour <[email protected]>
Quoted 56 lines
 docs/manual/adding-packages-generic.txt | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)
docs/manual/adding-packages-generic.txt +18 -14
index e3029ed..2e46d55 100644
@@ -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
#23 lore raw

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

In reply to Thomas Petazzoni
Quoted 29 lines · 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 <[email protected]>
---
 docs/manual/adding-packages-generic.txt | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)
docs/manual/adding-packages-generic.txt +0 -1
index e3029ed..2e46d55 100644
@@ -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

docs/manual/adding-packages-generic.txt information is (assuming the package name is +libfoo+) : · line 211

Quoted 10 lines
docs/manual/adding-packages-generic.txt +5 -5
-  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

Quoted 34 lines
docs/manual/adding-packages-generic.txt +13 -8
+  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
Signature · 6 lines
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
author
#24 lore raw

PATCH 7/7 perl: use <pkg>_EXTRA_DOWNLOADS

In reply to 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 <[email protected]>
---
 package/perl/perl.mk | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)
package/perl/perl.mk +6 -9
index 0636aba..cb99a98 100644
@@ -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) | \
Signature · 1 line
2.1.0
#25 lore raw

PATCH 7/7 perl: use <pkg>_EXTRA_DOWNLOADS

Hi Thomas,

Le 28/02/2015 19:15, Thomas Petazzoni a ?crit :

Quoted 7 lines

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

---

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 <[email protected]>
Quoted 37 lines
 package/perl/perl.mk | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)
package/perl/perl.mk +6 -9
index 0636aba..cb99a98 100644
@@ -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) | \
#26 lore raw

PATCH 7/7 perl: use <pkg>_EXTRA_DOWNLOADS

In reply to Thomas Petazzoni
Quoted 30 lines · 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 <[email protected]>
---
 package/perl/perl.mk | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)
package/perl/perl.mk +5 -0
index 0636aba..cb99a98 100644
@@ -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

package/perl/perl.mk PERL_CROSS_SOURCE = perl-$(PERL_CROSS_BASE_VERSION)-cross-$(PERL_CROSS_VERSION). · line 28

Quoted 10 lines
package/perl/perl.mk +1 -9
-# 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

Quoted 4 lines
package/perl/perl.mk +0 -0
 define PERL_CROSS_EXTRACT
 	$(call suitable-extractor,$(PERL_CROSS_SOURCE)) $(DL_DIR)/$(PERL_CROSS_SOURCE) | \
Signature · 6 lines
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