[Buildroot] [PATCH 1/2] package/zstd: avoid compilation during host-zstd install step

[Buildroot] [PATCH 1/2] package/zstd: avoid compilation during host-zstd install step

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

The host-zstd-build step was not actually compiling the library:

make[1]: Entering directory '/buildroot/output/build/host-zstd-1.4.5/lib'
make[1]: Nothing to be done for 'default'.
make[1]: Leaving directory '/buildroot/output/build/host-zstd-1.4.5/lib'

and the actual compilation was part of the install step.
This is not how other Buildroot packages work.

Make sure to specify which library targets we want instead. The total amount
of compiled files does not change with this patch.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 package/zstd/zstd.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/zstd/zstd.mk b/package/zstd/zstd.mk
index e2ba12b058..35002da332 100644
--- a/package/zstd/zstd.mk
+++ b/package/zstd/zstd.mk
@@ -71,7 +71,7 @@ endef
 # note: no 'HAVE_...' options for host library build only
 define HOST_ZSTD_BUILD_CMDS
 	$(HOST_MAKE_ENV) $(HOST_CONFIGURE_OPTS) $(MAKE) \
-		-C $(@D)/lib
+		-C $(@D)/lib libzstd.a libzstd
 	$(HOST_MAKE_ENV) $(HOST_CONFIGURE_OPTS) $(MAKE) \
 		-C $(@D) zstd
 endef
-- 
2.26.2

[Buildroot] [PATCH 2/2] package/zstd: link programs dynamically with libzstd to save space

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Even when a shared libzstd is built, the zstd command-line programs are
statically linked with libzstd, causing a large rootfs footprint.

While the cmake backend in zstd already supported a flag
ZSTD_PROGRAMS_LINK_SHARED, the make backend did not.

This commit adds support for ZSTD_PROGRAMS_LINK_SHARED in the make system
and applies it for the target compilation, unless only static libs are
supported.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 ...D_PROGRAMS_LINK_SHARED-to-link-zstd-.patch | 80 +++++++++++++++++++
 package/zstd/zstd.mk                          | 10 +++
 2 files changed, 90 insertions(+)
 create mode 100644 package/zstd/0002-make-support-ZSTD_PROGRAMS_LINK_SHARED-to-link-zstd-.patch

diff --git a/package/zstd/0002-make-support-ZSTD_PROGRAMS_LINK_SHARED-to-link-zstd-.patch b/package/zstd/0002-make-support-ZSTD_PROGRAMS_LINK_SHARED-to-link-zstd-.patch
new file mode 100644
index 0000000000..100cfaba95
--- /dev/null
+++ b/package/zstd/0002-make-support-ZSTD_PROGRAMS_LINK_SHARED-to-link-zstd-.patch
@@ -0,0 +1,80 @@
+From afc368e7247abfe89250def5b7673a9ccb79e0b1 Mon Sep 17 00:00:00 2001
+From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
+Date: Wed, 5 Aug 2020 15:35:01 +0200
+Subject: [PATCH] make: support ZSTD_PROGRAMS_LINK_SHARED to link zstd with
+ libzstd
+
+zstd allows building via make, cmake, meson, and others, but unfortunately
+the features and flags used for these build systems are inconsistent.
+
+The cmake version respects an option 'ZSTD_PROGRAMS_LINK_SHARED' to let the
+zstd binary link dynamically with its own libzstd. Without it, the zstd
+program uses static linking and thus has a big size.
+
+This option is not provided in the 'make' system. There does exist a target
+'zstd-dll' which intends to do exactly this, but it does not work out of the
+box due to linking issues. These in turn are caused because the lib makefile
+passes '-fvisibility=hidden' to the linker, which hides certain symbols that
+the zstd program needs. The cmake-based compilation does not pass this
+visibility flag, due to which all symbols are visible.
+
+Unfortunately, there is no 'install' rule that will install zstd-dll and
+create the derived programs like zstdless etc.
+
+With the above in mind, when ZSTD_PROGRAMS_LINK_SHARED is set in make
+context:
+- remove '-fvisibility=hidden' from the lib compilation
+- alter the 'zstd' target to not include the lib objects directly but link
+  dynamically with libzstd.
+
+See also https://github.com/facebook/zstd/issues/2261 which reported these
+inconsistencies between make and cmake compilation.
+
+Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
+
+---
+ lib/Makefile      | 5 ++++-
+ programs/Makefile | 8 +++++++-
+ 2 files changed, 11 insertions(+), 2 deletions(-)
+
+diff --git a/lib/Makefile b/lib/Makefile
+index 7c6dff02..8f9f36a6 100644
+--- a/lib/Makefile
++++ b/lib/Makefile
+@@ -204,7 +204,10 @@ $(LIBZSTD): $(ZSTD_FILES)
+ else
+ 
+ LIBZSTD = libzstd.$(SHARED_EXT_VER)
+-$(LIBZSTD): LDFLAGS += -shared -fPIC -fvisibility=hidden
++$(LIBZSTD): LDFLAGS += -shared -fPIC
++ifndef ZSTD_PROGRAMS_LINK_SHARED
++$(LIBZSTD): LDFLAGS += -fvisibility=hidden
++endif
+ $(LIBZSTD): $(ZSTD_FILES)
+ 	@echo compiling dynamic library $(LIBVER)
+ 	$(Q)$(CC) $(FLAGS) $^ $(LDFLAGS) $(SONAME_FLAGS) -o $@
+diff --git a/programs/Makefile b/programs/Makefile
+index 418ad4e6..8897dcf9 100644
+--- a/programs/Makefile
++++ b/programs/Makefile
+@@ -169,10 +169,16 @@ $(ZSTDDECOMP_O): CFLAGS += $(ALIGN_LOOP)
+ zstd : CPPFLAGS += $(THREAD_CPP) $(ZLIBCPP) $(LZMACPP) $(LZ4CPP)
+ zstd : LDFLAGS += $(THREAD_LD) $(ZLIBLD) $(LZMALD) $(LZ4LD) $(DEBUGFLAGS_LD)
+ zstd : CPPFLAGS += -DZSTD_LEGACY_SUPPORT=$(ZSTD_LEGACY_SUPPORT)
++ifdef ZSTD_PROGRAMS_LINK_SHARED
++# link dynamically against own library
++zstd : LDFLAGS += -L$(ZSTDDIR) -lzstd
++else
++zstd : $(ZSTDLIB_FILES)
++endif
+ ifneq (,$(filter Windows%,$(OS)))
+ zstd : $(RES_FILE)
+ endif
+-zstd : $(ZSTDLIB_FILES) $(ZSTD_CLI_OBJ)
++zstd : $(ZSTD_CLI_OBJ)
+ 	@echo "$(THREAD_MSG)"
+ 	@echo "$(ZLIB_MSG)"
+ 	@echo "$(LZMA_MSG)"
+-- 
+2.26.2
+
diff --git a/package/zstd/zstd.mk b/package/zstd/zstd.mk
index 35002da332..c7b224b002 100644
--- a/package/zstd/zstd.mk
+++ b/package/zstd/zstd.mk
@@ -43,9 +43,19 @@ ZSTD_INSTALL_LIBS = install-static
 else ifeq ($(BR2_SHARED_LIBS),y)
 ZSTD_BUILD_LIBS = libzstd
 ZSTD_INSTALL_LIBS = install-shared
+ZSTD_OPTS += ZSTD_PROGRAMS_LINK_SHARED=1
 else
 ZSTD_BUILD_LIBS = libzstd.a libzstd
 ZSTD_INSTALL_LIBS = install-static install-shared
+ZSTD_OPTS += ZSTD_PROGRAMS_LINK_SHARED=1
+endif
+
+# The HAVE_THREAD flag is read by the 'programs' makefile but not by  the 'lib'
+# one. Building a multi-threaded binary with a library (which defaults to
+# single-threaded) gives a runtime error when compressing files.
+# The 'lib' makefile provides specific '%-mt' targets for this purpose.
+ifeq ($(BR2_TOOLCHAIN_HAS_THREADS),y)
+ZSTD_BUILD_LIBS := $(addsuffix -mt,$(ZSTD_BUILD_LIBS))
 endif
 
 define ZSTD_BUILD_CMDS
-- 
2.26.2

[Buildroot] [PATCH 2/2] package/zstd: link programs dynamically with libzstd to save space

[Yann E. MORIN]

Thomas, All,

On 2020-09-28 13:42 +0200, Thomas De Schampheleire spake thusly:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> Even when a shared libzstd is built, the zstd command-line programs are
> statically linked with libzstd, causing a large rootfs footprint.
> 
> While the cmake backend in zstd already supported a flag
> ZSTD_PROGRAMS_LINK_SHARED, the make backend did not.

Any reason why we do not switch over to using cmake or meson? Both seem
to be officially supported, see README.md:

    ## Build instructions

    ### Makefile
    [...]

    ### cmake
    [...]

    ### meson
    [...]

So, I'd rather we expend the effort to switch to cmake, as it already
has ZSTD_PROGRAMS_LINK_SHARED (or to meson if it also fits the bill,
whatever floats your boat), rather than patching the Makefile-based
buildsystem.

Regards,
Yann E. MORIN.

> This commit adds support for ZSTD_PROGRAMS_LINK_SHARED in the make system
> and applies it for the target compilation, unless only static libs are
> supported.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> ---
>  ...D_PROGRAMS_LINK_SHARED-to-link-zstd-.patch | 80 +++++++++++++++++++
>  package/zstd/zstd.mk                          | 10 +++
>  2 files changed, 90 insertions(+)
>  create mode 100644 package/zstd/0002-make-support-ZSTD_PROGRAMS_LINK_SHARED-to-link-zstd-.patch
> 
> diff --git a/package/zstd/0002-make-support-ZSTD_PROGRAMS_LINK_SHARED-to-link-zstd-.patch b/package/zstd/0002-make-support-ZSTD_PROGRAMS_LINK_SHARED-to-link-zstd-.patch
> new file mode 100644
> index 0000000000..100cfaba95
> --- /dev/null
> +++ b/package/zstd/0002-make-support-ZSTD_PROGRAMS_LINK_SHARED-to-link-zstd-.patch
> @@ -0,0 +1,80 @@
> +From afc368e7247abfe89250def5b7673a9ccb79e0b1 Mon Sep 17 00:00:00 2001
> +From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> +Date: Wed, 5 Aug 2020 15:35:01 +0200
> +Subject: [PATCH] make: support ZSTD_PROGRAMS_LINK_SHARED to link zstd with
> + libzstd
> +
> +zstd allows building via make, cmake, meson, and others, but unfortunately
> +the features and flags used for these build systems are inconsistent.
> +
> +The cmake version respects an option 'ZSTD_PROGRAMS_LINK_SHARED' to let the
> +zstd binary link dynamically with its own libzstd. Without it, the zstd
> +program uses static linking and thus has a big size.
> +
> +This option is not provided in the 'make' system. There does exist a target
> +'zstd-dll' which intends to do exactly this, but it does not work out of the
> +box due to linking issues. These in turn are caused because the lib makefile
> +passes '-fvisibility=hidden' to the linker, which hides certain symbols that
> +the zstd program needs. The cmake-based compilation does not pass this
> +visibility flag, due to which all symbols are visible.
> +
> +Unfortunately, there is no 'install' rule that will install zstd-dll and
> +create the derived programs like zstdless etc.
> +
> +With the above in mind, when ZSTD_PROGRAMS_LINK_SHARED is set in make
> +context:
> +- remove '-fvisibility=hidden' from the lib compilation
> +- alter the 'zstd' target to not include the lib objects directly but link
> +  dynamically with libzstd.
> +
> +See also https://github.com/facebook/zstd/issues/2261 which reported these
> +inconsistencies between make and cmake compilation.
> +
> +Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> +
> +---
> + lib/Makefile      | 5 ++++-
> + programs/Makefile | 8 +++++++-
> + 2 files changed, 11 insertions(+), 2 deletions(-)
> +
> +diff --git a/lib/Makefile b/lib/Makefile
> +index 7c6dff02..8f9f36a6 100644
> +--- a/lib/Makefile
> ++++ b/lib/Makefile
> +@@ -204,7 +204,10 @@ $(LIBZSTD): $(ZSTD_FILES)
> + else
> + 
> + LIBZSTD = libzstd.$(SHARED_EXT_VER)
> +-$(LIBZSTD): LDFLAGS += -shared -fPIC -fvisibility=hidden
> ++$(LIBZSTD): LDFLAGS += -shared -fPIC
> ++ifndef ZSTD_PROGRAMS_LINK_SHARED
> ++$(LIBZSTD): LDFLAGS += -fvisibility=hidden
> ++endif
> + $(LIBZSTD): $(ZSTD_FILES)
> + 	@echo compiling dynamic library $(LIBVER)
> + 	$(Q)$(CC) $(FLAGS) $^ $(LDFLAGS) $(SONAME_FLAGS) -o $@
> +diff --git a/programs/Makefile b/programs/Makefile
> +index 418ad4e6..8897dcf9 100644
> +--- a/programs/Makefile
> ++++ b/programs/Makefile
> +@@ -169,10 +169,16 @@ $(ZSTDDECOMP_O): CFLAGS += $(ALIGN_LOOP)
> + zstd : CPPFLAGS += $(THREAD_CPP) $(ZLIBCPP) $(LZMACPP) $(LZ4CPP)
> + zstd : LDFLAGS += $(THREAD_LD) $(ZLIBLD) $(LZMALD) $(LZ4LD) $(DEBUGFLAGS_LD)
> + zstd : CPPFLAGS += -DZSTD_LEGACY_SUPPORT=$(ZSTD_LEGACY_SUPPORT)
> ++ifdef ZSTD_PROGRAMS_LINK_SHARED
> ++# link dynamically against own library
> ++zstd : LDFLAGS += -L$(ZSTDDIR) -lzstd
> ++else
> ++zstd : $(ZSTDLIB_FILES)
> ++endif
> + ifneq (,$(filter Windows%,$(OS)))
> + zstd : $(RES_FILE)
> + endif
> +-zstd : $(ZSTDLIB_FILES) $(ZSTD_CLI_OBJ)
> ++zstd : $(ZSTD_CLI_OBJ)
> + 	@echo "$(THREAD_MSG)"
> + 	@echo "$(ZLIB_MSG)"
> + 	@echo "$(LZMA_MSG)"
> +-- 
> +2.26.2
> +
> diff --git a/package/zstd/zstd.mk b/package/zstd/zstd.mk
> index 35002da332..c7b224b002 100644
> --- a/package/zstd/zstd.mk
> +++ b/package/zstd/zstd.mk
> @@ -43,9 +43,19 @@ ZSTD_INSTALL_LIBS = install-static
>  else ifeq ($(BR2_SHARED_LIBS),y)
>  ZSTD_BUILD_LIBS = libzstd
>  ZSTD_INSTALL_LIBS = install-shared
> +ZSTD_OPTS += ZSTD_PROGRAMS_LINK_SHARED=1
>  else
>  ZSTD_BUILD_LIBS = libzstd.a libzstd
>  ZSTD_INSTALL_LIBS = install-static install-shared
> +ZSTD_OPTS += ZSTD_PROGRAMS_LINK_SHARED=1
> +endif
> +
> +# The HAVE_THREAD flag is read by the 'programs' makefile but not by  the 'lib'
> +# one. Building a multi-threaded binary with a library (which defaults to
> +# single-threaded) gives a runtime error when compressing files.
> +# The 'lib' makefile provides specific '%-mt' targets for this purpose.
> +ifeq ($(BR2_TOOLCHAIN_HAS_THREADS),y)
> +ZSTD_BUILD_LIBS := $(addsuffix -mt,$(ZSTD_BUILD_LIBS))
>  endif
>  
>  define ZSTD_BUILD_CMDS
> -- 
> 2.26.2
> 
> _______________________________________________
> 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 2/2] package/zstd: link programs dynamically with libzstd to save space

[Yann E. MORIN]

Thomas, All,

Second review, as I forgot to read down to the end before replying...

On 2020-09-28 13:42 +0200, Thomas De Schampheleire spake thusly:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> Even when a shared libzstd is built, the zstd command-line programs are
> statically linked with libzstd, causing a large rootfs footprint.
> 
> While the cmake backend in zstd already supported a flag
> ZSTD_PROGRAMS_LINK_SHARED, the make backend did not.
> 
> This commit adds support for ZSTD_PROGRAMS_LINK_SHARED in the make system
> and applies it for the target compilation, unless only static libs are
> supported.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> ---
[--SNIP--]
> diff --git a/package/zstd/zstd.mk b/package/zstd/zstd.mk
> index 35002da332..c7b224b002 100644
> --- a/package/zstd/zstd.mk
> +++ b/package/zstd/zstd.mk
> @@ -43,9 +43,19 @@ ZSTD_INSTALL_LIBS = install-static
>  else ifeq ($(BR2_SHARED_LIBS),y)
>  ZSTD_BUILD_LIBS = libzstd
>  ZSTD_INSTALL_LIBS = install-shared
> +ZSTD_OPTS += ZSTD_PROGRAMS_LINK_SHARED=1
>  else
>  ZSTD_BUILD_LIBS = libzstd.a libzstd
>  ZSTD_INSTALL_LIBS = install-static install-shared
> +ZSTD_OPTS += ZSTD_PROGRAMS_LINK_SHARED=1
> +endif
> +
> +# The HAVE_THREAD flag is read by the 'programs' makefile but not by  the 'lib'
> +# one. Building a multi-threaded binary with a library (which defaults to
> +# single-threaded) gives a runtime error when compressing files.
> +# The 'lib' makefile provides specific '%-mt' targets for this purpose.
> +ifeq ($(BR2_TOOLCHAIN_HAS_THREADS),y)
> +ZSTD_BUILD_LIBS := $(addsuffix -mt,$(ZSTD_BUILD_LIBS))
>  endif

This last part seems unrelated, and should be in its own patch. If not,
then it should be explained in the commit log.

Regards,
Yann E. MORIN.

>  define ZSTD_BUILD_CMDS
> -- 
> 2.26.2
> 
> _______________________________________________
> 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 1/2] package/zstd: avoid compilation during host-zstd install step

[Yann E. MORIN]

Thomas, All,

On 2020-09-28 13:42 +0200, Thomas De Schampheleire spake thusly:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> The host-zstd-build step was not actually compiling the library:
> 
> make[1]: Entering directory '/buildroot/output/build/host-zstd-1.4.5/lib'
> make[1]: Nothing to be done for 'default'.
> make[1]: Leaving directory '/buildroot/output/build/host-zstd-1.4.5/lib'
> 
> and the actual compilation was part of the install step.
> This is not how other Buildroot packages work.
> 
> Make sure to specify which library targets we want instead. The total amount
> of compiled files does not change with this patch.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Applied to master, thanks.

Regards,
Yann E. MORIN.

> ---
>  package/zstd/zstd.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/zstd/zstd.mk b/package/zstd/zstd.mk
> index e2ba12b058..35002da332 100644
> --- a/package/zstd/zstd.mk
> +++ b/package/zstd/zstd.mk
> @@ -71,7 +71,7 @@ endef
>  # note: no 'HAVE_...' options for host library build only
>  define HOST_ZSTD_BUILD_CMDS
>  	$(HOST_MAKE_ENV) $(HOST_CONFIGURE_OPTS) $(MAKE) \
> -		-C $(@D)/lib
> +		-C $(@D)/lib libzstd.a libzstd
>  	$(HOST_MAKE_ENV) $(HOST_CONFIGURE_OPTS) $(MAKE) \
>  		-C $(@D) zstd
>  endef
> -- 
> 2.26.2
> 
> _______________________________________________
> 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 2/2] package/zstd: link programs dynamically with libzstd to save space

[Thomas De Schampheleire]

Hi Yann,



On Mon, Sep 28, 2020, 21:40 Yann E. MORIN <yann.morin.1998@free.fr> wrote:

> Thomas, All,
>
> On 2020-09-28 13:42 +0200, Thomas De Schampheleire spake thusly:
> > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >
> > Even when a shared libzstd is built, the zstd command-line programs are
> > statically linked with libzstd, causing a large rootfs footprint.
> >
> > While the cmake backend in zstd already supported a flag
> > ZSTD_PROGRAMS_LINK_SHARED, the make backend did not.
>
> Any reason why we do not switch over to using cmake or meson? Both seem
> to be officially supported, see README.md:
>
>     ## Build instructions
>
>     ### Makefile
>     [...]
>
>     ### cmake
>     [...]
>
>     ### meson
>     [...]
>
> So, I'd rather we expend the effort to switch to cmake, as it already
> has ZSTD_PROGRAMS_LINK_SHARED (or to meson if it also fits the bill,
> whatever floats your boat), rather than patching the Makefile-based
> buildsystem.
>

In fact, only the make system is 'official'. The other build systems are
considered 'community-maintained'.
See e.g.
https://github.com/facebook/zstd/issues/1512
https://github.com/facebook/zstd/issues/1503
https://github.com/facebook/zstd/issues/1401

So from the perspective of the authors, only make is guaranteed to work.

Even though I think their makefiles are definitely not a good example, the
cmake build rules are not equivalent in operation to make, see the issue I
created myself:
https://github.com/facebook/zstd/issues/2261

So I wouldn't conclude that we better switch away from the make build
system of zstd.

(I don't understand why they have rules for so many different build
systems, which are then not aligned nor officially maintained. IMHO they
better pick one and make sure it works well.)

Best regards
Thomas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20200928/7a0d4650/attachment.html>

[Buildroot] [PATCH 2/2] package/zstd: link programs dynamically with libzstd to save space

[Thomas De Schampheleire]

On Mon, Sep 28, 2020, 21:47 Yann E. MORIN <yann.morin.1998@free.fr> wrote:

> Thomas, All,
>
> Second review, as I forgot to read down to the end before replying...
>
> On 2020-09-28 13:42 +0200, Thomas De Schampheleire spake thusly:
> > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >
> > Even when a shared libzstd is built, the zstd command-line programs are
> > statically linked with libzstd, causing a large rootfs footprint.
> >
> > While the cmake backend in zstd already supported a flag
> > ZSTD_PROGRAMS_LINK_SHARED, the make backend did not.
> >
> > This commit adds support for ZSTD_PROGRAMS_LINK_SHARED in the make system
> > and applies it for the target compilation, unless only static libs are
> > supported.
> >
> > Signed-off-by: Thomas De Schampheleire <
> thomas.de_schampheleire at nokia.com>
> > ---
> [--SNIP--]
> > diff --git a/package/zstd/zstd.mk b/package/zstd/zstd.mk
> > index 35002da332..c7b224b002 100644
> > --- a/package/zstd/zstd.mk
> > +++ b/package/zstd/zstd.mk
> > @@ -43,9 +43,19 @@ ZSTD_INSTALL_LIBS = install-static
> >  else ifeq ($(BR2_SHARED_LIBS),y)
> >  ZSTD_BUILD_LIBS = libzstd
> >  ZSTD_INSTALL_LIBS = install-shared
> > +ZSTD_OPTS += ZSTD_PROGRAMS_LINK_SHARED=1
> >  else
> >  ZSTD_BUILD_LIBS = libzstd.a libzstd
> >  ZSTD_INSTALL_LIBS = install-static install-shared
> > +ZSTD_OPTS += ZSTD_PROGRAMS_LINK_SHARED=1
> > +endif
> > +
> > +# The HAVE_THREAD flag is read by the 'programs' makefile but not by
> the 'lib'
> > +# one. Building a multi-threaded binary with a library (which defaults
> to
> > +# single-threaded) gives a runtime error when compressing files.
> > +# The 'lib' makefile provides specific '%-mt' targets for this purpose.
> > +ifeq ($(BR2_TOOLCHAIN_HAS_THREADS),y)
> > +ZSTD_BUILD_LIBS := $(addsuffix -mt,$(ZSTD_BUILD_LIBS))
> >  endif
>
> This last part seems unrelated, and should be in its own patch. If not,
> then it should be explained in the commit log.
>

The comment tries to explain it but it's probably not clear enough: the
HAVE_THREAD flag that buildroot passes is interpreted by programs/Makefile.
Before this patch, that makefile will build and link the library files
directly together with the cli programs. This means that all these
compilations use flags defined by this programs/Makefile.

When we change to use dynamic libraries, such that libzstd.so is built
first, and that the programs just dynamically link with it, then there are
two makefiles of importance:
- lib/Makefile : this makefile does not interpret HAVE_DEBUG and thus
builds a single threaded library.
- programs/Makefile: would build a multithreaded program if Buildroot
requests it.

We are thus left with a multithreaded program that links dynamically with a
library which is build for singlethreaded operation, which is checked at
runtime, resulting in an error.

As lib/Makefile already provides -mt rules to produce the libs for
multithreaded operation, use them instead.

I think we could add this change as first patch, as it seems that today
Buildroot would produce a libzstd.so supporting single-threaded operation
only?

Best regards
Thomas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20200928/f20940c2/attachment.html>

[Buildroot] [PATCH 1/2] package/zstd: avoid compilation during host-zstd install step

[Peter Korsgaard]

>>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin@gmail.com> writes:

 > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
 > The host-zstd-build step was not actually compiling the library:

 > make[1]: Entering directory '/buildroot/output/build/host-zstd-1.4.5/lib'
 > make[1]: Nothing to be done for 'default'.
 > make[1]: Leaving directory '/buildroot/output/build/host-zstd-1.4.5/lib'

 > and the actual compilation was part of the install step.
 > This is not how other Buildroot packages work.

 > Make sure to specify which library targets we want instead. The total amount
 > of compiled files does not change with this patch.

 > Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Committed to 2020.02.x, 2020.05.x and 2020.08.x, thanks.

-- 
Bye, Peter Korsgaard