[Buildroot] [PATCH] bzip2: Rearrange build order

[Buildroot] [PATCH] bzip2: Rearrange build order

[Markos Chandras]

From: Markos Chandras <markos.chandras@imgtec.com>

Several object files are shared between the libbz2.so shared library
and the libbz2.a static one. MIPS will refuce to build a relocatable
object when creating a new shared library with the following error:

blocksort.o: relocation R_MIPS_HI16 against `__gnu_local_gp' can not be used
when making a shared object; recompile with -fPIC

This is because these files are build without -fPIC when creating the
static library and later on they are used to build the shared one.

This is easily fixed if we add the shared library build rule before
creating the static library so object files are always compiled with
-fPIC.

Signed-off-by: Markos Chandras <markos.chandras@imgtec.com>
---
 package/bzip2/bzip2.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/bzip2/bzip2.mk b/package/bzip2/bzip2.mk
index 5f8c35e..c49109a 100644
--- a/package/bzip2/bzip2.mk
+++ b/package/bzip2/bzip2.mk
@@ -18,9 +18,9 @@ endef
 endif
 
 define BZIP2_BUILD_CMDS
+	$(BZIP2_BUILD_SHARED_CMDS)
 	$(TARGET_MAKE_ENV)
 		$(MAKE) -C $(@D) libbz2.a bzip2 bzip2recover $(TARGET_CONFIGURE_OPTS)
-	$(BZIP2_BUILD_SHARED_CMDS)
 endef
 
 ifeq ($(BR2_PREFER_STATIC_LIB),)
-- 
1.8.2.1

[Buildroot] [PATCH] bzip2: Rearrange build order

[Thomas Petazzoni]

Dear Markos Chandras,

On Wed, 5 Jun 2013 13:56:16 +0100, Markos Chandras wrote:
> From: Markos Chandras <markos.chandras@imgtec.com>
> 
> Several object files are shared between the libbz2.so shared library
> and the libbz2.a static one. MIPS will refuce to build a relocatable
> object when creating a new shared library with the following error:
> 
> blocksort.o: relocation R_MIPS_HI16 against `__gnu_local_gp' can not be used
> when making a shared object; recompile with -fPIC
> 
> This is because these files are build without -fPIC when creating the
> static library and later on they are used to build the shared one.
> 
> This is easily fixed if we add the shared library build rule before
> creating the static library so object files are always compiled with
> -fPIC.
> 
> Signed-off-by: Markos Chandras <markos.chandras@imgtec.com>

This seems to make sense to me, so:

Reviewed-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH] bzip2: Rearrange build order

[Peter Korsgaard]

>>>>> "Markos" == Markos Chandras <markos.chandras@gmail.com> writes:

 Markos> From: Markos Chandras <markos.chandras@imgtec.com>
 Markos> Several object files are shared between the libbz2.so shared library
 Markos> and the libbz2.a static one. MIPS will refuce to build a relocatable
 Markos> object when creating a new shared library with the following error:

 Markos> blocksort.o: relocation R_MIPS_HI16 against `__gnu_local_gp' can not be used
 Markos> when making a shared object; recompile with -fPIC

 Markos> This is because these files are build without -fPIC when creating the
 Markos> static library and later on they are used to build the shared one.

 Markos> This is easily fixed if we add the shared library build rule before
 Markos> creating the static library so object files are always compiled with
 Markos> -fPIC.

This works, but is afaik less efficient for the static lib case.

The real fix is imho to build the object files twice, like how libtool
does it.

If you look at the Debian package, they work around it by adding a
seperate .c -> .sho build rule, which adds -fPIC, and then link the .so
file with the .sho files instead.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] bzip2: Rearrange build order

[Markos Chandras]

On 5 June 2013 14:50, Peter Korsgaard <jacmet@uclibc.org> wrote:
>>>>>> "Markos" == Markos Chandras <markos.chandras@gmail.com> writes:
>
>  Markos> From: Markos Chandras <markos.chandras@imgtec.com>
>  Markos> Several object files are shared between the libbz2.so shared library
>  Markos> and the libbz2.a static one. MIPS will refuce to build a relocatable
>  Markos> object when creating a new shared library with the following error:
>
>  Markos> blocksort.o: relocation R_MIPS_HI16 against `__gnu_local_gp' can not be used
>  Markos> when making a shared object; recompile with -fPIC
>
>  Markos> This is because these files are build without -fPIC when creating the
>  Markos> static library and later on they are used to build the shared one.
>
>  Markos> This is easily fixed if we add the shared library build rule before
>  Markos> creating the static library so object files are always compiled with
>  Markos> -fPIC.
>
> This works, but is afaik less efficient for the static lib case.
>
> The real fix is imho to build the object files twice, like how libtool
> does it.
>
> If you look at the Debian package, they work around it by adding a
> seperate .c -> .sho build rule, which adds -fPIC, and then link the .so
> file with the .sho files instead.
>
> --
> Bye, Peter Korsgaard

Hi Peter,

Ok I will send a new patch based on the debian patch.

--
Regards,
Markos Chandras

[Buildroot] [PATCH] bzip2: Rearrange build order

[Thomas Petazzoni]

Dear Peter Korsgaard,

On Wed, 05 Jun 2013 15:50:21 +0200, Peter Korsgaard wrote:

> This works, but is afaik less efficient for the static lib case.
> 
> The real fix is imho to build the object files twice, like how libtool
> does it.
> 
> If you look at the Debian package, they work around it by adding a
> seperate .c -> .sho build rule, which adds -fPIC, and then link the .so
> file with the .sho files instead.

Are you sure there are not already many packages that build things only
once with -fPIC and use that for both the static and the shared library?

What you're proposing here is quite the opposite to what you merged
(from me) in a33baa1ef9dadbec8e45d411c30d636fa6b8872a (icu: don't build
object files twice).

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH] bzip2: Rearrange build order

[Markos Chandras]

On 5 June 2013 15:04, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Peter Korsgaard,
>
> On Wed, 05 Jun 2013 15:50:21 +0200, Peter Korsgaard wrote:
>
>> This works, but is afaik less efficient for the static lib case.
>>
>> The real fix is imho to build the object files twice, like how libtool
>> does it.
>>
>> If you look at the Debian package, they work around it by adding a
>> seperate .c -> .sho build rule, which adds -fPIC, and then link the .so
>> file with the .sho files instead.
>
> Are you sure there are not already many packages that build things only
> once with -fPIC and use that for both the static and the shared library?
>
> What you're proposing here is quite the opposite to what you merged
> (from me) in a33baa1ef9dadbec8e45d411c30d636fa6b8872a (icu: don't build
> object files twice).
>
> Thomas
> --
> Thomas Petazzoni, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com

My understanding is that there is a small performance penalty in
static libraries because of the GOT presence introduced by the shared
object. Apart from that, nothing else should change in the static
library.
Gentoo does the same thing for bzip2. It builds the shared library first.

--
Regards,
Markos Chandras

[Buildroot] [PATCH] bzip2: Rearrange build order

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

 >> This works, but is afaik less efficient for the static lib case.
 >> 
 >> The real fix is imho to build the object files twice, like how libtool
 >> does it.
 >> 
 >> If you look at the Debian package, they work around it by adding a
 >> seperate .c -> .sho build rule, which adds -fPIC, and then link the .so
 >> file with the .sho files instead.

 Thomas> Are you sure there are not already many packages that build
 Thomas> things only once with -fPIC and use that for both the static
 Thomas> and the shared library?

 Thomas> What you're proposing here is quite the opposite to what you
 Thomas> merged (from me) in a33baa1ef9dadbec8e45d411c30d636fa6b8872a
 Thomas> (icu: don't build object files twice).

I've never claimed I was consistent ;) What I'm saying is simply that
the "correct" way to do this, is to build the object files twice similar
to how E.G. libtool does it.

For something as small (and possibly performance sensitive) as bzip2 I
think it is worthwhile doing it.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] bzip2: Rearrange build order

[Markos Chandras]

On 5 June 2013 15:15, Peter Korsgaard <jacmet@uclibc.org> wrote:
>>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:
>
>  >> This works, but is afaik less efficient for the static lib case.
>  >>
>  >> The real fix is imho to build the object files twice, like how libtool
>  >> does it.
>  >>
>  >> If you look at the Debian package, they work around it by adding a
>  >> seperate .c -> .sho build rule, which adds -fPIC, and then link the .so
>  >> file with the .sho files instead.
>
>  Thomas> Are you sure there are not already many packages that build
>  Thomas> things only once with -fPIC and use that for both the static
>  Thomas> and the shared library?
>
>  Thomas> What you're proposing here is quite the opposite to what you
>  Thomas> merged (from me) in a33baa1ef9dadbec8e45d411c30d636fa6b8872a
>  Thomas> (icu: don't build object files twice).
>
> I've never claimed I was consistent ;) What I'm saying is simply that
> the "correct" way to do this, is to build the object files twice similar
> to how E.G. libtool does it.
>
> For something as small (and possibly performance sensitive) as bzip2 I
> think it is worthwhile doing it.
>
> --
> Bye, Peter Korsgaard

Hi Peter,

The problem here is that Debian uses a single Makefile to build both
the static and the shared library but in buildroot we use
the Makefile for the static one and Makefile-libbz2_so for the shared
one. In this case, doing what Debian did is not possible
but what we can do is to remove the *.o files within the
Makefile-libbz2_so before we try to build the shared library.

--
Regards,
Markos Chandras

[Buildroot] [PATCH] bzip2: Rearrange build order

[Peter Korsgaard]

>>>>> "Markos" == Markos Chandras <markos.chandras@gmail.com> writes:

Hi,

 Markos> The problem here is that Debian uses a single Makefile to build both
 Markos> the static and the shared library but in buildroot we use
 Markos> the Makefile for the static one and Makefile-libbz2_so for the shared
 Markos> one. In this case, doing what Debian did is not possible
 Markos> but what we can do is to remove the *.o files within the
 Markos> Makefile-libbz2_so before we try to build the shared library.

Or we could just do something like:

define BZIP2_BUILD_CMDS
	$(TARGET_MAKE_ENV)
		$(MAKE) -C $(@D) libbz2.a bzip2 bzip2recover \
                        $(if $(BR2_PREFER_STATIC_LIB),,libbz2.so) \
                        $(TARGET_CONFIGURE_OPTS)
endef

E.G. only build the libbz2.so taget (which pulls in *.sho) if building
shared.

Or alternatively, keep Makefile-libbz2_so, but change it to use .sho
files instead of .o

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] bzip2: Rearrange build order

[Markos Chandras]

On 5 June 2013 15:48, Peter Korsgaard <jacmet@uclibc.org> wrote:
>>>>>> "Markos" == Markos Chandras <markos.chandras@gmail.com> writes:
>
> Hi,
>
>  Markos> The problem here is that Debian uses a single Makefile to build both
>  Markos> the static and the shared library but in buildroot we use
>  Markos> the Makefile for the static one and Makefile-libbz2_so for the shared
>  Markos> one. In this case, doing what Debian did is not possible
>  Markos> but what we can do is to remove the *.o files within the
>  Markos> Makefile-libbz2_so before we try to build the shared library.
>
> Or we could just do something like:
>
> define BZIP2_BUILD_CMDS
>         $(TARGET_MAKE_ENV)
>                 $(MAKE) -C $(@D) libbz2.a bzip2 bzip2recover \
>                         $(if $(BR2_PREFER_STATIC_LIB),,libbz2.so) \
>                         $(TARGET_CONFIGURE_OPTS)
> endef
>
> E.G. only build the libbz2.so taget (which pulls in *.sho) if building
> shared.
>
> Or alternatively, keep Makefile-libbz2_so, but change it to use .sho
> files instead of .o
>
> --
> Bye, Peter Korsgaard

Hi Peter,

Oh I've seen this e-mail a bit late. I just sent a new patch based on
the second proposal.

--
Regards,
Markos Chandras

[Buildroot] [PATCH] bzip2: Rearrange build order

[Thomas Petazzoni]

Dear Peter Korsgaard,

On Wed, 05 Jun 2013 16:15:34 +0200, Peter Korsgaard wrote:

>  Thomas> Are you sure there are not already many packages that build
>  Thomas> things only once with -fPIC and use that for both the static
>  Thomas> and the shared library?
> 
>  Thomas> What you're proposing here is quite the opposite to what you
>  Thomas> merged (from me) in a33baa1ef9dadbec8e45d411c30d636fa6b8872a
>  Thomas> (icu: don't build object files twice).
> 
> I've never claimed I was consistent ;) What I'm saying is simply that
> the "correct" way to do this, is to build the object files twice similar
> to how E.G. libtool does it.
> 
> For something as small (and possibly performance sensitive) as bzip2 I
> think it is worthwhile doing it.

I'm not sure I agree with your idea of making a different decision
depending on the package. Either we decide that all static libraries
should not be built with -fPIC, and we apply this rule on all packages,
or we decide that all static libraries are built with -fPIC, so that we
actually build object files only once.

In fact, I hadn't realized that libtool was building each and every .c
file twice, once without -fPIC for the static library, and once with
-fPIC for the shared library.

I believe we have three choices:

 (1) When !BR2_PREFER_STATIC_LIB, pass --enable-shared --disable-static
     instead of the current --enable-shared --enable-static. When I did
     009d8fceab4db7815502e4b0565fe0ef531d512c, I wasn't aware that
     having --enable-static was causing a double build of source files.
     Had I realized that, I would have probably suggested a different
     solution. If someone builds with !BR2_PREFER_STATIC_LIB, it's
     pretty unlikely that the static version of the libraries will be
     needed. There might be a few exceptions, but they can be handled
     by the user by adding --enable-static to the CONF_OPT of the
     specific packages he is interested in.

 (2) When !BR2_PREFER_STATIC_LIB, find a way of telling libtool not to
     build object files twice, and generate static libraries using the
     -fPIC capable object files. It's slightly less efficient, but if
     you're building !BR2_PREFER_STATIC_LIB, you're using shared
     libraries for most of your applications anyway, so having a small
     hit with the few static libraries isn't going to be really
     noticeable.

 (3) When !BR2_PREFER_STATIC_LIB, keep --enable-shared and
     --enable-static as we have today, and make sure that object files
     are always built twice, once without -fPIC, and once with. I
     believe this is making the build time longer, for situations
     (usage of static libraries when !BR2_PREFER_STATIC_LIB) that are
     fairly uncommon.

In any case, the solution of "some packages have their static library
objects built without -fPIC, some other packages have their static
library objects built with -fPIC" is not really nice. Where's the
boundary between the first and second category of packages?

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH] bzip2: Rearrange build order

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

Hi,

 Thomas> In fact, I hadn't realized that libtool was building each and
 Thomas> every .c file twice, once without -fPIC for the static library,
 Thomas> and once with -fPIC for the shared library.

 Thomas> I believe we have three choices:

Yes, like we discussed on IRC I think we need to end up with 3 options:

- Build only shared libraries (default)
- Build only static libraries (BR2_PREFER_STATIC_LIB)
- Build both, and prefer shared libraries (what we have today)

-- 
Bye, Peter Korsgaard