diff mbox series

[v3] package/pkg-utils.mk: add dl_dir to show-info output

Message ID 20190803140433.27967-1-arnout@mind.be
State Accepted
Headers show
Series [v3] package/pkg-utils.mk: add dl_dir to show-info output | expand

Commit Message

Arnout Vandecappelle Aug. 3, 2019, 2:04 p.m. UTC
It can be useful for scripts to be able to access a package's source
file after download. That used to be easy, just DL_DIR/PKG_SOURCE.
However, with the subdirectories in DL_DIR which can be overridden with
PKG_DL_SUBDIR, that is no longer easy.

Therefore, this patch adds dl_dir to the package information. It prints
just PKG_DL_SUBDIR, to avoid dumping absolute paths to the buildroot
directory in the show-info output.

It can be used with the following jq script to get a newline-separated
list of all downloaded files:

make show-info | jq -r '.[] | ("dl/" + .dl_dir + "/" + .downloads[]?.source)'

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
---
v2 -> v3:
 - Remove accidental change

v1 -> v2:
 - Use DL_SUBDIR (i.e. relative path) instead of DL_DIR (i.e. absolute
   path). (Yann)
 - Move the definition one level higher, because it is anyway the same
   for all sources. (Yann)
 - Update jq script in commit message to handle the above.
---
 package/pkg-utils.mk | 1 +
 1 file changed, 1 insertion(+)

Comments

Yann E. MORIN Aug. 3, 2019, 2:14 p.m. UTC | #1
Arnout, All,

On 2019-08-03 16:04 +0200, Arnout Vandecappelle (Essensium/Mind) spake thusly:
> It can be useful for scripts to be able to access a package's source
> file after download. That used to be easy, just DL_DIR/PKG_SOURCE.
> However, with the subdirectories in DL_DIR which can be overridden with
> PKG_DL_SUBDIR, that is no longer easy.
> 
> Therefore, this patch adds dl_dir to the package information. It prints
> just PKG_DL_SUBDIR, to avoid dumping absolute paths to the buildroot
> directory in the show-info output.
> 
> It can be used with the following jq script to get a newline-separated
> list of all downloaded files:
> 
> make show-info | jq -r '.[] | ("dl/" + .dl_dir + "/" + .downloads[]?.source)'
> 
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>

Acked-by: Yann E. MORIN <yann.morin.1998@free.fr>

Thanks for the jq script! :-)

Regards,
Yann E. MORIN.

> ---
> v2 -> v3:
>  - Remove accidental change
> 
> v1 -> v2:
>  - Use DL_SUBDIR (i.e. relative path) instead of DL_DIR (i.e. absolute
>    path). (Yann)
>  - Move the definition one level higher, because it is anyway the same
>    for all sources. (Yann)
>  - Update jq script in commit message to handle the above.
> ---
>  package/pkg-utils.mk | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
> index b7280e930f..ef93345595 100644
> --- a/package/pkg-utils.mk
> +++ b/package/pkg-utils.mk
> @@ -94,6 +94,7 @@ endef
>  define _json-info-pkg-details
>  	"version": "$($(1)_DL_VERSION)",
>  	"licenses": "$($(1)_LICENSE)",
> +	"dl_dir": "$($(1)_DL_SUBDIR)",
>  	"downloads": [
>  	$(foreach dl,$(sort $($(1)_ALL_DOWNLOADS)),
>  		{
> -- 
> 2.21.0
>
Thomas Petazzoni Sept. 25, 2019, 8:02 p.m. UTC | #2
On Sat,  3 Aug 2019 16:04:33 +0200
"Arnout Vandecappelle (Essensium/Mind)" <arnout@mind.be> wrote:

> It can be useful for scripts to be able to access a package's source
> file after download. That used to be easy, just DL_DIR/PKG_SOURCE.
> However, with the subdirectories in DL_DIR which can be overridden with
> PKG_DL_SUBDIR, that is no longer easy.
> 
> Therefore, this patch adds dl_dir to the package information. It prints
> just PKG_DL_SUBDIR, to avoid dumping absolute paths to the buildroot
> directory in the show-info output.
> 
> It can be used with the following jq script to get a newline-separated
> list of all downloaded files:
> 
> make show-info | jq -r '.[] | ("dl/" + .dl_dir + "/" + .downloads[]?.source)'
> 
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> ---
> v2 -> v3:
>  - Remove accidental change

Applied to master, thanks.

I asked on IRC to Yann what was the difference between this and "make
external-deps". make external-deps only returns the file name, but not
its location inside $(BR2_DL_DIR). Perhaps we should change
external-deps, as it is no longer very useful as it is ? But of course
that's kind of an API breakage... but moving files into subfolders in
$(BR2_DL_DIR) was also kind of an API breakage anyway.

Thoughts?

Thomas
Yann E. MORIN Sept. 25, 2019, 8:41 p.m. UTC | #3
Thomas, Arnout, All,

On 2019-09-25 22:02 +0200, Thomas Petazzoni spake thusly:
> On Sat,  3 Aug 2019 16:04:33 +0200
> "Arnout Vandecappelle (Essensium/Mind)" <arnout@mind.be> wrote:
[--SNIP--]
> > make show-info | jq -r '.[] | ("dl/" + .dl_dir + "/" + .downloads[]?.source)'
[--SNIP--]
> I asked on IRC to Yann what was the difference between this and "make
> external-deps". make external-deps only returns the file name, but not
> its location inside $(BR2_DL_DIR). Perhaps we should change
> external-deps, as it is no longer very useful as it is ? But of course
> that's kind of an API breakage... but moving files into subfolders in
> $(BR2_DL_DIR) was also kind of an API breakage anyway.
> 
> Thoughts?

As much as I like to keep backward compatibility when it makes sense,
external-deps has been broken for ~18 months now (since we moved
downloads to sub-directories), and no one complained. It's even been in
an LTS for 7 months now, and still no complain so far...

Besides, show-info is a versatile and easily extendable solution to
extract this kind of information from Buildroot.

So, what's the point in keeping external-deps?

Regards,
Yann E. MORIN.
Arnout Vandecappelle Sept. 25, 2019, 9:28 p.m. UTC | #4
On 25/09/2019 22:41, Yann E. MORIN wrote:
> Thomas, Arnout, All,
> 
> On 2019-09-25 22:02 +0200, Thomas Petazzoni spake thusly:
>> On Sat,  3 Aug 2019 16:04:33 +0200
>> "Arnout Vandecappelle (Essensium/Mind)" <arnout@mind.be> wrote:
> [--SNIP--]
>>> make show-info | jq -r '.[] | ("dl/" + .dl_dir + "/" + .downloads[]?.source)'
> [--SNIP--]
>> I asked on IRC to Yann what was the difference between this and "make
>> external-deps". make external-deps only returns the file name, but not
>> its location inside $(BR2_DL_DIR). Perhaps we should change
>> external-deps, as it is no longer very useful as it is ? But of course
>> that's kind of an API breakage... but moving files into subfolders in
>> $(BR2_DL_DIR) was also kind of an API breakage anyway.
>>
>> Thoughts?
> 
> As much as I like to keep backward compatibility when it makes sense,
> external-deps has been broken for ~18 months now (since we moved
> downloads to sub-directories), and no one complained. It's even been in
> an LTS for 7 months now, and still no complain so far...
> 
> Besides, show-info is a versatile and easily extendable solution to
> extract this kind of information from Buildroot.
> 
> So, what's the point in keeping external-deps?

 Kill it with fire!

 Regards,
 Arnout
Thomas Petazzoni Sept. 26, 2019, 6:55 a.m. UTC | #5
On Wed, 25 Sep 2019 23:28:05 +0200
Arnout Vandecappelle <arnout@mind.be> wrote:

> > As much as I like to keep backward compatibility when it makes sense,
> > external-deps has been broken for ~18 months now (since we moved
> > downloads to sub-directories), and no one complained. It's even been in
> > an LTS for 7 months now, and still no complain so far...
> > 
> > Besides, show-info is a versatile and easily extendable solution to
> > extract this kind of information from Buildroot.
> > 
> > So, what's the point in keeping external-deps?  
> 
>  Kill it with fire!

Do we really want to do that? Remember that we removed source-check,
and Thomas DS wanted to reintroduce it because it was useful in his
use-case ?

I'm all for dropping duplicated functionality to keep a sane code base
and a clear user experience (when there's gazillions of ways to achieve
the same thing, it's very confusing for users). But breaking stuff for
existing users is not nice, and I know our BDFL is also not happy with
breaking things.

Thomas
Yann E. MORIN Sept. 26, 2019, 8:58 a.m. UTC | #6
Thomas, Arnout, Yann, All,

On 2019-09-26 08:55 +0200, Thomas Petazzoni spake thusly:
> On Wed, 25 Sep 2019 23:28:05 +0200
> Arnout Vandecappelle <arnout@mind.be> wrote:
> > > So, what's the point in keeping external-deps?  
> >  Kill it with fire!
> Do we really want to do that? Remember that we removed source-check,
> and Thomas DS wanted to reintroduce it because it was useful in his
> use-case ?
> 
> I'm all for dropping duplicated functionality to keep a sane code base
> and a clear user experience (when there's gazillions of ways to achieve
> the same thing, it's very confusing for users). But breaking stuff for
> existing users is not nice, and I know our BDFL is also not happy with
> breaking things.

So I would disagree with my out-of-work alter-ego, and I would consider
that external-deps is not broken: it has always explicitly just returned
the basename of the downloads and continues to do so. It is different
from what show-info provides.

Whether some people find it useful or not (I don't) is another subject.

Regards,
Yann E. MORIN.

PS. Yes, keeping this dichotomy is getting me a bit schizophrenic! ;-)
YEM.
diff mbox series

Patch

diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
index b7280e930f..ef93345595 100644
--- a/package/pkg-utils.mk
+++ b/package/pkg-utils.mk
@@ -94,6 +94,7 @@  endef
 define _json-info-pkg-details
 	"version": "$($(1)_DL_VERSION)",
 	"licenses": "$($(1)_LICENSE)",
+	"dl_dir": "$($(1)_DL_SUBDIR)",
 	"downloads": [
 	$(foreach dl,$(sort $($(1)_ALL_DOWNLOADS)),
 		{