[Cluster-devel] [PATCH] Fix libdlm static build

[Cluster-devel] [PATCH] Fix libdlm static build

[Fabio M. Di Nitto]

Hi David,

the objects included in the static version of dlm are different from the 
one in the shared lib. This is generally wrong as they should be the same.

the patch addresses the problem by killing .po files in favour of more 
common .o and changing the linking targets.

the patch applies to both master and stable2. rhel branches are also 
affected by other problems (_lt is linked to pthread) but i don't think 
it's wise to change it.

Please review and apply.

Thanks
Fabio

diff --git a/dlm/libdlm/Makefile b/dlm/libdlm/Makefile
index 0cc1f7b..734eaa9 100644
--- a/dlm/libdlm/Makefile
+++ b/dlm/libdlm/Makefile
@@ -42,26 +42,20 @@ $(TARGET)_lt.a: $(TARGET)_lt.o
  	${AR} cru $@ $^
  	${RANLIB} $@

-$(TARGET).so.${SOMAJOR}.${SOMINOR}: $(TARGET).po $(AISTARGET).po
+$(TARGET).so.${SOMAJOR}.${SOMINOR}: $(TARGET).o $(AISTARGET).o
  	$(CC) -shared -o $@ -Wl,-soname=$(TARGET).so.$(SOMAJOR) $< $(PTHREAD_LDFLAGS) $(LDFLAGS)
  	ln -sf $(TARGET).so.$(SOMAJOR).$(SOMINOR) $(TARGET).so
  	ln -sf $(TARGET).so.$(SOMAJOR).$(SOMINOR) $(TARGET).so.$(SOMAJOR)

-$(TARGET)_lt.so.${SOMAJOR}.${SOMINOR}: $(TARGET)_lt.po
+$(TARGET)_lt.so.${SOMAJOR}.${SOMINOR}: $(TARGET)_lt.o
  	$(CC) -shared -o $@ -Wl,-soname=$(TARGET)_lt.so.$(SOMAJOR) $< $(LDFLAGS)
  	ln -sf $(TARGET)_lt.so.$(SOMAJOR).$(SOMINOR) $(TARGET)_lt.so
  	ln -sf $(TARGET)_lt.so.$(SOMAJOR).$(SOMINOR) $(TARGET)_lt.so.$(SOMAJOR)

  %_lt.o: $(S)/%.c
-	$(CC) $(CFLAGS) -c -o $@ $<
-
-%_lt.po: $(S)/%.c
  	$(CC) $(CFLAGS) -fPIC -c -o $@ $<

  %.o: $(S)/%.c
-	$(CC) $(CFLAGS) -D_REENTRANT -c -o $@ $<
-
-%.po: $(S)/%.c
  	$(CC) $(CFLAGS) -fPIC -D_REENTRANT -c -o $@ $<

  clean: generalclean

--
I'm going to make him an offer he can't refuse.

[Cluster-devel] [PATCH] Fix libdlm static build

[Christine Caulfield]

Fabio M. Di Nitto wrote:
> 
> Hi David,
> 
> the objects included in the static version of dlm are different from the 
> one in the shared lib. This is generally wrong as they should be the same.
> 
> the patch addresses the problem by killing .po files in favour of more 
> common .o and changing the linking targets.
> 
> the patch applies to both master and stable2. rhel branches are also 
> affected by other problems (_lt is linked to pthread) but i don't think 
> it's wise to change it.
> 
> Please review and apply.
> 
> Thanks
> Fabio
> 
> diff --git a/dlm/libdlm/Makefile b/dlm/libdlm/Makefile
> index 0cc1f7b..734eaa9 100644
> --- a/dlm/libdlm/Makefile
> +++ b/dlm/libdlm/Makefile
> @@ -42,26 +42,20 @@ $(TARGET)_lt.a: $(TARGET)_lt.o
>      ${AR} cru $@ $^
>      ${RANLIB} $@
> 
> -$(TARGET).so.${SOMAJOR}.${SOMINOR}: $(TARGET).po $(AISTARGET).po
> +$(TARGET).so.${SOMAJOR}.${SOMINOR}: $(TARGET).o $(AISTARGET).o
>      $(CC) -shared -o $@ -Wl,-soname=$(TARGET).so.$(SOMAJOR) $< 
> $(PTHREAD_LDFLAGS) $(LDFLAGS)
>      ln -sf $(TARGET).so.$(SOMAJOR).$(SOMINOR) $(TARGET).so
>      ln -sf $(TARGET).so.$(SOMAJOR).$(SOMINOR) $(TARGET).so.$(SOMAJOR)
> 
> -$(TARGET)_lt.so.${SOMAJOR}.${SOMINOR}: $(TARGET)_lt.po
> +$(TARGET)_lt.so.${SOMAJOR}.${SOMINOR}: $(TARGET)_lt.o
>      $(CC) -shared -o $@ -Wl,-soname=$(TARGET)_lt.so.$(SOMAJOR) $< 
> $(LDFLAGS)
>      ln -sf $(TARGET)_lt.so.$(SOMAJOR).$(SOMINOR) $(TARGET)_lt.so
>      ln -sf $(TARGET)_lt.so.$(SOMAJOR).$(SOMINOR) 
> $(TARGET)_lt.so.$(SOMAJOR)
> 
>  %_lt.o: $(S)/%.c
> -    $(CC) $(CFLAGS) -c -o $@ $<
> -
> -%_lt.po: $(S)/%.c
>      $(CC) $(CFLAGS) -fPIC -c -o $@ $<
> 
>  %.o: $(S)/%.c
> -    $(CC) $(CFLAGS) -D_REENTRANT -c -o $@ $<
> -
> -%.po: $(S)/%.c
>      $(CC) $(CFLAGS) -fPIC -D_REENTRANT -c -o $@ $<
> 
>  clean: generalclean
> 

I don't understand the problem you are trying to fix here. Having PIC 
objects in the dynamic library and non-PIC in the static is perfectly 
standard practice.

-- 

Chrissie

[Cluster-devel] [PATCH] Fix libdlm static build

[Fabio M. Di Nitto]

On Thu, 3 Jul 2008, Christine Caulfield wrote:

> Fabio M. Di Nitto wrote:
>> 
>> Hi David,
>> 
>> the objects included in the static version of dlm are different from the 
>> one in the shared lib. This is generally wrong as they should be the same.
>> 
>> the patch addresses the problem by killing .po files in favour of more 
>> common .o and changing the linking targets.
>> 
>> the patch applies to both master and stable2. rhel branches are also 
>> affected by other problems (_lt is linked to pthread) but i don't think 
>> it's wise to change it.
>> 
>> Please review and apply.
>> 
>> Thanks
>> Fabio
>>

[SNIP]

> I don't understand the problem you are trying to fix here.

To have the same objects in both.

> Having PIC objects 
> in the dynamic library and non-PIC in the static is perfectly standard 
> practice.

Why does dlm needs to have it different from the rest of our libraries? Is 
there a special reason?

Fabio

--
I'm going to make him an offer he can't refuse.

[Cluster-devel] [PATCH] Fix libdlm static build

[Lon Hohberger]

On Thu, 2008-07-03 at 14:06 +0100, Christine Caulfield wrote:

> 
> I don't understand the problem you are trying to fix here. Having PIC 
> objects in the dynamic library and non-PIC in the static is perfectly 
> standard practice.
> 

Well, you can't build a loadable module using a static library w/o
building PIC.

-- Lon

[Cluster-devel] [PATCH] Fix libdlm static build

[Christine Caulfield]

Lon Hohberger wrote:
> On Thu, 2008-07-03 at 14:06 +0100, Christine Caulfield wrote:
> 
>> I don't understand the problem you are trying to fix here. Having PIC 
>> objects in the dynamic library and non-PIC in the static is perfectly 
>> standard practice.
>>
> 
> Well, you can't build a loadable module using a static library w/o
> building PIC.

Good. Then build against the dynamic one like you're supposed to!


Chrissie

[Cluster-devel] [PATCH] Fix libdlm static build

[Fabio M. Di Nitto]

On Fri, 4 Jul 2008, Christine Caulfield wrote:

> Lon Hohberger wrote:
>> On Thu, 2008-07-03 at 14:06 +0100, Christine Caulfield wrote:
>> 
>>> I don't understand the problem you are trying to fix here. Having PIC 
>>> objects in the dynamic library and non-PIC in the static is perfectly 
>>> standard practice.
>>> 
>> 
>> Well, you can't build a loadable module using a static library w/o
>> building PIC.
>
> Good. Then build against the dynamic one like you're supposed to!

Generally this is right, we still want to ship a working version of the 
static one :)

Fabio

--
I'm going to make him an offer he can't refuse.

[Cluster-devel] [PATCH] Fix libdlm static build

[Christine Caulfield]

Fabio M. Di Nitto wrote:
> On Fri, 4 Jul 2008, Christine Caulfield wrote:
> 
>> Lon Hohberger wrote:
>>> On Thu, 2008-07-03 at 14:06 +0100, Christine Caulfield wrote:
>>>
>>>> I don't understand the problem you are trying to fix here. Having 
>>>> PIC objects in the dynamic library and non-PIC in the static is 
>>>> perfectly standard practice.
>>>>
>>>
>>> Well, you can't build a loadable module using a static library w/o
>>> building PIC.
>>
>> Good. Then build against the dynamic one like you're supposed to!
> 
> Generally this is right, we still want to ship a working version of the 
> static one :)

You still haven't said just what it is that is broken about the static 
library. Given that my test programs in dlm/tests/usertest all use it, 
it can't be totally broken surely ??

-- 

Chrissie

[Cluster-devel] [PATCH] Fix libdlm static build

[Fabio M. Di Nitto]

On Fri, 4 Jul 2008, Christine Caulfield wrote:

> Fabio M. Di Nitto wrote:
>> On Fri, 4 Jul 2008, Christine Caulfield wrote:
>> 
>>> Lon Hohberger wrote:
>>>> On Thu, 2008-07-03 at 14:06 +0100, Christine Caulfield wrote:
>>>> 
>>>>> I don't understand the problem you are trying to fix here. Having PIC 
>>>>> objects in the dynamic library and non-PIC in the static is perfectly 
>>>>> standard practice.
>>>>> 
>>>> 
>>>> Well, you can't build a loadable module using a static library w/o
>>>> building PIC.
>>> 
>>> Good. Then build against the dynamic one like you're supposed to!
>> 
>> Generally this is right, we still want to ship a working version of the 
>> static one :)
>
> You still haven't said just what it is that is broken about the static 
> library. Given that my test programs in dlm/tests/usertest all use it, it 
> can't be totally broken surely ??

Just to summarize a bit what we discussed on IRC:

-fPIC changes the way in which the code is generated and built by gcc. So 
building the static version the same way as the shared, will keep it the 
same across. This is important on some architectures like powerpc 
(according to gcc man page).

What lon said in the other email is also a valid point.

As extra bonus:

the makefile becomes simpler and should be possible to collapse like the 
others (didn't check this in details yet since dlm is the only one 
generating more than one shared lib from the same dir.).

I don't dare to say (just kidding here!) that it takes less time to build 
since you will build 50% less objects ;)

Anyway, enough said, it's a trivial change to keep things aligned and I 
see no reason to have libdlm having this special exception.

It's up to David or you (i still don't know who of you owns what in dlm) 
if you want it or not.

Fabio

--
I'm going to make him an offer he can't refuse.