On Sun, 2011-09-18 at 09:28 -0700, Chase Douglas wrote: > 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 :).
So did I. I was a bit out of context, but if it is a side piece, I don't object. Other than the learning curve, the correct solution is really no more work. > > -- Chase
signature.asc
Description: This is a digitally signed message part
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
