[cairo] Conditionally include debug functions with ENABLE_DEBUG_FUNCS

[Carl Worth]

On Thu, 18 Oct 2007 23:13:51 +0200, "=?ISO-8859-1?Q?BJ=F6rn_Lindqvist?=" wrote:
>
> Here is a patch that introduces a ENABLE_DEBUG_FUNCS macro define. Its
> purpose is to allow for debug stuff to be conditionally included in
> Cairo proper. Things that must be exposed for testing to work, but
> that should not be included in the final and really public API.

Hi Björn,

Thanks for the patch!

Could you please put a CAIRO_ prefix into the macro name? And also,
I'm a little bit concerned about the name "enable debug funcs" since
it suggests that a function like cairo_debug_reset_stat_data would not
exist unless this option were set, (which is not the case). Does
anybody have an alternate naming suggestion for the new macro?

Similarly, the cairo_xlib_surface_disable_render function should be
named such that it is clearly associated with the new option.

>  	make check
>  
> +   This requires that Cairo is configured with --enable-debug-funcs.
> +

Can't we make the test suite work unconditionally, but perhaps with
less coverage in the case of the debugging function not existing? That
is, just sprinkle some "#if CAIRO_ENABLE_DEBUG_FUNCS" throughout
cairo-boilerplate as necessary?

Though it might not hurt to have a warning message when running "make
check" in that case that its testing won't be complete as otherwise.

If you don't go this route, I'd at least like to see a kind warning
from "make check" explaining how to build cairo properly for testing,
(as opposed to just an obscure build failure due to an undefined
symbol as I think will result with the current patch).

> +if test "x$enable_debug_funcs" = "xyes"; then
> +  AC_DEFINE(ENABLE_DEBUG_FUNCS, 1, [include diagnostic functions needed by the
> +                                    performance and test suites])
> +fi

This warning message gives some naming hints. I don't think
"diagnostic" is accurate and it should just be dropped. But the fact
that the functions are needed only by the test suites is quite
accurate. So maybe that's the direction we want for naming?

Maybe something like this?

	--enable-test-functions

	CAIRO_HAS_TEST_FUNCTIONS

	cairo_xlib_surface_test_disable_render

And actually, the existing functionality of --enable-test-surfaces
should probably be combined as well. There's nothing useful that I can
see in keeping these separate.

So maybe it's --enable-test-build ? CAIRO_IS_TEST_BUILD?

Something like that anyway.

-Carl
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.cairographics.org/archives/cairo/attachments/20071018/8bcc13f6/attachment.pgp