[Libguestfs] [PATCH v2] common/mlstdutils: Build a bytecode version of this library.

[Richard W.M. Jones]

On Mon, Jul 24, 2017 at 01:03:00PM +0200, Pino Toscano wrote:
> On Thursday, 13 July 2017 16:38:14 CEST Richard W.M. Jones wrote:
> > Even if ocamlopt is available, always build a bytecode version of
> > ‘common/mlstdutils’.
> > 
> > Furthermore, because this library is pure OCaml, we should not be
> > using ‘ocamlmklib’.  We should use ‘ocaml{c,opt} -a’ instead.  This
> > doesn't make any difference for native code, but for bytecode it was
> > building a broken library.
> > 
> > The original reason for making this change is because the generator is
> > always built as bytecode, and it depended on
> > ‘../common/mlstdutils/guestfs_config.cmo’ and
> > ‘../common/mlstdutils/std_utils.cmo’.  On native code platforms these
> > were not built before the generator and so the generator races to
> > build the .cmi and .cmo files.  Since the generator doesn't have
> > correct dependencies covering the ‘common/mlstdutils’ directory you
> > can get a broken link on fast machines:
> > 
> >   File "../common/mlstdutils/std_utils.ml", line 1:
> >   Error: Corrupted compiled interface
> >   ../common/mlstdutils/guestfs_config.cmi
> >   make[2]: *** [Makefile:1993: ../common/mlstdutils/std_utils.cmo] Error 2
> >   make[2]: *** Waiting for unfinished jobs....
> >   make[2]: Leaving directory '/builddir/build/BUILD/libguestfs-1.37.17/generator'
> > ---
> >  common/mlstdutils/Makefile.am | 11 ++++++++---
> >  generator/Makefile.am         |  9 ++++++---
> >  2 files changed, 14 insertions(+), 6 deletions(-)
> > 
> > diff --git a/common/mlstdutils/Makefile.am b/common/mlstdutils/Makefile.am
> > index 9e0b34d42..67441f769 100644
> > --- a/common/mlstdutils/Makefile.am
> > +++ b/common/mlstdutils/Makefile.am
> > @@ -48,7 +48,9 @@ else
> >  MLSTDUTILS_CMA = mlstdutils.cmxa
> >  endif
> >  
> > -noinst_DATA = $(MLSTDUTILS_CMA)
> > +# Just for this library, we need to build both bytecode and native
> > +# code because the generator always requires the bytecode version.
> > +noinst_DATA = mlstdutils.cma mlstdutils.cmxa
> >  
> >  libmlstdutils_a_SOURCES = dummy.c
> >  libmlstdutils_a_CPPFLAGS = \
> > @@ -79,8 +81,11 @@ endif
> >  
> >  libmlstdutils_a_DEPENDENCIES = $(OBJECTS)
> >  
> > -$(MLSTDUTILS_CMA): $(OBJECTS)
> > -	$(OCAMLFIND) mklib $(OCAMLPACKAGES) $(OBJECTS) -o mlstdutils
> > +mlstdutils.cma: $(BOBJECTS)
> > +	$(OCAMLFIND) ocamlc $(OCAMLPACKAGES) -a $^ -o $@
> > +
> > +mlstdutils.cmxa: $(XOBJECTS)
> > +	$(OCAMLFIND) ocamlopt $(OCAMLPACKAGES) -a $^ -o $@
> >  
> >  # This OCaml module has to be generated by make (configure will put
> >  # unexpanded prefix macro in).
> > diff --git a/generator/Makefile.am b/generator/Makefile.am
> > index 3f54ad51d..d4a21fe97 100644
> > --- a/generator/Makefile.am
> > +++ b/generator/Makefile.am
> > @@ -115,8 +115,6 @@ sources = \
> >  # In build dependency order.
> >  objects = \
> >  	$(OCAML_BYTES_COMPAT_CMO) \
> > -	../common/mlstdutils/guestfs_config.cmo \
> > -	../common/mlstdutils/std_utils.cmo \
> 
> Most probably also $(OCAML_BYTES_COMPAT_CMO) should be removed, since
> it's part of mlstdutils too.  This also means the variable itself can
> go.

I'll try that.

> >  	types.cmo \
> >  	utils.cmo \
> >  	proc_nr.cmo \
> > @@ -180,7 +178,12 @@ noinst_PROGRAM = generator
> >  if HAVE_OCAML
> >  
> >  generator: $(objects)
> > -	$(OCAMLFIND) ocamlc $(OCAMLFLAGS) $(OCAMLPACKAGES) -linkpkg $^ -o $@
> > +#	We need the recursive make here because the generator is
> > +#	rebuilt in ‘make clean’.
> > +	$(MAKE) -C $(top_builddir)/common/mlstdutils mlstdutils.cma
> > +	$(OCAMLFIND) ocamlc $(OCAMLFLAGS) \
> > +	    $(OCAMLPACKAGES) -linkpkg mlstdutils.cma \
> > +	    $^ -o $@
> 
> Hm it is (i.e. rebuilt on clean)?  I don't remember this behaviour, and
> issuing `make clean` in generator just removes files here.

If you do 'make clean' at the top level then the generator gets
rebuilt (because of generator_built dependencies in other
directories).  This could be a bug, but it's not this bug ...

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org