<!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.26.0"> </HEAD> <BODY> On Thu, 2009-12-17 at 16:59 -0800, Dan Nicholson wrote: <BLOCKQUOTE TYPE=CITE> <PRE> On Thu, Dec 17, 2009 at 4:48 PM, Gaetan Nadon <<A HREF="mailto:gaetan.nadon@videotron.ca">gaetan.nadon@videotron.ca</A>> wrote: > Hi, > > There are 41 driver modules with the line: > > sdkdir=$(pkg-config --variable=sdkdir xorg-server) > > Following up on review > <A HREF="http://lists.x.org/archives/xorg-devel/2009-November/003711.html">http://lists.x.org/archives/xorg-devel/2009-November/003711.html</A> > > From Dan Nicholson: > This should be > > sdkdir=`$PKG_CONFIG --variable=sdkdir xorg-server` > > 1. The user may need to use a specific pkg-config or pass custom > arguments to it. Think multiarch or cross-compiling scenarios. > > 2. Backticks (``) are supported by all shells while $() command > substitution is for strictly POSIX conforming shells. We don't need to > limit ourselves to that subset. > > There are 4 modules who are not using that line at all. Rather than making > dead code more portable, this patch removes this statement. I have done > extensive grepping and tested with make distcheck: > > apm, ark, dummy, vesa > > In addition, the sdkdir was passed in INCLUDES. The sdkdir is already > included in XORG_CFLAGS. There were 2 copies of sdkdir in the Makefile and > after the patch there is only one. That seems fine, but it would be good if you mentioned that it's already in XORG_CFLAGS in the commit message. That's the real reason it's not needed here. </PRE> </BLOCKQUOTE> I intended to... Now done. Thanks <BLOCKQUOTE TYPE=CITE> <PRE> -- Dan _______________________________________________ xorg-devel mailing list <A HREF="mailto:xorg-devel@lists.x.org">xorg-devel@lists.x.org</A> <A HREF="http://lists.x.org/mailman/listinfo/xorg-devel">http://lists.x.org/mailman/listinfo/xorg-devel</A> </PRE> </BLOCKQUOTE> </BODY> </HTML>