[PATCH 2/2] libXft: remove AC_PROG_CC as it overrides AC_PROG_C_C99

[Gaetan Nadon]

On Wed, 2011-01-19 at 13:53 -0800, Dan Nicholson wrote:

> On Wed, Jan 19, 2011 at 1:25 PM, Gaetan Nadon <memsize at videotron.ca> wrote:
> > On Wed, 2011-01-19 at 12:51 -0800, Dan Nicholson wrote:
> >
> > This seems like more of a problem with the macros. I don't know why
> > XORG_STRICT_OPTION doesn't just AC_REQUIRE AC_PROG_CC so that we don't
> > get these kinds of issues.
> >
> > I have not found a way to avoid these issues. The last call wins.
> > If C99 is invoked before AC_REQUIRE C89, C89 will win simply because
> > it is not C99. They don't know about each other, even if some of the work
> > they
> > do is cached and reused by the other.
> >
> > However, for this particular case I think
> > it would be better if we just moved the XORG_DEFAULT_OPTIONS call
> > below AC_PROG_CC.
> >
> > The statements become order sensitive, which is hard to maintain, even
> > when commented. The day will come where there will be a dead lock.
> >
> > If we comment or remove AC_PROG_CC, then it becomes
> > really unclear that we're getting it implicitly through
> > XORG_DEFAULT_OPTIONS.
> >
> > This is also true for any macro that is being invoked from
> > XORG_DEFAULT_OPTIONS.
> >
> > The XORG_STRICT_OPTION was intended to "enforce" C99 by virtue of inclusion
> > in configure.ac. It ceased to be "optional" once it got included in
> > XORG_DEFAULT_OPTIONS.
> >
> > Other comments from reviewers so far indicate there is no objections to C99,
> > with some pros and cons.
> >
> > The reason why AC_PROG_CC are removed is for the module to be compiled C99.
> >
> > If we find something better to do, it should be applied to all 240 modules,
> > unless there
> > is a reason to configure the modules with different compiler levels.
> 
> And if we later remove XORG_STRICT_OPTION from XORG_DEFAULT_OPTIONS
> (which isn't a crazy suggestion), then there is nothing calling
> AC_PROG_CC*. That's why I was suggesting to just move the
> XORG_DEFAULT_OPTIONS call later so that the original call is still
> there and it's not some implicit thing buried in another macro.

Removing XORG_STRICT_OPTION will never pass review as it would break
backward
compatibility. In fact, we can only add or fix bugs.

Suppose we switch order, no harm done. C99 will prevail. It will be very
confusing.
Reading configure.ac one will thing C89 is in effect, not knowing C99 is
called
somewhere buried in a macro.

> 
> > To find out which version it is compiled with:
> >
> > grep "^CC=" `find . -name config.log -type f -print`
> >
> > For a C99:
> >
> > CC='gcc -std=gnu99'
> 
> We want to try to ensure that we get C99 if XORG_STRICT_OPTION is
> used. 

The intention of the author was to always use C99 and this was
accomplished
by moving XORG_STRICT_OPTION in XORG_DEFAULT_OPTIONS. 
The only optional part is the usage of -pedantic -Werror flags.


> So, if we AC_REQUIRE([AC_PROG_CC]) before
> AC_REQUIRE([AC_PROG_CC_C99]) in XORG_STRICT_OPTION, then it should
> ensure the ordering, right? Or can you just keep calling them and the
> last one wins? I haven't checked.
> 

The last one wins. 

If one call is in configure.ac and the other call is buried, it is
confusing to the reader.
If neither is called in configure.ac, it's like any other statement that
is buried in util-macros.

I guess it's like any reusable code you can't see from the calling
program. You don't know what
it does until you dig in.

There are pros and cons. The module developer can concentrate on the
configuration that is unique
to the module and not have to deal with dozens of details that are the
same in all modules.

Some modules print a summary of the important configuration aspects of
the module at the
end of the configuration. That would be a nice feature.



> --
> Dan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.x.org/archives/xorg-devel/attachments/20110119/b45a7a50/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
URL: <http://lists.x.org/archives/xorg-devel/attachments/20110119/b45a7a50/attachment.pgp>