[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.



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
>


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