<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 TRANSITIONAL//EN"> <HTML> <HEAD> <META HTTP-EQUIV="Content-Type" CONTENT="text/html; CHARSET=UTF-8"> <META NAME="GENERATOR" CONTENT="GtkHTML/3.32.2"> </HEAD> <BODY> On Sun, 2011-09-18 at 09:28 -0700, Chase Douglas wrote: <BLOCKQUOTE TYPE=CITE> <PRE> 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 <<A HREF="mailto:chase.douglas@canonical.com">chase.douglas@canonical.com</A> <<A HREF="mailto:chase.douglas@canonical.com">mailto:chase.douglas@canonical.com</A>> <<A HREF="mailto:chase.douglas@canonical.com">mailto:chase.douglas@canonical.com</A>>> >> >> > --- >> >> > 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: > > <A HREF="http://www.gnu.org/software/automake/manual/automake.html#User-Variables">http://www.gnu.org/software/automake/manual/automake.html#User-Variables</A> > > 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. > > <A HREF="http://www.gnu.org/software/automake/manual/automake.html#Flag-Variables-Ordering">http://www.gnu.org/software/automake/manual/automake.html#Flag-Variables-Ordering</A> > > 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 :). </PRE> </BLOCKQUOTE> So did I. I was a bit out of context, but if it is a side piece, I don't object.<BR> Other than the learning curve, the correct solution is really no more work.<BR> <BR> <BLOCKQUOTE TYPE=CITE> <PRE> -- Chase </PRE> </BLOCKQUOTE> <BR> </BODY> </HTML>