[PATCH] tools/ocaml: Fix library generation

[PATCH] tools/ocaml: Fix library generation

[Vincent Bernardoff]

From: Vincent Bernardoff <vincent.bernardoff@citrix.com>

Fix the commands given to the OCaml compiler to make the OCaml
bindings to Xen usable outside the build environment.

Signed-off-by: Vincent Bernardoff <vincent.bernardoff@citrix.com>
---
 tools/Rules.mk             |    8 ++++----
 tools/ocaml/Makefile.rules |    6 +++---
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/Rules.mk b/tools/Rules.mk
index 8d55e03..91a5d02 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -20,15 +20,15 @@ XEN_LIBVCHAN       = $(XEN_ROOT)/tools/libvchan
 CFLAGS_xeninclude = -I$(XEN_INCLUDE)
 
 CFLAGS_libxenctrl = -I$(XEN_LIBXC) $(CFLAGS_xeninclude)
-LDLIBS_libxenctrl = $(XEN_LIBXC)/libxenctrl.so
+LDLIBS_libxenctrl = -L$(XEN_LIBXC) -lxenctrl
 SHLIB_libxenctrl  = -Wl,-rpath-link=$(XEN_LIBXC)
 
 CFLAGS_libxenguest = -I$(XEN_LIBXC) $(CFLAGS_xeninclude)
-LDLIBS_libxenguest = $(XEN_LIBXC)/libxenguest.so
+LDLIBS_libxenguest = -L$(XEN_LIBXC) -lxenguest
 SHLIB_libxenguest  = -Wl,-rpath-link=L$(XEN_LIBXC)
 
 CFLAGS_libxenstore = -I$(XEN_XENSTORE) $(CFLAGS_xeninclude)
-LDLIBS_libxenstore = $(XEN_XENSTORE)/libxenstore.so
+LDLIBS_libxenstore = -L$(XEN_XENSTORE) -lxenstore
 SHLIB_libxenstore  = -Wl,-rpath-link=$(XEN_XENSTORE)
 
 CFLAGS_libxenstat  = -I$(XEN_LIBXENSTAT)
@@ -56,7 +56,7 @@ SHLIB_libblktapctl  =
 endif
 
 CFLAGS_libxenlight = -I$(XEN_XENLIGHT) $(CFLAGS_libxenctrl) $(CFLAGS_xeninclude)
-LDLIBS_libxenlight = $(XEN_XENLIGHT)/libxenlight.so $(SHLIB_libxenctrl) $(SHLIB_libxenstore) $(SHLIB_libblktapctl)
+LDLIBS_libxenlight = -L$(XEN_XENLIGHT) -lxenlight $(SHLIB_libxenctrl) $(SHLIB_libxenstore) $(SHLIB_libblktapctl)
 SHLIB_libxenlight  = -Wl,-rpath-link=$(XEN_XENLIGHT)
 
 CFLAGS += -D__XEN_TOOLS__
diff --git a/tools/ocaml/Makefile.rules b/tools/ocaml/Makefile.rules
index ff19067..28b81a9 100644
--- a/tools/ocaml/Makefile.rules
+++ b/tools/ocaml/Makefile.rules
@@ -45,7 +45,7 @@ ALL_OCAML_OBJ_SOURCES=$(addsuffix .ml, $(ALL_OCAML_OBJS))
 	$(call quiet-command, $(OCAMLDEP) $(ALL_OCAML_OBJ_SOURCES) *.mli $o,MLDEP,)
 
 clean: $(CLEAN_HOOKS)
-	$(Q)rm -f .*.d *.o *.so *.a *.cmo *.cmi *.cma *.cmx *.cmxa *.annot *.spot *.spit $(LIBS) $(PROGRAMS) $(GENERATED_FILES) .ocamldep.make
+	$(Q)rm -f .*.d *.o *.so *.a *.cmo *.cmi *.cma *.cmx *.cmxa *.annot *.spot *.spit $(LIBS) $(PROGRAMS) $(GENERATED_FILES) .ocamldep.make META
 
 quiet-command = $(if $(V),$1,@printf " %-8s %s\n" "$2" "$3" && $1)
 
@@ -54,7 +54,7 @@ mk-caml-lib-bytecode = $(call quiet-command, $(OCAMLC) $(OCAMLCFLAGS) -a -o $1 $
 
 mk-caml-stubs = $(call quiet-command, $(OCAMLMKLIB) -o `basename $1 .a` $2,MKLIB,$1)
 mk-caml-lib-stubs = \
-	$(call quiet-command, $(AR) rcs $1 $2 && $(OCAMLMKLIB) -o `basename $1 .a | sed -e 's/^lib//'` $2,MKLIB,$1)
+	$(call quiet-command, $(OCAMLMKLIB) -o `basename $1 .a | sed -e 's/^lib//'` $2 $3,MKLIB,$1)
 
 # define a library target <name>.cmxa and <name>.cma
 define OCAML_LIBRARY_template
@@ -65,7 +65,7 @@ define OCAML_LIBRARY_template
  $(1)_stubs.a: $(foreach obj,$$($(1)_C_OBJS),$(obj).o)
 	$(call mk-caml-stubs,$$@, $$+)
  lib$(1)_stubs.a: $(foreach obj,$($(1)_C_OBJS),$(obj).o)
-	$(call mk-caml-lib-stubs,$$@, $$+)
+	$(call mk-caml-lib-stubs,$$@, $$+, $(foreach lib,$(LIBS_$(1)),$(lib)))
 endef
 
 define OCAML_NOC_LIBRARY_template
-- 
1.7.10.4

Re: [PATCH] tools/ocaml: Fix library generation

[Andrew Cooper]

On 08/04/13 18:26, Vincent Bernardoff wrote:
> From: Vincent Bernardoff <vincent.bernardoff@citrix.com>
>
> Fix the commands given to the OCaml compiler to make the OCaml
> bindings to Xen usable outside the build environment.
>
> Signed-off-by: Vincent Bernardoff <vincent.bernardoff@citrix.com>

Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>

Ian: This supersedes the precurser of this patch in the thread "ocaml
bindings" and my patch by the same name.

> ---
>  tools/Rules.mk             |    8 ++++----
>  tools/ocaml/Makefile.rules |    6 +++---
>  2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/tools/Rules.mk b/tools/Rules.mk
> index 8d55e03..91a5d02 100644
> --- a/tools/Rules.mk
> +++ b/tools/Rules.mk
> @@ -20,15 +20,15 @@ XEN_LIBVCHAN       = $(XEN_ROOT)/tools/libvchan
>  CFLAGS_xeninclude = -I$(XEN_INCLUDE)
>  
>  CFLAGS_libxenctrl = -I$(XEN_LIBXC) $(CFLAGS_xeninclude)
> -LDLIBS_libxenctrl = $(XEN_LIBXC)/libxenctrl.so
> +LDLIBS_libxenctrl = -L$(XEN_LIBXC) -lxenctrl
>  SHLIB_libxenctrl  = -Wl,-rpath-link=$(XEN_LIBXC)
>  
>  CFLAGS_libxenguest = -I$(XEN_LIBXC) $(CFLAGS_xeninclude)
> -LDLIBS_libxenguest = $(XEN_LIBXC)/libxenguest.so
> +LDLIBS_libxenguest = -L$(XEN_LIBXC) -lxenguest
>  SHLIB_libxenguest  = -Wl,-rpath-link=L$(XEN_LIBXC)
>  
>  CFLAGS_libxenstore = -I$(XEN_XENSTORE) $(CFLAGS_xeninclude)
> -LDLIBS_libxenstore = $(XEN_XENSTORE)/libxenstore.so
> +LDLIBS_libxenstore = -L$(XEN_XENSTORE) -lxenstore
>  SHLIB_libxenstore  = -Wl,-rpath-link=$(XEN_XENSTORE)
>  
>  CFLAGS_libxenstat  = -I$(XEN_LIBXENSTAT)
> @@ -56,7 +56,7 @@ SHLIB_libblktapctl  =
>  endif
>  
>  CFLAGS_libxenlight = -I$(XEN_XENLIGHT) $(CFLAGS_libxenctrl) $(CFLAGS_xeninclude)
> -LDLIBS_libxenlight = $(XEN_XENLIGHT)/libxenlight.so $(SHLIB_libxenctrl) $(SHLIB_libxenstore) $(SHLIB_libblktapctl)
> +LDLIBS_libxenlight = -L$(XEN_XENLIGHT) -lxenlight $(SHLIB_libxenctrl) $(SHLIB_libxenstore) $(SHLIB_libblktapctl)
>  SHLIB_libxenlight  = -Wl,-rpath-link=$(XEN_XENLIGHT)
>  
>  CFLAGS += -D__XEN_TOOLS__
> diff --git a/tools/ocaml/Makefile.rules b/tools/ocaml/Makefile.rules
> index ff19067..28b81a9 100644
> --- a/tools/ocaml/Makefile.rules
> +++ b/tools/ocaml/Makefile.rules
> @@ -45,7 +45,7 @@ ALL_OCAML_OBJ_SOURCES=$(addsuffix .ml, $(ALL_OCAML_OBJS))
>  	$(call quiet-command, $(OCAMLDEP) $(ALL_OCAML_OBJ_SOURCES) *.mli $o,MLDEP,)
>  
>  clean: $(CLEAN_HOOKS)
> -	$(Q)rm -f .*.d *.o *.so *.a *.cmo *.cmi *.cma *.cmx *.cmxa *.annot *.spot *.spit $(LIBS) $(PROGRAMS) $(GENERATED_FILES) .ocamldep.make
> +	$(Q)rm -f .*.d *.o *.so *.a *.cmo *.cmi *.cma *.cmx *.cmxa *.annot *.spot *.spit $(LIBS) $(PROGRAMS) $(GENERATED_FILES) .ocamldep.make META
>  
>  quiet-command = $(if $(V),$1,@printf " %-8s %s\n" "$2" "$3" && $1)
>  
> @@ -54,7 +54,7 @@ mk-caml-lib-bytecode = $(call quiet-command, $(OCAMLC) $(OCAMLCFLAGS) -a -o $1 $
>  
>  mk-caml-stubs = $(call quiet-command, $(OCAMLMKLIB) -o `basename $1 .a` $2,MKLIB,$1)
>  mk-caml-lib-stubs = \
> -	$(call quiet-command, $(AR) rcs $1 $2 && $(OCAMLMKLIB) -o `basename $1 .a | sed -e 's/^lib//'` $2,MKLIB,$1)
> +	$(call quiet-command, $(OCAMLMKLIB) -o `basename $1 .a | sed -e 's/^lib//'` $2 $3,MKLIB,$1)
>  
>  # define a library target <name>.cmxa and <name>.cma
>  define OCAML_LIBRARY_template
> @@ -65,7 +65,7 @@ define OCAML_LIBRARY_template
>   $(1)_stubs.a: $(foreach obj,$$($(1)_C_OBJS),$(obj).o)
>  	$(call mk-caml-stubs,$$@, $$+)
>   lib$(1)_stubs.a: $(foreach obj,$($(1)_C_OBJS),$(obj).o)
> -	$(call mk-caml-lib-stubs,$$@, $$+)
> +	$(call mk-caml-lib-stubs,$$@, $$+, $(foreach lib,$(LIBS_$(1)),$(lib)))
>  endef
>  
>  define OCAML_NOC_LIBRARY_template

Re: [PATCH] tools/ocaml: Fix library generation [and 1 more messages]

[Ian Jackson]

Vincent Bernardoff writes ("[Xen-devel] [PATCH] tools/ocaml: Fix library generation"):
> From: Vincent Bernardoff <vincent.bernardoff@citrix.com>
> 
> Fix the commands given to the OCaml compiler to make the OCaml
> bindings to Xen usable outside the build environment.
> 
> Signed-off-by: Vincent Bernardoff <vincent.bernardoff@citrix.com>

It would have been good to point out somewhere that the changes to
Rules.mk don't just affect the commands given to the ocaml compiler
but to all users of libxc, libxl etc.

And I'm afraid, viewing the changes in that light, this amounts to
reverting b7ee8d2f432f726a1154d172016d3f2b22757fe3 "tools build: link
to specific library version (libxs, libxl, xenstore, xenstat)".
(CCing Roger, author of that commit.)

So I'm afraid this patch is not acceptable in this form.

Is there no way to get the ocaml compiler to link against a specified
library file without embedding the path into the executable ?

Ian.

Re: [PATCH] tools/ocaml: Fix library generation [and 1 more messages]

[Andrew Cooper]

On 09/04/13 15:30, Ian Jackson wrote:
> Vincent Bernardoff writes ("[Xen-devel] [PATCH] tools/ocaml: Fix library generation"):
>> From: Vincent Bernardoff <vincent.bernardoff@citrix.com>
>>
>> Fix the commands given to the OCaml compiler to make the OCaml
>> bindings to Xen usable outside the build environment.
>>
>> Signed-off-by: Vincent Bernardoff <vincent.bernardoff@citrix.com>
> It would have been good to point out somewhere that the changes to
> Rules.mk don't just affect the commands given to the ocaml compiler
> but to all users of libxc, libxl etc.
>
> And I'm afraid, viewing the changes in that light, this amounts to
> reverting b7ee8d2f432f726a1154d172016d3f2b22757fe3 "tools build: link
> to specific library version (libxs, libxl, xenstore, xenstat)".
> (CCing Roger, author of that commit.)
>
> So I'm afraid this patch is not acceptable in this form.
>
> Is there no way to get the ocaml compiler to link against a specified
> library file without embedding the path into the executable ?
>
> Ian.

I was not aware of Rogers commit, but at the very least it break static
linking.

Furthermore,  the library directory paths are specified on the command
line, which take priority over system library paths.  The only way I can
see, given a brief read of the gcc manual, for the wrong versions to be
linked is if there are stale versions of the libraries present in the
build directory.

As far as I am concerned, having had to explicitly fix this up in
XenServer, commit b7ee8d2f432f726a1154d172016d3f2b22757fe3 should be
reverted, unless I am really misunderstanding the point of the patch.

~Andrew

Re: [PATCH] tools/ocaml: Fix library generation [and 1 more messages]

[Ian Jackson]

Andrew Cooper writes ("Re: [Xen-devel] [PATCH] tools/ocaml: Fix library generat> As far as I am concerned, having had to explicitly fix this up in
> XenServer, commit b7ee8d2f432f726a1154d172016d3f2b22757fe3 should be
> reverted, unless I am really misunderstanding the point of the patch.

Going back to when this was done,
  http://lists.xen.org/archives/html/xen-devel/2011-10/msg00322.html
says
  Minor build process fixes, to allow the use of LDFALGS and CFLAGS
  with folders that contain older versions of xen libraries and added
  a specific env variable to point to yajl header and library files.

So AIUI the problem is that you have an LDFLAGS which mentions -L<dir>
where <dir> contains other versions of the Xen libraries, you end up
linking against them.

Ian.

Re: [PATCH] tools/ocaml: Fix library generation [and 1 more messages]

[Ian Campbell]

On Tue, 2013-04-09 at 16:10 +0100, Ian Jackson wrote:
> Andrew Cooper writes ("Re: [Xen-devel] [PATCH] tools/ocaml: Fix library generat> As far as I am concerned, having had to explicitly fix this up in
> > XenServer, commit b7ee8d2f432f726a1154d172016d3f2b22757fe3 should be
> > reverted, unless I am really misunderstanding the point of the patch.
> 
> Going back to when this was done,
>   http://lists.xen.org/archives/html/xen-devel/2011-10/msg00322.html
> says
>   Minor build process fixes, to allow the use of LDFALGS and CFLAGS
>   with folders that contain older versions of xen libraries and added
>   a specific env variable to point to yajl header and library files.
> 
> So AIUI the problem is that you have an LDFLAGS which mentions -L<dir>
> where <dir> contains other versions of the Xen libraries, you end up
> linking against them.

There was also an issue with linking against the libraries in /usr/lib
instead of the fresh ones in the build tree, which IIRC was the real
impetus behind this change.

Ian.

Re: [PATCH] tools/ocaml: Fix library generation [and 1 more messages]

[Ian Campbell]

On Tue, 2013-04-09 at 15:50 +0100, Andrew Cooper wrote:
> 
> I was not aware of Rogers commit, but at the very least it break
> static linking. 

How? The change is only about dynamic linking against libraries in the
current tree.

Ian.

Re: [PATCH] tools/ocaml: Fix library generation [and 1 more messages]

[Andrew Cooper]

On 10/04/13 12:33, Ian Campbell wrote:
> On Tue, 2013-04-09 at 15:50 +0100, Andrew Cooper wrote:
>> I was not aware of Rogers commit, but at the very least it break
>> static linking. 
> How? The change is only about dynamic linking against libraries in the
> current tree.
>
> Ian.
>

This is the issue I reported 6 months ago in

http://lists.xen.org/archives/html/xen-devel/2012-10/msg01166.html

Although back then I was simply trying to make Xen 4.2 to compile in the
XenServer build system, and did not pay too much attention to why the
build had regressed from 4.1

~Andrew

Re: [PATCH] tools/ocaml: Fix library generation [and 1 more messages]

[Ian Campbell]

On Wed, 2013-04-10 at 13:01 +0100, Andrew Cooper wrote:
> On 10/04/13 12:33, Ian Campbell wrote:
> > On Tue, 2013-04-09 at 15:50 +0100, Andrew Cooper wrote:
> >> I was not aware of Rogers commit, but at the very least it break
> >> static linking. 
> > How? The change is only about dynamic linking against libraries in the
> > current tree.
> >
> > Ian.
> >
> 
> This is the issue I reported 6 months ago in
> 
> http://lists.xen.org/archives/html/xen-devel/2012-10/msg01166.html

I replied to this at the time, my last comment in
http://lists.xen.org/archives/html/xen-devel/2012-10/msg01176.html was:
        However taking  a step back I think the underlying bug here is actually
        the use of $(LDLIBS_libxenstore) where we should be using $(LIBXENSTORE)
        when linking and not just as a dependency.

Ian.