[v2] lz4: improve static only build support

The current method of supporting static only build, removal of all lines
that match the SHARED regex from lib/Makefile, is crude and fragile.
Instead, patch lib/Makefile to allow disable of shared libraries.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
v2: Leave DESTDIR and PREFIX alone (Thomas P)
---
 ...ib-allow-to-disable-shared-libraries.patch | 59 +++++++++++++++++++
 package/lz4/lz4.mk                            | 12 ++--
 2 files changed, 64 insertions(+), 7 deletions(-)
 create mode 100644 package/lz4/0002-lib-allow-to-disable-shared-libraries.patch

Hello,

On Fri, 20 Apr 2018 07:52:46 +0300, Baruch Siach wrote:
> The current method of supporting static only build, removal of all lines
> that match the SHARED regex from lib/Makefile, is crude and fragile.
> Instead, patch lib/Makefile to allow disable of shared libraries.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>

Thanks for the new version, I've applied, but I have one question below.

> ---
> v2: Leave DESTDIR and PREFIX alone (Thomas P)
> ---
>  ...ib-allow-to-disable-shared-libraries.patch | 59 +++++++++++++++++++
>  package/lz4/lz4.mk                            | 12 ++--
>  2 files changed, 64 insertions(+), 7 deletions(-)
>  create mode 100644 package/lz4/0002-lib-allow-to-disable-shared-libraries.patch
> 
> diff --git a/package/lz4/0002-lib-allow-to-disable-shared-libraries.patch b/package/lz4/0002-lib-allow-to-disable-shared-libraries.patch
> new file mode 100644
> index 000000000000..4f89e85577b3
> --- /dev/null
> +++ b/package/lz4/0002-lib-allow-to-disable-shared-libraries.patch
> @@ -0,0 +1,59 @@
> +From 95bde2a4ae4a92e984a5783ca1f09f44bf04fadb Mon Sep 17 00:00:00 2001
> +From: Baruch Siach <baruch@tkos.co.il>
> +Date: Thu, 19 Apr 2018 12:28:11 +0300
> +Subject: [PATCH] lib: allow to disable shared libraries
> +
> +Just like BUILD_STATIC=no disables static libraries, BUILD_SHARED=no
> +disabled shared libraries. This is useful to support toolchains that do
> +not support shared libraries.

Why aren't we using BUILD_STATIC=no when BR2_SHARED_LIBS=y, to disable
the build of the static library ?

Admittedly, it isn't a huge improvement, but for consistency it would
be good to do that.

Thanks again,

Thomas

Hello,

On Fri, 20 Apr 2018 07:52:46 +0300, Baruch Siach wrote:
> The current method of supporting static only build, removal of all lines
> that match the SHARED regex from lib/Makefile, is crude and fragile.
> Instead, patch lib/Makefile to allow disable of shared libraries.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>

Not sure if it is due to this patch or the one bumping to 1.8.1.2, but
lz4 now fails to build in some configurations:

  http://autobuild.buildroot.net/results/a8d/a8d956ff420f6a265c5c00b33646dbbc24ce2d48/build-end.log

Best regards,

Thomas

Hi Thomas,

On Sat, Apr 21, 2018 at 02:45:10PM +0200, Thomas Petazzoni wrote:
> On Fri, 20 Apr 2018 07:52:46 +0300, Baruch Siach wrote:
> > The current method of supporting static only build, removal of all lines
> > that match the SHARED regex from lib/Makefile, is crude and fragile.
> > Instead, patch lib/Makefile to allow disable of shared libraries.
> > 
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> 
> Not sure if it is due to this patch or the one bumping to 1.8.1.2, but
> lz4 now fails to build in some configurations:
> 
>   http://autobuild.buildroot.net/results/a8d/a8d956ff420f6a265c5c00b33646dbbc24ce2d48/build-end.log

Sees to be a parallel build issue caused by the version bump patch. I'll post 
a patch.

baruch

Hi Thomas,

On Fri, Apr 20, 2018 at 08:41:43AM +0200, Thomas Petazzoni wrote:
> On Fri, 20 Apr 2018 07:52:46 +0300, Baruch Siach wrote:
> > +Just like BUILD_STATIC=no disables static libraries, BUILD_SHARED=no
> > +disabled shared libraries. This is useful to support toolchains that do
> > +not support shared libraries.
> 
> Why aren't we using BUILD_STATIC=no when BR2_SHARED_LIBS=y, to disable
> the build of the static library ?
> 
> Admittedly, it isn't a huge improvement, but for consistency it would
> be good to do that.

Many other packages build both shared and static libraries when 
BR2_SHARED_LIBS=y. This particular case is easy enough to fix, though.

Is there any advantage to not building the static library when 
BR2_SHARED_LIBS=y other than the small build time reduction?

baruch

Hello Baruch,

On Sun, 22 Apr 2018 00:05:10 +0300, Baruch Siach wrote:

> > Admittedly, it isn't a huge improvement, but for consistency it would
> > be good to do that.  
> 
> Many other packages build both shared and static libraries when 
> BR2_SHARED_LIBS=y.

Yes, I know. I don't know how strict we want to be about this. Probably
not too much because it doesn't matter that much.

> This particular case is easy enough to fix, though.

Yes, that's the point: for this package, it's super easy to fix, so
it's nice to do it.

> Is there any advantage to not building the static library when 
> BR2_SHARED_LIBS=y other than the small build time reduction?

In some cases there is more than a "small build time reduction".

The object files that go in a shared library must be built with -fPIC.
Such object files can also be used in a static library, but the code is
not as optimized as it could be, and therefore object files that go in
a static library should ideally not be built with -fPIC. I believe some
packages (but not many) do build their object files twice, once with
-fPIC for the shared library, once without -fPIC for the static
library. This creates more than a "small build time reduction".

From the commit log of commit f1d3e09895b245da9d54bbaef36e5de95269034e:

    For example, a static+shared build of libglib2 takes 1 minutes and 59
    seconds, with a final build directory of 96 MB. A shared-only build of
    libglib2 takes only 1 minutes and 31 seconds (almost a 25% reduction
    of the build time), and the final build directory weights 89 MB (a
    reduction of almost 8%).

Best regards,

Thomas