[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH 3/4] ocaml: Use automake to build the C part of the bindings.



On Wed, Jan 18, 2012 at 05:15:16PM +0100, Jim Meyering wrote:
> Richard W.M. Jones wrote:
> > By arranging the C part of the bindings into a library, we can get
> > automake to build it instead of using $(CC) directly.
> 
> > diff --git a/ocaml/Makefile.am b/ocaml/Makefile.am
> ...
> > -AM_CPPFLAGS = -I$(top_builddir) -I$(OCAMLLIB) -I$(top_srcdir)/ocaml \
> > -  -I$(top_srcdir)/src -I$(top_builddir)/src \
> > -  $(WARN_CFLAGS) $(WERROR_CFLAGS)
> > -
> >  if HAVE_OCAML
> >
> >  OCAMLCFLAGS = -g -warn-error CDEFLMPSUVYZX
> > @@ -46,20 +42,26 @@ OCAMLOPTFLAGS = $(OCAMLCFLAGS)
> >
> >  noinst_DATA = mlguestfs.cma mlguestfs.cmxa META
> >
> > -OBJS = guestfs_c.o guestfs_c_actions.o guestfs.cmo
> > +# Build the C part into a library, so that automake handles the C
> > +# compilation step for us.  Note that we don't directly use this
> > +# library; we link with the object files that it generates.
> > +noinst_LIBRARIES = libguestfsocaml.a
> > +
> > +OBJS = libguestfsocaml.a guestfs.cmo
> >  XOBJS = $(OBJS:.cmo=.cmx)
> >
> >  mlguestfs.cma: $(OBJS)
> > -	$(OCAMLMKLIB) -o mlguestfs $^ -L$(top_builddir)/src/.libs -lguestfs
> > +	$(OCAMLMKLIB) -o mlguestfs libguestfsocaml_a-guestfs_c_actions.o libguestfsocaml_a-guestfs_c.o guestfs.cmo -L$(top_builddir)/src/.libs -lguestfs
> 
> Nice clean-up/simplification.
> Personally I'd find it easier to read/review
> if you split the two new lines that are longer than 80 columns.
> 
> 	$(OCAMLMKLIB) -o mlguestfs \
>           libguestfsocaml_a-guestfs_c_actions.o \
>           libguestfsocaml_a-guestfs_c.o \
>           guestfs.cmo -L$(top_builddir)/src/.libs -lguestfs
> 
> That also helps to see the common "libguestfsocaml_a-" prefix,
> which if there were 3 or more I'd suggest to factor it out and use
> GNU make's $(addprefix ...), assuming you already depend on
> GNU make.  But, seeing the $< in other lines, maybe you do already.
> But see below...
> 
> >  mlguestfs.cmxa: $(XOBJS)
> > -	$(OCAMLMKLIB) -o mlguestfs $^ -L$(top_builddir)/src/.libs -lguestfs
> > -
> > -guestfs_c.o: guestfs_c.c
> > -	$(CC) $(AM_CPPFLAGS) $(CFLAGS) -fPIC -Wall -c $<
> > -
> > -guestfs_c_actions.o: guestfs_c_actions.c
> > -	$(CC) $(AM_CPPFLAGS) $(CFLAGS) -fPIC -Wall -c $(srcdir)/$<
> > +	$(OCAMLMKLIB) -o mlguestfs libguestfsocaml_a-guestfs_c_actions.o libguestfsocaml_a-guestfs_c.o guestfs.cmx -L$(top_builddir)/src/.libs -lguestfs
> 
> Oh.  Actually, seeing this pair of long-named .o files again,
> and knowing that they are the components of an automake library,
> I realized that there should be an automake-provided variable for
> the pair of them, poked the generated Makefile and found this:
> 
> am_libguestfsocaml_a_OBJECTS =  \
> 	libguestfsocaml_a-guestfs_c.$(OBJEXT) \
> 	libguestfsocaml_a-guestfs_c_actions.$(OBJEXT)
> 
> So you can simplify further by using $(am_libguestfsocaml_a_OBJECTS)
> in both of those rules.
> 
> > +libguestfsocaml_a_CFLAGS = \
> > +	-I$(top_builddir) -I$(OCAMLLIB) -I$(top_srcdir)/ocaml \
> > +	-I$(top_srcdir)/src -I$(top_builddir)/src \
> > +	$(WARN_CFLAGS) $(WERROR_CFLAGS) \
> > +	-fPIC
> > +libguestfsocaml_a_SOURCES = guestfs_c.c guestfs_c_actions.c
> >
> >  if HAVE_OCAMLDOC
> >

Thanks, I will make the changes as suggested.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]