pkg-infra: differentiate remote and local tarball filenames (branch yem/download)

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

Cc Maxime Hadjinlian <[email protected]>

Cc Samuel Martin <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

---
 package/pkg-download.mk | 3 ++-
 package/pkg-generic.mk  | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

package/pkg-download.mk +2 -1

index f3409bd..7e932e9 100644
@@ -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

package/pkg-generic.mk +0 -0

index 9643a30..543cb60 100644

     $(foreach p,$($(PKG)_PATCH),\
         $(if $(findstring ://,$(p)),\

Quoted 34 lines

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

Cc Maxime Hadjinlian <[email protected]>

Cc Samuel Martin <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

---
 package/pkg-download.mk | 3 ++-
 package/pkg-generic.mk  | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

package/pkg-download.mk +2 -1

index f3409bd..7e932e9 100644
@@ -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
 

Quoted 6 lines

package/pkg-download.mk +0 -0

 define DOWNLOAD_INNER

package/pkg-generic.mk +0 -0

index 9643a30..543cb60 100644

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

Quoted 8 lines

package/pkg-generic.mk +1 -1

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

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

Cc Maxime Hadjinlian <[email protected]>

Cc Samuel Martin <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

---
 package/pkg-generic.mk | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

package/pkg-generic.mk +1 -1

index 543cb60..30bce59 100644
@@ -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)),\

package/pkg-generic.mk +4 -0

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

Quoted 9 lines

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

Quoted 5 lines

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

On 2014-11-18 21:54 +0100, Arnout Vandecappelle spake thusly:

Quoted 13 lines · Yann E. MORIN

Quoted 12 lines

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.

Quoted 4 lines

Quoted 2 lines

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.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Maxime Hadjinlian <[email protected]>

Cc Samuel Martin <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

---
 docs/manual/adding-packages-generic.txt | 11 +++++++++++
 1 file changed, 11 insertions(+)

docs/manual/adding-packages-generic.txt +0 -0

index 67a7453..5295681 100644
@@ -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+
 

Quoted 19 lines

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Maxime Hadjinlian <[email protected]>

Cc Samuel Martin <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

---
 docs/manual/adding-packages-generic.txt | 11 +++++++++++
 1 file changed, 11 insertions(+)

docs/manual/adding-packages-generic.txt +0 -0

index 67a7453..5295681 100644
@@ -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+
 

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

Quoted 10 lines

docs/manual/adding-packages-generic.txt +10 -0

+* +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?.

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.

Dear Yann E. MORIN,

Quoted 12 lines · Yann E. MORIN

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.

Quoted 6 lines

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

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.

Dear Arnout Vandecappelle,

Quoted 4 lines · Arnout Vandecappelle

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.

Quoted 1 line

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

Dear Arnout Vandecappelle,

Quoted 4 lines · Arnout Vandecappelle

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.

Quoted 1 line

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.