Bug 24242

On Wed, Sep 30, 2009 at 06:12:14PM -0700, bugzilla-daemon@freedesktop.org wrote:
> I think the intent was to deploy them to all components. Here would be the
> place to mention exceptions. Perhaps util/macros itself. The proto which only
> have header files. 

the macro was mostly introduced for those. Right now, changing anything
requires changing all modules, and it's the same check all over.
with that, we can sneak in changes into the macro and everyone gets it.
IMO, the only exception is obviously util/macros (cant it reference itself?)
and those modules where the maintainer disagrees. This may apply to xserver
and some drivers but I doubt even that.
the protos definitely fall into the "need default options" category.

I've been deploying on a case-by-case basis in the apps and other
modules I've been preparing for releases for the 7.5 release cycle.

So far I've hesitated on adding it to modules that don't currently
run any of the macros that need to do the compiler checks, since it
calls them and would cause longer configuration cycles for things 
that don't need it - these would mainly be the docs/*, font/* and proto/*,
but perhaps I'm just being overly cautious there and the benefit of
a single point of configuration overweighs that small cost.

(In reply to comment #2)
> I've been deploying on a case-by-case basis in the apps and other
> modules I've been preparing for releases for the 7.5 release cycle.
> 
> So far I've hesitated on adding it to modules that don't currently
> run any of the macros that need to do the compiler checks, since it
> calls them and would cause longer configuration cycles for things 
> that don't need it - these would mainly be the docs/*, font/* and proto/*,
> but perhaps I'm just being overly cautious there and the benefit of
> a single point of configuration overweighs that small cost.
> 

I think you touched on a limitation of having all defaults in one meta-macro. The more defaults you put it, the less applicable it becomes. I just went through this mental exercise with the .gitignore template.

I tried to categorize the default values, but could only come down with two. I realized afterwards that they match the GNU Autotools domain: autoconf and automake. Or if you prefer, the configuration versus the development activities.

Let's see if the pattern applies to XORG defaults macro. One for configuration aspect of the component which would apply to all components (minus sisters) and one that would apply to components using development tools such as AC_PROG_CC.

XORG_CONF_DEFAULTS could contain (for illustration purpose, not tested):

AC_CONFIG_SRCDIR([Makefile.am])
AM_INIT_AUTOMAKE([dist-bzip2 foreign])
AM_MAINTAINER_MODE
m4_ifndef([XORG_MACROS_VERSION], [AC_FATAL([must install xorg-macros 1.3 or later before running autoconf/autogen])])
XORG_MACROS_VERSION(1.3)
XORG_RELEASE_VERSION
XORG_MANPAGE_SECTIONS
XORG_CHANGELOG

So we would not be running around to add missing foreign or maintainer mode macros.

XORG_DEV_DEFAULTS could contain (for illustration purpose, not tested):
AC_PROG_CC
AC_PROG_CC_C99
XORG_CWARNFLAGS
XORG_STRICT_OPTION

If the concept is sound, I could development further by analyzing the components configuration and determine the rate of applicability.

(In reply to comment #3)
> I think you touched on a limitation of having all defaults in one meta-macro.
> The more defaults you put it, the less applicable it becomes. 

It's a trade-off that's true for any generic solution. At some point, the costs outweigh the benefits and we need to be mindful of that point.

The way I see it is that adding XORG_DEFAULT_OPTIONS will increase build time for some modules but reduce maintainership costs.
Build time is passively spent time, if it increases it's annoying but that's about it. Maintainership OTOH is actively spent time. It wastes someone's time who could be doing something more important, it is prone to human error (forgetting to update a module), etc.

Your suggestion with the DEV and the CONF macros is a good one, but--right now--I don't really see the need for splitting it. I guess I'm worried about trying to over-engineer something that's not a real problem yet. (that's my personal opinion of course).

What I do like about the CONF macro is that they can unify the header, so most modules may just get away with something like XORG_CONFIGURE_AC([modulename], [version]) as the sole line in configure.ac
It's going to be interesting to see how the check for macros works out if you need the macros to check for the macros :)

I was having second thoughts while compiling the numbers:

Out of 261 modules having configure.ac (8 being in sister projects)
AM_AUTOMAKE_INIT	260 OK, mesa does not use Automake
AM_MAINTAINER_MODE	220
AC_CONFIG_SRCDIR	 69

XORG_MACROS_VERSION	216
XORG_CHANGELOG		213
XORG_RELEASE_VERSION	247
XORG_MANPAGE_SECTIONS	158
XORG_CWARNFLAGS		131
XORG_DEFAULT_OPTIONS	  6 

AC_PROG_CC		216
AC_PROG_CC_C99		  0 (directly)
AC_CANONICAL_HOST	  8 (directly)

My main reservation about adding AC/AM macros in the xorg default macros was that it reduce the visibility in the ac file. You don't know what's in there unless you start digging in the macros.This has the side-effect of increasing maintenance time.

It's also true, perhaps to a lesser extend, with the xorg_ macros. It's not obvious that you have warning flags enabled, one might just add them again. Duplicate statements are misleading.

It came to mind that if we have some AC/AM macros, it should not change over time, i.e not a container for other macros. That way it can be commented and would not reduce readability. So your idea about  XORG_CONFIGURE_AC fits perfectly. 

So that would be something like:

AC_PREREQ([2.57])
XORG_CONFIGURE_AC([mkfontscale], [1.0.6])
XORG_DEFAULTS_OPTIONS

AC_PROG_INSTALL
.
.
.
AC_OUTPUT([Makefile])

I left out XORG_MACROS_VERSION...

---------------------------------------------
OR
---------------------------------------------
AC_PREREQ([2.57])
AC_INIT(mkfontscale, [1.0.6], [https:...], mkfontscale)

# Some comment about this group
m4_ifndef([XORG_MACROS_VERSION], [AC_FATAL([must install xorg-macros 1.3 or later before running autoconf/autogen])])
XORG_MACROS_VERSION(1.3)
XORG_CONFIGURE_AC
XORG_DEFAULTS_OPTIONS
# end of group

AC_PROG_INSTALL
.
.
.
AC_OUTPUT([Makefile])

(In reply to comment #2)
> I've been deploying on a case-by-case basis in the apps and other
> modules I've been preparing for releases for the 7.5 release cycle.
> 
> So far I've hesitated on adding it to modules that don't currently
> run any of the macros that need to do the compiler checks, since it
> calls them and would cause longer configuration cycles for things 
> that don't need it - these would mainly be the docs/*, font/* and proto/*,
> but perhaps I'm just being overly cautious there and the benefit of
> a single point of configuration overweighs that small cost.
> 

Some numbers may help. Performing initial configuration takes 6 secs without xorg_default_options and with it, it takes 13 secs. It performs about 30 more checks associated to the compiler.

The strange thing is that running 'autoscan', which creates a starter configure.ac file, writes the AC_PROG_CC statement, probably because of the presence of header files. 

With AC_PROG_CC:
...
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for ANSI C header files... yes
checking for sys/types.h... yes
checking for sys/stat.h... yes
...
It may be worthed to make these checks given all the target platforms.

I am proceeding one component at a time, so we have time to think through the questions as they arise.

Summary of changes
------------------

Patches have been created locally (not released yet) such that all 251 xorg/* modules contain the XORG_DEFAULT_OPTIONS. It now contains XORG_INSTALL which copies the standard INSTALL file. 

In practice, it is difficult not to use this macro. It will be easier to remove it if an issue arise in some modules.

All patches applied.