[Buildroot] [PATCH 0/4] pkg-infra: differentiate remote and local tarball filenames (branch yem/download)

[Buildroot] [PATCH 0/4] pkg-infra: differentiate remote and local tarball filenames (branch yem/download)

[Yann E. MORIN]

Hello All!

This series introduces a new variable, FOO_UPSTREAM_SOURCE, so as to be
able to differentiate remote and local tarball filenames.

This is needed when upstream tarballs are not named the way we expect them
to be. For example, the GitHub API only provides a request like:
    GET /repos/:owner/:repo/:archive_format/:ref

So far, we're not using this API, and directly use a non-stable scheme
to retrieve the tarballs. That scheme has proven to break from time to
time, so we'll soon need to switch to the API, which is guaranteed to be
stable.

So, we need to be able to differentiate the filename remote knows the
tarball by, and the local filename we want to save that tarball as.

Changes to the GitHub helper will come in a later series, because it is
a bit more involved, and still needs some ironing out. Alternate forges
may also get added in the future, which would require such a feature.

Regards,
Yann E. MORIN.


The following changes since commit f0bd293ffd227908e1d09d61e08486d5259b5cfb:

  host-qemu: fix install (2014-11-15 14:28:45 +0100)

are available in the git repository at:

  git://git.busybox.net/~ymorin/git/buildroot yem/download

for you to fetch changes up to 51470d856dde581d14c2e20bad669cd36332ae5b:

  docs/manual: document the new variable FOO_UPSTREAM_SOURCE (2014-11-15 16:55:39 +0100)

----------------------------------------------------------------
Yann E. MORIN (4):
      pkg-infra: always specify the local tarball name when calling DOWNLOAD
      pkg-infra: squash DOWNLOAD_INNER into DOWNLOAD
      pkg-infra: differentiate remote tarball name from local filename
      docs/manual: document the new variable FOO_UPSTREAM_SOURCE

 docs/manual/adding-packages-generic.txt | 11 +++++++++++
 package/pkg-download.mk                 |  5 +----
 package/pkg-generic.mk                  |  6 +++++-
 3 files changed, 17 insertions(+), 5 deletions(-)

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

[Buildroot] [PATCH 1/4] pkg-infra: always specify the local tarball name when calling DOWNLOAD

[Yann E. MORIN]

This will be needed to be able to differentiate the upstream filename
from the local filename, which may differ in some cases.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
Cc: Samuel Martin <s.martin49@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Peter Korsgaard <jacmet@uclibc.org>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
---
 package/pkg-download.mk | 3 ++-
 package/pkg-generic.mk  | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index f3409bd..7e932e9 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -245,13 +245,14 @@ endef
 # 3) BR2_BACKUP_SITE if enabled, unless BR2_PRIMARY_SITE_ONLY is set
 #
 # Argument 1 is the source location
+# Argument 2 is the local filename, without any path component
 #
 # E.G. use like this:
 # $(call DOWNLOAD,$(FOO_SITE))
 ################################################################################
 
 define DOWNLOAD
-	$(call DOWNLOAD_INNER,$(1),$(notdir $(1)))
+	$(call DOWNLOAD_INNER,$(1),$(2))
 endef
 
 define DOWNLOAD_INNER
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 9643a30..543cb60 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -84,7 +84,7 @@ ifeq ($(DL_MODE),DOWNLOAD)
 		done ; \
 	fi
 endif
-	$(if $($(PKG)_SOURCE),$(call DOWNLOAD,$($(PKG)_SITE:/=)/$($(PKG)_SOURCE)))
+	$(if $($(PKG)_SOURCE),$(call DOWNLOAD,$($(PKG)_SITE:/=)/$($(PKG)_SOURCE),$($(PKG)_SOURCE)))
 	$(foreach p,$($(PKG)_EXTRA_DOWNLOADS),$(call DOWNLOAD,$($(PKG)_SITE:/=)/$(p))$(sep))
 	$(foreach p,$($(PKG)_PATCH),\
 		$(if $(findstring ://,$(p)),\
-- 
1.9.1

[Buildroot] [PATCH 2/4] pkg-infra: squash DOWNLOAD_INNER into DOWNLOAD

[Yann E. MORIN]

We no longer have a need for a DOWNLOAD wrapper around DOWNLOAD_INNER,
since all it does is call DOWNLOAD_INNER with the exact same arguments
it was called with.

So, just squash the two macros into a single one.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
---
 package/pkg-download.mk | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 7e932e9..e10ae36 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -252,10 +252,6 @@ endef
 ################################################################################
 
 define DOWNLOAD
-	$(call DOWNLOAD_INNER,$(1),$(2))
-endef
-
-define DOWNLOAD_INNER
 	$(Q)if test -n "$(call qstrip,$(BR2_PRIMARY_SITE))" ; then \
 		case "$(call geturischeme,$(BR2_PRIMARY_SITE))" in \
 			scp) $(call $(DL_MODE)_SCP,$(BR2_PRIMARY_SITE)/$(2),$(2)) && exit ;; \
-- 
1.9.1

[Buildroot] [PATCH 3/4] pkg-infra: differentiate remote tarball name from local filename

[Yann E. MORIN]

Some upstreams may use a naming scheme that does not fit well in how
Buildroot wants to handle filenames.

For example, GitHub used to have a scheme like:
    https://github.com/USER/REPO/archive/VERSION.tar.gz

which means we would have a local file named VERSION.tar.gz, when we
want to have PKG-VERSION.tar.gz

Other forges are also known to have similar schemes. Google Code, for
example, may also use similarly named files.

Introduce a new variable, FOO_UPSTREAM_SOURCE, which the package may set
in that case. If not set, it defaults to FOO_SOURCE.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
Cc: Samuel Martin <s.martin49@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Peter Korsgaard <jacmet@uclibc.org>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
---
 package/pkg-generic.mk | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 543cb60..30bce59 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -84,7 +84,7 @@ ifeq ($(DL_MODE),DOWNLOAD)
 		done ; \
 	fi
 endif
-	$(if $($(PKG)_SOURCE),$(call DOWNLOAD,$($(PKG)_SITE:/=)/$($(PKG)_SOURCE),$($(PKG)_SOURCE)))
+	$(if $($(PKG)_SOURCE),$(call DOWNLOAD,$($(PKG)_SITE:/=)/$($(PKG)_UPSTREAM_SOURCE),$($(PKG)_SOURCE)))
 	$(foreach p,$($(PKG)_EXTRA_DOWNLOADS),$(call DOWNLOAD,$($(PKG)_SITE:/=)/$(p))$(sep))
 	$(foreach p,$($(PKG)_PATCH),\
 		$(if $(findstring ://,$(p)),\
@@ -361,6 +361,10 @@ ifndef $(2)_SOURCE
  endif
 endif
 
+ifndef $(2)_UPSTREAM_SOURCE
+  $(2)_UPSTREAM_SOURCE = $$($(2)_SOURCE)
+endif
+
 ifndef $(2)_PATCH
  ifdef $(3)_PATCH
   $(2)_PATCH = $$($(3)_PATCH)
-- 
1.9.1

[Buildroot] [PATCH 4/4] docs/manual: document the new variable FOO_UPSTREAM_SOURCE

[Yann E. MORIN]

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
Cc: Samuel Martin <s.martin49@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Peter Korsgaard <jacmet@uclibc.org>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
---
 docs/manual/adding-packages-generic.txt | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/docs/manual/adding-packages-generic.txt b/docs/manual/adding-packages-generic.txt
index 67a7453..5295681 100644
--- a/docs/manual/adding-packages-generic.txt
+++ b/docs/manual/adding-packages-generic.txt
@@ -211,6 +211,17 @@ information is (assuming the package name is +libfoo+) :
   +libfoo-$(LIBFOO_VERSION).tar.gz+. +
   Example: +LIBFOO_SOURCE = foobar-$(LIBFOO_VERSION).tar.bz2+
 
+* +LIBFOO_UPSTREAM_SOURCE+ may contain the name of the tarball as
+  known by upstream. If +HOST_LIBFOO_UPSTREAM_SOURCE+ is not specified,
+  it defaults to +LIBFOO_UPSTREAM_SOURCE+. If none is specified, then
+  the value is assumed to be the same as +LIBFOO_SOURCE+.
+
+.Note
++LIBFOO_SOURCE+ is the filename we want to save the tarball as, while
++LIBFOO_UPSTREAM_SOURCE+ is the name by which upstream names that
+tarball. The two are usually the same, except in very rare cases, so you
+normally should not have to set +LIBFOO_UPSTREAM_SOURCE+.
+
 * +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
-- 
1.9.1

[Buildroot] [PATCH 4/4] docs/manual: document the new variable FOO_UPSTREAM_SOURCE

[Baruch Siach]

Hi Yann,

On Sat, Nov 15, 2014 at 05:19:29PM +0100, Yann E. MORIN wrote:
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
> Cc: Samuel Martin <s.martin49@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Peter Korsgaard <jacmet@uclibc.org>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> ---
>  docs/manual/adding-packages-generic.txt | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/docs/manual/adding-packages-generic.txt b/docs/manual/adding-packages-generic.txt
> index 67a7453..5295681 100644
> --- a/docs/manual/adding-packages-generic.txt
> +++ b/docs/manual/adding-packages-generic.txt
> @@ -211,6 +211,17 @@ information is (assuming the package name is +libfoo+) :
>    +libfoo-$(LIBFOO_VERSION).tar.gz+. +
>    Example: +LIBFOO_SOURCE = foobar-$(LIBFOO_VERSION).tar.bz2+
>  
> +* +LIBFOO_UPSTREAM_SOURCE+ may contain the name of the tarball as
> +  known by upstream. If +HOST_LIBFOO_UPSTREAM_SOURCE+ is not specified,
> +  it defaults to +LIBFOO_UPSTREAM_SOURCE+. If none is specified, then
> +  the value is assumed to be the same as +LIBFOO_SOURCE+.
> +
> +.Note
> ++LIBFOO_SOURCE+ is the filename we want to save the tarball as, while
> ++LIBFOO_UPSTREAM_SOURCE+ is the name by which upstream names that
> +tarball. The two are usually the same, except in very rare cases, so you
> +normally should not have to set +LIBFOO_UPSTREAM_SOURCE+.

I think it is worth clarifying which name the .hash file should refer in this 
case. I guess this is LIBFOO_SOURCE, isn't it?.

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 0/4] pkg-infra: differentiate remote and local tarball filenames (branch yem/download)

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Sat, 15 Nov 2014 17:19:24 +0100, Yann E. MORIN wrote:

> This series introduces a new variable, FOO_UPSTREAM_SOURCE, so as to be
> able to differentiate remote and local tarball filenames.
> 
> This is needed when upstream tarballs are not named the way we expect them
> to be. For example, the GitHub API only provides a request like:
>     GET /repos/:owner/:repo/:archive_format/:ref
> 
> So far, we're not using this API, and directly use a non-stable scheme
> to retrieve the tarballs. That scheme has proven to break from time to
> time, so we'll soon need to switch to the API, which is guaranteed to be
> stable.
> 
> So, we need to be able to differentiate the filename remote knows the
> tarball by, and the local filename we want to save that tarball as.
> 
> Changes to the GitHub helper will come in a later series, because it is
> a bit more involved, and still needs some ironing out. Alternate forges
> may also get added in the future, which would require such a feature.

Not directly related, but somewhat related: I have always though that
our split between <foo>_SITE and <foo>_SOURCE was a bit stupid. Why
don't we simply give a full URL instead of splitting that between SITE
and SOURCE ?

The way we do things today also forces the things in <foo>_PATCH and
<foo>_EXTRA_DOWNLOADS to be under the exact same <foo>_SITE, which
prevents applying patches from other locations than the original
package site.

Shouldn't we simply get rid of <pkg>_SITE, and make <pkg>_SOURCE the
full URL to the tarball / git repo?

Best regards,

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

[Buildroot] [PATCH 4/4] docs/manual: document the new variable FOO_UPSTREAM_SOURCE

[Yann E. MORIN]

Baruch, All,

On 2014-11-16 08:19 +0200, Baruch Siach spake thusly:
> On Sat, Nov 15, 2014 at 05:19:29PM +0100, Yann E. MORIN wrote:
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
> > Cc: Samuel Martin <s.martin49@gmail.com>
> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > Cc: Peter Korsgaard <jacmet@uclibc.org>
> > Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> > Cc: Arnout Vandecappelle <arnout@mind.be>
> > ---
> >  docs/manual/adding-packages-generic.txt | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/docs/manual/adding-packages-generic.txt b/docs/manual/adding-packages-generic.txt
> > index 67a7453..5295681 100644
> > --- a/docs/manual/adding-packages-generic.txt
> > +++ b/docs/manual/adding-packages-generic.txt
> > @@ -211,6 +211,17 @@ information is (assuming the package name is +libfoo+) :
> >    +libfoo-$(LIBFOO_VERSION).tar.gz+. +
> >    Example: +LIBFOO_SOURCE = foobar-$(LIBFOO_VERSION).tar.bz2+
> >  
> > +* +LIBFOO_UPSTREAM_SOURCE+ may contain the name of the tarball as
> > +  known by upstream. If +HOST_LIBFOO_UPSTREAM_SOURCE+ is not specified,
> > +  it defaults to +LIBFOO_UPSTREAM_SOURCE+. If none is specified, then
> > +  the value is assumed to be the same as +LIBFOO_SOURCE+.
> > +
> > +.Note
> > ++LIBFOO_SOURCE+ is the filename we want to save the tarball as, while
> > ++LIBFOO_UPSTREAM_SOURCE+ is the name by which upstream names that
> > +tarball. The two are usually the same, except in very rare cases, so you
> > +normally should not have to set +LIBFOO_UPSTREAM_SOURCE+.
> 
> I think it is worth clarifying which name the .hash file should refer in this 
> case. I guess this is LIBFOO_SOURCE, isn't it?.

Nice catch, indeed! :-)

Yes, this should be referencing the local tarball filename. I'll add
this to the manual.

Thank! :-)

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCH 0/4] pkg-infra: differentiate remote and local tarball filenames (branch yem/download)

[Arnout Vandecappelle]

On 16/11/14 12:22, Thomas Petazzoni wrote:
> Dear Yann E. MORIN,
>
> On Sat, 15 Nov 2014 17:19:24 +0100, Yann E. MORIN wrote:
>
> > This series introduces a new variable, FOO_UPSTREAM_SOURCE, so as to be
> > able to differentiate remote and local tarball filenames.
> >
> > This is needed when upstream tarballs are not named the way we expect them
> > to be. For example, the GitHub API only provides a request like:
> >     GET /repos/:owner/:repo/:archive_format/:ref
> >
> > So far, we're not using this API, and directly use a non-stable scheme
> > to retrieve the tarballs. That scheme has proven to break from time to
> > time, so we'll soon need to switch to the API, which is guaranteed to be
> > stable.
> >
> > So, we need to be able to differentiate the filename remote knows the
> > tarball by, and the local filename we want to save that tarball as.
> >
> > Changes to the GitHub helper will come in a later series, because it is
> > a bit more involved, and still needs some ironing out. Alternate forges
> > may also get added in the future, which would require such a feature.
>
> Not directly related, but somewhat related: I have always though that
> our split between <foo>_SITE and <foo>_SOURCE was a bit stupid. Why
> don't we simply give a full URL instead of splitting that between SITE
> and SOURCE ?
>
> The way we do things today also forces the things in <foo>_PATCH and
> <foo>_EXTRA_DOWNLOADS to be under the exact same <foo>_SITE, which
> prevents applying patches from other locations than the original
> package site.
>
> Shouldn't we simply get rid of <pkg>_SITE, and make <pkg>_SOURCE the
> full URL to the tarball / git repo?

 I think there are two reasons for the split:

1. _SOURCE can be dropped if it is <pkg>-<version>.tar.gz - this would be
difficult if we only had _SITE.

2. _PATH and _EXTRA_DOWNLOAD don't need to specify the full URL.


 In my opinion, these reasons are not strong enough to keep the split between
_SITE and _SOURCE.

 So, +1 from me to get rid of _SOURCE.


 Regards,
 Arnout

>
> Best regards,
>
> Thomas
>


-- 
Arnout Vandecappelle arnout at mind be
Senior Embedded Software Architect +32-16-286500
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH 1/4] pkg-infra: always specify the local tarball name when calling DOWNLOAD

[Arnout Vandecappelle]

On 15/11/14 17:19, Yann E. MORIN wrote:
> This will be needed to be able to differentiate the upstream filename
> from the local filename, which may differ in some cases.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
> Cc: Samuel Martin <s.martin49@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Peter Korsgaard <jacmet@uclibc.org>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> ---
>  package/pkg-download.mk | 3 ++-
>  package/pkg-generic.mk  | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index f3409bd..7e932e9 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -245,13 +245,14 @@ endef
>  # 3) BR2_BACKUP_SITE if enabled, unless BR2_PRIMARY_SITE_ONLY is set
>  #
>  # Argument 1 is the source location
> +# Argument 2 is the local filename, without any path component
>  #
>  # E.G. use like this:
>  # $(call DOWNLOAD,$(FOO_SITE))
>  ################################################################################
>  
>  define DOWNLOAD
> -    $(call DOWNLOAD_INNER,$(1),$(notdir $(1)))
> +    $(call DOWNLOAD_INNER,$(1),$(2))
>  endef
>  
>  define DOWNLOAD_INNER
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 9643a30..543cb60 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -84,7 +84,7 @@ ifeq ($(DL_MODE),DOWNLOAD)
>          done ; \
>      fi
>  endif
> -    $(if $($(PKG)_SOURCE),$(call DOWNLOAD,$($(PKG)_SITE:/=)/$($(PKG)_SOURCE)))
> +    $(if $($(PKG)_SOURCE),$(call
> DOWNLOAD,$($(PKG)_SITE:/=)/$($(PKG)_SOURCE),$($(PKG)_SOURCE)))
>      $(foreach p,$($(PKG)_EXTRA_DOWNLOADS),$(call
> DOWNLOAD,$($(PKG)_SITE:/=)/$(p))$(sep))

 So, why didn't you replace this second instance of DOWNLOAD? And there are
several others scattered around the code...


 Regards,
 Arnout

>      $(foreach p,$($(PKG)_PATCH),\
>          $(if $(findstring ://,$(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 3/4] pkg-infra: differentiate remote tarball name from local filename

[Arnout Vandecappelle]

On 15/11/14 17:19, Yann E. MORIN wrote:
> Some upstreams may use a naming scheme that does not fit well in how
> Buildroot wants to handle filenames.
>
> For example, GitHub used to have a scheme like:
>     https://github.com/USER/REPO/archive/VERSION.tar.gz
>
> which means we would have a local file named VERSION.tar.gz, when we
> want to have PKG-VERSION.tar.gz
>
> Other forges are also known to have similar schemes. Google Code, for
> example, may also use similarly named files.
>
> Introduce a new variable, FOO_UPSTREAM_SOURCE, which the package may set
> in that case. If not set, it defaults to FOO_SOURCE.

 Instead of this, I was thinking of the reverse approach: define FOO_LOCAL_SOURCE.
It could possibly even be calculated automatically in pkg-generic.mk. E.g.

$(2)_LOCAL_SOURCE = \
    $$(if $$(findstring $(3),$$($(2)_SOURCE)),
        $$($(2)_SOURCE),
        $(3)-$$($(2)_SOURCE))

(and something similar for adding the version, so it should probably be factored
into a function).


 Regards,
 Arnout

>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
> Cc: Samuel Martin <s.martin49@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Peter Korsgaard <jacmet@uclibc.org>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> ---
>  package/pkg-generic.mk | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 543cb60..30bce59 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -84,7 +84,7 @@ ifeq ($(DL_MODE),DOWNLOAD)
>          done ; \
>      fi
>  endif
> -    $(if $($(PKG)_SOURCE),$(call
> DOWNLOAD,$($(PKG)_SITE:/=)/$($(PKG)_SOURCE),$($(PKG)_SOURCE)))
> +    $(if $($(PKG)_SOURCE),$(call
> DOWNLOAD,$($(PKG)_SITE:/=)/$($(PKG)_UPSTREAM_SOURCE),$($(PKG)_SOURCE)))
>      $(foreach p,$($(PKG)_EXTRA_DOWNLOADS),$(call
> DOWNLOAD,$($(PKG)_SITE:/=)/$(p))$(sep))
>      $(foreach p,$($(PKG)_PATCH),\
>          $(if $(findstring ://,$(p)),\
> @@ -361,6 +361,10 @@ ifndef $(2)_SOURCE
>   endif
>  endif
>  
> +ifndef $(2)_UPSTREAM_SOURCE
> +  $(2)_UPSTREAM_SOURCE = $$($(2)_SOURCE)
> +endif
> +
>  ifndef $(2)_PATCH
>   ifdef $(3)_PATCH
>    $(2)_PATCH = $$($(3)_PATCH)

-- 
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 0/4] pkg-infra: differentiate remote and local tarball filenames (branch yem/download)

[Thomas Petazzoni]

Dear Arnout Vandecappelle,

On Tue, 18 Nov 2014 21:38:58 +0100, Arnout Vandecappelle wrote:

>  I think there are two reasons for the split:
> 
> 1. _SOURCE can be dropped if it is <pkg>-<version>.tar.gz - this would be
> difficult if we only had _SITE.

True. But on the other hand, the fact that we use
<pkg>-<version>.tar.gz by default has always confused newcomers a bit,
and with .tar.xz becoming more and more adopted, having .tar.gz as the
default is less obvious than it used to be.

> 2. _PATH and _EXTRA_DOWNLOAD don't need to specify the full URL.

Well, yes, but that's specifically the issue I raised: since you don't
specify the full URL, there's no way for the package to download
patches or other stuff from other places than <pkg>_SITE.

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

[Buildroot] [PATCH 0/4] pkg-infra: differentiate remote and local tarball filenames (branch yem/download)

[Arnout Vandecappelle]

On 18/11/14 22:03, Thomas Petazzoni wrote:
> Dear Arnout Vandecappelle,
>
> On Tue, 18 Nov 2014 21:38:58 +0100, Arnout Vandecappelle wrote:
>
> >  I think there are two reasons for the split:
> >
> > 1. _SOURCE can be dropped if it is <pkg>-<version>.tar.gz - this would be
> > difficult if we only had _SITE.
>
> True. But on the other hand, the fact that we use
> <pkg>-<version>.tar.gz by default has always confused newcomers a bit,
> and with .tar.xz becoming more and more adopted, having .tar.gz as the
> default is less obvious than it used to be.
>
> > 2. _PATH and _EXTRA_DOWNLOAD don't need to specify the full URL.
>
> Well, yes, but that's specifically the issue I raised: since you don't
> specify the full URL, there's no way for the package to download
> patches or other stuff from other places than <pkg>_SITE.

 I completely agree, just pointing out the (historical) reasons why the split
exists.

 Regards,
 Arnout

>
> Thomas
>


-- 
Arnout Vandecappelle arnout at mind be
Senior Embedded Software Architect +32-16-286500
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH 1/4] pkg-infra: always specify the local tarball name when calling DOWNLOAD

[Yann E. MORIN]

Arnout, All,

On 2014-11-18 21:41 +0100, Arnout Vandecappelle spake thusly:
> On 15/11/14 17:19, Yann E. MORIN wrote:
> > This will be needed to be able to differentiate the upstream filename
> > from the local filename, which may differ in some cases.
> >
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
> > Cc: Samuel Martin <s.martin49@gmail.com>
> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > Cc: Peter Korsgaard <jacmet@uclibc.org>
> > Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> > Cc: Arnout Vandecappelle <arnout@mind.be>
> > ---
> >  package/pkg-download.mk | 3 ++-
> >  package/pkg-generic.mk  | 2 +-
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> > index f3409bd..7e932e9 100644
> > --- a/package/pkg-download.mk
> > +++ b/package/pkg-download.mk
> > @@ -245,13 +245,14 @@ endef
> >  # 3) BR2_BACKUP_SITE if enabled, unless BR2_PRIMARY_SITE_ONLY is set
> >  #
> >  # Argument 1 is the source location
> > +# Argument 2 is the local filename, without any path component
> >  #
> >  # E.G. use like this:
> >  # $(call DOWNLOAD,$(FOO_SITE))
> >  ################################################################################
> >  
> >  define DOWNLOAD
> > -    $(call DOWNLOAD_INNER,$(1),$(notdir $(1)))
> > +    $(call DOWNLOAD_INNER,$(1),$(2))
> >  endef
> >  
> >  define DOWNLOAD_INNER
> > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> > index 9643a30..543cb60 100644
> > --- a/package/pkg-generic.mk
> > +++ b/package/pkg-generic.mk
> > @@ -84,7 +84,7 @@ ifeq ($(DL_MODE),DOWNLOAD)
> >          done ; \
> >      fi
> >  endif
> > -    $(if $($(PKG)_SOURCE),$(call DOWNLOAD,$($(PKG)_SITE:/=)/$($(PKG)_SOURCE)))
> > +    $(if $($(PKG)_SOURCE),$(call
> > DOWNLOAD,$($(PKG)_SITE:/=)/$($(PKG)_SOURCE),$($(PKG)_SOURCE)))
> >      $(foreach p,$($(PKG)_EXTRA_DOWNLOADS),$(call
> > DOWNLOAD,$($(PKG)_SITE:/=)/$(p))$(sep))
> 
>  So, why didn't you replace this second instance of DOWNLOAD?

The idea was that we have a different local filename only for the
_SOURCE file, not for extra downloads or patches.

Of course, this patch is obviously flawed because then we missed the
local filename for those extra downloads or patches.

Thus, in the new series, I'll keep the ability to not specify the local
filename:

    define DOWNLOAD
        $(call DOWNLOAD_INNER,$(1),$(if $(2),$(2),$(notdir $(1))))
    endef

> And there are
> several others scattered around the code...

Yes, hence the reason to still default to the basename of $(1) if $(2)
is not specified.

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCH 3/4] pkg-infra: differentiate remote tarball name from local filename

[Yann E. MORIN]

Arnout, All,

On 2014-11-18 21:54 +0100, Arnout Vandecappelle spake thusly:
> On 15/11/14 17:19, Yann E. MORIN wrote:
> > Some upstreams may use a naming scheme that does not fit well in how
> > Buildroot wants to handle filenames.
> >
> > For example, GitHub used to have a scheme like:
> >     https://github.com/USER/REPO/archive/VERSION.tar.gz
> >
> > which means we would have a local file named VERSION.tar.gz, when we
> > want to have PKG-VERSION.tar.gz
> >
> > Other forges are also known to have similar schemes. Google Code, for
> > example, may also use similarly named files.
> >
> > Introduce a new variable, FOO_UPSTREAM_SOURCE, which the package may set
> > in that case. If not set, it defaults to FOO_SOURCE.
> 
>  Instead of this, I was thinking of the reverse approach: define FOO_LOCAL_SOURCE.

OK, will change.

> It could possibly even be calculated automatically in pkg-generic.mk. E.g.
> 
> $(2)_LOCAL_SOURCE = \
>     $$(if $$(findstring $(3),$$($(2)_SOURCE)),
>         $$($(2)_SOURCE),
>         $(3)-$$($(2)_SOURCE))
> 
> (and something similar for adding the version, so it should probably be factored
> into a function).

Well, why not just simply revert what I did with _UPSTREAM_SOURCE:

    ifndef $(2)_LOCAL_SOURCE
      $(2)_LOCAL_SOURCE = $$($(2)_SOURCE)
    endif

That's much more readable and simple than the above, given that
$(2)_SOURCE is already auto-computed by tje pkg-generic infra.

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCH 3/4] pkg-infra: differentiate remote tarball name from local filename

[Yann E. MORIN]

Arnout, All,

On 2014-11-23 18:06 +0100, Yann E. MORIN spake thusly:
> On 2014-11-18 21:54 +0100, Arnout Vandecappelle spake thusly:
> > On 15/11/14 17:19, Yann E. MORIN wrote:
> > > Some upstreams may use a naming scheme that does not fit well in how
> > > Buildroot wants to handle filenames.
> > >
> > > For example, GitHub used to have a scheme like:
> > >     https://github.com/USER/REPO/archive/VERSION.tar.gz
> > >
> > > which means we would have a local file named VERSION.tar.gz, when we
> > > want to have PKG-VERSION.tar.gz
> > >
> > > Other forges are also known to have similar schemes. Google Code, for
> > > example, may also use similarly named files.
> > >
> > > Introduce a new variable, FOO_UPSTREAM_SOURCE, which the package may set
> > > in that case. If not set, it defaults to FOO_SOURCE.
> > 
> >  Instead of this, I was thinking of the reverse approach: define FOO_LOCAL_SOURCE.
> 
> OK, will change.

Well, actually, I'm not convinced.

What I've done is add FOO_LOCAL_SOURCE, which defaults to FOO_SOURCE if
not defined:

    ifndef $(2)_LOCAL_SOURCE
        $(2)_LOCAL_SOURCE = $$($(2)_SOURCE)
    endif

But then, here's what I thought about:

  - if both upstream and local are the same, we just have to define
    FOO_SOURCE and be done with it;

  - furthermore, if the value of FOO_SOURCE is the default
    PKG-VERSION.tar.gz, we need not specify it at all;

  - if upstream and local differ, then we must specify both, even though
    FOO_LOCAL_SOURCE is in the format we default to.

So, I prefer we keep FOO_UPSTREAM_SOURCE, to really emp[hasise that it
is upstream that is doing weird things, not us.

I'm still open for discussion on this, though.

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCH 0/4] pkg-infra: differentiate remote and local tarball filenames (branch yem/download)

[Yann E. MORIN]

Thomas, All,

On 2014-11-18 22:03 +0100, Thomas Petazzoni spake thusly:
> Dear Arnout Vandecappelle,
> 
> On Tue, 18 Nov 2014 21:38:58 +0100, Arnout Vandecappelle wrote:
> 
> >  I think there are two reasons for the split:
> > 
> > 1. _SOURCE can be dropped if it is <pkg>-<version>.tar.gz - this would be
> > difficult if we only had _SITE.
> 
> True. But on the other hand, the fact that we use
> <pkg>-<version>.tar.gz by default has always confused newcomers a bit,
> and with .tar.xz becoming more and more adopted, having .tar.gz as the
> default is less obvious than it used to be.
> 
> > 2. _PATH and _EXTRA_DOWNLOAD don't need to specify the full URL.
> 
> Well, yes, but that's specifically the issue I raised: since you don't
> specify the full URL, there's no way for the package to download
> patches or other stuff from other places than <pkg>_SITE.

Well, I still find the split worthwhile.

Still, we could allow FOO_EXTRA_DOWNLOADS and FOO_PATCH to be
fully-qualified URIs. If they are not, prepend FOO_SITE to generate a
full URI.

Yes, I am writing a package that will download most of its files via
_EXTRA_DOWNLOADS (don't ask why, upstream is not doing release tarballs
for that component), and it would be *very* cumbersome to repeat the
_SITE for all of those files.

Regards,
Yann E. MORIN.

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