[Buildroot] [PATCH 1/1] package/libsodium: add config for minimal build

[Adrien Gallouët]

Hi Arnout,

On Sat, Sep 21, 2019 at 8:17 PM Arnout Vandecappelle <arnout at mind.be> wrote:
>
>  Hi Adrien,
>
> On 21/09/2019 11:11, Adrien Gallouët wrote:
> > Signed-off-by: Adrien Gallouët <adrien at gallouet.fr>
> > ---
> >  package/libsodium/Config.in    | 9 +++++++++
> >  package/libsodium/libsodium.mk | 4 ++++
> >  2 files changed, 13 insertions(+)
> >
> > diff --git a/package/libsodium/Config.in b/package/libsodium/Config.in
> > index 47b9bb350c..e9b9960dae 100644
> > --- a/package/libsodium/Config.in
> > +++ b/package/libsodium/Config.in
> > @@ -4,3 +4,12 @@ config BR2_PACKAGE_LIBSODIUM
> >         A modern and easy-to-use crypto library.
> >
> >         http://libsodium.org/
> > +
> > +if BR2_PACKAGE_LIBSODIUM
> > +
> > +config BR2_PACKAGE_LIBSODIUM_MINIMAL
> > +     bool "minimal"
> > +     help
> > +       Build minimal version.
>
>  Could you extend the help text a little with what this implies, i.e. which
> features are not available in the minimal case?
>
>  Also, in general, it's better to have options that add something rather than
> remove something. Imagine that there is some package that requires a feature
> that is not included in the minimal set. It can then not do something like
> "select !BR2_PACKAGE_LIBSODIUM_MINIMAL".
>
>  So, another way to do this would be to add an option
> BR2_PACKAGE_LIBSODIUM_FULL, and pass --enable-minimal if it is *not* set. This
> option should then default to y to make sure existing configs will still work.
>
> > +
> > +endif
> > diff --git a/package/libsodium/libsodium.mk b/package/libsodium/libsodium.mk
> > index a94a8271c5..17bccb5796 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_MINIMAL),y)
> > +LIBSODIUM_CONF_OPTS += --enable-minimal
>
>  Is there also a --disable-minimal? If yes, please use it; if no, please explain
> either in the commit message or a comment.
>
>  Regards,
>  Arnout
>
> > +endif
> > +
> >  $(eval $(autotools-package))
> >  $(eval $(host-autotools-package))
> >

I submitted a new patch with a 'full' option. I also think it is
better like this,
projects that use obselete, not recommended or very specific features
should specify their needs.

Best regards,
Adrien