[Spice-devel] [PATCH v2 12/43] Generate some definition for dissector

[Christophe Fergeau]

On Tue, Jul 21, 2015 at 11:54:06AM -0400, Frediano Ziglio wrote:
> > From: "Christophe Fergeau" <cfergeau at redhat.com>
> > This test.o dep (and the similar one in another commit) is odd. Missing
> > BUILT_SOURCES?
> > 
> 
> No, it's just manual, see
> http://www.delorie.com/gnu/docs/automake/automake_69.html and
> http://www.delorie.com/gnu/docs/automake/automake_70.html, basically
> not all cases are covered by automatic generated dependency.

Stuff like "Adding explicit dependencies like this can be a bit
dangerous if you are not careful enough." "Always check the generated
`Makefile.in' if you do this." would make me strongly favour use of
BUILT_SOURCES, especially as "make" in the codegen directory will only
generate the needed sources (make check will then build the code).

> 
> > > +
> > > +test.c: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
> > > +	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-dissector
> > > --client $< $@ >/dev/null
> > > +
> > > +test.h: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
> > > +	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-dissector
> > > --client $< --header $@ >/dev/null
> > > +
> > > +TESTS = dissector_test
> > > +check_PROGRAMS = dissector_test
> > > +
> > > +dissector_test_SOURCES = dissector_test.c test.c test.h
> > > +
> > > +EXTRA_DIST =				\
> > > +	$(NULL)
> > > +
> > > +-include $(top_srcdir)/git.mk
> > 
> > 
> > > diff --git a/configure.ac b/configure.ac
> > > index 4287f92..a156cae 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -38,6 +38,7 @@ SPICE_CHECK_PIXMAN(SPICE_COMMON)
> > >  SPICE_CHECK_SMARTCARD(SPICE_COMMON)
> > >  SPICE_CHECK_CELT051(SPICE_COMMON)
> > >  SPICE_CHECK_GLIB2(SPICE_COMMON)
> > > +PKG_CHECK_MODULES(WIRESHARK, wireshark)
> > 
> > This should be optional.
> > 
> 
> Oh... is it not in this way? I though so.

Nope, this errors out if wireshark is not found (easy to test by
changing 'wireshark' to something random). If you want a
--enable-dissector argument to configure, you can take a look at
m4/spice-deps.m4 for an example of how to handle it (defaults to
auto-detection, errors out if --enable-dissector is passed but wireshark
is not found, always disables it if --disable-dissector is passed).
Here, I think just doing
PKG_CHECK_MODULES(WIRESHARK, [have_wireshark=yes], [have_wireshark=no])
AM_CONDITIONAL([HAVE_WIRESHARK], [test "x$have_wireshark" = "xyes"])
should be enough (and then add the appropriate ifdef to the Makefile.am)

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150722/321f0d97/attachment.sig>