PATCH app,driver,libs 0/43 - Replace deprecated Automake INCLUDES variable

by Gaetan Nadonon 2010-03-19T18:42:17+00:00

Rather than posting 43 patches, I'll summarize the changes and include
the diffs here.
Automake produces a warning about the INCLUDES variable being
deprecated. There were only 43 out of 240 modules still using it. It has
been replaced with AM-CPPFLAGS. In some modules, both INCLUDES and
AM-CPPFLAGS were used.
Further more, there isn't a good separation between pre-processor and
compiler flags. The lint tool only accepts pre-processor flags, so the
lint target had to be reworked. Separation is also required to allow
user to override with CFLAGS and CPPFLAGS on the configure invocation.
In addition of replacing/merging INCLUDES and AM-CPPFLAGS, compiler and
pre-processor flags have been regrouped in AM-CFLAGS and AM-CPPFLAGS
respectively. The order of -I has been preserved the INCLUDES variable
taking precedence over AM-CPPFLAGS. Attention has been given so as not
alter the final gcc command emitted. One exception is that Automake does
not pass pre-processor flags when linking a library which were
previously lumped together with C flags.
For apps and drivers, a couple of patches cover all cases:
**************************** app-bdftopcf *************
~/xorg/src ~/xorg/src
diff
For lib patches that are different than the one above.
This one has per-target flags mixed-in.
**************************** lib-libXvMC *************
~/xorg/src ~/xorg/src
diff
Another example:
**************************** lib-libXt *************
~/xorg/src ~/xorg/src
diff





Rather than posting 43 patches, I'll summarize the changes and include the diffs here.
Automake produces a warning about the INCLUDES variable being deprecated. There were only 43 out of 240 modules still using it. It has been replaced with AM-CPPFLAGS. In some modules, both INCLUDES and AM-CPPFLAGS were used.
Further more, there isn't a good separation between pre-processor and compiler flags. The lint tool only accepts pre-processor flags, so the lint target had to be reworked. Separation is also required to allow user to override with CFLAGS and CPPFLAGS on the configure invocation.
In addition of replacing/merging INCLUDES and AM-CPPFLAGS, compiler and pre-processor flags have been regrouped in AM-CFLAGS and AM-CPPFLAGS respectively. The order of -I has been preserved the INCLUDES variable taking precedence over AM-CPPFLAGS. Attention has been given so as not alter the final gcc command emitted. One exception is that Automake does not pass pre-processor flags when linking a library which were previously lumped together with C flags.
For apps and drivers, a couple of patches cover all cases:
****************************   app-bdftopcf   *************
~/xorg/src ~/xorg/src
diff ****************************   app-iceauth   *************
~/xorg/src ~/xorg/src
diff
For lib patches that are different than the one above.
This one has per-target flags mixed-in.
****************************   lib-libXvMC   *************
~/xorg/src ~/xorg/src
diff
Another example:
****************************   lib-libXt   *************
~/xorg/src ~/xorg/src
diff



On Fri, Mar 19, 2010 at 11:42 AM, Gaetan Nadon
I'm not sure that putting the package CFLAGS into AM-CPPFLAGS won't
cause problems. While it's very likely that $(PKG-CFLAGS) only
contains preprocessor directives, we don't know that and have no way
of enforcing it. I think it would be better to take the safe route and
put them in AM-CFLAGS even if it means -I include paths will be passed
when linking.

It's been a real pain to decide. Either way there are potential
problems. Tools like lint don't like compiler flags, but you can
workaround that. I have seen some projects making patches essentially
for two reasons. One is that some compilers will break, which does not
seem to be our case, and that users can't override correctly with CFLAGS
and CPPFLAGS, which may be for the same first reason.
The server sticks -fvisibility compiler flag in the pkg Cflags which
does not help while 99% of pkg-config files on my distro are compiler
flags free. I don't see an elegant solution to this. There are a few
modules where some improvement can be made, I'lll submit those
individually.
Thanks




On Fri, Mar 19, 2010 at 11:42 AM, Gaetan Nadon
>
> Rather than posting 43 patches, I'll summarize the changes and include the
> diffs here.
>
> Automake produces a warning about the INCLUDES variable being deprecated.
> There were only 43 out of 240 modules still using it. It has been replaced
> with AM-CPPFLAGS. In some modules, both INCLUDES and AM-CPPFLAGS were used.
>
> Further more, there isn't a good separation between pre-processor and
> compiler flags. The lint tool only accepts pre-processor flags, so the lint
> target had to be reworked. Separation is also required to allow user to
> override with CFLAGS and CPPFLAGS on the configure invocation.
>
> In addition of replacing/merging INCLUDES and AM-CPPFLAGS, compiler and
> pre-processor flags have been regrouped in AM-CFLAGS and AM-CPPFLAGS
> respectively. The order of -I has been preserved the INCLUDES variable
> taking precedence over AM-CPPFLAGS. Attention has been given so as not alter
> the final gcc command emitted. One exception is that Automake does not pass
> pre-processor flags when linking a library which were previously lumped
> together with C flags.
I'm not sure that putting the package CFLAGS into AM-CPPFLAGS won't
cause problems. While it's very likely that $(PKG-CFLAGS) only
contains preprocessor directives, we don't know that and have no way
of enforcing it. I think it would be better to take the safe route and
put them in AM-CFLAGS even if it means -I include paths will be passed
when linking.
It's been a real pain to decide. Either way there are potential problems. Tools like lint don't like compiler flags, but you can workaround that. I have seen some projects making patches essentially for two reasons. One is that some compilers will break, which does not seem to be our case, and that users can't override correctly with CFLAGS and CPPFLAGS, which may be for the same first reason.
The server sticks -fvisibility compiler flag in the pkg Cflags which does not help while 99% of pkg-config files on my distro are compiler flags free. I don't see an elegant solution to this. There are a few modules where some improvement can be made, I'lll submit those individually.
Thanks

Gaetan Nadon wrote:
Which is probably a bug in itself - I end up removing @symbol-visibility@
from xorg-server.pc.in when building our packages, since we support both
gcc & Sun Studio, and they take different flags for that option.

I tend to agree, providing compiler flags such as this one is pushing
the "convenience" envelop. This flag should be specified in the driver
module, but it is significant work. There is no facility to specify a
portable compiler flag. In this particular case, it could be supplied
through a server macro and included in each driver makefile.
If we are willing to make the assumption that no other modules will pass
a compiler flag in pkgconfig, then we can have pre-processor/compiler
flags separation in apps and libs.
As a side note, it looks like the server config handles the SUN
compilers.
AC-CHECK-DECL([



> The server sticks -fvisibility compiler flag in the pkg Cflags which
> does not help while 99% of pkg-config files on my distro are compiler
> flags free.
Which is probably a bug in itself - I end up removing @symbol-visibility@
from xorg-server.pc.in when building our packages, since we support both
gcc & Sun Studio, and they take different flags for that option.

I tend to agree, providing compiler flags such as this one is pushing the "convenience" envelop. This flag should be specified in the driver module, but it is significant work. There is no facility to specify a portable compiler flag. In this particular case, it could be supplied through a server macro and included in each driver makefile.
If we are willing to make the assumption that no other modules will pass a compiler flag in pkgconfig, then we can have pre-processor/compiler flags separation in apps and libs.
As a side note, it looks like the server config handles the SUN compilers.
AC-CHECK-DECL([
Gaetan Nadon wrote:
Yes, the visibility cflags should probably have been added to xorg-macros
(since it's useful for libraries as well as driver modules) and the macro
added to the drivers.
section, no one can build drivers with gcc (including me, which is why I found
this, since some of the drivers I build cannot be built with Sun compilers and
must use gcc).

to fix, but there will be the issue
of older modules configuring with newer servers without visibility flag.
But it does not break anything,
just larger object code file with the default visibility.




> On Fri, 2010-03-19 at 13:53 -0700, Alan Coopersmith wrote:
>> > The server sticks -fvisibility compiler flag in the pkg Cflags which
>> > does not help while 99% of pkg-config files on my distro are compiler
>> > flags free.
>>
>> Which is probably a bug in itself - I end up removing @symbol-visibility@
>> from xorg-server.pc.in when building our packages, since we support both
>> gcc & Sun Studio, and they take different flags for that option.
>>
>
> I tend to agree, providing compiler flags such as this one is pushing
> the "convenience" envelop. This flag should be specified in the driver
> module, but it is significant work. There is no facility to specify a
> portable compiler flag. In this particular case, it could be supplied
> through a server macro and included in each driver makefile.
Yes, the visibility cflags should probably have been added to xorg-macros
(since it's useful for libraries as well as driver modules) and the macro
added to the drivers.
> As a side note, it looks like the server config handles the SUN compilers.
>
> AC-CHECK-DECL([Yes, so if I build the X server with Sun compilers and don't patch out that
section, no one can build drivers with gcc (including me, which is why I found
this, since some of the drivers I build cannot be built with Sun compilers and
must use gcc).
Ah, I came close to that scenario. Isn't that a bug then? It seems easy to fix, but there will be the issue
of older modules configuring with newer servers without visibility flag. But it does not break anything,
just larger object code file with the default visibility.