libblktapctl.so is unversioned

libblktapctl.so is unversioned

[Olaf Hering]

Hello,

In my builds /usr/sbin/tap-ctl and /usr/sbin/xl link against this shared
lib.  Is there a reason why libblktapctl.so has no SONAME and no version
number, like libblktapctl.so.1?

Olaf

Re: libblktapctl.so is unversioned

[Keir Fraser]

It ought to be versioned, especially since I think the interface is going to
change to hide some OS dependencies.

 K.

On 28/07/2010 16:23, "Olaf Hering" <olaf@aepfle.de> wrote:

> 
> Hello,
> 
> In my builds /usr/sbin/tap-ctl and /usr/sbin/xl link against this shared
> lib.  Is there a reason why libblktapctl.so has no SONAME and no version
> number, like libblktapctl.so.1?
> 
> Olaf
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

[PATCH] add SONAME to libblktapctl.so

[Olaf Hering]

Add an SONAME to libblktapctl.so.
Install static library as data to avoid executable permissions in the .a file.

Signed-off-by: Olaf Hering <olaf@aepfle.de>

---
 tools/blktap2/control/Makefile |   35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

--- xen-unstable.hg-4.1.21890.orig/tools/blktap2/control/Makefile
+++ xen-unstable.hg-4.1.21890/tools/blktap2/control/Makefile
@@ -1,6 +1,11 @@
 XEN_ROOT := ../../../
 include $(XEN_ROOT)/tools/Rules.mk
 
+MAJOR              = 1.0
+MINOR              = 0
+LIBNAME            = libblktapctl
+LIBSONAME          = $(LIBNAME).so.$(MAJOR)
+
 IBIN               = tap-ctl
 
 CFLAGS            += -Werror
@@ -35,29 +40,39 @@ CTL_PICS  = $(patsubst %.o,%.opic,$(CTL_
 OBJS = $(CTL_OBJS) tap-ctl.o
 PICS = $(CTL_PICS)
 
-LIBS = libblktapctl.a libblktapctl.so
+LIB_STATIC = $(LIBNAME).a
+LIB_SHARED = $(LIBSONAME).$(MINOR)
 IBIN = tap-ctl
 
 all: build
 
-build: $(IBIN) $(LIBS)
+build: $(IBIN) $(LIB_STATIC) $(LIB_SHARED)
+
+$(LIBNAME).so: $(LIBSONAME)
+	ln -sf $< $@
+
+$(LIBSONAME): $(LIB_SHARED)
+	ln -sf $< $@
 
-tap-ctl: tap-ctl.o libblktapctl.so
-	$(CC) $(CFLAGS) -o $@ $^
+tap-ctl: tap-ctl.o $(LIBNAME).so
+	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^
 
-libblktapctl.a: $(CTL_OBJS)
+$(LIB_STATIC): $(CTL_OBJS)
 	$(AR) r $@ $^
 
-libblktapctl.so: $(CTL_PICS)
-	$(CC) $(CFLAGS) -fPIC -shared -rdynamic $^ -o $@
+$(LIB_SHARED): $(CTL_PICS)
+	$(CC) $(CFLAGS) $(LDFLAGS) -fPIC  -Wl,$(SONAME_LDFLAG) -Wl,$(LIBSONAME) $(SHLIB_LDFLAGS) -rdynamic $^ -o $@
 
-install: $(IBIN) $(LIBS)
+install: $(IBIN) $(LIB_STATIC) $(LIB_SHARED)
 	$(INSTALL_DIR) -p $(DESTDIR)$(SBINDIR)
 	$(INSTALL_PROG) $(IBIN) $(DESTDIR)$(SBINDIR)
-	$(INSTALL_PROG) $(LIBS) $(DESTDIR)$(LIBDIR)
+	$(INSTALL_DATA) $(LIB_STATIC) $(DESTDIR)$(LIBDIR)
+	$(INSTALL_PROG) $(LIB_SHARED) $(DESTDIR)$(LIBDIR)
+	ln -sf $(LIBSONAME) $(DESTDIR)$(LIBDIR)/$(LIBNAME).so
+	ln -sf $(LIB_SHARED) $(DESTDIR)$(LIBDIR)/$(LIBSONAME)
 
 clean:
-	rm -f $(OBJS) $(PICS) $(DEPS) $(IBIN) $(LIBS)
+	rm -f $(OBJS) $(PICS) $(DEPS) $(IBIN) $(LIB_STATIC) $(LIB_SHARED)
 	rm -f *~
 
 .PHONY: all build clean install

Re: [PATCH] add SONAME to libblktapctl.so

[Ian Jackson]

Olaf Hering writes ("[Xen-devel] [PATCH] add SONAME to libblktapctl.so"):
> Add an SONAME to libblktapctl.so.  Install static library as data to
> avoid executable permissions in the .a file.

Thanks.  I have a few comments:

> -libblktapctl.so: $(CTL_PICS)
> -	$(CC) $(CFLAGS) -fPIC -shared -rdynamic $^ -o $@
> +$(LIB_SHARED): $(CTL_PICS)
> +	$(CC) $(CFLAGS) $(LDFLAGS) -fPIC  -Wl,$(SONAME_LDFLAG) -Wl,$(LIBSONAME) $(SHLIB_LDFLAGS) -rdynamic $^ -o $@

I'm not sure I understand why -fPIC appears here as a literal, rather
than being included via some more general make variable.  And does it
have any effect on the shared library creation link line anyway ?
Other places that make shared libraries don't have -fPIC there.

Likewise they don't say -rdynamic.

I appreciate that you're not doing anything here that wasn't in the
makefiles before.  So really I'm just asking if you or anyone else
knows anything about the reasons for this.

Thanks,
Ian.

Re: [PATCH] add SONAME to libblktapctl.so

[Olaf Hering]

On Wed, Aug 11, Ian Jackson wrote:

> > -libblktapctl.so: $(CTL_PICS)
> > -	$(CC) $(CFLAGS) -fPIC -shared -rdynamic $^ -o $@
> > +$(LIB_SHARED): $(CTL_PICS)
> > +	$(CC) $(CFLAGS) $(LDFLAGS) -fPIC  -Wl,$(SONAME_LDFLAG) -Wl,$(LIBSONAME) $(SHLIB_LDFLAGS) -rdynamic $^ -o $@
> 
> I'm not sure I understand why -fPIC appears here as a literal, rather
> than being included via some more general make variable.  And does it
> have any effect on the shared library creation link line anyway ?
> Other places that make shared libraries don't have -fPIC there.
> 
> Likewise they don't say -rdynamic.

Ian,

maybe someone has over-done things here. I'm sure libblktapctl is most
likely not different from all the other xen libraries. So I would just
drop -fPIC and -rdynamic from the options.

The gcc manual states that linking should be done with the same options
used to compile the individual objects, to get predictable results. So
if for example -fPIC wasnt there before (in CFLAGS), it wont make the
resulting lib any better.

Olaf

Re: [PATCH] add SONAME to libblktapctl.so

[Ian Jackson]

Olaf Hering writes ("Re: [Xen-devel] [PATCH] add SONAME to libblktapctl.so"):
> maybe someone has over-done things here. I'm sure libblktapctl is most
> likely not different from all the other xen libraries. So I would just
> drop -fPIC and -rdynamic from the options.
> 
> The gcc manual states that linking should be done with the same options
> used to compile the individual objects, to get predictable results. So
> if for example -fPIC wasnt there before (in CFLAGS), it wont make the
> resulting lib any better.

That's a convincing explanation.  In which case, as regards your
patch,

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

and we should have a separate changeset to remove -fPIC and -rdynamic.

Ian.

Re: [PATCH] add SONAME to libblktapctl.so

[Ian Jackson]

Ian Jackson writes ("Re: [Xen-devel] [PATCH] add SONAME to libblktapctl.so"):
> That's a convincing explanation.  In which case, as regards your
> patch,
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> and we should have a separate changeset to remove -fPIC and -rdynamic.

And here that one is:


tools/blktap2: Remove unnecessary -fPIC and -rdynamic from link line

Where these options are needed they will be in the appropriate *FLAGS
variables; they should not appear in specific Makefiles.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>


diff -r ddb9f47ef2e2 tools/blktap2/control/Makefile
--- a/tools/blktap2/control/Makefile	Thu Aug 12 18:56:20 2010 +0100
+++ b/tools/blktap2/control/Makefile	Fri Aug 13 13:34:49 2010 +0100
@@ -61,7 +61,7 @@ tap-ctl: tap-ctl.o $(LIBNAME).so
 	$(AR) r $@ $^
 
 $(LIB_SHARED): $(CTL_PICS)
-	$(CC) $(CFLAGS) $(LDFLAGS) -fPIC  -Wl,$(SONAME_LDFLAG) -Wl,$(LIBSONAME) $(SHLIB_LDFLAGS) -rdynamic $^ -o $@
+	$(CC) $(CFLAGS) $(LDFLAGS) -Wl,$(SONAME_LDFLAG) -Wl,$(LIBSONAME) $(SHLIB_LDFLAGS) $^ -o $@
 
 install: $(IBIN) $(LIB_STATIC) $(LIB_SHARED)
 	$(INSTALL_DIR) -p $(DESTDIR)$(SBINDIR)