[Buildroot] [PATCH] package/swugenerator: add missing dependencies

[Buildroot] [PATCH] package/swugenerator: add missing dependencies

[Giulio Benetti]

From: Giulio Benetti <giulio.benetti+tekvox@benettiengineering.com>

At the moment swugenerator package lacks host-openssl, host-gzip and
host-zstd. All their final executables like openssl, gzip and zstd
can be used by swugenerator if the correct flag is passed so let's
add these missing dependencies.

Signed-off-by: Giulio Benetti <giulio.benetti+tekvox@benettiengineering.com>
Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
---
 package/swugenerator/swugenerator.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/swugenerator/swugenerator.mk b/package/swugenerator/swugenerator.mk
index f4ead1d229..141a6a96c5 100644
--- a/package/swugenerator/swugenerator.mk
+++ b/package/swugenerator/swugenerator.mk
@@ -9,6 +9,6 @@ SWUGENERATOR_SITE = $(call github,sbabic,swugenerator,v$(SWUGENERATOR_VERSION))
 SWUGENERATOR_LICENSE = GPL-3.0
 SWUGENERATOR_LICENSE_FILES = LICENSE
 HOST_SWUGENERATOR_SETUP_TYPE = setuptools
-HOST_SWUGENERATOR_DEPENDENCIES = host-python-libconf
+HOST_SWUGENERATOR_DEPENDENCIES = host-gzip host-openssl host-python-libconf
 
 $(eval $(host-python-package))
-- 
2.34.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/swugenerator: add missing dependencies

[Thomas Petazzoni via buildroot]

Hello Giulio,

On Sat,  2 Dec 2023 02:51:23 +0100
Giulio Benetti <giulio.benetti@benettiengineering.com> wrote:

> From: Giulio Benetti <giulio.benetti+tekvox@benettiengineering.com>
> 
> At the moment swugenerator package lacks host-openssl, host-gzip and
> host-zstd. All their final executables like openssl, gzip and zstd
> can be used by swugenerator if the correct flag is passed so let's
> add these missing dependencies.

Could you clarify whether these are runtime dependencies? If there are
build time dependencies, do we see build failures in the autobuilders?

> diff --git a/package/swugenerator/swugenerator.mk b/package/swugenerator/swugenerator.mk
> index f4ead1d229..141a6a96c5 100644
> --- a/package/swugenerator/swugenerator.mk
> +++ b/package/swugenerator/swugenerator.mk
> @@ -9,6 +9,6 @@ SWUGENERATOR_SITE = $(call github,sbabic,swugenerator,v$(SWUGENERATOR_VERSION))
>  SWUGENERATOR_LICENSE = GPL-3.0
>  SWUGENERATOR_LICENSE_FILES = LICENSE
>  HOST_SWUGENERATOR_SETUP_TYPE = setuptools
> -HOST_SWUGENERATOR_DEPENDENCIES = host-python-libconf
> +HOST_SWUGENERATOR_DEPENDENCIES = host-gzip host-openssl host-python-libconf

In your commit log, you mention host-zstd being needed, but you don't
add it to the dependencies here. Could you clarify this as well?

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/swugenerator: add missing dependencies

[Giulio Benetti]

Hello Thomas,

On 24/12/23 12:11, Thomas Petazzoni wrote:
> Hello Giulio,
> 
> On Sat,  2 Dec 2023 02:51:23 +0100
> Giulio Benetti <giulio.benetti@benettiengineering.com> wrote:
> 
>> From: Giulio Benetti <giulio.benetti+tekvox@benettiengineering.com>
>>
>> At the moment swugenerator package lacks host-openssl, host-gzip and
>> host-zstd. All their final executables like openssl, gzip and zstd
>> can be used by swugenerator if the correct flag is passed so let's
>> add these missing dependencies.
> 
> Could you clarify whether these are runtime dependencies? If there are
> build time dependencies, do we see build failures in the autobuilders?

I haven't thought about it. They are runtime dependencies, so we can't
see failure in the autobuilders.

>> diff --git a/package/swugenerator/swugenerator.mk b/package/swugenerator/swugenerator.mk
>> index f4ead1d229..141a6a96c5 100644
>> --- a/package/swugenerator/swugenerator.mk
>> +++ b/package/swugenerator/swugenerator.mk
>> @@ -9,6 +9,6 @@ SWUGENERATOR_SITE = $(call github,sbabic,swugenerator,v$(SWUGENERATOR_VERSION))
>>   SWUGENERATOR_LICENSE = GPL-3.0
>>   SWUGENERATOR_LICENSE_FILES = LICENSE
>>   HOST_SWUGENERATOR_SETUP_TYPE = setuptools
>> -HOST_SWUGENERATOR_DEPENDENCIES = host-python-libconf
>> +HOST_SWUGENERATOR_DEPENDENCIES = host-gzip host-openssl host-python-libconf
> 
> In your commit log, you mention host-zstd being needed, but you don't
> add it to the dependencies here. Could you clarify this as well?

Ah right, I simply need to add host-zstd because swugenerator can use
gzip or zstd at runtime.

Thank you for reviewing.

Best regards
-- 
Giulio Benetti
CEO&CTO@Benetti Engineering sas
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/swugenerator: add missing dependencies

[Thomas Petazzoni via buildroot]

Hello Giulio,

On Tue, 26 Dec 2023 22:28:01 +0100
Giulio Benetti <giulio.benetti@benettiengineering.com> wrote:

> > Could you clarify whether these are runtime dependencies? If there are
> > build time dependencies, do we see build failures in the autobuilders?  
> 
> I haven't thought about it. They are runtime dependencies, so we can't
> see failure in the autobuilders.

OK. Then ideally they should be identified as such so we don't forget
in the future :)

> >> +HOST_SWUGENERATOR_DEPENDENCIES = host-gzip host-openssl host-python-libconf  
> > 
> > In your commit log, you mention host-zstd being needed, but you don't
> > add it to the dependencies here. Could you clarify this as well?  
> 
> Ah right, I simply need to add host-zstd because swugenerator can use
> gzip or zstd at runtime.

So how to we handle this? We put only one of the two (which one?) or
both?

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v2] package/swugenerator: add missing runtime dependencies

[Giulio Benetti]

From: Giulio Benetti <giulio.benetti+tekvox@benettiengineering.com>

At the moment swugenerator package lacks host-openssl, host-gzip and
host-zstd runtime dependency so let's add them to prevent swugenerator from
failing to run.

Signed-off-by: Giulio Benetti <giulio.benetti+tekvox@benettiengineering.com>
---
V1->V2:
* add host-zstd and clarify in commit log these are runtime dependencies as
  pointed by Thomas Petazzoni
---
 package/swugenerator/swugenerator.mk | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/package/swugenerator/swugenerator.mk b/package/swugenerator/swugenerator.mk
index f4ead1d229..466fe1f046 100644
--- a/package/swugenerator/swugenerator.mk
+++ b/package/swugenerator/swugenerator.mk
@@ -9,6 +9,10 @@ SWUGENERATOR_SITE = $(call github,sbabic,swugenerator,v$(SWUGENERATOR_VERSION))
 SWUGENERATOR_LICENSE = GPL-3.0
 SWUGENERATOR_LICENSE_FILES = LICENSE
 HOST_SWUGENERATOR_SETUP_TYPE = setuptools
-HOST_SWUGENERATOR_DEPENDENCIES = host-python-libconf
+HOST_SWUGENERATOR_DEPENDENCIES = \
+	host-gzip \
+	host-openssl \
+	host-python-libconf \
+	host-zstd
 
 $(eval $(host-python-package))
-- 
2.34.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/swugenerator: add missing runtime dependencies

[Thomas Petazzoni via buildroot]

Hello Giulio,

Thanks for the v2!

On Tue, 26 Dec 2023 22:40:37 +0100
Giulio Benetti <giulio.benetti@benettiengineering.com> wrote:

> From: Giulio Benetti <giulio.benetti+tekvox@benettiengineering.com>
> 
> At the moment swugenerator package lacks host-openssl, host-gzip and
> host-zstd runtime dependency so let's add them to prevent swugenerator from
> failing to run.

I'm going to nitpick a bit (yes, I admit), but this commit log explains
what the commit does, not why it does it. The *why* is important. The
problem is not that swugenerator lacks host-openssl, host-gzip and
host-zstd.

The problem is that swugenerator can (apparently) use openssl, gzip and
zstd on the host (under which conditions?), and therefore we should add
host-openssl, host-gzip and host-zstd as runtime dependencies.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/swugenerator: add missing runtime dependencies

[Giulio Benetti]

Hello Thomas,

> Il giorno 5 gen 2024, alle ore 14:07, Thomas Petazzoni <thomas.petazzoni@bootlin.com> ha scritto:
> 
> Hello Giulio,
> 
> Thanks for the v2!
> 
>> On Tue, 26 Dec 2023 22:40:37 +0100
>> Giulio Benetti <giulio.benetti@benettiengineering.com> wrote:
>> 
>> From: Giulio Benetti <giulio.benetti+tekvox@benettiengineering.com>
>> 
>> At the moment swugenerator package lacks host-openssl, host-gzip and
>> host-zstd runtime dependency so let's add them to prevent swugenerator from
>> failing to run.
> 
> I'm going to nitpick a bit (yes, I admit), but this commit log explains
> what the commit does, not why it does it. The *why* is important. The
> problem is not that swugenerator lacks host-openssl, host-gzip and
> host-zstd.

Absolutely,

> 
> The problem is that swugenerator can (apparently) use openssl, gzip and
> zstd on the host (under which conditions?), and therefore we should add
> host-openssl, host-gzip and host-zstd as runtime dependencies.

Yes. To tell the truth I don’t even remember why I have sent V2 since I still have to answer to previous email, including the runtime test to check for host dependencies.

So thank you for pointing how to improve commit log, I will take care of all.

Best regards
Giulio

> 
> Thomas
> --
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering and training
> https://bootlin.com
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/swugenerator: add missing runtime dependencies

[Thomas Petazzoni via buildroot]

Hello Giulio,

On Fri, 5 Jan 2024 14:22:15 +0100
Giulio Benetti <giulio.benetti@benettiengineering.com> wrote:

> Yes. To tell the truth I don’t even remember why I have sent V2 since
> I still have to answer to previous email, including the runtime test
> to check for host dependencies.

A runtime test would obviously be good, but I clearly to make that a
requirement to merge this patch. Only to have a slightly better commit
log that explains how/why openssl/gzip/zstd are used by swugenerator.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v3] package/swugenerator: add missing runtime dependencies

[Giulio Benetti]

From: Giulio Benetti <giulio.benetti+tekvox@benettiengineering.com>

Package swugenerator can use openssl to encrypt files in .swu(cpio)
resulting output archive and gzip or zlib to compress the same files but
we're not sure build environment have or not openssl, gzip and zlib
installed so add them as host dependencies like:
* host-openssl
* host-gzip
* host-zlib

Signed-off-by: Giulio Benetti <giulio.benetti+tekvox@benettiengineering.com>
---
V1->V2:
* V1->V2:
* add host-zstd and clarify in commit log these are runtime dependencies as
  pointed by Thomas Petazzoni
V2->V3:
* improve commit log explaining why we need this patch as pointed by
  Thomas Petazzoni
---
 package/swugenerator/swugenerator.mk | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/package/swugenerator/swugenerator.mk b/package/swugenerator/swugenerator.mk
index f4ead1d229..466fe1f046 100644
--- a/package/swugenerator/swugenerator.mk
+++ b/package/swugenerator/swugenerator.mk
@@ -9,6 +9,10 @@ SWUGENERATOR_SITE = $(call github,sbabic,swugenerator,v$(SWUGENERATOR_VERSION))
 SWUGENERATOR_LICENSE = GPL-3.0
 SWUGENERATOR_LICENSE_FILES = LICENSE
 HOST_SWUGENERATOR_SETUP_TYPE = setuptools
-HOST_SWUGENERATOR_DEPENDENCIES = host-python-libconf
+HOST_SWUGENERATOR_DEPENDENCIES = \
+	host-gzip \
+	host-openssl \
+	host-python-libconf \
+	host-zstd
 
 $(eval $(host-python-package))
-- 
2.34.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3] package/swugenerator: add missing runtime dependencies

[Giulio Benetti]

Re-reading again...

On 07/01/24 12:59, Giulio Benetti wrote:
> From: Giulio Benetti <giulio.benetti+tekvox@benettiengineering.com>
> 
> Package swugenerator can use openssl to encrypt files in .swu(cpio)
> resulting output archive and gzip or zlib to compress the same files but
> we're not sure build environment have or not openssl, gzip and zlib
> installed 

and this can lead to runtime execution failure,

> so add them as host dependencies like:
> * host-openssl
> * host-gzip
> * host-zlib
> 

Could please a Maintainer reword this commit log adding that missing 
line that completes the answer to *why* we need *what*(add missing 
dependencies)?

I hope this time I've got close to a sufficient commit log :-)

Best regards
-- 
Giulio Benetti
CEO&CTO@Benetti Engineering sas

> Signed-off-by: Giulio Benetti <giulio.benetti+tekvox@benettiengineering.com>
> ---
> V1->V2:
> * V1->V2:
> * add host-zstd and clarify in commit log these are runtime dependencies as
>    pointed by Thomas Petazzoni
> V2->V3:
> * improve commit log explaining why we need this patch as pointed by
>    Thomas Petazzoni
> ---
>   package/swugenerator/swugenerator.mk | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/package/swugenerator/swugenerator.mk b/package/swugenerator/swugenerator.mk
> index f4ead1d229..466fe1f046 100644
> --- a/package/swugenerator/swugenerator.mk
> +++ b/package/swugenerator/swugenerator.mk
> @@ -9,6 +9,10 @@ SWUGENERATOR_SITE = $(call github,sbabic,swugenerator,v$(SWUGENERATOR_VERSION))
>   SWUGENERATOR_LICENSE = GPL-3.0
>   SWUGENERATOR_LICENSE_FILES = LICENSE
>   HOST_SWUGENERATOR_SETUP_TYPE = setuptools
> -HOST_SWUGENERATOR_DEPENDENCIES = host-python-libconf
> +HOST_SWUGENERATOR_DEPENDENCIES = \
> +	host-gzip \
> +	host-openssl \
> +	host-python-libconf \
> +	host-zstd
>   
>   $(eval $(host-python-package))

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3] package/swugenerator: add missing runtime dependencies

[Thomas Petazzoni via buildroot]

On Sun,  7 Jan 2024 12:59:58 +0100
Giulio Benetti <giulio.benetti@benettiengineering.com> wrote:

> From: Giulio Benetti <giulio.benetti+tekvox@benettiengineering.com>
> 
> Package swugenerator can use openssl to encrypt files in .swu(cpio)
> resulting output archive and gzip or zlib to compress the same files but
> we're not sure build environment have or not openssl, gzip and zlib
> installed so add them as host dependencies like:
> * host-openssl
> * host-gzip
> * host-zlib
> 
> Signed-off-by: Giulio Benetti <giulio.benetti+tekvox@benettiengineering.com>
> ---
> V1->V2:
> * V1->V2:
> * add host-zstd and clarify in commit log these are runtime dependencies as
>   pointed by Thomas Petazzoni
> V2->V3:
> * improve commit log explaining why we need this patch as pointed by
>   Thomas Petazzoni
> ---
>  package/swugenerator/swugenerator.mk | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot