* [Buildroot] [PATCH] pkg-download: make package name optional in github helper
@ 2017-10-04 8:57 Ismael Luceno
2017-10-04 16:35 ` Arnout Vandecappelle
2017-10-05 10:22 ` Thomas Petazzoni
0 siblings, 2 replies; 8+ messages in thread
From: Ismael Luceno @ 2017-10-04 8:57 UTC (permalink / raw)
To: buildroot
Signed-off-by: Ismael Luceno <ismael@iodev.co.uk>
---
package/pkg-download.mk | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index dc4ff1c8c755..78307eee8a33 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -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
--
2.14.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Buildroot] [PATCH] pkg-download: make package name optional in github helper
2017-10-04 8:57 [Buildroot] [PATCH] pkg-download: make package name optional in github helper Ismael Luceno
@ 2017-10-04 16:35 ` Arnout Vandecappelle
2017-10-05 10:13 ` Ismael Luceno
2017-10-05 10:23 ` Thomas Petazzoni
2017-10-05 10:22 ` Thomas Petazzoni
1 sibling, 2 replies; 8+ messages in thread
From: Arnout Vandecappelle @ 2017-10-04 16:35 UTC (permalink / raw)
To: buildroot
Hi Ismael,
On 04-10-17 10:57, Ismael Luceno wrote:
> Signed-off-by: Ismael Luceno <ismael@iodev.co.uk>
> ---
> package/pkg-download.mk | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index dc4ff1c8c755..78307eee8a33 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -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).
> +# 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
>
> # Expressly do not check hashes for those files
> # Exported variables default to immediately expanded in some versions of
>
--
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Buildroot] [PATCH] pkg-download: make package name optional in github helper
2017-10-04 16:35 ` Arnout Vandecappelle
@ 2017-10-05 10:13 ` Ismael Luceno
2017-10-05 17:50 ` Arnout Vandecappelle
2017-10-05 10:23 ` Thomas Petazzoni
1 sibling, 1 reply; 8+ messages in thread
From: Ismael Luceno @ 2017-10-05 10:13 UTC (permalink / raw)
To: buildroot
On 04/Oct/2017 18:35, Arnout Vandecappelle wrote:
<...>
> The solution (probably) is to use $(PKG) instead of $(pkgname).
I see.
> > +# 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.
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Buildroot] [PATCH] pkg-download: make package name optional in github helper
2017-10-04 8:57 [Buildroot] [PATCH] pkg-download: make package name optional in github helper Ismael Luceno
2017-10-04 16:35 ` Arnout Vandecappelle
@ 2017-10-05 10:22 ` Thomas Petazzoni
1 sibling, 0 replies; 8+ messages in thread
From: Thomas Petazzoni @ 2017-10-05 10:22 UTC (permalink / raw)
To: buildroot
Hello,
On Wed, 4 Oct 2017 05:57:45 -0300, Ismael Luceno wrote:
> Signed-off-by: Ismael Luceno <ismael@iodev.co.uk>
An empty commit log is definitely not good for such a commit. Why do we
want this?
Thanks!
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Buildroot] [PATCH] pkg-download: make package name optional in github helper
2017-10-04 16:35 ` Arnout Vandecappelle
2017-10-05 10:13 ` Ismael Luceno
@ 2017-10-05 10:23 ` Thomas Petazzoni
2017-10-05 12:57 ` Arnout Vandecappelle
1 sibling, 1 reply; 8+ messages in thread
From: Thomas Petazzoni @ 2017-10-05 10:23 UTC (permalink / raw)
To: buildroot
Hello,
On Wed, 4 Oct 2017 18:35:07 +0200, Arnout Vandecappelle wrote:
> 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
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Buildroot] [PATCH] pkg-download: make package name optional in github helper
2017-10-05 10:23 ` Thomas Petazzoni
@ 2017-10-05 12:57 ` Arnout Vandecappelle
0 siblings, 0 replies; 8+ messages in thread
From: Arnout Vandecappelle @ 2017-10-05 12:57 UTC (permalink / raw)
To: buildroot
On 05-10-17 12:23, Thomas Petazzoni wrote:
> Hello,
>
> On Wed, 4 Oct 2017 18:35:07 +0200, Arnout Vandecappelle wrote:
>
>> 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" ?
Yes, "it" was meant to mean "making the argument optional".
Regards,
Arnout
--
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Buildroot] [PATCH] pkg-download: make package name optional in github helper
2017-10-05 10:13 ` Ismael Luceno
@ 2017-10-05 17:50 ` Arnout Vandecappelle
2017-10-05 21:20 ` Ismael Luceno
0 siblings, 1 reply; 8+ messages in thread
From: Arnout Vandecappelle @ 2017-10-05 17:50 UTC (permalink / raw)
To: buildroot
On 05-10-17 12:13, Ismael Luceno wrote:
> On 04/Oct/2017 18:35, Arnout Vandecappelle wrote:
> <...>
>> The solution (probably) is to use $(PKG) instead of $(pkgname).
>
> I see.
>
>>> +# 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.
>> 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
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
--
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Buildroot] [PATCH] pkg-download: make package name optional in github helper
2017-10-05 17:50 ` Arnout Vandecappelle
@ 2017-10-05 21:20 ` Ismael Luceno
0 siblings, 0 replies; 8+ messages in thread
From: Ismael Luceno @ 2017-10-05 21:20 UTC (permalink / raw)
To: buildroot
On 05/Oct/2017 19:50, Arnout Vandecappelle wrote:
> On 05-10-17 12:13, Ismael Luceno wrote:
<...>
> > 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.
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-10-05 21:20 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-04 8:57 [Buildroot] [PATCH] pkg-download: make package name optional in github helper Ismael Luceno
2017-10-04 16:35 ` Arnout Vandecappelle
2017-10-05 10:13 ` Ismael Luceno
2017-10-05 17:50 ` Arnout Vandecappelle
2017-10-05 21:20 ` Ismael Luceno
2017-10-05 10:23 ` Thomas Petazzoni
2017-10-05 12:57 ` Arnout Vandecappelle
2017-10-05 10:22 ` Thomas Petazzoni
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.