* [Buildroot] [PATCH v4 1/1] package/libsodium: add config for full build
@ 2019-09-23 8:29 Adrien Gallouët
2019-09-28 21:19 ` Thomas Petazzoni
0 siblings, 1 reply; 8+ messages in thread
From: Adrien Gallouët @ 2019-09-23 8:29 UTC (permalink / raw)
To: buildroot
A new option is added to build all functions.
This option was implicitly used before, so it is activated by default to
avoid unpleasant surprises.
Signed-off-by: Adrien Gallou?t <adrien@gallouet.fr>
---
Changes v3 -> v4:
- better commit log (suggested by Thomas Petazzoni)
Changes v2 -> v3:
- add a better help (suggested by Arnout Vandecappelle)
Changes v1 -> v2:
- full build (suggested by Arnout Vandecappelle)
---
package/libsodium/Config.in | 12 ++++++++++++
package/libsodium/libsodium.mk | 4 ++++
2 files changed, 16 insertions(+)
diff --git a/package/libsodium/Config.in b/package/libsodium/Config.in
index 47b9bb350c..518b0532ed 100644
--- a/package/libsodium/Config.in
+++ b/package/libsodium/Config.in
@@ -4,3 +4,15 @@ config BR2_PACKAGE_LIBSODIUM
A modern and easy-to-use crypto library.
http://libsodium.org/
+
+if BR2_PACKAGE_LIBSODIUM
+
+config BR2_PACKAGE_LIBSODIUM_FULL
+ bool "all functions"
+ default y
+ help
+ Build all seldom used and obsolete functions. Without
+ this option, only the minimal set of normally used
+ functions is built.
+
+endif
diff --git a/package/libsodium/libsodium.mk b/package/libsodium/libsodium.mk
index a94a8271c5..9afa09ff37 100644
--- a/package/libsodium/libsodium.mk
+++ b/package/libsodium/libsodium.mk
@@ -14,5 +14,9 @@ ifeq ($(BR2_TOOLCHAIN_SUPPORTS_PIE),)
LIBSODIUM_CONF_OPTS += --disable-pie
endif
+ifeq ($(BR2_PACKAGE_LIBSODIUM_FULL),)
+LIBSODIUM_CONF_OPTS += --enable-minimal
+endif
+
$(eval $(autotools-package))
$(eval $(host-autotools-package))
--
2.19.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Buildroot] [PATCH v4 1/1] package/libsodium: add config for full build
2019-09-23 8:29 [Buildroot] [PATCH v4 1/1] package/libsodium: add config for full build Adrien Gallouët
@ 2019-09-28 21:19 ` Thomas Petazzoni
2019-09-28 21:49 ` Adrien Gallouët
0 siblings, 1 reply; 8+ messages in thread
From: Thomas Petazzoni @ 2019-09-28 21:19 UTC (permalink / raw)
To: buildroot
Hello,
On Mon, 23 Sep 2019 08:29:03 +0000
Adrien Gallou?t <adrien@gallouet.fr> wrote:
> +config BR2_PACKAGE_LIBSODIUM_FULL
> + bool "all functions"
> + default y
> + help
> + Build all seldom used and obsolete functions. Without
> + this option, only the minimal set of normally used
> + functions is built.
> +
> +endif
> diff --git a/package/libsodium/libsodium.mk b/package/libsodium/libsodium.mk
> index a94a8271c5..9afa09ff37 100644
> --- a/package/libsodium/libsodium.mk
> +++ b/package/libsodium/libsodium.mk
> @@ -14,5 +14,9 @@ ifeq ($(BR2_TOOLCHAIN_SUPPORTS_PIE),)
> LIBSODIUM_CONF_OPTS += --disable-pie
> endif
>
> +ifeq ($(BR2_PACKAGE_LIBSODIUM_FULL),)
> +LIBSODIUM_CONF_OPTS += --enable-minimal
But then, does it make sense to invert the option and name it
BR2_PACKAGE_LIBSODIUM_MINIMAL ? It would match the
--enable-minimal/--disable-minimal autoconf options of libsodium.
Also, we need to pass --disable-minimal in an "else" clause.
Thanks,
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Buildroot] [PATCH v4 1/1] package/libsodium: add config for full build
2019-09-28 21:19 ` Thomas Petazzoni
@ 2019-09-28 21:49 ` Adrien Gallouët
2019-09-29 7:04 ` Thomas Petazzoni
0 siblings, 1 reply; 8+ messages in thread
From: Adrien Gallouët @ 2019-09-28 21:49 UTC (permalink / raw)
To: buildroot
Hello,
On Sat, Sep 28, 2019 at 11:19 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello,
>
> On Mon, 23 Sep 2019 08:29:03 +0000
> Adrien Gallou?t <adrien@gallouet.fr> wrote:
>
> > +config BR2_PACKAGE_LIBSODIUM_FULL
> > + bool "all functions"
> > + default y
> > + help
> > + Build all seldom used and obsolete functions. Without
> > + this option, only the minimal set of normally used
> > + functions is built.
> > +
> > +endif
> > diff --git a/package/libsodium/libsodium.mk b/package/libsodium/libsodium.mk
> > index a94a8271c5..9afa09ff37 100644
> > --- a/package/libsodium/libsodium.mk
> > +++ b/package/libsodium/libsodium.mk
> > @@ -14,5 +14,9 @@ ifeq ($(BR2_TOOLCHAIN_SUPPORTS_PIE),)
> > LIBSODIUM_CONF_OPTS += --disable-pie
> > endif
> >
> > +ifeq ($(BR2_PACKAGE_LIBSODIUM_FULL),)
> > +LIBSODIUM_CONF_OPTS += --enable-minimal
>
> But then, does it make sense to invert the option and name it
> BR2_PACKAGE_LIBSODIUM_MINIMAL ? It would match the
> --enable-minimal/--disable-minimal autoconf options of libsodium.
That was my first patch :) Do you want me to resubmit it ?
>
> Also, we need to pass --disable-minimal in an "else" clause.
>
> Thanks,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Regards,
Adrien
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Buildroot] [PATCH v4 1/1] package/libsodium: add config for full build
2019-09-28 21:49 ` Adrien Gallouët
@ 2019-09-29 7:04 ` Thomas Petazzoni
2019-09-29 10:51 ` Arnout Vandecappelle
0 siblings, 1 reply; 8+ messages in thread
From: Thomas Petazzoni @ 2019-09-29 7:04 UTC (permalink / raw)
To: buildroot
On Sat, 28 Sep 2019 23:49:56 +0200
Adrien Gallou?t <adrien@gallouet.fr> wrote:
> > But then, does it make sense to invert the option and name it
> > BR2_PACKAGE_LIBSODIUM_MINIMAL ? It would match the
> > --enable-minimal/--disable-minimal autoconf options of libsodium.
>
> That was my first patch :) Do you want me to resubmit it ?
I don't have a very clear-cut opinion.
On one hand, it is weird to have a Config.in option to reduce/remove
something. But on the other hand, it makes sense to have Config.in
options that are as much as possible mapped to the corresponding
options provided by the package build system.
Arnout, Peter, any preference ?
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Buildroot] [PATCH v4 1/1] package/libsodium: add config for full build
2019-09-29 7:04 ` Thomas Petazzoni
@ 2019-09-29 10:51 ` Arnout Vandecappelle
2019-09-30 12:04 ` Peter Korsgaard
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Arnout Vandecappelle @ 2019-09-29 10:51 UTC (permalink / raw)
To: buildroot
On 29/09/2019 09:04, Thomas Petazzoni wrote:
> On Sat, 28 Sep 2019 23:49:56 +0200
> Adrien Gallou?t <adrien@gallouet.fr> wrote:
>
>>> But then, does it make sense to invert the option and name it
>>> BR2_PACKAGE_LIBSODIUM_MINIMAL ? It would match the
>>> --enable-minimal/--disable-minimal autoconf options of libsodium.
>>
>> That was my first patch :) Do you want me to resubmit it ?
>
> I don't have a very clear-cut opinion.
>
> On one hand, it is weird to have a Config.in option to reduce/remove
> something. But on the other hand, it makes sense to have Config.in
> options that are as much as possible mapped to the corresponding
> options provided by the package build system.
>
> Arnout, Peter, any preference ?
It was me who suggested to turn around the logic.
I believe we have had problems in the past where there was some MINIMAL option,
but then some other package needed something from the non-minimal set. Then it
has to do a 'depends on' instead of a 'select'.
So it's not a matter of being weird, it's a matter of making sure we don't
create problems in the future.
>> Also, we need to pass --disable-minimal in an "else" clause.
This is still true. Adrien, I believe I did say something to that effect in an
earlier iteration, and I also believe you replied to it. But my memory is
worthless :-)
Regards,
Arnout
^ permalink raw reply [flat|nested] 8+ messages in thread* [Buildroot] [PATCH v4 1/1] package/libsodium: add config for full build
2019-09-29 10:51 ` Arnout Vandecappelle
@ 2019-09-30 12:04 ` Peter Korsgaard
2019-09-30 12:29 ` Thomas Petazzoni
2019-10-24 8:30 ` Peter Korsgaard
2 siblings, 0 replies; 8+ messages in thread
From: Peter Korsgaard @ 2019-09-30 12:04 UTC (permalink / raw)
To: buildroot
>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:
Hi,
>> Arnout, Peter, any preference ?
> It was me who suggested to turn around the logic.
> I believe we have had problems in the past where there was some MINIMAL option,
> but then some other package needed something from the non-minimal set. Then it
> has to do a 'depends on' instead of a 'select'.
> So it's not a matter of being weird, it's a matter of making sure we don't
> create problems in the future.
Yes, indeed - I would prefer to keep it like this (a "positive" option).
>>> Also, we need to pass --disable-minimal in an "else" clause.
> This is still true. Adrien, I believe I did say something to that effect in an
> earlier iteration, and I also believe you replied to it. But my memory is
> worthless :-)
Heh ;)
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Buildroot] [PATCH v4 1/1] package/libsodium: add config for full build
2019-09-29 10:51 ` Arnout Vandecappelle
2019-09-30 12:04 ` Peter Korsgaard
@ 2019-09-30 12:29 ` Thomas Petazzoni
2019-10-24 8:30 ` Peter Korsgaard
2 siblings, 0 replies; 8+ messages in thread
From: Thomas Petazzoni @ 2019-09-30 12:29 UTC (permalink / raw)
To: buildroot
On Sun, 29 Sep 2019 12:51:35 +0200
Arnout Vandecappelle <arnout@mind.be> wrote:
> I believe we have had problems in the past where there was some MINIMAL option,
> but then some other package needed something from the non-minimal set. Then it
> has to do a 'depends on' instead of a 'select'.
That makes a lot of sense indeed. So we should always have option that
"add" more stuff, and never option that "remove" stuff for that reason.
Thanks for the feedback, I hadn't thought about this.
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Buildroot] [PATCH v4 1/1] package/libsodium: add config for full build
2019-09-29 10:51 ` Arnout Vandecappelle
2019-09-30 12:04 ` Peter Korsgaard
2019-09-30 12:29 ` Thomas Petazzoni
@ 2019-10-24 8:30 ` Peter Korsgaard
2 siblings, 0 replies; 8+ messages in thread
From: Peter Korsgaard @ 2019-10-24 8:30 UTC (permalink / raw)
To: buildroot
>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:
Hi,
>> I don't have a very clear-cut opinion.
>>
>> On one hand, it is weird to have a Config.in option to reduce/remove
>> something. But on the other hand, it makes sense to have Config.in
>> options that are as much as possible mapped to the corresponding
>> options provided by the package build system.
>>
>> Arnout, Peter, any preference ?
> It was me who suggested to turn around the logic.
> I believe we have had problems in the past where there was some MINIMAL option,
> but then some other package needed something from the non-minimal set. Then it
> has to do a 'depends on' instead of a 'select'.
> So it's not a matter of being weird, it's a matter of making sure we don't
> create problems in the future.
Agreed, having options to disable stuff gets tricky when you have other
packages needing the disabled stuff.
>>> Also, we need to pass --disable-minimal in an "else" clause.
> This is still true. Adrien, I believe I did say something to that effect in an
> earlier iteration, and I also believe you replied to it. But my memory is
> worthless :-)
Getting old? :P
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-10-24 8:30 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-09-23 8:29 [Buildroot] [PATCH v4 1/1] package/libsodium: add config for full build Adrien Gallouët
2019-09-28 21:19 ` Thomas Petazzoni
2019-09-28 21:49 ` Adrien Gallouët
2019-09-29 7:04 ` Thomas Petazzoni
2019-09-29 10:51 ` Arnout Vandecappelle
2019-09-30 12:04 ` Peter Korsgaard
2019-09-30 12:29 ` Thomas Petazzoni
2019-10-24 8:30 ` Peter Korsgaard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox