[Buildroot] [PATCH v2 1/2] pkg-generic: Depend on host-{xz, lzip} only for fitting archives

[Buildroot] [PATCH v2 1/2] pkg-generic: Depend on host-{xz, lzip} only for fitting archives

[Norbert Lange]

results in cleaner dependency graphs, and usually omits host-lzip
from being built.

Signed-off-by: Norbert Lange <nolange79@gmail.com>
---
 package/pkg-generic.mk | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 4353bd3868..89da43d5e5 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -607,12 +607,16 @@ $(2)_EXTRACT_DEPENDENCIES += $$(BR2_TAR_HOST_DEPENDENCY)
 endif
 
 ifeq ($$(filter host-tar host-skeleton host-xz host-lzip host-fakedate,$(1)),)
+ifneq ($$(filter .xz .lzma,$$(suffix $$($(2)_SOURCE))),)
 $(2)_EXTRACT_DEPENDENCIES += $$(BR2_XZCAT_HOST_DEPENDENCY)
 endif
+endif
 
 ifeq ($$(filter host-tar host-skeleton host-xz host-lzip host-fakedate,$(1)),)
+ifeq ($$(suffix $$($(2)_SOURCE)),.lz)
 $(2)_EXTRACT_DEPENDENCIES += $$(BR2_LZIP_HOST_DEPENDENCY)
 endif
+endif
 
 ifeq ($$(BR2_CCACHE),y)
 ifeq ($$(filter host-tar host-skeleton host-xz host-lzip host-fakedate host-ccache,$(1)),)
-- 
2.20.1

[Buildroot] [PATCH v2 2/2] pkg-generic: Cleanup some redundant checks

[Norbert Lange]

With the last change, packages will depend
only on host-{xz,lzip} if the source archives
have filenames requesting the corresponding compressor.

This allows using a single guard.

Signed-off-by: Norbert Lange <nolange79@gmail.com>
---
 package/pkg-generic.mk | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 89da43d5e5..11f16cab18 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -604,18 +604,15 @@ endif
 
 ifeq ($$(filter host-tar host-skeleton host-fakedate,$(1)),)
 $(2)_EXTRACT_DEPENDENCIES += $$(BR2_TAR_HOST_DEPENDENCY)
-endif
 
-ifeq ($$(filter host-tar host-skeleton host-xz host-lzip host-fakedate,$(1)),)
 ifneq ($$(filter .xz .lzma,$$(suffix $$($(2)_SOURCE))),)
 $(2)_EXTRACT_DEPENDENCIES += $$(BR2_XZCAT_HOST_DEPENDENCY)
 endif
-endif
 
-ifeq ($$(filter host-tar host-skeleton host-xz host-lzip host-fakedate,$(1)),)
 ifeq ($$(suffix $$($(2)_SOURCE)),.lz)
 $(2)_EXTRACT_DEPENDENCIES += $$(BR2_LZIP_HOST_DEPENDENCY)
 endif
+
 endif
 
 ifeq ($$(BR2_CCACHE),y)
-- 
2.20.1

[Buildroot] [PATCH v2 2/2] pkg-generic: Cleanup some redundant checks

[Yann E. MORIN]

Norbert, All,

On 2019-03-17 22:20 +0100, Norbert Lange spake thusly:
> With the last change, packages will depend
> only on host-{xz,lzip} if the source archives
> have filenames requesting the corresponding compressor.
> 
> This allows using a single guard.

This commit log is an improvement agaisnt the previous one, yet I still
had to think a bit too hard to understand the reason that works.

And I think there is an issue with that. Not today, but that opens up a
case where we can introduce a subtil bug in the future.

If, say, xz changes its distribution archive from .bz2 to .lz, iand lzip
changes theirs from .gz to .xz, then we'd introduce a circular
dependency at the make level, and make silently and arbitrarily drops
one of the dependencies.

However, now that I think about it, that is of not big consequence: in
either case, the build woulld break on the first one we try to extract,
and we would notice quite early and quite easily.

Still, I'd like we think a bit harder about those special cases.

Regards,
Yann E. MORIN.

> Signed-off-by: Norbert Lange <nolange79@gmail.com>
> ---
>  package/pkg-generic.mk | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 89da43d5e5..11f16cab18 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -604,18 +604,15 @@ endif
>  
>  ifeq ($$(filter host-tar host-skeleton host-fakedate,$(1)),)
>  $(2)_EXTRACT_DEPENDENCIES += $$(BR2_TAR_HOST_DEPENDENCY)
> -endif
>  
> -ifeq ($$(filter host-tar host-skeleton host-xz host-lzip host-fakedate,$(1)),)
>  ifneq ($$(filter .xz .lzma,$$(suffix $$($(2)_SOURCE))),)
>  $(2)_EXTRACT_DEPENDENCIES += $$(BR2_XZCAT_HOST_DEPENDENCY)
>  endif
> -endif
>  
> -ifeq ($$(filter host-tar host-skeleton host-xz host-lzip host-fakedate,$(1)),)
>  ifeq ($$(suffix $$($(2)_SOURCE)),.lz)
>  $(2)_EXTRACT_DEPENDENCIES += $$(BR2_LZIP_HOST_DEPENDENCY)
>  endif
> +
>  endif
>  
>  ifeq ($$(BR2_CCACHE),y)
> -- 
> 2.20.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

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

[Buildroot] [PATCH v2 2/2] pkg-generic: Cleanup some redundant checks

[Norbert Lange]

Am Di., 19. M?rz 2019 um 23:03 Uhr schrieb Yann E. MORIN
<yann.morin.1998@free.fr>:
>
> Norbert, All,
>
> On 2019-03-17 22:20 +0100, Norbert Lange spake thusly:
> > With the last change, packages will depend
> > only on host-{xz,lzip} if the source archives
> > have filenames requesting the corresponding compressor.
> >
> > This allows using a single guard.
>
> This commit log is an improvement agaisnt the previous one, yet I still
> had to think a bit too hard to understand the reason that works.
>
> And I think there is an issue with that. Not today, but that opens up a
> case where we can introduce a subtil bug in the future.
>
> If, say, xz changes its distribution archive from .bz2 to .lz, iand lzip
> changes theirs from .gz to .xz, then we'd introduce a circular
> dependency at the make level, and make silently and arbitrarily drops
> one of the dependencies.

What would you do in that case anyway?
(besides I dont think lzip is getting any traction, or that a sane compressor
package should use anything but gzip for "boostrapping").

> However, now that I think about it, that is of not big consequence: in
> either case, the build woulld break on the first one we try to extract,
> and we would notice quite early and quite easily.
>
> Still, I'd like we think a bit harder about those special cases.

I really don't think the patch makes things worse.

Norbert

[Buildroot] [PATCH v2 2/2] pkg-generic: Cleanup some redundant checks

[Thomas Petazzoni]

Hello Norbert,

On Sun, 17 Mar 2019 22:20:14 +0100
Norbert Lange <nolange79@gmail.com> wrote:

> With the last change, packages will depend
> only on host-{xz,lzip} if the source archives
> have filenames requesting the corresponding compressor.
> 
> This allows using a single guard.
> 
> Signed-off-by: Norbert Lange <nolange79@gmail.com>
> ---
>  package/pkg-generic.mk | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)

I hesitated a bit on this. Originally, I was quite in favor of the
change and found Yann's opinion a bit too strict. However, after
applying the patch, and looking at the code, I ended up feeling a bit
like Yann: even though there are less lines of code, the conditions
become a bit tricky, while currently, they are perfectly clear.

So I'll mark this patch as Rejected. Of course, if other BR maintainers
object, we can revisit, my choice is not super strong.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2 2/2] pkg-generic: Cleanup some redundant checks

[Arnout Vandecappelle]

On 17/03/2019 22:20, Norbert Lange wrote:
> With the last change, packages will depend
> only on host-{xz,lzip} if the source archives
> have filenames requesting the corresponding compressor.
> 
> This allows using a single guard.
> 
> Signed-off-by: Norbert Lange <nolange79@gmail.com>

 I disagree with Yann and Thomas that this makes it less clear. Except, I don't
think you should have even the single guard...

 Regarding the potential circular dependency that Yann mentioned: yes, that's
true. But conversely, by adding the filter like we have now, you remove a
potentially required dependency. So in the end it's the same thing. And for
circular dependencies we have graph-depends that detects them.

> ---
>  package/pkg-generic.mk | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 89da43d5e5..11f16cab18 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -604,18 +604,15 @@ endif
>  
>  ifeq ($$(filter host-tar host-skeleton host-fakedate,$(1)),)

 This condition is needed because skeleton and fakedate are dependencies of tar,
but host-skeleton and host-fakedate don't actually need tar. However, since
there's nothing that detects that host-skeleton and host-fakedate don't have a
tarball to extract, we have to remove those dependencies by hand.

>  $(2)_EXTRACT_DEPENDENCIES += $$(BR2_TAR_HOST_DEPENDENCY)
> -endif
>  
> -ifeq ($$(filter host-tar host-skeleton host-xz host-lzip host-fakedate,$(1)),)
>  ifneq ($$(filter .xz .lzma,$$(suffix $$($(2)_SOURCE))),)

 Here, however, there is an explicit dependency. So the condition on tar,
skeleton, fakedate is not needed.

 Bottom line: I don't think the endif above should be removed.


 Regards,
 Arnout


>  $(2)_EXTRACT_DEPENDENCIES += $$(BR2_XZCAT_HOST_DEPENDENCY)
>  endif
> -endif
>  
> -ifeq ($$(filter host-tar host-skeleton host-xz host-lzip host-fakedate,$(1)),)
>  ifeq ($$(suffix $$($(2)_SOURCE)),.lz)
>  $(2)_EXTRACT_DEPENDENCIES += $$(BR2_LZIP_HOST_DEPENDENCY)
>  endif
> +
>  endif
>  
>  ifeq ($$(BR2_CCACHE),y)
> 

[Buildroot] [PATCH v2 1/2] pkg-generic: Depend on host-{xz, lzip} only for fitting archives

[Yann E. MORIN]

Norbert, All,

On 2019-03-17 22:20 +0100, Norbert Lange spake thusly:
> results in cleaner dependency graphs, and usually omits host-lzip
> from being built.

As for dependency graphs, there will still be a lot of package that may
depend on host-xz, and that would still clutter it a bit more than
necessary (we have much less packages needing host-lzip, 4 exactly).

But that sure is still a nice improvement.

> Signed-off-by: Norbert Lange <nolange79@gmail.com>
> ---
>  package/pkg-generic.mk | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 4353bd3868..89da43d5e5 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -607,12 +607,16 @@ $(2)_EXTRACT_DEPENDENCIES += $$(BR2_TAR_HOST_DEPENDENCY)
>  endif
>  
>  ifeq ($$(filter host-tar host-skeleton host-xz host-lzip host-fakedate,$(1)),)
> +ifneq ($$(filter .xz .lzma,$$(suffix $$($(2)_SOURCE))),)
>  $(2)_EXTRACT_DEPENDENCIES += $$(BR2_XZCAT_HOST_DEPENDENCY)
>  endif
> +endif
>  
>  ifeq ($$(filter host-tar host-skeleton host-xz host-lzip host-fakedate,$(1)),)
> +ifeq ($$(suffix $$($(2)_SOURCE)),.lz)

Please use the same pattern here as for the other one. It helps with
readability and maintainability.

Regards,
Yann E. MORIN.

>  $(2)_EXTRACT_DEPENDENCIES += $$(BR2_LZIP_HOST_DEPENDENCY)
>  endif
> +endif
>  
>  ifeq ($$(BR2_CCACHE),y)
>  ifeq ($$(filter host-tar host-skeleton host-xz host-lzip host-fakedate host-ccache,$(1)),)
> -- 
> 2.20.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

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

[Buildroot] [PATCH v2 1/2] pkg-generic: Depend on host-{xz, lzip} only for fitting archives

[Thomas Petazzoni]

On Sun, 17 Mar 2019 22:20:13 +0100
Norbert Lange <nolange79@gmail.com> wrote:

> results in cleaner dependency graphs, and usually omits host-lzip
> from being built.
> 
> Signed-off-by: Norbert Lange <nolange79@gmail.com>
> ---
>  package/pkg-generic.mk | 4 ++++
>  1 file changed, 4 insertions(+)

I improved the commit log, modified the code as suggested by Yann, and
applied. Thanks, this is a very useful change!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2 1/2] pkg-generic: Depend on host-{xz, lzip} only for fitting archives

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes:

 > On Sun, 17 Mar 2019 22:20:13 +0100
 > Norbert Lange <nolange79@gmail.com> wrote:

 >> results in cleaner dependency graphs, and usually omits host-lzip
 >> from being built.
 >> 
 >> Signed-off-by: Norbert Lange <nolange79@gmail.com>
 >> ---
 >> package/pkg-generic.mk | 4 ++++
 >> 1 file changed, 4 insertions(+)

 > I improved the commit log, modified the code as suggested by Yann, and
 > applied. Thanks, this is a very useful change!

Committed to 2019.02.x, thanks.

-- 
Bye, Peter Korsgaard