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

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 >