On 09/17/2011 03:31 PM, Gaetan Nadon wrote: > On Sat, 2011-09-17 at 09:54 -0700, Chase Douglas wrote: >> On 09/17/2011 06:53 AM, Gaetan Nadon wrote: >> > On Sat, 2011-09-17 at 06:52 +1000, Peter Hutterer wrote: >> >> On Wed, Sep 14, 2011 at 10:33:56PM -0700, Chase Douglas wrote: >> >> > Signed-off-by: Chase Douglas <[email protected] >> >> > <mailto:[email protected]> >> >> > <mailto:[email protected]>> >> >> > --- >> >> > configure.ac | 3 +++ >> >> > 1 files changed, 3 insertions(+), 0 deletions(-) >> >> > >> >> > diff --git a/configure.ac b/configure.ac >> >> > index 6c2f731..f3d6d8e 100644 >> >> > --- a/configure.ac >> >> > +++ b/configure.ac >> >> > @@ -39,6 +39,9 @@ if ! test "x$UNSTABLE_LIB" = xyes; then >> >> > AC_MSG_ERROR([This branch contains elements which have not yet >> >> > been finalised. When this branch is updated, you will probably need to >> >> > recompile both the any clients using the library, and may experience >> >> > crashes or undefined behaviour if you do not.]) >> >> > fi >> >> > >> >> > +# Define macros for compiling with unstable protocols >> >> > +AC_SUBST(CFLAGS, "-DXINPUT2_1_USE_UNSTABLE_PROTOCOL >> >> > -DXINPUT2_2_USE_UNSTABLE_PROTOCOL") >> > CFLAGS is a user environment and should never be set in configure.ac. In >> > some case it is, for a configure test, but restored to the original >> > value. Refer to Automake documentation for a complete discussion on env >> > variable usage. >> >> A couple points: >> >> * This is only temporary while the protocol is under development. It >> will not be part of any official libXi release. > I suppose it goes in git master, so it makes no difference to me. It is > expose in broad day light for any one to conclude that it is ok to do > and replicate in some of the other 240 packages.
These patches will first go into a separate branch as we ready XI 2.2 support. This particular patch should never be merged into libXi master. It is only necessary during development. >> * When I have used AC_SUBST before, it has only appended flags to the >> CFLAGS env var. This is exactly what we want. > And that maybe exactly what someone else does not want. The way I > understand Automake, is that the user is king and must have the final > word. This of course might conflict with the developer who wants to > enforce certain flags. > Note that CFLAGS is written after AM_CFLAGS to uphold the user priority > over the configuration. In this particular case, the library will not build without the flag being set. Allowing the user to override this won't really help anyone. >> * I can't find any documentation providing guidelines, so if you have >> any please share :). > I was in a hurry to leave, sorry. Here it is: > > http://www.gnu.org/software/automake/manual/automake.html#User-Variables > > Some Makefile variables are reserved by the GNU Coding Standards > for the use of the “user”—the person building the package. > For instance, CFLAGS is one such variable. > > http://www.gnu.org/software/automake/manual/automake.html#Flag-Variables-Ordering > > Follow the links for more discussion. Several xorg packages were setting > CFLAGS in configure.ac and patches have been submitted and reviewed > multiple times by people more knowledgeable than I am. That's how I got > my current understanding. Basically, I was educated by reviewers on this > list. > > Unless I have misunderstood Automake, the proper way to handle this > situation is to create a new variable in configure.ac and add it to > AM_CFLAGS in the makefile. I'd be curious to know if the motivation was > to ensure the builder does not change the flags or if it was done > because it is convenient and has always been working. It's due to both. We don't want the developer to change the flags because then the library wouldn't build. I implemented it this way because it was quick and easy, and this change should never reach master or a released version of the package. It doesn't seem useful to worry about getting this implemented 100% correctly when it's just a temporary measure. Does that seem reasonable to you? I don't really care one way or the other, I'm just trying to save a bit of work. I'd rather people be happy with what gets committed than feel like they were ignored though, so if it's important to you I'm flexible. > Thanks for listening, and greetings to all the folks at Canonical. Thanks! I appreciate the insight. I learned something new today :). -- Chase _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
