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>
---
Changed since v1:
* tools/Rules.mk is not modified, changes are now in
bottom-level Makefiles of OCaml libraries
---
tools/ocaml/Makefile.rules | 6 +++---
tools/ocaml/libs/eventchn/Makefile | 2 +-
tools/ocaml/libs/xc/Makefile | 2 +-
tools/ocaml/libs/xl/Makefile | 2 +-
4 files changed, 6 insertions(+), 6 deletions(-)
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
diff --git a/tools/ocaml/libs/eventchn/Makefile
b/tools/ocaml/libs/eventchn/Makefile
index 2d8d618..ddd2ace 100644
--- a/tools/ocaml/libs/eventchn/Makefile
+++ b/tools/ocaml/libs/eventchn/Makefile
@@ -8,7 +8,7 @@ OBJS = xeneventchn
INTF = $(foreach obj, $(OBJS),$(obj).cmi)
LIBS = xeneventchn.cma xeneventchn.cmxa
-LIBS_xeneventchn = $(LDLIBS_libxenctrl)
+LIBS_xeneventchn = -L$(XEN_LIBXC) -lxenctrl
all: $(INTF) $(LIBS) $(PROGRAMS)
diff --git a/tools/ocaml/libs/xc/Makefile b/tools/ocaml/libs/xc/Makefile
index 239c187..2e55849 100644
--- a/tools/ocaml/libs/xc/Makefile
+++ b/tools/ocaml/libs/xc/Makefile
@@ -9,7 +9,7 @@ OBJS = xenctrl
INTF = xenctrl.cmi
LIBS = xenctrl.cma xenctrl.cmxa
-LIBS_xenctrl = $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest)
+LIBS_xenctrl = -L$(XEN_LIBXC) -lxenctrl -lxenguest
xenctrl_OBJS = $(OBJS)
xenctrl_C_OBJS = xenctrl_stubs
diff --git a/tools/ocaml/libs/xl/Makefile b/tools/ocaml/libs/xl/Makefile
index c9e5274..897921c 100644
--- a/tools/ocaml/libs/xl/Makefile
+++ b/tools/ocaml/libs/xl/Makefile
@@ -10,7 +10,7 @@ OBJS = xenlight
INTF = xenlight.cmi
LIBS = xenlight.cma xenlight.cmxa
-LIBS_xenlight = $(LDLIBS_libxenlight)
+LIBS_xenlight = -L$(XEN_XENLIGHT) -lxenlight
xenlight_OBJS = $(OBJS)
xenlight_C_OBJS = xenlight_stubs
--
1.7.10.4
On Mon, 2013-04-15 at 11:09 +0100, Vincent Bernardoff wrote:> 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> > > --- > Changed since v1: > * tools/Rules.mk is not modified, changes are now in > bottom-level Makefiles of OCaml librariesHow does this relate to the patch which Andy Cooper posted in <fe2d14f39de68ab01ac2.1365012176@andrewcoop.uk.xensource.com> ?> diff --git a/tools/ocaml/libs/eventchn/Makefile b/tools/ocaml/libs/eventchn/Makefile > index 2d8d618..ddd2ace 100644 > --- a/tools/ocaml/libs/eventchn/Makefile > +++ b/tools/ocaml/libs/eventchn/Makefile > @@ -8,7 +8,7 @@ OBJS = xeneventchn > INTF = $(foreach obj, $(OBJS),$(obj).cmi) > LIBS = xeneventchn.cma xeneventchn.cmxa > > -LIBS_xeneventchn = $(LDLIBS_libxenctrl) > +LIBS_xeneventchn = -L$(XEN_LIBXC) -lxenctrlThe problem with this is that it seems to reintroduce a form of the problem solved by b7ee8d2f432f, that is accidental linking against libraries in /usr/lib (or elsewhere) instead of the freshly built ones in the source tree. Andy''s version of the patch seems to have solved that issue, I was just hoping for a brief explanation of how (per <1365607338.27868.87.camel@zakaz.uk.xensource.com>) before I through it in the tree. Ian.
On 15/04/13 11:21, Ian Campbell wrote:> On Mon, 2013-04-15 at 11:09 +0100, Vincent Bernardoff wrote: >> 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> >> >> --- >> Changed since v1: >> * tools/Rules.mk is not modified, changes are now in >> bottom-level Makefiles of OCaml libraries > How does this relate to the patch which Andy Cooper posted in > <fe2d14f39de68ab01ac2.1365012176@andrewcoop.uk.xensource.com> ?As stated somewhere on one of the two threads, this patch from Vincent supersedes mine.> >> diff --git a/tools/ocaml/libs/eventchn/Makefile b/tools/ocaml/libs/eventchn/Makefile >> index 2d8d618..ddd2ace 100644 >> --- a/tools/ocaml/libs/eventchn/Makefile >> +++ b/tools/ocaml/libs/eventchn/Makefile >> @@ -8,7 +8,7 @@ OBJS = xeneventchn >> INTF = $(foreach obj, $(OBJS),$(obj).cmi) >> LIBS = xeneventchn.cma xeneventchn.cmxa >> >> -LIBS_xeneventchn = $(LDLIBS_libxenctrl) >> +LIBS_xeneventchn = -L$(XEN_LIBXC) -lxenctrl > The problem with this is that it seems to reintroduce a form of the > problem solved by b7ee8d2f432f, that is accidental linking against > libraries in /usr/lib (or elsewhere) instead of the freshly built ones > in the source tree. > > Andy''s version of the patch seems to have solved that issue, I was just > hoping for a brief explanation of how (per > <1365607338.27868.87.camel@zakaz.uk.xensource.com>) before I through it > in the tree. > > Ian. >"My patch" was simply an upstreaming of JonL''s patch to make XCP build against Debian. I have no particular knowledge of the Ocaml build gubbins. It unfortunatly did not fix the problem in general. ~Andrew
On Mon, 2013-04-15 at 11:27 +0100, Andrew Cooper wrote:> On 15/04/13 11:21, Ian Campbell wrote: > > On Mon, 2013-04-15 at 11:09 +0100, Vincent Bernardoff wrote: > >> 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> > >> > >> --- > >> Changed since v1: > >> * tools/Rules.mk is not modified, changes are now in > >> bottom-level Makefiles of OCaml libraries > > How does this relate to the patch which Andy Cooper posted in > > <fe2d14f39de68ab01ac2.1365012176@andrewcoop.uk.xensource.com> ? > > As stated somewhere on one of the two threads, this patch from Vincent > supersedes mine.Ah, I''d missed one of the threads (despite replying to it at one point!)> > > >> diff --git a/tools/ocaml/libs/eventchn/Makefile b/tools/ocaml/libs/eventchn/Makefile > >> index 2d8d618..ddd2ace 100644 > >> --- a/tools/ocaml/libs/eventchn/Makefile > >> +++ b/tools/ocaml/libs/eventchn/Makefile > >> @@ -8,7 +8,7 @@ OBJS = xeneventchn > >> INTF = $(foreach obj, $(OBJS),$(obj).cmi) > >> LIBS = xeneventchn.cma xeneventchn.cmxa > >> > >> -LIBS_xeneventchn = $(LDLIBS_libxenctrl) > >> +LIBS_xeneventchn = -L$(XEN_LIBXC) -lxenctrl > > The problem with this is that it seems to reintroduce a form of the > > problem solved by b7ee8d2f432f, that is accidental linking against > > libraries in /usr/lib (or elsewhere) instead of the freshly built ones > > in the source tree. > > > > Andy''s version of the patch seems to have solved that issue, I was just > > hoping for a brief explanation of how (per > > <1365607338.27868.87.camel@zakaz.uk.xensource.com>) before I through it > > in the tree. > > > > Ian. > > > > "My patch" was simply an upstreaming of JonL''s patch to make XCP build > against Debian. I have no particular knowledge of the Ocaml build > gubbins. It unfortunatly did not fix the problem in general.Adding some CCs, please can we try and retain these for any subsequent threads/postings of this patch. So what is the correct fix? How bad are the shortcomings of your proposed fix? What we need to avoid is either linking a new set of bindings against stale libraries present on the system outside of the build directory or the bindings subsequently linking against libraries other than the ones they were built/linked against. Doing either is likely to result in crashes for the user. This suggests that whatever is baked into the ocaml module needs to include at least a sufficient portion of the SONAME to ensure that the library which gets used is actually compatible with the one the bindings were build against. This requires that we don''t embed "-lfoo" into the module since that translates to libfoo.so not libfoo.so.VERSION (or whichever scheme is used). If it isn''t possible to separate out the command for linking the module from the one for using it then some hacks might be needed. Ian.
On 15/04/13 12:04, Ian Campbell wrote:> On Mon, 2013-04-15 at 11:27 +0100, Andrew Cooper wrote: >> On 15/04/13 11:21, Ian Campbell wrote: >>> On Mon, 2013-04-15 at 11:09 +0100, Vincent Bernardoff wrote: >>>> 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> >>>> >>>> --- >>>> Changed since v1: >>>> * tools/Rules.mk is not modified, changes are now in >>>> bottom-level Makefiles of OCaml libraries >>> How does this relate to the patch which Andy Cooper posted in >>> <fe2d14f39de68ab01ac2.1365012176@andrewcoop.uk.xensource.com> ? >> As stated somewhere on one of the two threads, this patch from Vincent >> supersedes mine. > Ah, I''d missed one of the threads (despite replying to it at one point!) > >>>> diff --git a/tools/ocaml/libs/eventchn/Makefile b/tools/ocaml/libs/eventchn/Makefile >>>> index 2d8d618..ddd2ace 100644 >>>> --- a/tools/ocaml/libs/eventchn/Makefile >>>> +++ b/tools/ocaml/libs/eventchn/Makefile >>>> @@ -8,7 +8,7 @@ OBJS = xeneventchn >>>> INTF = $(foreach obj, $(OBJS),$(obj).cmi) >>>> LIBS = xeneventchn.cma xeneventchn.cmxa >>>> >>>> -LIBS_xeneventchn = $(LDLIBS_libxenctrl) >>>> +LIBS_xeneventchn = -L$(XEN_LIBXC) -lxenctrl >>> The problem with this is that it seems to reintroduce a form of the >>> problem solved by b7ee8d2f432f, that is accidental linking against >>> libraries in /usr/lib (or elsewhere) instead of the freshly built ones >>> in the source tree. >>> >>> Andy''s version of the patch seems to have solved that issue, I was just >>> hoping for a brief explanation of how (per >>> <1365607338.27868.87.camel@zakaz.uk.xensource.com>) before I through it >>> in the tree. >>> >>> Ian. >>> >> "My patch" was simply an upstreaming of JonL''s patch to make XCP build >> against Debian. I have no particular knowledge of the Ocaml build >> gubbins. It unfortunatly did not fix the problem in general. > Adding some CCs, please can we try and retain these for any subsequent > threads/postings of this patch. > > So what is the correct fix? How bad are the shortcomings of your > proposed fix?The shortcomings of the patch sent by me is that it completely breaks the bytecode libraries. As XCP was only needing to get the native compile working, that''s all that got accounted for.> > What we need to avoid is either linking a new set of bindings against > stale libraries present on the system outside of the build directory or > the bindings subsequently linking against libraries other than the ones > they were built/linked against. Doing either is likely to result in > crashes for the user. > > This suggests that whatever is baked into the ocaml module needs to > include at least a sufficient portion of the SONAME to ensure that the > library which gets used is actually compatible with the one the bindings > were build against. This requires that we don''t embed "-lfoo" into the > module since that translates to libfoo.so not libfoo.so.VERSION (or > whichever scheme is used). > > If it isn''t possible to separate out the command for linking the module > from the one for using it then some hacks might be needed. > > Ian. >To the best of my understanding: Ocaml .cma/.cmxa libraries can ether be loaded at runtime, or subsequently linked against later to create complete programs. To facilitate that, gcc command line options are embedded in the cma/cmxa so the correct .so''s can be found. The problem we have is that oxenstored is complied and linked completely to an executable in the build system, which means it needs the build system .so locations, while at the same time, the cma/cmxa''s need to have the runtime system library locations so they can be included/linked against later. ~Andrew
On Mon, 2013-04-15 at 13:19 +0100, Andrew Cooper wrote:> To the best of my understanding: > > Ocaml .cma/.cmxa libraries can ether be loaded at runtime, or > subsequently linked against later to create complete programs. > > To facilitate that, gcc command line options are embedded in the > cma/cmxa so the correct .so''s can be found.This matches my understanding. One thing which isn''t clear to me is if the command line options which are embedded are also used as part of building the module itself or if they are only used when the module is linked against.> The problem we have is that oxenstored is complied and linked completely > to an executable in the build system, which means it needs the build > system .so locations, while at the same time, the cma/cmxa''s need to > have the runtime system library locations so they can be included/linked > against later.Right, and in this latter case when linking against the ocaml module we needs to be sure we are picking up a version of the library which is consistent with what the module was built against (including headers etc). i.e. we need the right ABI. This means that -lfoo and /path/to/libfoo.so are both equally wrong, it needs to be the thing which libfoo.so links to, or the intermediate "MAJOR" version SONAME variant iff that declares sufficient ABI compatibility. e.g. libxenctrl.so -> libxenctrl.so.4.2 libxenctrl.so.4.2 -> libxenctrl.so.4.2.0 libxenctrl.so.4.2.0 So specifying libxenctrl.so is wrong (as is -lxenctrl, which is ~equivalent). Either or libxenctrl.so.4.2 or libxenctrl.so.4.2.0 would be OK. A C application ends up with a NEEDED value of libxenctrl.so.4.2 so that''s probably the one to use here too. How do the dllFOO_stubs.so libraries: $ find tools/ocaml/ -name dll\*stub\*so tools/ocaml/libs/xb/dllxenbus_stubs.so tools/ocaml/libs/eventchn/dllxeneventchn_stubs.so tools/ocaml/libs/xl/dllxenlight_stubs.so tools/ocaml/libs/xc/dllxenctrl_stubs.so tools/ocaml/libs/mmap/dllxenmmap_stubs.so tools/ocaml/xenstored/dllsyslog_stubs.so fit in? They do not appear to have a suitable ELF NEEDED entry for the library which they are stubbing for -- could that be the actual root cause of this issue? I''m not entirely sure how those are built or used but given that dllxenctrl_stubs.so uses symbols from libxenctrl.so I''d have said it ought to have a NEEDED libxenctrl.so.4.2 in it. However I''m not sure how these stub libraries get used, oxenstored appears to dynamically link against libxenctrls.so.4.2 and not the dllxenctrl_stubs.so -- so I guess it gets embedded at link time? So I''m not at all sure if a NEEDED header in dllxentrl_stubs.so would get propagated to the final application binary (arguably it should). Ian.