Skip to contentSkip to conversation

pkg-download: make package name optional in github helper

PATCH New Ismael Luceno 3 participants 8 messages Oct 4, 2017 – Oct 5, 2017

lore
Share & tools
lore

Ismael Luceno

author
#0 lore raw

PATCH 1/1 pkg-download: make package name optional in github helper

Signed-off-by Ismael Luceno <[email protected]>
---
 package/pkg-download.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
package/pkg-download.mk +2 -2
index dc4ff1c8c755..78307eee8a33 100644
@@ -52,8 +52,8 @@ notdomain = $(patsubst $(call domain,$(1),$(2))$(call domainseparator,$(2))%,%,$
 # default domainseparator is /, specify alternative value as first argument
 domainseparator = $(if $(1),$(1),/)
 
-# github(user,package[,version]): returns site of GitHub repository
-github = https://github.com/$(1)/$(2)/archive/$(or $(3),$($(call UPPERCASE,$(pkgname))_VERSION))
+# github(user[,package[,version]]): returns site of GitHub repository
+github = https://github.com/$(1)/$(or $(2),$(pkgname))/archive/$(or $(3),$($(call UPPERCASE,$(pkgname))_VERSION))
 
 # Expressly do not check hashes for those files
 # Exported variables default to immediately expanded in some versions of
Signature · 1 line
2.14.2
#1 lore raw

PATCH 1/1 pkg-download: make package name optional in github helper

Hi Ismael,

Quoted 10 lines · Ismael Luceno
Signed-off-by Ismael Luceno <[email protected]>
---
 package/pkg-download.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
package/pkg-download.mk +0 -0
index dc4ff1c8c755..78307eee8a33 100644

package/pkg-download.mk notdomain = $(patsubst $(call domain,$(1),$(2))$(call domainseparator,$(2))%,%,$ · line 56

Quoted 5 lines
package/pkg-download.mk +0 -2
@@ -52,8 +52,8 @@ notdomain = $(patsubst $(call domain,$(1),$(2))$(call domainseparator,$(2))%,%,$
 # default domainseparator is /, specify alternative value as first argument
 domainseparator = $(if $(1),$(1),/)
 
-# github(user,package[,version]): returns site of GitHub repository
-github = https://github.com/$(1)/$(2)/archive/$(or $(3),$($(call UPPERCASE,$(pkgname))_VERSION))

As noticed by Peter, this was already wrong in the original patch: it only
works if the github helper is used in a := assignment, otherwise $(pkgname) evaluates to empty by the time the variable is evaluated.

The solution (probably) is to use $(PKG) instead of $(pkgname).

package/pkg-download.mk notdomain = $(patsubst $(call domain,$(1),$(2))$(call domainseparator,$(2))%,%,$ · line 56

Quoted 2 lines
package/pkg-download.mk +2 -0
+# github(user[,package[,version]]): returns site of GitHub repository
+github = https://github.com/$(1)/$(or $(2),$(pkgname))/archive/$(or $(3),$($(call UPPERCASE,$(pkgname))_VERSION))

Could you estimate how many packages could be simplified with this?

I'm not actually sure this is a good idea anyway. The _VERSION is OK because it
works for *all* packages, the third argument is only needed when you use the github helper outside a package (e.g. for a custom kernel URL). But this change will apply only to a limited number of packages on the one hand, and on the other hand it makes it even more difficult for people to understand what is meant with this $(call github,foo) construct.

Regards,
Arnout

Quoted 4 lines
package/pkg-download.mk +0 -0
 # Expressly do not check hashes for those files
 # Exported variables default to immediately expanded in some versions of
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:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
author
#2 lore raw

PATCH 1/1 pkg-download: make package name optional in github helper

On 04/Oct/2017 18:35, Arnout Vandecappelle wrote:
<...>

Quoted 1 line

The solution (probably) is to use $(PKG) instead of $(pkgname).

I see.

Quoted 4 lines

package/pkg-download.mk notdomain = $(patsubst $(call domain,$(1),$(2))$(call domainseparator,$(2))%,%,$ · line 56

Quoted 2 lines
package/pkg-download.mk +2 -0
+# github(user[,package[,version]]): returns site of GitHub repository
+github = https://github.com/$(1)/$(or $(2),$(pkgname))/archive/$(or $(3),$($(call UPPERCASE,$(pkgname))_VERSION))

Could you estimate how many packages could be simplified with this?

A great deal, including axel, in 219 out of 380 the repo matches the package name, with 30 also matching the username.

Quoted 7 lines

I'm not actually sure this is a good idea anyway. The _VERSION is OK because it works for *all* packages, the third argument is only needed when you use the github helper outside a package (e.g. for a custom kernel URL). But this change will apply only to a limited number of packages on the one hand, and on the other hand it makes it even more difficult for people to understand what is meant with this $(call github,foo) construct.

It's "magic" anyway. It would perhaps make sense to implement this kind of rules via rewriting AXEL_SITE, it could be something descriptive like:

AXEL_SITE = @ github user:axel-download-accelerator repo:libtls

Here's a mockup: http://tmp.iodev.co.uk/rtest.mk

#3 lore raw

PATCH 1/1 pkg-download: make package name optional in github helper

Quoted 6 lines · Ismael Luceno

On 04/Oct/2017 18:35, Arnout Vandecappelle wrote:
<...>

Quoted 1 line

The solution (probably) is to use $(PKG) instead of $(pkgname).

I see.

Quoted 7 lines
Quoted 4 lines

package/pkg-download.mk notdomain = $(patsubst $(call domain,$(1),$(2))$(call domainseparator,$(2))%,%,$ · line 56

Quoted 2 lines
package/pkg-download.mk +2 -0
+# github(user[,package[,version]]): returns site of GitHub repository
+github = https://github.com/$(1)/$(or $(2),$(pkgname))/archive/$(or $(3),$($(call UPPERCASE,$(pkgname))_VERSION))

Could you estimate how many packages could be simplified with this?

A great deal, including axel, in 219 out of 380 the repo matches the package name, with 30 also matching the username.

Hm, not convincing enough for me. It saves just a tiny little bit of typing, it
does not make things more readable, and it adds complexity to the github macro itself. No, I don't think it's worth it.

Quoted 8 lines
Quoted 7 lines

I'm not actually sure this is a good idea anyway. The _VERSION is OK because it works for *all* packages, the third argument is only needed when you use the github helper outside a package (e.g. for a custom kernel URL). But this change will apply only to a limited number of packages on the one hand, and on the other hand it makes it even more difficult for people to understand what is meant with this $(call github,foo) construct.

Quoted 6 lines

It's "magic" anyway. It would perhaps make sense to implement this kind of rules via rewriting AXEL_SITE, it could be something descriptive like:

AXEL_SITE = @ github user:axel-download-accelerator repo:libtls

Here's a mockup: http://tmp.iodev.co.uk/rtest.mk

Here also I don't think the added complexity in the implementation is worth it.
It is indeed a bit more explicit, but now you have to remember that the arguments are called user: and repo:. For me,
$(call github,axel-download-accelerator,libtls) is an easy-to-remember way to refer to https://github.com/axel-download-accelerator/libtls.

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:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
author
#4 lore raw

PATCH 1/1 pkg-download: make package name optional in github helper

Quoted 1 line · Arnout Vandecappelle

On 05-10-17 12:13, Ismael Luceno wrote:

<...>

Quoted 5 lines
Quoted 5 lines

It's "magic" anyway. It would perhaps make sense to implement this kind of rules via rewriting AXEL_SITE, it could be something descriptive like:

AXEL_SITE = @ github user:axel-download-accelerator repo:libtls

Quoted 7 lines
Quoted 1 line

Here's a mockup: http://tmp.iodev.co.uk/rtest.mk

Here also I don't think the added complexity in the implementation is worth it. It is indeed a bit more explicit, but now you have to remember that the arguments are called user: and repo:. For me, $(call github,axel-download-accelerator,libtls) is an easy-to-remember way to refer to https://github.com/axel-download-accelerator/libtls.

Well, it's an interesting exercise anyway; I've simplified it a little bit (same URL), I think this could be useful for other purposes.

It's perhaps not so obvious where to look for to see how it works, but documentation fixes that.

#5 lore raw

PATCH 1/1 pkg-download: make package name optional in github helper

In reply to Arnout Vandecappelle

Hello,

Quoted 2 lines · Arnout Vandecappelle

As noticed by Peter, this was already wrong in the original patch: it only
works if the github helper is used in a := assignment,

Doh? We're using the github helper almost exclusively in = assignments:

package/assimp/assimp.mk:ASSIMP_SITE = $(call github,assimp,assimp,$(ASSIMP_VERSION)) package/asterisk/asterisk.mk:ASTERISK_SITE = $(call github,asterisk,asterisk,$(ASTERISK_VERSION)) package/atest/atest.mk:ATEST_SITE = $(call github,amouiche,atest,$(ATEST_VERSION)) package/aufs/aufs.mk:AUFS_SITE = $(call github,sfjro,aufs4-standalone,$(AUFS_VERSION)) package/avrdude/avrdude.mk:AVRDUDE_SITE = $(call github,kcuzner,avrdude,$(AVRDUDE_VERSION)) package/axfsutils/axfsutils.mk:AXFSUTILS_SITE = $(call github,jaredeh,axfs,$(AXFSUTILS_VERSION)) package/azmq/azmq.mk:AZMQ_SITE = $(call github,zeromq,azmq,$(AZMQ_VERSION))

Am I missing something in your explanation ?

Or perhaps you meant "when the third argument is not passed, we need to use a := assignment" ?

Best regards,

Thomas

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

PATCH 1/1 pkg-download: make package name optional in github helper

Quoted 17 lines · Thomas Petazzoni

Hello,

Quoted 2 lines · Arnout Vandecappelle

As noticed by Peter, this was already wrong in the original patch: it only
works if the github helper is used in a := assignment,

Doh? We're using the github helper almost exclusively in = assignments:

package/assimp/assimp.mk:ASSIMP_SITE = $(call github,assimp,assimp,$(ASSIMP_VERSION)) package/asterisk/asterisk.mk:ASTERISK_SITE = $(call github,asterisk,asterisk,$(ASTERISK_VERSION)) package/atest/atest.mk:ATEST_SITE = $(call github,amouiche,atest,$(ATEST_VERSION)) package/aufs/aufs.mk:AUFS_SITE = $(call github,sfjro,aufs4-standalone,$(AUFS_VERSION)) package/avrdude/avrdude.mk:AVRDUDE_SITE = $(call github,kcuzner,avrdude,$(AVRDUDE_VERSION)) package/axfsutils/axfsutils.mk:AXFSUTILS_SITE = $(call github,jaredeh,axfs,$(AXFSUTILS_VERSION)) package/azmq/azmq.mk:AZMQ_SITE = $(call github,zeromq,azmq,$(AZMQ_VERSION))

Quoted 4 lines

Am I missing something in your explanation ?

Or perhaps you meant "when the third argument is not passed, we need to use a := assignment" ?

Yes, "it" was meant to mean "making the argument optional".

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:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
#7 lore raw

PATCH 1/1 pkg-download: make package name optional in github helper

In reply to Ismael Luceno

Hello,

Quoted 1 line · Ismael Luceno
Signed-off-by Ismael Luceno <[email protected]>

An empty commit log is definitely not good for such a commit. Why do we want this?

Thanks!

Thomas

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