Patchwork [WIP,2/5] gcc-final: disable shared build for static

login
register
mail settings
Submitter Gustavo Zacarias
Date May 25, 2014, 10:12 p.m.
Message ID <1401055980-28742-3-git-send-email-gustavo@zacarias.com.ar>
Download mbox | patch
Permalink /patch/352302/
State Accepted
Headers show

Comments

Gustavo Zacarias - May 25, 2014, 10:12 p.m.
Disable shared build for host-gcc-final when building for static targets.
We really want static or shared, there's no such thing as "preferring static"
since we can't choose with any degree of granularity for which packages.
And it confuses linking scripts having both available at the same time. Fixes:
http://autobuild.buildroot.net/results/c54/c54bdf88eff6d60c7001cb0e2cb6792cc75178db/

Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
---
 package/gcc/gcc-final/gcc-final.mk | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)
Thomas Petazzoni - May 26, 2014, 9:37 a.m.
Dear Gustavo Zacarias,

On Sun, 25 May 2014 19:12:57 -0300, Gustavo Zacarias wrote:
> Disable shared build for host-gcc-final when building for static targets.
> We really want static or shared, there's no such thing as "preferring static"
> since we can't choose with any degree of granularity for which packages.
> And it confuses linking scripts having both available at the same time. Fixes:
> http://autobuild.buildroot.net/results/c54/c54bdf88eff6d60c7001cb0e2cb6792cc75178db/

While on principle I'm probably fine with the patch, I kinda disagree
that it fixes the autobuilder issues. Because the same autobuilder
issue will appear with an external toolchain that has both libstdc++.so
and libstdc++.a.

Thomas
Thomas Petazzoni - May 26, 2014, 9:38 a.m.
Dear Gustavo Zacarias,

On Sun, 25 May 2014 19:12:57 -0300, Gustavo Zacarias wrote:

>  ifneq ($(HOST_GCC_FINAL_USR_LIBS),)
> +ifeq ($(BR2_PREFER_STATIC_LIB),y)
> +define HOST_GCC_FINAL_INSTALL_USR_LIBS
> +	mkdir -p $(TARGET_DIR)/usr/lib
> +	for i in $(HOST_GCC_FINAL_USR_LIBS) ; do \
> +		cp -dpf $(HOST_DIR)/usr/$(GNU_TARGET_NAME)/lib*/$${i}.a \
> +			$(STAGING_DIR)/usr/lib/ ; \
> +	done
> +endef

No, we want to install .a files as well in my opinion. In fact, the
patch series I have to rename BR2_PREFER_STATIC_LIB makes things even
clearer, with three possibilities: pure static, static and shared, pure
shared.

Thomas
Gustavo Zacarias - May 26, 2014, 10:04 a.m.
On 05/26/2014 06:37 AM, Thomas Petazzoni wrote:

> While on principle I'm probably fine with the patch, I kinda disagree
> that it fixes the autobuilder issues. Because the same autobuilder
> issue will appear with an external toolchain that has both libstdc++.so
> and libstdc++.a.

This is a way to fix it for internal toolchains, i can build up to mpd
static with audiofile support with this, but it's not complete, that's
for sure.
Regards.
Gustavo Zacarias - May 26, 2014, 10:06 a.m.
On 05/26/2014 06:38 AM, Thomas Petazzoni wrote:

> No, we want to install .a files as well in my opinion. In fact, the
> patch series I have to rename BR2_PREFER_STATIC_LIB makes things even
> clearer, with three possibilities: pure static, static and shared, pure
> shared.

Sorry but i didn't see your patchset so relax a bit :)
That being said do we really want a pure dyn scenario? Since we purge .a
files from the target anyway since they're not required for runtime it
wouldn't hurt a bit to keep the dual mode and just static i think.
Regards.
Thomas Petazzoni - May 26, 2014, 10:07 a.m.
Dear Gustavo Zacarias,

On Mon, 26 May 2014 07:04:14 -0300, Gustavo Zacarias wrote:

> > While on principle I'm probably fine with the patch, I kinda disagree
> > that it fixes the autobuilder issues. Because the same autobuilder
> > issue will appear with an external toolchain that has both libstdc++.so
> > and libstdc++.a.
> 
> This is a way to fix it for internal toolchains, i can build up to mpd
> static with audiofile support with this, but it's not complete, that's
> for sure.

Yeah, I read on the rest of your patches, and the patch changing
--static to -static indeed makes it clear that it fixes the problem for
external toolchains. IMO, switching from --static to -static is the
real fix: the build process should not be confused by the presence of
libstdc++.so even if we're doing a pure static build.

Also, about reverting back from --static to -static, would it be useful
to contact Andy Kennedy to have more details about why he made the
change? His commit log lacks the details on why he changed from -static
to --static.

Best regards,

Thomas
Thomas Petazzoni - May 26, 2014, 10:24 a.m.
Dear Gustavo Zacarias,

On Mon, 26 May 2014 07:06:08 -0300, Gustavo Zacarias wrote:

> > No, we want to install .a files as well in my opinion. In fact, the
> > patch series I have to rename BR2_PREFER_STATIC_LIB makes things
> > even clearer, with three possibilities: pure static, static and
> > shared, pure shared.
> 
> Sorry but i didn't see your patchset so relax a bit :)

Yeah, no problem :)

> That being said do we really want a pure dyn scenario? Since we
> purge .a files from the target anyway since they're not required for
> runtime it wouldn't hurt a bit to keep the dual mode and just static
> i think. Regards.

There is an advantage with the pure dynamic scenario: build time.

When you have dyn+static, you build all object files (for packages that
do it correctly): once for static without -fPIC, and once for dynamic
with -fPIC. Here is what I have in my commit log about this:

    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%).

A 25% reduction in build time is certainly nice for users who don't
care about static libraries, no?

Best regards,

Thomas
Gustavo Zacarias - May 26, 2014, 11:02 a.m.
On 05/26/2014 07:07 AM, Thomas Petazzoni wrote:

> Yeah, I read on the rest of your patches, and the patch changing
> --static to -static indeed makes it clear that it fixes the problem for
> external toolchains. IMO, switching from --static to -static is the
> real fix: the build process should not be confused by the presence of
> libstdc++.so even if we're doing a pure static build.
> 
> Also, about reverting back from --static to -static, would it be useful
> to contact Andy Kennedy to have more details about why he made the
> change? His commit log lacks the details on why he changed from -static
> to --static.

In tests for the internal toolchain audiofile builds but mpd fails to
link with that change alone (+host-gcc-final static works, actually with
host-gcc-fnial static alone it works).
For an external toolchain it works with the -static change.
Regards.
Gustavo Zacarias - May 26, 2014, 11:03 a.m.
On 05/26/2014 07:24 AM, Thomas Petazzoni wrote:
> There is an advantage with the pure dynamic scenario: build time.
> 
> When you have dyn+static, you build all object files (for packages that
> do it correctly): once for static without -fPIC, and once for dynamic
> with -fPIC. Here is what I have in my commit log about this:
> 
>     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%).
> 
> A 25% reduction in build time is certainly nice for users who don't
> care about static libraries, no?

Ok, so then we can go for it, i don't think anyone will care about the
space saving though, but time is a whole different matter.
Regards.
Thomas Petazzoni - May 26, 2014, 12:05 p.m.
Dear Gustavo Zacarias,

On Mon, 26 May 2014 08:03:48 -0300, Gustavo Zacarias wrote:

> >     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%).
> > 
> > A 25% reduction in build time is certainly nice for users who don't
> > care about static libraries, no?
> 
> Ok, so then we can go for it, i don't think anyone will care about the
> space saving though, but time is a whole different matter.

Agreed, the disk space saving nobody really cares, especially if it's
only ~8%. But a build time saving of 25% is quite nice, in my opinion.
Normally, all packages using libtool should be affected, but I haven't
checked.

Best regards,

Thomas

Patch

diff --git a/package/gcc/gcc-final/gcc-final.mk b/package/gcc/gcc-final/gcc-final.mk
index 622dcf2..366dc42 100644
--- a/package/gcc/gcc-final/gcc-final.mk
+++ b/package/gcc/gcc-final/gcc-final.mk
@@ -26,7 +26,7 @@  HOST_GCC_FINAL_SUBDIR = build
 
 HOST_GCC_FINAL_PRE_CONFIGURE_HOOKS += HOST_GCC_CONFIGURE_SYMLINK
 
-define  HOST_GCC_FINAL_CONFIGURE_CMDS
+define HOST_GCC_FINAL_CONFIGURE_CMDS
         (cd $(HOST_GCC_FINAL_SRCDIR) && rm -rf config.cache; \
                 $(HOST_CONFIGURE_OPTS) \
                 CFLAGS="$(HOST_CFLAGS)" \
@@ -35,7 +35,7 @@  define  HOST_GCC_FINAL_CONFIGURE_CMDS
                 ./configure \
                 --prefix="$(HOST_DIR)/usr" \
                 --sysconfdir="$(HOST_DIR)/etc" \
-                --enable-shared --enable-static \
+                --enable-static \
                 $(QUIET) $(HOST_GCC_FINAL_CONF_OPT) \
         )
 endef
@@ -54,6 +54,13 @@  HOST_GCC_FINAL_CONF_OPT = \
 	$(DISABLE_LARGEFILE) \
 	--with-build-time-tools=$(HOST_DIR)/usr/$(GNU_TARGET_NAME)/bin
 
+# Disable shared libs like libstdc++ if we do static since it confuses linking
+ifeq ($(BR2_PREFER_STATIC_LIB),y)
+HOST_GCC_FINAL_CONF_OPT += --disable-shared
+else
+HOST_GCC_FINAL_CONF_OPT += --enable-shared
+endif
+
 ifeq ($(BR2_GCC_ENABLE_OPENMP),y)
 HOST_GCC_FINAL_CONF_OPT += --enable-libgomp
 else
@@ -141,6 +148,15 @@  endif
 endif
 
 ifneq ($(HOST_GCC_FINAL_USR_LIBS),)
+ifeq ($(BR2_PREFER_STATIC_LIB),y)
+define HOST_GCC_FINAL_INSTALL_USR_LIBS
+	mkdir -p $(TARGET_DIR)/usr/lib
+	for i in $(HOST_GCC_FINAL_USR_LIBS) ; do \
+		cp -dpf $(HOST_DIR)/usr/$(GNU_TARGET_NAME)/lib*/$${i}.a \
+			$(STAGING_DIR)/usr/lib/ ; \
+	done
+endef
+else
 define HOST_GCC_FINAL_INSTALL_USR_LIBS
 	mkdir -p $(TARGET_DIR)/usr/lib
 	for i in $(HOST_GCC_FINAL_USR_LIBS) ; do \
@@ -152,6 +168,7 @@  define HOST_GCC_FINAL_INSTALL_USR_LIBS
 			$(TARGET_DIR)/usr/lib/ ; \
 	done
 endef
+endif
 HOST_GCC_FINAL_POST_INSTALL_HOOKS += HOST_GCC_FINAL_INSTALL_USR_LIBS
 endif