[Buildroot] [PATCH v2 4/5] package/zstd: Change Build options

[Norbert Lange]

Am Mo., 14. Juni 2021 um 21:56 Uhr schrieb Arnout Vandecappelle
<arnout at mind.be>:
>
>
>
> On 25/05/2021 19:26, Norbert Lange wrote:
> > Disable the legacy format, these are just needed for
> > decompressing files created with pre-release version.
> >
> > Use Buildroot's setting for optimization, zstd's build system
> > overrides CFLAGS, but MOREFLAGS can override again.
> > Quick tests show that using -O2 (like buildroot)
> > is actually a little faster than -O3 on x86_64 Atoms.
> >
> > Signed-off-by: Norbert Lange <nolange79 at gmail.com>
> > ---
> >  package/zstd/zstd.mk | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/package/zstd/zstd.mk b/package/zstd/zstd.mk
> > index a7a5ba4e50..a9499df4d0 100644
> > --- a/package/zstd/zstd.mk
> > +++ b/package/zstd/zstd.mk
> > @@ -13,6 +13,10 @@ ZSTD_CPE_ID_VENDOR = facebook
> >  ZSTD_CPE_ID_PRODUCT = zstandard
> >
> >  ZSTD_OPTS += PREFIX=/usr
> > +ZSTD_OPTS += ZSTD_LEGACY_SUPPORT=0
> > +
> > +# zstd will append -O3 after $(CFLAGS), use MOREFLAGS to override again
> > +ZSTD_OPTS_MOREFLAGS += $(TARGET_OPTIMIZATION)
>
>  Wouldn't it make more sense to apply *all* buildroot's flags here, i.e.
>
> ZSTD_OPTS += MOREFLAGS="$(TARGET_CFLAGS)"

The CFLAGS are already picked up from the environment,
the package normally should be able to add options on top.

>
> (also note:
>  - intermediate variable ZSTD_OPTS_MOREFLAGS is not needed for anything;

Not yet. There are a couple of interesting options available with macros,
ex. ZSTD_LIB_MINIFY. see lib/README.md
But I think ill cut that out for now.

>  - we usually quote only the "$(TARGET_CFLAGS)" part, not including the
> MOREFLAGS= part)

OK.

>
>
>  Regards,
>  Arnout
>
> >
> >  ifeq ($(BR2_TOOLCHAIN_HAS_THREADS),y)
> >  ZSTD_OPTS += HAVE_THREAD=1
> > @@ -41,6 +45,8 @@ else
> >  ZSTD_OPTS += HAVE_LZ4=0
> >  endif
> >
> > +ZSTD_OPTS += "MOREFLAGS=$(ZSTD_OPTS_MOREFLAGS)"
> > +
> >  ZSTD_BUILD_PROG_TARGET := zstd-release
> >
> >  # Since v1.5.0 the dynamic library is built for
> >