[Buildroot] [PATCH v4 0/3] Cleanup trailing slashes from FOO_SITE

[Buildroot] [PATCH v4 0/3] Cleanup trailing slashes from FOO_SITE

[Luca Ceresoli]

Hi,

here's the respin of my patch set to clean up the form of FOO_SITE
variables by removing all trailing slashes.

Those slashes are useless and potentially armful, which led to introducing a
workaround to strip them:

    commit 1cbffbd015106ea90fe49e27433375769dc1035b
    Author: Shawn J. Goff <shawn7400@gmail.com>
    Date:   Fri Apr 12 09:40:30 2013 +0000

        eliminate double slashes caused by FOO_SITE ending in a slash
    
        When a FOO_SITE variable ends in a slash and gets joined with a
        FOO_SOURCE variable like $(FOO_SITE)/$(FOO_SOURCE), the resulting URI
        has a double slash. While double-slashes are fine in unix paths, they
        are reserved in URIs - the part following '//' must be an authority.
    
        Signed-off-by: Shawn J. Goff <shawn7400@gmail.com>
        Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>

v1 removed all these slashes but did not add a check to prevent some
more ones from slipping in new pagkages, and of course they did. v3
proposed a check, but taht was not ideal. So here's the new, shiny,
fully-make-based version that Arnout's headache produced! :)

Patches from v3 have already be merged that remove the slashes in
existing packages and to state in tha manual that trailing slashes are
incorrect. Here's what's left.

Patch 1 fixes toolchain-external, which introduces a trailing slash on
its own. Patch 2 adds a the check to block trailing slashes before
building. Finally patch 3 removes the workaround that is not needed
anymore.

This work has been inspired by this comment from Arnout Vandecappelle [1]:
>>>>>>>>>>>> >>>>>> >>> >> > +else
>>>>>>>>>>>> >>>>>> >>> >> > +ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
>>>>>>>>>>>> >>>>>> >>> >> > +          $(call DOWNLOAD,$$($(2)_ACTUAL_SOURCE_SITE:/=)/$$($(2)$($(PKG)_SITE:/=)_ACTUAL_SOURCE_TARBALL))
>>>> >> > > 
>>>> >> > >  I think the $($(PKG)_SITE:/=) construct was just introduced because for some
>>>> >> > > packages, the _SITE ends with a / and that should be stripped, and we were too
>>>> >> > > lazy to fix the packages. Hm, looks like all the the external toolchain _SITEs
>>>> >> > > end with a /...

[0] http://lists.busybox.net/pipermail/buildroot/2015-March/121502.html
[1] https://patchwork.ozlabs.org/patch/424980/

Luca Ceresoli (3):
  toolchain-external: strip trailing slash from autogenerated FOO_SITE
  pkg-generic: prevent _SITE URLs with a trailing slash
  download: get rid of trailing slash removal hack for FOO_SITE

 package/pkg-generic.mk                             | 6 +++++-
 toolchain/toolchain-external/toolchain-external.mk | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

-- 
1.9.1

[Buildroot] [PATCH v4 1/3] toolchain-external: strip trailing slash from autogenerated FOO_SITE

[Luca Ceresoli]

Trailing slashes are going to be declared illegal from FOO_SITE
variables.

But Buildroot internally generates such a variable when using a custom
external toolchain (i.e. BR2_TOOLCHAIN_EXTERNAL_CUSTOM). This is
because TOOLCHAIN_EXTERNAL_SITE is set to
$(dir $(call qstrip,$(BR2_TOOLCHAIN_EXTERNAL_URL))), and $(dir)
leaves a trailing slash.

Fix it using patsubst, just like linux and the bootloaders do.

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Arnout Vandecappelle <arnout@mind.be>

---

This patch is new in v4.
---
 toolchain/toolchain-external/toolchain-external.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/toolchain/toolchain-external/toolchain-external.mk b/toolchain/toolchain-external/toolchain-external.mk
index 79afdaa..4a3a3d0 100644
--- a/toolchain/toolchain-external/toolchain-external.mk
+++ b/toolchain/toolchain-external/toolchain-external.mk
@@ -416,7 +416,7 @@ endif
 TOOLCHAIN_EXTERNAL_SOURCE = arc_gnu_2014.12_prebuilt_uclibc_$(TOOLCHAIN_EXTERNAL_SYNOPSYS_ENDIANESS)_$(TOOLCHAIN_EXTERNAL_SYNOPSYS_CORE)_linux_install.tar.gz
 else
 # Custom toolchain
-TOOLCHAIN_EXTERNAL_SITE = $(dir $(call qstrip,$(BR2_TOOLCHAIN_EXTERNAL_URL)))
+TOOLCHAIN_EXTERNAL_SITE = $(patsubst %/,%,$(dir $(call qstrip,$(BR2_TOOLCHAIN_EXTERNAL_URL))))
 TOOLCHAIN_EXTERNAL_SOURCE = $(notdir $(call qstrip,$(BR2_TOOLCHAIN_EXTERNAL_URL)))
 # We can't check hashes for custom downloaded toolchains
 BR_NO_CHECK_HASH_FOR += $(TOOLCHAIN_EXTERNAL_SOURCE)
-- 
1.9.1

[Buildroot] [PATCH v4 2/3] pkg-generic: prevent _SITE URLs with a trailing slash

[Luca Ceresoli]

A trailing slash in FOO_SITE is useless, since Buildroot automatically adds
a slash between FOO_SITE and the filename as appropriate.

Moreover it is potentially harmful, which led to introducing a workaround
to strip them:

    commit 1cbffbd015106ea90fe49e27433375769dc1035b
    Author: Shawn J. Goff <shawn7400@gmail.com>
    Date:   Fri Apr 12 09:40:30 2013 +0000

        eliminate double slashes caused by FOO_SITE ending in a slash

        When a FOO_SITE variable ends in a slash and gets joined with a
        FOO_SOURCE variable like $(FOO_SITE)/$(FOO_SOURCE), the resulting URI
        has a double slash. While double-slashes are fine in unix paths, they
        are reserved in URIs - the part following '//' must be an authority.

So let's ban trailing slashes entirely. They have all been removed in
a 7b0e757fb85fd, now add a check to error out loudly in case a new one
is added.

Example commands to test this check:

  $ make busybox-dirclean busybox-source
  rm -Rf /home/murray/devel/buildroot/output/build/busybox-1.23.2
  busybox-1.23.2.tar.bz2: OK (md5: 7925683d7dd105aabe9b6b618d48cc73)
  busybox-1.23.2.tar.bz2: OK (sha1: 7f37193cb249f27630e0b2a2c6c9bbb7b1d24c16)
  $
  $ make BUSYBOX_SITE=http://www.busybox.net/downloads/ busybox-dirclean busybox-source
  rm -Rf /home/murray/devel/buildroot/output/build/busybox-1.23.2
  BUSYBOX_SITE (http://www.busybox.net/downloads/) cannot have a trailing slash
  make[1]: *** [/home/murray/devel/buildroot/output/build/busybox-1.23.2/.stamp_downloaded] Error 1
  make: *** [_all] Error 2
  $

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Baruch Siach <baruch@tkos.co.il>

---

Changed v3 -> v4:
 - Completely reimplement with make code, not shell code (Arnout actually
   wrote this patch, I was the typing monkey).
 - Slightly improve commit log and fix typos (Baruch, me).

Changed v2 -> v3:
 - Simplify the check logic removing a useless "&& true".
 - Expand commit log message. This is a pretty important change, so it
   should be well justified.
---
 package/pkg-generic.mk | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 5201fca..de2fb07 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -847,6 +847,10 @@ endif
 	$(1)-source \
 	$(1)-source-check
 
+ifeq ($$(patsubst %/,ERROR,$$($(2)_SITE)),ERROR)
+$$(error $(2)_SITE ($$($(2)_SITE)) cannot have a trailing slash)
+endif
+
 endif # $(2)_KCONFIG_VAR
 endef # inner-generic-package
 
-- 
1.9.1

[Buildroot] [PATCH v4 3/3] download: get rid of trailing slash removal hack for FOO_SITE

[Luca Ceresoli]

Not needed anymore since the URLs have been cleared, the manual states
they are illegal and a check has been introduced to notice future
mistakes.

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>

---
No changes in v4.

Changes v2 -> v3:
 - move after the commit on docs/manual, update commit message.

Changes v1 -> v2:
 - redo on current master.
---
 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 de2fb07..3942a89 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -388,7 +388,7 @@ endif
 $(2)_ALL_DOWNLOADS = \
 	$$(foreach p,$$($(2)_SOURCE) $$($(2)_PATCH) $$($(2)_EXTRA_DOWNLOADS),\
 		$$(if $$(findstring ://,$$(p)),$$(p),\
-			$$($(2)_SITE:/=)/$$(p)))
+			$$($(2)_SITE)/$$(p)))
 
 ifndef $(2)_SITE
  ifdef $(3)_SITE
-- 
1.9.1

[Buildroot] [PATCH v4 1/3] toolchain-external: strip trailing slash from autogenerated FOO_SITE

[Arnout Vandecappelle]

On 03-10-15 18:22, Luca Ceresoli wrote:
> Trailing slashes are going to be declared illegal from FOO_SITE
> variables.
> 
> But Buildroot internally generates such a variable when using a custom
> external toolchain (i.e. BR2_TOOLCHAIN_EXTERNAL_CUSTOM). This is
> because TOOLCHAIN_EXTERNAL_SITE is set to
> $(dir $(call qstrip,$(BR2_TOOLCHAIN_EXTERNAL_URL))), and $(dir)
> leaves a trailing slash.
> 
> Fix it using patsubst, just like linux and the bootloaders do.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>

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

 Regards,
 Arnout

> 
> ---
> 
> This patch is new in v4.
> ---
>  toolchain/toolchain-external/toolchain-external.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/toolchain/toolchain-external/toolchain-external.mk b/toolchain/toolchain-external/toolchain-external.mk
> index 79afdaa..4a3a3d0 100644
> --- a/toolchain/toolchain-external/toolchain-external.mk
> +++ b/toolchain/toolchain-external/toolchain-external.mk
> @@ -416,7 +416,7 @@ endif
>  TOOLCHAIN_EXTERNAL_SOURCE = arc_gnu_2014.12_prebuilt_uclibc_$(TOOLCHAIN_EXTERNAL_SYNOPSYS_ENDIANESS)_$(TOOLCHAIN_EXTERNAL_SYNOPSYS_CORE)_linux_install.tar.gz
>  else
>  # Custom toolchain
> -TOOLCHAIN_EXTERNAL_SITE = $(dir $(call qstrip,$(BR2_TOOLCHAIN_EXTERNAL_URL)))
> +TOOLCHAIN_EXTERNAL_SITE = $(patsubst %/,%,$(dir $(call qstrip,$(BR2_TOOLCHAIN_EXTERNAL_URL))))
>  TOOLCHAIN_EXTERNAL_SOURCE = $(notdir $(call qstrip,$(BR2_TOOLCHAIN_EXTERNAL_URL)))
>  # We can't check hashes for custom downloaded toolchains
>  BR_NO_CHECK_HASH_FOR += $(TOOLCHAIN_EXTERNAL_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:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v4 2/3] pkg-generic: prevent _SITE URLs with a trailing slash

[Arnout Vandecappelle]

On 03-10-15 18:22, Luca Ceresoli wrote:
> A trailing slash in FOO_SITE is useless, since Buildroot automatically adds
> a slash between FOO_SITE and the filename as appropriate.
> 
> Moreover it is potentially harmful, which led to introducing a workaround
> to strip them:
> 
>     commit 1cbffbd015106ea90fe49e27433375769dc1035b
>     Author: Shawn J. Goff <shawn7400@gmail.com>
>     Date:   Fri Apr 12 09:40:30 2013 +0000
> 
>         eliminate double slashes caused by FOO_SITE ending in a slash
> 
>         When a FOO_SITE variable ends in a slash and gets joined with a
>         FOO_SOURCE variable like $(FOO_SITE)/$(FOO_SOURCE), the resulting URI
>         has a double slash. While double-slashes are fine in unix paths, they
>         are reserved in URIs - the part following '//' must be an authority.
> 
> So let's ban trailing slashes entirely. They have all been removed in
> a 7b0e757fb85fd, now add a check to error out loudly in case a new one
> is added.
> 
> Example commands to test this check:
> 
>   $ make busybox-dirclean busybox-source
>   rm -Rf /home/murray/devel/buildroot/output/build/busybox-1.23.2
>   busybox-1.23.2.tar.bz2: OK (md5: 7925683d7dd105aabe9b6b618d48cc73)
>   busybox-1.23.2.tar.bz2: OK (sha1: 7f37193cb249f27630e0b2a2c6c9bbb7b1d24c16)
>   $
>   $ make BUSYBOX_SITE=http://www.busybox.net/downloads/ busybox-dirclean busybox-source
>   rm -Rf /home/murray/devel/buildroot/output/build/busybox-1.23.2
>   BUSYBOX_SITE (http://www.busybox.net/downloads/) cannot have a trailing slash
>   make[1]: *** [/home/murray/devel/buildroot/output/build/busybox-1.23.2/.stamp_downloaded] Error 1
>   make: *** [_all] Error 2
>   $
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Baruch Siach <baruch@tkos.co.il>

 I'm not going to give it my Rev-by tag since I partly wrote it myself but it
does look good to me.

> 
> ---
> 
> Changed v3 -> v4:
>  - Completely reimplement with make code, not shell code (Arnout actually
>    wrote this patch, I was the typing monkey).
>  - Slightly improve commit log and fix typos (Baruch, me).
> 
> Changed v2 -> v3:
>  - Simplify the check logic removing a useless "&& true".
>  - Expand commit log message. This is a pretty important change, so it
>    should be well justified.
> ---
>  package/pkg-generic.mk | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 5201fca..de2fb07 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -847,6 +847,10 @@ endif
>  	$(1)-source \
>  	$(1)-source-check
>  
> +ifeq ($$(patsubst %/,ERROR,$$($(2)_SITE)),ERROR)

 Note that by subsituting with ERROR instead of empty, we make sure that we
don't error when _SITE is empty.

 Regards,
 Arnout

> +$$(error $(2)_SITE ($$($(2)_SITE)) cannot have a trailing slash)
> +endif
> +
>  endif # $(2)_KCONFIG_VAR
>  endef # inner-generic-package
>  
> 


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v4 3/3] download: get rid of trailing slash removal hack for FOO_SITE

[Arnout Vandecappelle]

On 03-10-15 18:22, Luca Ceresoli wrote:
> Not needed anymore since the URLs have been cleared, the manual states
> they are illegal and a check has been introduced to notice future
> mistakes.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>

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


 Regards,
 Arnout

> 
> ---
> No changes in v4.
> 
> Changes v2 -> v3:
>  - move after the commit on docs/manual, update commit message.
> 
> Changes v1 -> v2:
>  - redo on current master.
> ---
>  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 de2fb07..3942a89 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -388,7 +388,7 @@ endif
>  $(2)_ALL_DOWNLOADS = \
>  	$$(foreach p,$$($(2)_SOURCE) $$($(2)_PATCH) $$($(2)_EXTRA_DOWNLOADS),\
>  		$$(if $$(findstring ://,$$(p)),$$(p),\
> -			$$($(2)_SITE:/=)/$$(p)))
> +			$$($(2)_SITE)/$$(p)))
>  
>  ifndef $(2)_SITE
>   ifdef $(3)_SITE
> 


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v4 2/3] pkg-generic: prevent _SITE URLs with a trailing slash

[Luca Ceresoli]

Dear Arnout,

thanks for the series review.

Il 04/10/2015 14:38, Arnout Vandecappelle ha scritto:
> On 03-10-15 18:22, Luca Ceresoli wrote:
>> A trailing slash in FOO_SITE is useless, since Buildroot automatically adds
>> a slash between FOO_SITE and the filename as appropriate.
>>
>> Moreover it is potentially harmful, which led to introducing a workaround
>> to strip them:
>>
>>      commit 1cbffbd015106ea90fe49e27433375769dc1035b
>>      Author: Shawn J. Goff <shawn7400@gmail.com>
>>      Date:   Fri Apr 12 09:40:30 2013 +0000
>>
>>          eliminate double slashes caused by FOO_SITE ending in a slash
>>
>>          When a FOO_SITE variable ends in a slash and gets joined with a
>>          FOO_SOURCE variable like $(FOO_SITE)/$(FOO_SOURCE), the resulting URI
>>          has a double slash. While double-slashes are fine in unix paths, they
>>          are reserved in URIs - the part following '//' must be an authority.
>>
>> So let's ban trailing slashes entirely. They have all been removed in
>> a 7b0e757fb85fd, now add a check to error out loudly in case a new one
>> is added.
>>
>> Example commands to test this check:
>>
>>    $ make busybox-dirclean busybox-source
>>    rm -Rf /home/murray/devel/buildroot/output/build/busybox-1.23.2
>>    busybox-1.23.2.tar.bz2: OK (md5: 7925683d7dd105aabe9b6b618d48cc73)
>>    busybox-1.23.2.tar.bz2: OK (sha1: 7f37193cb249f27630e0b2a2c6c9bbb7b1d24c16)
>>    $
>>    $ make BUSYBOX_SITE=http://www.busybox.net/downloads/ busybox-dirclean busybox-source
>>    rm -Rf /home/murray/devel/buildroot/output/build/busybox-1.23.2
>>    BUSYBOX_SITE (http://www.busybox.net/downloads/) cannot have a trailing slash
>>    make[1]: *** [/home/murray/devel/buildroot/output/build/busybox-1.23.2/.stamp_downloaded] Error 1
>>    make: *** [_all] Error 2
>>    $
>>
>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
>> Cc: Arnout Vandecappelle <arnout@mind.be>
>> Cc: Baruch Siach <baruch@tkos.co.il>
>
>   I'm not going to give it my Rev-by tag since I partly wrote it myself but it
> does look good to me.

Should I add your Signed-off-by and resend?

-- 
Luca

[Buildroot] [PATCH v4 0/3] Cleanup trailing slashes from FOO_SITE

[Thomas Petazzoni]

Dear Luca Ceresoli,

On Sat,  3 Oct 2015 19:22:15 +0200, Luca Ceresoli wrote:

> Luca Ceresoli (3):
>   toolchain-external: strip trailing slash from autogenerated FOO_SITE
>   pkg-generic: prevent _SITE URLs with a trailing slash
>   download: get rid of trailing slash removal hack for FOO_SITE

All applied, thanks!

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