Re: [PATCH:libX11] Xcms file parsing should not require the impossible to succeed
On Oct 23, 2013 10:49 PM, Alan Coopersmith alan.coopersm...@oracle.com wrote: On 10/23/13 01:45 PM, Alan Coopersmith wrote: This has gone unnoticed since 1991, until gcc -Wlogicalop came to our rescue, and https://bugs.freedesktop.org/show_bug.cgi?id=70803 was filed. And I probably should have mentioned it's gone unnoticed because the effect is truly minor - it's only hit on lines in Xcms.txt that start with whitespace, which no color entry line shipped in any X.Org release ever did. You can see this with our current Xcms.txt if you edit it and put a space at the start of the cms red color line - after doing so, without this fix, 'xlogo -bg cms red' will fail, with this fix (or without the added space), you'll get a red background in the logo display. I'm guessing that's exactly why no entries ever started with whitespace - they wouldn't actually work. :) Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH modular] Remove video-newport failing driver from the build scripts
On Sep 18, 2013 7:50 AM, Gaetan Nadon mems...@videotron.ca wrote:
This driver has not received server updates such as mibstore removal
and conditional compiling of xaa architecture. This indicate an implicite
decisison to remove the driver from active support and future releases.
This driver is used for SGI workstations Indy and Indigo2. Manufacturing
stopped in 1997 and SGI support ended in 2012.
Signed-off-by: Gaetan Nadon mems...@videotron.ca
---
build.sh |1 -
xorg.modules |9 -
2 files changed, 10 deletions(-)
diff --git a/build.sh b/build.sh
index 2093644..e36b6a1 100755
--- a/build.sh
+++ b/build.sh
@@ -890,7 +890,6 @@ build_driver_video() {
build driver xf86-video-mach64
build driver xf86-video-mga
build driver xf86-video-neomagic
-build driver xf86-video-newport
build driver xf86-video-nv
build driver xf86-video-rendition
build driver xf86-video-r128
diff --git a/xorg.modules b/xorg.modules
index d9d5cd0..9827f47 100644
--- a/xorg.modules
+++ b/xorg.modules
@@ -1809,14 +1809,6 @@
/dependencies
/autotools
- autotools id=xf86-video-newport
-branch module=xorg/driver/xf86-video-newport
-checkoutdir=xorg/driver/xf86-video-newport/
-dependencies
- dep package=xserver/
-/dependencies
- /autotools
-
autotools id=xf86-video-nouveau
branch module=nouveau/xf86-video-nouveau
checkoutdir=xorg/driver/xf86-video-nouveau/
@@ -2058,7 +2050,6 @@
dep package=xf86-video-i740/
dep package=xf86-video-impact/
dep package=xf86-video-imstt/
- dep package=xf86-video-newport/
dep package=xf86-video-s3/
dep package=xf86-video-s3virge/
dep package=xf86-video-siliconmotion/
I'd suggest leaving the module definition in there but just removing the
dep. Unless the code is actually removed, someone might want to try
building and fixing it.
Dan
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH:luit 0/3] configure.ac cleanups bug fix
On Jul 15, 2013 10:46 PM, Alan Coopersmith alan.coopersm...@oracle.com wrote: On 07/14/13 01:33 PM, Thomas Dickey wrote: On Sun, Jul 14, 2013 at 09:45:01PM +0200, Matthieu Herrb wrote: Note however that as is luit is currently broken on OpenBSD. I had to commit a patch to OpenBSD's xenocara to disable grantpt() check in configure.ac to avoid picking the support for it that was added to OpenBSD 5.3. I don't know yet if it's OpenBSD grantpt code that's broken or its use in luit. fwiw, luit 2.0 works on OpenBSD So does anyone remember why we still have two versions of luit? We happily deprecated X.Org's xterm in favor of Thomas's many years ago, but somehow kept our own fork of luit in the process. Since the only user of luit is xterm, I think it would make sense to deprecate X.org's version. Thomas clearly has more incentive to maintain it. When I worked on Linux from Scratch, our i18n guru declared that Thomas's version worked better than X.org's and we used his. Not sure how the situation is now. Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] config: provide example configuration for multi-seat setups
On Jul 11, 2013 7:37 PM, Peter Hutterer peter.hutte...@who-t.net wrote: Seats other than seat0 need custom configuration. Provide that with a default configuration file so we can share it across distros. This file intentionally does not end in .conf so it won't get picked up by the server after a normal installation. gdm, or whatever starts up the servers will have to explicitly specifiy this config file. This file replaces the one currently written by systemd's multi-seat-x binary: http://cgit.freedesktop.org/systemd/systemd/tree/src/login/multi-seat-x.c CC: Lennart Poettering lenn...@poettering.net Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- config/Makefile.am | 2 +- config/non-seat0.conf.multi-seat | 18 ++ 2 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 config/non-seat0.conf.multi-seat diff --git a/config/Makefile.am b/config/Makefile.am index da81d77..327d07e 100644 --- a/config/Makefile.am +++ b/config/Makefile.am @@ -44,4 +44,4 @@ endif # CONFIG_NEED_DBUS endif # !CONFIG_UDEV -EXTRA_DIST = xorg-server.conf x11-input.fdi 10-evdev.conf fdi2iclass.py 10-quirks.conf +EXTRA_DIST = xorg-server.conf x11-input.fdi 10-evdev.conf non-seat0.conf.multi-seat fdi2iclass.py 10-quirks.conf diff --git a/config/non-seat0.conf.multi-seat b/config/non-seat0.conf.multi-seat new file mode 100644 index 000..34008ce --- /dev/null +++ b/config/non-seat0.conf.multi-seat @@ -0,0 +1,18 @@ +# This is the default configuration for servers on seat-1 and above. +# +# Start the server with -config non-seat0.conf.multi-seat, or alternatively +# rename the file to end in .conf and put it in the standard config +# directory (though it will apply to _all_ seats!). +# +# * Disable VT switching with Ctrl-Alt-F1 +# * Force a grab on all input devices to detach them from the VT subsystem +# to avoid event leakage. + +Section ServerFlags +Option DontVTSwitch on +EndSection + +Section InputClass +Identifier Force input devices to seat +Option GrabDevice on +EndSection Is that all there is to it? Looking at this makes me sad that you can't make sections filtered by seat. A project for another day, I guess. Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH:xman 1/2] Provide a fallback mkstemp so we don't need to maintain 2 API versions
On Sat, Apr 20, 2013 at 10:08 AM, Alan Coopersmith alan.coopersm...@oracle.com wrote: Unifies the file handling API's to always return an open fd or FILE *, instead of delaying the open when mkstemp was not available. Signed-off-by: Alan Coopersmith alan.coopersm...@oracle.com Wow, that old code is pretty ugly. It looked pretty good to me, but I didn't try extremely hard to navigate the previous #ifdef maze. Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH:xman 2/2] configure: check for groff and enable groff extensions if found
On Sat, Apr 20, 2013 at 10:08 AM, Alan Coopersmith alan.coopersm...@oracle.com wrote: Check for groff never got translated from imake to autoconf Signed-off-by: Alan Coopersmith alan.coopersm...@oracle.com Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH:xman 2/2] configure: check for groff and enable groff extensions if found
On Sat, Apr 20, 2013 at 11:28 AM, Gaetan Nadon mems...@videotron.ca wrote: On 13-04-20 01:08 PM, Alan Coopersmith wrote: Check for groff never got translated from imake to autoconf Signed-off-by: Alan Coopersmith alan.coopersm...@oracle.com --- configure.ac |5 + defs.h |4 2 files changed, 9 insertions(+) diff --git a/configure.ac b/configure.ac index 74a6fc8..b9920bd 100644 --- a/configure.ac +++ b/configure.ac @@ -37,6 +37,11 @@ AC_CONFIG_HEADERS([config.h]) AC_CANONICAL_HOST +AC_CHECK_PROG([GROFF], [groff], [found], [missing]) +if test x$GROFF = xfound ; then + AC_DEFINE([HAS_GROFF], 1, [Define to 1 if you have the groff package.]) +fi + AC_CHECK_FUNCS([mkstemp]) AC_ARG_WITH(helpdir, diff --git a/defs.h b/defs.h index b4cd434..fe09b6b 100644 --- a/defs.h +++ b/defs.h @@ -34,6 +34,10 @@ from the X Consortium. * Created: October 22, 1987 */ +#ifdef HAVE_CONFIG_H +# include config.h +#endif + #ifndef HELPFILE #define HELPFILE /usr/lib/X11/xman.help /* name of the default helpfile. */ #endif Alternatively, you can use XORG_WITH_GROFF macro and benefit from all the features. This was used in many docs before the move to DocBook. In any case you probably want to use HAVE_xx to follow the convention used by Automake elsewhere. That's a good point, but the usage of groff is different in this case. groff isn't being used to generate any documentation, it's simply being used to detect if xman should use groff features in it's output. Alan's patch simply uses the existence of groff on the system to decide whether to build the groff features into xman or not. So, the macro would check for a lot of things not actually used. On the other hand, you'd get a --with-groff for free to give the user full control of whether to use groff or not... -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH util/modular 2/7] xorg.modules: mesa is now autotooled
On Feb 28, 2013 7:08 AM, Jon TURNEY jon.tur...@dronecode.org.uk wrote: mesa is now sufficently autotooled that: - it doesn't need makedepend - non-srcdir builds work - it doesn't need skip-autogen - the check target exists Signed-off-by: Jon TURNEY jon.tur...@dronecode.org.uk libGL doesn't need makedepend I haven't checked on the mesa automake conversion in a while, but I think it works in the default configuration. Are all the old Makefiles (which require makedepend) removed at this point? Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH util/modular 6/7] xorg.modules: Add split xcb util repositories so they get tinderboxed
On Feb 28, 2013 7:10 AM, Jon TURNEY jon.tur...@dronecode.org.uk wrote: Add split xcb util repositories and a xcb metamodule Signed-off-by: Jon TURNEY jon.tur...@dronecode.org.uk --- xorg.modules | 32 1 files changed, 32 insertions(+), 0 deletions(-) diff --git a/xorg.modules b/xorg.modules index ada28b1..309fa74 100644 --- a/xorg.modules +++ b/xorg.modules @@ -14,6 +14,7 @@ checkoutdir=fontconfig/ /autotools + !-- xcb -- autotools id=pthread-stubs branch module=xcb/pthread-stubs checkoutdir=xcb/pthread-stubs/ @@ -42,6 +43,36 @@ /dependencies /autotools + autotools id=xcb-util-image + branch module=xcb/util-image checkoutdir=xcb/xcb-util-image/ + /autotools Do the util modules depend on libxcb proper? I can't recall offhand. Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH util/modular 0/7] Various updates to xorg.modules
On Feb 28, 2013 7:05 AM, Jon TURNEY jon.tur...@dronecode.org.uk wrote: Jon TURNEY (7): xorg.modules: Add libXaw3d so it gets tinderboxed xorg.modules: mesa is now autotooled xorg.modules: Add mesa-glut so it gets tinderboxed xorg.modules: Add mesa-glu so it gets tinderboxed xorg.modules: Add mesa metamodule xorg.modules: Add split xcb util repositories so they get tinderboxed xorg.modules: Add xf86-video-nested so it gets tinderboxed Except for a couple questions, these look good to me. Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH lib/libX11] Fix config check for loadable modules
On Mon, Jan 7, 2013 at 5:14 AM, Jon TURNEY jon.tur...@dronecode.org.uk wrote: The config check of the results of testing for dlcfn.h or dl.h just tests the value of the ac_cv_ variables, which will be 'yes' or 'no', rather than checking it is 'yes', so loadable module support would always be detected. This is neccessary for successful compilation for the MinGW target without the optional dlfcn-win32 library. Signed-off-by: Jon TURNEY jon.tur...@dronecode.org.uk --- configure.ac |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/configure.ac b/configure.ac index fe31799..6c3534e 100644 --- a/configure.ac +++ b/configure.ac @@ -177,7 +177,7 @@ else AC_DEFINE(HAVE_DLOPEN,1,[Use dlopen to load shared libraries]) AC_CHECK_HEADERS([dlfcn.h]) fi -if test x$ac_cv_header_dlcfn_h -o x$ac_cv_header_dl_h; then +if test x$ac_cv_header_dlcfn_h = xyes -o x$ac_cv_header_dl_h = xyes; then HAVE_LOADABLE_MODULES=yes else HAVE_LOADABLE_MODULES=no Looks right. Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH synaptics] Free mtdev device as well as closing it
On Fri, Dec 28, 2012 at 9:24 PM, Daniel Stone dan...@fooishbar.org wrote: mtdev_close_delete() is to mtdev_new_open() as mtdev_close() is to mtdev_open(). So, since we're using mtdev_new_open(), we need to use mtdev_close_delete() instead of just mtdev_close() to actually free everything. Fixes an eventual failure to open the touchpad device after a lot of suspend/resume cycles. Signed-off-by: Daniel Stone dan...@fooishbar.org --- src/eventcomm.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/eventcomm.c b/src/eventcomm.c index b1d5460..8508e6a 100644 --- a/src/eventcomm.c +++ b/src/eventcomm.c @@ -122,7 +122,7 @@ UninitializeTouch(InputInfoPtr pInfo) proto_data-last_mt_vals = NULL; } -mtdev_close(proto_data-mtdev); +mtdev_close_delete(proto_data-mtdev); proto_data-mtdev = NULL; proto_data-num_touches = 0; } From what I can tell, evdev_query_touch() has the same imbalanced mtdev_new_open/mtdev_close imbalance. With that fix, too, Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xts] Build blowup
On Tue, Dec 18, 2012 at 10:44 AM, Peter Harris phar...@opentext.com wrote: Rename from pixval/blowup to bin/xts-blowup, add Makefile.am, and delete old-style Makefile. --- Was blowup really not ported to the new build system yet, or is there some alternative tool that people use these days? I think when I was doing the port, blowup seemed to be only a fringe tool and I figured I'd get back to it later. 3 years later... Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libXi 1/2] Fix compiler warnings
= (XDeviceStateNotifyEvent
*) save;
deviceStateNotify *sev = (deviceStateNotify *) event;
char *data;
diff --git a/src/XGetFCtl.c b/src/XGetFCtl.c
index 2961034..43afa00 100644
--- a/src/XGetFCtl.c
+++ b/src/XGetFCtl.c
@@ -181,18 +181,18 @@ XGetFeedbackControl(
}
case IntegerFeedbackClass:
{
- xIntegerFeedbackState *i;
+ xIntegerFeedbackState *ifs;
XIntegerFeedbackState *I;
- i = (xIntegerFeedbackState *) f;
+ ifs = (xIntegerFeedbackState *) f;
I = (XIntegerFeedbackState *) Feedback;
- I-class = i-class;
+ I-class = ifs-class;
I-length = sizeof(XIntegerFeedbackState);
- I-id = i-id;
- I-resolution = i-resolution;
- I-minVal = i-min_value;
- I-maxVal = i-max_value;
+ I-id = ifs-id;
+ I-resolution = ifs-resolution;
+ I-minVal = ifs-min_value;
+ I-maxVal = ifs-max_value;
break;
}
case StringFeedbackClass:
diff --git a/src/XIQueryVersion.c b/src/XIQueryVersion.c
index 225737f..1fbda50 100644
--- a/src/XIQueryVersion.c
+++ b/src/XIQueryVersion.c
@@ -60,15 +60,15 @@ _xiQueryVersion(Display * dpy, int *major, int *minor,
XExtDisplayInfo *info)
if (_XiCheckExtInit(dpy, XInput_2_0, info) == -1)
{
XExtensionVersion *ext;
-XExtDisplayInfo *info = XInput_find_display(dpy);
+XExtDisplayInfo *extinfo = XInput_find_display(dpy);
-if (!info || !info-data) {
+if (!extinfo || !info-data) {
I believe this should be extinfo-data, right?
Looks good otherwise.
Reviewed-by: Dan Nicholson dbn.li...@gmail.com
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libXi 2/2] Fix const compiler warnings
On Thu, Dec 6, 2012 at 9:38 PM, Peter Hutterer peter.hutte...@who-t.net wrote:
XExtInt.c:80:38: warning: initialization discards 'const' qualifier from
pointer target type [enabled by default]
XExtInt.c:150:5: warning: initialization discards 'const' qualifier from
pointer target type [enabled by default]
XExtInt.c:151:5: warning: initialization discards 'const' qualifier from
pointer target type [enabled by default]
XExtInt.c:152:5: warning: initialization discards 'const' qualifier from
pointer target type [enabled by default]
XExtInt.c:153:5: warning: initialization discards 'const' qualifier from
pointer target type [enabled by default]
XExtInt.c:154:5: warning: initialization discards 'const' qualifier from
pointer target type [enabled by default]
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
src/XExtInt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/XExtInt.c b/src/XExtInt.c
index fd1ae6c..1c668c7 100644
--- a/src/XExtInt.c
+++ b/src/XExtInt.c
@@ -77,7 +77,7 @@ int copy_classes(XIDeviceInfo *to, xXIAnyInfo* from, int
*nclasses);
int size_classes(xXIAnyInfo* from, int nclasses);
static XExtensionInfo *xinput_info;
-static /* const */ char *xinput_extension_name = INAME;
+static const char *xinput_extension_name = INAME;
static int XInputClose(
Display * /* dpy */,
@@ -143,7 +143,7 @@ static /* const */ XExtensionHooks xinput_extension_hooks
= {
XInputError, /* error_string */
};
-static char *XInputErrorList[] = {
+static const char *XInputErrorList[] = {
BadDevice, invalid or uninitialized input device,/* BadDevice
*/
BadEvent, invalid event type,/* BadEvent */
BadMode, invalid mode parameter, /* BadMode */
I didn't actually look through the code to see all the usage of these
two variables, but I imagine you would have got more compiler warnings
if it wasn't OK to make these const.
Reviewed-by: Dan Nicholson dbn.li...@gmail.com
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] XKB: Remove component listing support
!= '-')
-status = BadImplementation;
-tmp++;
-}
-if (status != Success)
-break;
-if (!isspace(*tmp)) {
-status = BadImplementation;
-break;
-}
-else
-tmp++;
-for (i = 0; (i 8) (status == Success); i++) { /* read
the component flags */
-if (isalpha(*tmp))
-flags |= (1L (i + 8));
-else if (*tmp != '-')
-status = BadImplementation;
-tmp++;
-}
-if (status != Success)
-break;
-if (isspace(*tmp)) {
-while (isspace(*tmp)) {
-tmp++;
-}
-}
-else {
-status = BadImplementation;
-break;
-}
-status = _AddListComponent(list, what, flags, tmp, client);
-}
-#ifndef WIN32
-if (haveDir)
-fclose(in);
-else if ((rval = Pclose(in)) != 0) {
-if (xkbDebugFlags)
-ErrorF([xkb] xkbcomp returned exit code %d\n, rval);
-}
-#else
-fclose(in);
-unlink(tmpname);
-#endif
-free(buf);
-return status;
-}
-
-/******/
-
-/* ARGSUSED */
-Status
-XkbDDXList(DeviceIntPtr dev, XkbSrvListInfoPtr list, ClientPtr client)
-{
-Status status;
-
-status = XkbDDXListComponent(dev, _XkbListKeycodes, list, client);
-if (status == Success)
-status = XkbDDXListComponent(dev, _XkbListTypes, list, client);
-if (status == Success)
-status = XkbDDXListComponent(dev, _XkbListCompat, list, client);
-if (status == Success)
-status = XkbDDXListComponent(dev, _XkbListSymbols, list, client);
-if (status == Success)
-status = XkbDDXListComponent(dev, _XkbListGeometry, list,
client);
-return status;
-}
diff --git a/xkb/xkb.c b/xkb/xkb.c
index 4440a98..0d3bda3 100644
--- a/xkb/xkb.c
+++ b/xkb/xkb.c
@@ -5621,9 +5621,9 @@ ProcXkbListComponents(ClientPtr client)
DeviceIntPtr dev;
xkbListComponentsReply rep;
unsigned len;
-int status;
unsigned char *str;
-XkbSrvListInfoRec list;
+uint8_t size;
+int i;
REQUEST(xkbListComponentsReq);
REQUEST_AT_LEAST_SIZE(xkbListComponentsReq);
@@ -5633,40 +5633,27 @@ ProcXkbListComponents(ClientPtr client)
CHK_KBD_DEVICE(dev, stuff-deviceSpec, client, DixGetAttrAccess);
-status = Success;
str = (unsigned char *) stuff[1];
-memset(list, 0, sizeof(XkbSrvListInfoRec));
-list.maxRtrn = stuff-maxNames;
-list.pattern[_XkbListKeycodes] = GetComponentSpec(str, FALSE,
status);
-list.pattern[_XkbListTypes] = GetComponentSpec(str, FALSE, status);
-list.pattern[_XkbListCompat] = GetComponentSpec(str, FALSE,
status);
-list.pattern[_XkbListSymbols] = GetComponentSpec(str, FALSE,
status);
-list.pattern[_XkbListGeometry] = GetComponentSpec(str, FALSE,
status);
-if (status != Success)
-return status;
+for (i = 0; i _XkbListNumComponents; i++) {
+size = *((uint8_t *)str);
+str += (size + 1);
+}
This seems wrong, although I'm not extremely familiar with how the server
writes things on the wire. Overall, I agree with your assessment of the
usefulness of this code. It's far more correct to do this from the XML on
the client side.
Acked-by: Dan Nicholson dbn.li...@gmail.com
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] config/udev: get driver suggestions from udev
On Oct 17, 2012 5:36 PM, Peter Hutterer peter.hutte...@who-t.net wrote:
On Thu, Oct 18, 2012 at 09:40:11AM +1000, Dave Airlie wrote:
On Thu, Oct 18, 2012 at 9:15 AM, Aaron Plattner aplatt...@nvidia.com
wrote:
On 10/17/2012 04:12 PM, Peter Hutterer wrote:
On Wed, Oct 17, 2012 at 03:02:53PM -0700, Aaron Plattner wrote:
If the udev device corresponding to a platform drm device has an
xorg_drivers
property associated with it, treat that as a comma-separated list of
driver
suggestions to use. Otherwise, fall back to the existing PCI ID
table.
This lets people create a udev rule to associate an X driver with a
given
drm
device using rules that look like this:
SUBSYSTEM==drm, DRIVERS==i915, ENV{xorg_drivers}=intel
Signed-off-by: Aaron Plattner aplatt...@nvidia.com
we had a similar suggestion for input drivers when we added udev
support.
but to keep the xorg configuration in X, we then added InputClass and
xorg.conf.d snippets. With this patch, you'd have xorg configuration
in
udev. Is this really what you want, or is it worth investigating
VideoClass
support?
Hmm, maybe. That would be a lot more work, but might be more
flexible in
the long run. I'll see if I can find some time to work on that.
Though I'm not sure we really VideoClass type situations, we generally
have a real driver or fallback to modesetting per set of devices.
we don't really have the synaptics situation where one driver can
drive loads of non-synaptics.
Except evdev, we generally don't use catchall classes. wacom and
synaptics
only get applied for such devices, the rest of the InputClass features is
overloading configurations. e.g. for a product of this name, set that
option. I think this is quite similar to what would apply here.
The above example Aaron gave would be something like
Section VideoClass
Identifier blah
MatchDRMDriver i915
Driver intel
EndSection
This was something I always intended to work on after InputClass but never
got started. At the very least it would be far nicer than the hardcoded PCI
table. It would be a good project for somebody.
Dan
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] configure: Stop using AM_MAINTAINER_MODE
LOn Oct 2, 2012 1:09 PM, Julien Cristau jcris...@debian.org wrote: On Sun, Sep 30, 2012 at 16:45:17 -0400, Gaetan Nadon wrote: On 12-09-29 04:37 PM, Dan Nicholson wrote: Some distros may prefer maintainer mode. A way to appease everyone is: AM_MAINTAINER_MODE([enable]) Oh, yeah. That would be the best. The historical background is that mode was added as a CVS tarball timestamp loss workaround before the git days. Whoever wishes to keep this workaround should demonstrate that they extract tarballs from CVS and want to prevent the build to re-create the configuration because the timestamp was lost and files appear to be out-of-date. Well for us as I recall it was not so much cvs as - extract tarball - patch configure.ac and configure (or Makefile.am and Makefile.in, whatever) - run configure make - make tries to run some autotool because the timestamps are screwed up and it thinks the generated file is out of date - that fails because the autotools aren't installed in your build chroot. Yes, that's what I was thinking of. -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] configure: Stop using AM_MAINTAINER_MODE
On Sep 29, 2012 11:19 AM, Chase Douglas chase.doug...@ubuntu.com wrote: On Thu, Sep 27, 2012 at 2:12 PM, Adam Jackson a...@redhat.com wrote: All this does is make it so editing configure.ac or Makefile.am doesn't rebuild the makefiles. Which is just stupid. Signed-off-by: Adam Jackson a...@redhat.com --- configure.ac | 1 - 1 file changed, 1 deletion(-) diff --git a/configure.ac b/configure.ac index ac3bf26..b37e608 100644 --- a/configure.ac +++ b/configure.ac @@ -31,7 +31,6 @@ RELEASE_DATE=2012-09-05 RELEASE_NAME=Iced Tea AC_CONFIG_SRCDIR([Makefile.am]) AM_INIT_AUTOMAKE([foreign dist-bzip2]) -AM_MAINTAINER_MODE # Require xorg-macros minimum of 1.14 for XORG_COMPILER_BRAND in XORG_DEFAULT_OPTIONS m4_ifndef([XORG_MACROS_VERSION], Some distros may prefer maintainer mode. A way to appease everyone is: AM_MAINTAINER_MODE([enable]) Oh, yeah. That would be the best. Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] configure: Stop using AM_MAINTAINER_MODE
On Sep 28, 2012 7:45 AM, Adam Jackson a...@redhat.com wrote: On 9/27/12 7:13 PM, Keith Packard wrote: Adam Jackson a...@redhat.com writes: All this does is make it so editing configure.ac or Makefile.am doesn't rebuild the makefiles. Which is just stupid. At one point, I'm almost certain that not having AM_MAINTAINER_MODE meant that you *never* got correct behaviour. When did that change? Maybe you're remembering you need --enable-maintainer-mode to get correct behaviour? Which is true, if you've said AM_MAINTAINER_MODE in configure.ac. It's more correct for downstream in a sense. You don't accidentally get the autotools pulled in during your tarball build if you patched something. There are ways to accomplish that (touch) without maintainer mode, though. I recall Julien said they wanted this in Debian. I think this patch is the right thing to do. Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: Installing proto pc files in /usr/share/pkgconfig?
On Mon, Sep 24, 2012 at 8:04 AM, Yaakov (Cygwin/X) yselkow...@users.sourceforge.net wrote: On Mon, 2012-09-24 at 07:15 -0700, Dan Nicholson wrote: On Sep 23, 2012 9:59 PM, Yaakov (Cygwin/X) wrote: On 2012-09-23 12:35, Matt Turner wrote: The proto packages install their pc files in $(libdir)/pkgconfig, but this leads to files being installed in /usr/lib32 or /usr/lib64 when there's nothing ABI specific about them. Would it be reasonable to install them to $(prefix)/share/pkgconfig instead? Absolutely NOT. If you do this, then they will be picked up when cross-compiling, adding e.g. -I/usr/include to CFLAGS, which will then cause the build-system's headers to be used instead of the cross-host's. Can you explain how this affects cross-compiling? Unlike /usr/lib/pkgconfig, /usr/share/pkgconfig is meant to be used even when cross-compiling. If you put the *proto.pc in the latter, then cross-compile anything which requires *proto, pkg-config will use the native proto and do one of two things: I'm sorry, but I don't think this policy makes sense for cross-compiling. 1. As you demonstrated (and speaking as a pkg-config maintainer), pkg-config does not treat /usr/share/pkgconfig the way you want it to. It's mean to be for a multiarch system sharing the same root, not for a cross-compile scenario where you're trying to minimize pollution from the host root. pkg-config has no code that guarantee's things in /usr/share/pkgconfig are cross-compile safe. 2. If you're going to keep using .pc files from the host, then why is it wrong to use pathways from the host? What other information besides pathways under /usr would be returned from a .pc file in /usr? Either pkg-config is telling your compiler to use -I/usr/include or it's doing it implicitly like your host compiler would. Either way, it sounds like you want to minimize pollution of the host system to your cross-build. In that case, why are you using the host .pc files at all? It sort of sounds like you shouldn't be using the host pkg-config path at all if you want to guarantee you won't pollute your build environment. 3. I think making this change doesn't actually affect your situation. Either way you need to build and install the package supplying the .pc file to your cross-root. Suppose the proto packages stay in /usr/lib/pkgconfig and you avoid them for your setup. You still need to build those packages with your cross-root paths. If these proto packages start showing up in /usr/share/pkgconfig, you still need to build the packages for your new root so the .pc files are found earlier in the path than /usr/share/pkgconfig. I imagine you're building all the proto packages now for your cross setup, right? How does change anything? There is also the matter of xproto and xtrans not being truly cross-platform (e.g. NARROWPROTO and FUNCPROTO in Xfuncproto.h), so each platform anyway needs their own copy. Obviously, if a proto package isn't truly arch-independent, it wouldn't get moved over. The vast majority are completely arch-independent. -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: Installing proto pc files in /usr/share/pkgconfig?
On Sep 23, 2012 9:59 PM, Yaakov (Cygwin/X) yselkow...@users.sourceforge.net wrote: On 2012-09-23 12:35, Matt Turner wrote: The proto packages install their pc files in $(libdir)/pkgconfig, but this leads to files being installed in /usr/lib32 or /usr/lib64 when there's nothing ABI specific about them. Would it be reasonable to install them to $(prefix)/share/pkgconfig instead? Absolutely NOT. If you do this, then they will be picked up when cross-compiling, adding e.g. -I/usr/include to CFLAGS, which will then cause the build-system's headers to be used instead of the cross-host's. Where the pc file is installed has nothing to do with what flags are returned. This is only an issue if people don't put $datadir/pkgconfig in their path. That used to happen frequently, but now there are lots of arch-independent packages who put their pc files there. The likelihood this breaks now is pretty low. Can you explain how this affects cross-compiling? Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: Installing proto pc files in /usr/share/pkgconfig?
On Sep 23, 2012 10:35 AM, Matt Turner matts...@gmail.com wrote: Hi Gaetan, The proto packages install their pc files in $(libdir)/pkgconfig, but this leads to files being installed in /usr/lib32 or /usr/lib64 when there's nothing ABI specific about them. Would it be reasonable to install them to $(prefix)/share/pkgconfig instead? I think people always recognized that was correct, but were scared of builds randomly failing because people didn't put that directory in PKG_CONFIG_PATH. Nowadays I say just bite the bullet. I think enough packages have blazed that trail at this point. Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libX11] Allow overriding location of keysymdef.h
On Tue, Sep 11, 2012 at 9:43 AM, Ross Burton ross.bur...@intel.com wrote: Currently keysymdef.h is found by using the includedir of xproto. This doesn't work when cross-compiling with a sysroot as that ends up being /usr/include/X11, not a path into the cross-build environment. So, add an option to allow explicitly specifying the location of keysymdef.h, and verify that the specified or found path exists. (original patch by Martin Jansa martin.ja...@gmail.com, revised by myself) Signed-off-by: Ross Burton ross.bur...@intel.com It's unfortunate that PKG_CONFIG_SYSROOT_DIR doesn't help you here, but it only does the mangling at the Cflags/Libs level rather than at individual variables. --- configure.ac | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 48a0c8a..200db15 100644 --- a/configure.ac +++ b/configure.ac @@ -306,7 +306,18 @@ AC_CHECK_FUNC(poll, [AC_DEFINE(USE_POLL, 1, [poll() function is available])], ) # Find keysymdef.h # AC_MSG_CHECKING([keysym definitions]) -KEYSYMDEFDIR=`$PKG_CONFIG --variable=includedir xproto`/X11 +AC_ARG_WITH(keysymdefdir, +AC_HELP_STRING([--with-keysymdefdir=DIR], [The location of keysymdef.h (defaults to xproto include dir)]), +KEYSYMDEFDIR=$withval, KEYSYMDEFDIR=) + +if test x$KEYSYMDEFDIR = x; then + KEYSYMDEFDIR=`$PKG_CONFIG --variable=includedir xproto`/X11 +fi You could compact these together so the pkg-config query is just the option default. Plus all the stuff in the macros should be quoted to be safe when autoconf expands it. AC_ARG_WITH([keysymdefdir], [AC_HELP_STRING([--with-keysymdefdir=DIR], [The location of keysymdef.h (defaults to xproto include dir)])], [KEYSYMDEFDIR=$withval], [KEYSYMDEFDIR=`$PKG_CONFIG --variable=includedir xproto`/X11]) +if test ! -d $KEYSYMDEFDIR; then I'd quote the variable here so stupid shell errors don't happen if someone throws in a directory with spaces. + AC_MSG_ERROR([$KEYSYMDEFDIR doesn't exist or isn't a directory]) +fi + FILES=keysymdef.h XF86keysym.h Sunkeysym.h DECkeysym.h HPkeysym.h for i in $FILES; do if test -f $KEYSYMDEFDIR/$i; then Other than those nitpicks, it seems like a good fix to me. Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH V2] Allow overriding location of keysymdef.h
On Sep 12, 2012 6:40 AM, Ross Burton ross.bur...@intel.com wrote: Currently keysymdef.h is found by using the includedir of xproto. This doesn't work when cross-compiling with a sysroot as that ends up being /usr/include/X11, not a path into the cross-build environment. So, add an option to allow explicitly specifying the location of keysymdef.h, and verify that the specified or found path exists. (original patch by Martin Jansa martin.ja...@gmail.com, revised by myself) Signed-off-by: Ross Burton ross.bur...@intel.com Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: xkbcommon in the server (was: Re: [PATCH] Cache xkbcomp output)
Sorry for the slow reply. This was interesting to read, but I got busy. On Fri, Jul 13, 2012 at 7:50 AM, Daniel Stone dan...@fooishbar.org wrote: Hi, On 13 July 2012 13:34, Dan Nicholson dbn.li...@gmail.com wrote: Not that I actually have the time to work on this, but I'd been thinking about xkbcommon lately. Do you think it's possible to build a compatibility layer around the current code? Either the compat code could live in the server or it could be an optional extra library shipped in xkbcommon. I'm not sure if the changes in xkbcommon are too deep to be able to build a keymap for current XKB or not. What do you think? It seems silly not to be making use of xkbcommon in X. I realise it seems a bit counterintuitive, but yeah, I don't think we can really support it. The problem is that the server -- due to wonderful things like xmodmap -- needs to be able to just obliterate random parts of the keymap on the fly. There's quite a bit of server code to support this, which we've had to pull in to xkbcommon to have it generate an even remotely coherent map (rather than just half-complete) for clients. Stuff like actually binding virtual modifiers to real modifiers and thus having working types. :) In terms of what we need for the server, the API surface area is, well, pretty much the entire source tree. I don't think that's really sustainable as a library. Yeah, I remember my disappointment when I discovered all the protocol for modifying an existing keymap rather than just starting a new one. A couple things came to mind which may be crack since I still only have a rudimentary understanding of XKB. 1. Would it be possible to just drop support for all the on-the-fly protocol and fix the major clients like xmodmap to just build a new keymap and send it to the server? It would kind of suck, but maybe it could be coordinated for a katamari. 2. Could the server still use xkbcommon for keymap creation and other utilities below its current piles of XKB code? 3. If 2. doesn't work because it needs more access to xkbcommon internals, could a separate compat library live in the xkbcommon tree that would be solely for supporting XKB1? Just some ideas. My goal with xkbcommon's changed to be able to provide a useful and easy keyboard handling library, with a clear, simple, and supportable API. One of the tradeoffs I had to make there was making the maps immutable, which to be honest I'm completely fine with. I'm also hoping to use it to experiment with other, more comprehensible, file formats, and I don't think that's doable if the entire library is wide open to clients to mangle. As a happy side effect of this, we can now see what clients are actually _doing_ with the keymap and thus find out which bits we really need to support. It seems like you guys are doing the right thing with xkbcommon to make it a useful project. At least if XKB2 ever comes along you'll have a solid idea of what's actually needed and some of the esoteric bits can be dropped. I still think having (something like) xkbcommon in the library is a really good idea, but I just don't see much benefit in sharing the library, given the two wildly disparate usecases. I'm hoping to fix it up to at least import maps from an X connection, so we can have all clients (be they X, Wayland, DirectFB, whatever) using xkbcommon, and just the X server using its own special codebase for its own special requirements. I hope I haven't completely run your project into the ground! No, no. It was and is your project. I just got interested long enough to smash some code together and am happy enough to have this effort recorded in git. -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] Cache xkbcomp output
On Jul 19, 2012 3:38 PM, Daniel Stone dan...@fooishbar.org wrote: Hi, On 19 July 2012 22:53, Keith Packard kei...@keithp.com wrote: Adam Jackson a...@nwnk.net writes: I think there's a combination of factors that pile on to make it worse than you'd expect. And, caching is completely free, so it seems sensible to just do it It's free to not unlink the file, but you still have the overhead of opening and reparsing the file every time. A more sensible interim strategy (IMO) would be to always fork xkbcomp once during startup, preserve the resulting XkbDescRec, then hand that back every time someone asks for the same keymap, which is pretty much all the time. It's really not a lot of code either. Don't we already do this? I wrote that patch a couple years ago. You still have the initial xkbcomp fork that's slow on a cold disk, but the in memory keymap is pretty much instant for subsequent devices. I'm on my phone and can't check the archives right now, though. Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] Cache xkbcomp output
On Thu, Jul 12, 2012 at 3:16 PM, Daniel Stone dan...@fooishbar.org wrote: Hi, On 12 July 2012 23:07, Keith Packard kei...@keithp.com wrote: Construct a unique filename based on the display name and the RMLVO values. If that file contains valid contents, use it. Otherwise, compile the keymap to that file and don't unlink it so that it will be re-used the next time the server runs. You need to do at least some rudimentary stat() work so that you'll rebuild the keymap if the files it uses changes. Aside from that, it's our best hope short of forking xkbcommon, adding back some of the bits we removed as pointless, and smashing that into the server (volunteers?), so, why not. Not that I actually have the time to work on this, but I'd been thinking about xkbcommon lately. Do you think it's possible to build a compatibility layer around the current code? Either the compat code could live in the server or it could be an optional extra library shipped in xkbcommon. I'm not sure if the changes in xkbcommon are too deep to be able to build a keymap for current XKB or not. What do you think? It seems silly not to be making use of xkbcommon in X. -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] Change autogen.sh scripts to respect NOCONFIGURE
On Fri, Jun 8, 2012 at 3:20 PM, Daniel Stone dan...@fooishbar.org wrote: Gaetan is our build system maintainer (sorry Gaetan) I like how the anointing of build system maintainership requires an apology. :) -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] xkb: warn if XKB SlowKeys have been automatically enabled
On Tue, Jun 5, 2012 at 9:57 PM, Peter Hutterer peter.hutte...@who-t.net wrote:
Slow keys are enabled when the XKB AccessX features are generally enabled
(ctrls-enabled_ctrls XkbAccessXKeysMask) and either shift key is held for
8 seconds. For the unsuspecting user this appears as if the keyboard
suddenly stops working.
Print a warning to the log, so we can later tell them told you so.
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
xkb/xkbAccessX.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/xkb/xkbAccessX.c b/xkb/xkbAccessX.c
index 95e28e7..fe28e12 100644
--- a/xkb/xkbAccessX.c
+++ b/xkb/xkbAccessX.c
@@ -295,10 +295,15 @@ AccessXKRGExpire(OsTimerPtr timer, CARD32 now, pointer
arg)
cn.eventType = 0;
cn.requestMajor = 0;
cn.requestMinor = 0;
- if (xkbi-desc-ctrls-enabled_ctrls XkbSlowKeysMask)
+ if (xkbi-desc-ctrls-enabled_ctrls XkbSlowKeysMask) {
AccessXKRGTurnOff((DeviceIntPtr) arg, cn);
- else
+ LogMessage(X_INFO, XKB SlowKeys are disabled.\n);
+ }
+ else {
AccessXKRGTurnOn((DeviceIntPtr) arg, XkbSlowKeysMask, cn);
+ LogMessage(X_INFO, XKB SlowKeys are now enabled. Hold shift to
disable.\n);
+ }
+
return 0;
}
I never knew about that. Seems like exactly the type of feature to be
buried deep in the server...
Reviewed-by: Dan Nicholson dbn.li...@gmail.com
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [Mesa-dev] [PATCH 20/20] tests/glx: Add unit tests for GLX_ARB_create_context GLX protocol
2012/1/4 Ian Romanick i...@freedesktop.org: On 01/04/2012 11:05 AM, Michel Dänzer wrote: On Mit, 2012-01-04 at 10:56 -0800, Ian Romanick wrote: On 01/04/2012 10:55 AM, Daniel Stone wrote: Hi, On 4 January 2012 18:45, Ian Romanicki...@freedesktop.org wrote: Okay, I looked back at your build output, and I think I see the problem: * econf: updating Mesa-/bin/config.sub with /usr/share/gnuconfig/config.sub * econf: updating Mesa-/bin/config.guess with /usr/share/gnuconfig/config.guess ./configure --prefix=/usr --build=x86_64-pc-linux-gnu [...] Since it's a raw GIT tree, this should be 'autogen.sh' instead of 'configure'. The Makefile.in files are generated by autoreconf (run by autogen.sh) and consumed by configure. Since they haven't been generated, configure can't find them and gets angry. The ebuild scripts need to either run ./autogen.sh or run 'autoreconf -v --install' before running configure. I bet that will fix it, and I bet that's why only Gentoo users are still having problems. Can you give that a try? See immediately before that: Preparing source in /var/tmp/portage/media-libs/mesa-/work/Mesa- ... Running eautoreconf in '/var/tmp/portage/media-libs/mesa-/work/Mesa-' ... Running aclocal ... [ok] Running autoconf ... [ok] Source prepared. Okay. I give up. I have no clue why it's not working. Patches welcome. :( It's not running automake (just like the Mesa toplevel Makefile isn't...). Right... autoreconf does aclocal, autoconf, and automake all in one shot. That makes sense. About adding automake to the toplevel Makefile, where should that go? It seems like adding it to 'default' or similar will break the non-autotool builds. Doing that so close before a release seems mean. Since the automake business is only necessary for 'make check', would putting it in the 'check' target be sufficient? Hi Ian, I'm a little busy at the moment, but just wanted to mention that the automake integration looks OK on a first glance. The problems being faced here seems to be gentoo's eautoreconf script. I'm guessing that since the toplevel Makefile is not Makefile.am, it doesn't think it needs to run automake. That's lame and it's the whole reason the regular autoreconf script exists in the first place. It can tell that you added AM_INIT_AUTOMAKE to configure.ac and therefore it needs to run automake. There's no automake to add in the toplevel Makefile. autogen.sh/autoreconf will do the right thing. That said, you could definitely hack in these changes without pulling in automake. -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] Disable building of tests requiring DDX functions when Xorg is not built
On Nov 28, 2011 8:40 PM, Alan Coopersmith alan.coopersm...@oracle.com wrote: Some test cases require linking with some sort of DDX - ideally we'd have a stub ddx for testing, but for now, since we link with the Xorg ddx, disable those tests when configured with --disable-xorg Fixes https://bugs.freedesktop.org/show_bug.cgi?id=43320 Signed-off-by: Alan Coopersmith alan.coopersm...@oracle.com Looks right, but it would be REALLY great for the build if that stub DDX existed. Most of my experimenting with building Xorg smarter broke down because it also needed to build from test. Anyway, Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libX11 v2 1/4] Add _XGetRequest as substitute for GetReq/GetReqExtra
On Oct 27, 2011 4:26 AM, Peter Hutterer peter.hutte...@who-t.net wrote:
On Thu, Oct 27, 2011 at 10:41:06AM +0200, walter harms wrote:
Am 27.10.2011 09:55, schrieb Jamey Sharp:
On Thu, Oct 27, 2011 at 09:15:54AM +0200, walter harms wrote:
Am 27.10.2011 06:21, schrieb Peter Hutterer:
+void *_XGetRequest(Display *dpy, CARD8 type, size_t len)
+{
+ xReq *req;
+
+ WORD64ALIGN
+
+ if (dpy-bufptr + len dpy-bufmax)
+ _XFlush(dpy);
+
+ if (len % 4)
+ fprintf(stderr,
+ Xlib: request %d length %zd not a multiple of 4.\n,
+ type, len);
Does it make sense to continue here ?
perhaps you want a add a return NULL ?
It doesn't make sense to continue, but there's no way to report the
error that any caller can handle. If you return NULL here, the caller is
guaranteed to segfault.
Since these errors are already possible today, but aren't being even
noticed, I think Peter's choice of a printf is the best we can do. At
least it allows the possibility of somebody noticing the bug.
It'd be nice if we could get more information than the minor opcode for
extension requests, but nothing else is immediately obvious to me here.
what is bad with segfault ? the macro version would more or less make
sure that len % 4 is 0 by using sizeof. Everybody else has a serious
problem.
Better you stop here (by returning NULL) than at a random place.
all the existing code still uses the macros so the same assumptions are
true. Only new code could use _XGetRequest directly and you'd hope that
whoever writes that code runs it at least once to see the error message.
Drive by reviewing ...
What about an assert? Than the user's incorrect program will at least
stop with a usefulish message. Seems better then a segfault.
--
Dan
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 3/5] parser: free scandir's list
On Tue, Nov 1, 2011 at 9:27 PM, Peter Hutterer peter.hutte...@who-t.net wrote:
On Tue, Nov 01, 2011 at 11:12:36AM -0200, przan...@gmail.com wrote:
From: Paulo Zanoni paulo.r.zan...@intel.com
It seems appropriate to make the function that frees the list elements
also free the list.
80 bytes in 1 blocks are definitely lost in loss record 411 of 631
at 0x4C2779D: malloc (vgpreload_memcheck-amd64-linux.so)
by 0x4C27927: realloc (vgpreload_memcheck-amd64-linux.so)
by 0x696A80D: scandir (scandir.c:108)
by 0x4D8828: OpenConfigDir (scan.c:854)
by 0x4D8A43: xf86openConfigDirFiles (scan.c:952)
by 0x49031F: xf86HandleConfigFile (xf86Config.c:2327)
by 0x49A9E3: InitOutput (xf86Init.c:365)
by 0x425A7A: main (main.c:204)
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
hw/xfree86/parser/scan.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/hw/xfree86/parser/scan.c b/hw/xfree86/parser/scan.c
index 668237b..96ea703 100644
--- a/hw/xfree86/parser/scan.c
+++ b/hw/xfree86/parser/scan.c
@@ -818,6 +818,7 @@ AddConfigDirFiles(const char *dirpath, struct dirent
**list, int num)
numFiles++;
}
+ free(list);
not a big fan of doing this. why can't the caller (which allocates the list)
free it? imo it'd make much more sense to the occasional reader of the code
Yeah, that seems right.
return openedFile;
}
@@ -856,7 +857,6 @@ OpenConfigDir(const char *path, const char *cmdline,
const char *projroot,
if (!found) {
free(dirpath);
dirpath = NULL;
- free(list);
}
}
If you just move this free(list) outside the conditional, it's covered, right?
--
Dan
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 4/5] parser: free val.str after xf86getBoolValue
On Nov 1, 2011 6:14 AM, przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com After we convert the value to a boolean, we discard the string. This is just one example: 3 bytes in 1 blocks are definitely lost in loss record 5 of 657 at 0x4C2779D: malloc (vgpreload_memcheck-amd64-linux.so) by 0x4D744D: xf86getToken (scan.c:400) by 0x4D75F1: xf86getSubToken (scan.c:462) by 0x4DB3E0: xf86parseInputClassSection (InputClass.c:189) by 0x4D664C: xf86readConfigFile (read.c:184) by 0x490556: xf86HandleConfigFile (xf86Config.c:2360) by 0x49AA77: InitOutput (xf86Init.c:365) by 0x425A7A: main (main.c:204) Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com I guess I thought the parser was gonna clean those up. Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 2/5] Correctly free config file names
(configBuf); diff --git a/hw/xfree86/parser/xf86Parser.h b/hw/xfree86/parser/xf86Parser.h index a8785c5..c31fdc4 100644 --- a/hw/xfree86/parser/xf86Parser.h +++ b/hw/xfree86/parser/xf86Parser.h @@ -487,10 +487,10 @@ xf86ConfigSymTabRec, *xf86ConfigSymTabPtr; * prototypes for public functions */ extern void xf86initConfigFiles(void); -extern const char *xf86openConfigFile(const char *path, const char *cmdline, - const char *projroot); -extern const char *xf86openConfigDirFiles(const char *path, const char *cmdline, - const char *projroot); +extern char *xf86openConfigFile(const char *path, const char *cmdline, + const char *projroot); +extern char *xf86openConfigDirFiles(const char *path, const char *cmdline, + const char *projroot); extern void xf86setBuiltinConfig(const char *config[]); extern XF86ConfigPtr xf86readConfigFile(void); extern void xf86closeConfigFile(void); Seems pretty reasonable, but do you know why xf86openConfigDirFiles is called twice? Oh, I guess it's because we also look at the system config dir. Now I recall thinking this would leak but not really caring to fix it at the time. :) I certainly like the idea of passing around variables when possible and not poking globals. These functions also get called from hw/xwin/winconfig.c. I'm not sure how used that code is anymore, but I was trying to keep it updated at the time. Besides that, Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 5/5] parser: free val.str after xstrtokenize
On Nov 1, 2011 6:15 AM, przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com After we tokenize val.str, we discard it. This is just one example: 6 bytes in 1 blocks are definitely lost in loss record 24 of 652 at 0x4C2779D: malloc (in vgpreload_memcheck-amd64-linux.so) by 0x4D744D: xf86getToken (scan.c:400) by 0x4D75F1: xf86getSubToken (scan.c:462) by 0x4DB060: xf86parseInputClassSection (InputClass.c:145) by 0x4D664C: xf86readConfigFile (read.c:184) by 0x490556: xf86HandleConfigFile (xf86Config.c:2360) by 0x49AA77: InitOutput (xf86Init.c:365) by 0x425A7A: main (main.c:204) Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] xfree86: reduce calls to input_option_get_key/value
On Oct 24, 2011 7:00 PM, Peter Hutterer peter.hutte...@who-t.net wrote: No functional changes. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Nice cleanup. Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libXi multitouch 3/4] Define macros for unstable protocol development
On Sep 17, 2011 9:54 AM, Chase Douglas chase.doug...@canonical.com 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 chase.doug...@canonical.com mailto: chase.doug...@canonical.com --- 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. * When I have used AC_SUBST before, it has only appended flags to the CFLAGS env var. This is exactly what we want. I suggest just using AC_DEFINE for macros. Then it doesn't toy with environment variables and the setting is stored in a file (config.h) for you. This will break with make CFLAGS=-O or anything like that. Carl Worth has said some words on this before, but I'm on my phone right now. -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 3/4] input: switch InputOption to use XF86OptionRec storage.
On Wed, Aug 10, 2011 at 8:20 PM, Peter Hutterer peter.hutte...@who-t.net wrote: Use the same struct for both InputOption and XF86OptionRec so we don't need to convert to and fro the two in the config backends. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Now that I see this, I'm wondering why the generic structure isn't called Option since it clearly isn't input configuration specific. It would be really nice if we could just kill XF86OptionRec/Ptr, drop the awkward GenericListRec, and use a generic Option with nt_list* everywhere in the code. I realize this would break the xf86 API, though. Maybe another day. Anyway, Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 4/4] xfree86: use NewInputDeviceRequest for xorg.conf devices too
On Wed, Aug 10, 2011 at 8:20 PM, Peter Hutterer
peter.hutte...@who-t.net wrote:
Only use one init path for input devices - through NIDR.
This requires that inp_driver and inp_identifier from the
XF86ConfInputRec are copied over into the options for NIDR to see them.
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
hw/xfree86/common/xf86Config.c | 8
hw/xfree86/common/xf86Init.c | 29 +
2 files changed, 9 insertions(+), 28 deletions(-)
diff --git a/hw/xfree86/common/xf86Config.c b/hw/xfree86/common/xf86Config.c
index b831d9a..f93724b 100644
--- a/hw/xfree86/common/xf86Config.c
+++ b/hw/xfree86/common/xf86Config.c
@@ -1239,6 +1239,10 @@ checkCoreInputDevices(serverLayoutPtr servlayoutp,
Bool implicitLayout)
if (foundPointer) {
Pointer-options = xf86AddNewOption(Pointer-options,
CorePointer, on);
+ Pointer-options = xf86AddNewOption(Pointer-options,
+ driver,
confInput-inp_driver);
+ Pointer-options = xf86AddNewOption(Pointer-options,
+ identifier,
confInput-inp_identifier);
servlayoutp-inputs = addDevice(servlayoutp-inputs, Pointer);
}
}
@@ -1329,6 +1333,10 @@ checkCoreInputDevices(serverLayoutPtr servlayoutp,
Bool implicitLayout)
if (foundKeyboard) {
Keyboard-options = xf86AddNewOption(Keyboard-options,
CoreKeyboard, on);
+ Keyboard-options = xf86AddNewOption(Keyboard-options,
+ driver,
confInput-inp_driver);
+ Keyboard-options = xf86AddNewOption(Keyboard-options,
+ identifier,
confInput-inp_identifier);
servlayoutp-inputs = addDevice(servlayoutp-inputs, Keyboard);
}
}
Are these two hunks based on some other code I'm not seeing? The
checkCoreInputDevices in master doesn't look like this, and there's no
addDevice().
diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c
index 89bc82a..de342bd 100644
--- a/hw/xfree86/common/xf86Init.c
+++ b/hw/xfree86/common/xf86Init.c
@@ -810,21 +810,6 @@ InitOutput(ScreenInfo *pScreenInfo, int argc, char
**argv)
NULL);
}
-static InputInfoPtr
-duplicateDevice(InputInfoPtr pInfo)
-{
- InputInfoPtr dup = calloc(1, sizeof(InputInfoRec));
- if (dup) {
- dup-name = strdup(pInfo-name);
- dup-driver = strdup(pInfo-driver);
- dup-options = xf86OptionListDuplicate(pInfo-options);
- /* type_name is a const string */
- dup-type_name = pInfo-type_name;
- dup-fd = -1;
- }
- return dup;
-}
-
/**
* Initialize all supported input devices present and referenced in the
* xorg.conf.
@@ -841,20 +826,8 @@ InitInput(int argc, char **argv)
/* Initialize all configured input devices */
for (pInfo = xf86ConfigLayout.inputs; pInfo *pInfo; pInfo++) {
- InputInfoPtr dup;
- /* Replace obsolete keyboard driver with kbd */
- if (!xf86NameCmp((*pInfo)-driver, keyboard)) {
- strcpy((*pInfo)-driver, kbd);
- }
-
- /* Data passed into xf86NewInputDevice will be freed on shutdown.
- * Duplicate from xf86ConfigLayout.inputs, otherwise we don't have
any
- * xorg.conf input devices in the second generation
- */
- dup = duplicateDevice(*pInfo);
-
/* If one fails, the others will too */
- if (xf86NewInputDevice(dup, dev, TRUE) == BadAlloc)
+ if (NewInputDeviceRequest((*pInfo)-options, NULL, dev) == BadAlloc)
break;
}
This part looks good though.
--
Dan
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH:luit] Remove check for zlib in configure script
On Mon, Aug 15, 2011 at 9:49 PM, Alan Coopersmith alan.coopersm...@oracle.com wrote: If libfontenc needs it, it will either be linked with it or provide -lz in its pkg-config library requirements. Signed-off-by: Alan Coopersmith alan.coopersm...@oracle.com Good catch. Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xts] tet: Ignore SIGWINCH (terminal window size change)
On Fri, Aug 12, 2011 at 3:51 PM, Aaron Plattner aplatt...@nvidia.com wrote:
This signal is asserted when the controlling terminal changes size,
for example if you're running xts from an xterm and you resize the
window. This causes test tests to fail:
70||VSW5TESTSUITE CASE FillPoly 3
10|34 /Xproto/pFillPoly 15:28:48|TC Start, scenario ref 71-0
15|34 3.3-lite 3|TCM Start
510|34|system 0: Abandoning testset: caught unexpected signal 28 (unknown
signal)
80|34 1 15:28:49|TC End, scenario ref 71-0
These signals can't affect the test environment, so just ignore them
all the time.
Signed-off-by: Aaron Plattner aplatt...@nvidia.com
---
Tracking down test intermittency only to find that it's caused by me
unconsciously rearranging my windows all the time gets really old really
quickly.
src/tet3/tcm/dtcm.c | 8
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/src/tet3/tcm/dtcm.c b/src/tet3/tcm/dtcm.c
index ecfc0a3..3d56e8d 100644
--- a/src/tet3/tcm/dtcm.c
+++ b/src/tet3/tcm/dtcm.c
@@ -720,6 +720,14 @@ void (*func)();
{
sig_init(TET_SIG_IGN, sig_ign);
sig_init(TET_SIG_LEAVE, sig_leave);
+#if defined(SIGWINCH)
+ /*
+ * Ignore terminal window size changes, which don't affect the
+ * test environment.
+ */
+ sigaddset(sig_ign, SIGWINCH);
+#endif
+
init_done = 1;
}
Hah, good catch.
Reviewed-by: Dan Nicholson dbn.li...@gmail.com
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH v2 2/4] input: make InputOption opaque, provide interface functions.
On Sun, Aug 14, 2011 at 5:30 PM, Peter Hutterer peter.hutte...@who-t.net wrote: InputOptions is not switched to use struct list for a future patch to unify it with the XF86OptionRec. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- Changes to v1: - add required fixes for hal and dbus backends - expand testing to test key/value changes and insertion-overwrite - remove pointless error: label in input_option_new - use xf86AddNewOption in NIDR, it strdups for us Looks good now. I'll try to review the rest of this series today, but time is scarce. I really like the idea, though. Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH proxymngr] More LBX proxy avoidance.
On Sat, Aug 13, 2011 at 9:54 AM, Alan Coopersmith
alan.coopersm...@oracle.com wrote:
I don't think I've ever used proxymngr or known anyone who has, but some
distros
still ship lbxproxy, so I'd slightly prefer leaving support for those that do.
What if you leave Makefile.am doing the substitution (i.e. revert that file to
what's currently there), but change configure.ac to:
AC_PATH_PROG(LBXPROXY, lbxproxy)
if [[ -z $LBXPROXY ]] ; then
LBXPROXY=${bindir}/lbxproxy
fi
Even simpler, let autoconf do the fallback for you:
AC_PATH_PROG(LBXPROXY, lbxproxy, ${bindir}/lbxproxy)
--
Dan
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 2/4] input: make InputOption opaque, provide interface functions.
/input.c
@@ -1308,6 +1308,63 @@ static void dix_get_master(void)
}
+static void input_option_test(void)
+{
+ InputOption *list = NULL;
+ InputOption *opt;
+ const char *val;
+
+ printf(Testing input_option list interface\n);
+
+ list = input_option_new(list, key, value);
+ assert(list);
+ opt = input_option_find(list, key);
+ val = input_option_get_value(opt);
+ assert(strcmp(val, value) == 0);
+
+ list = input_option_new(list, 2, v2);
+ opt = input_option_find(list, key);
+ val = input_option_get_value(opt);
+ assert(strcmp(val, value) == 0);
+
+ opt = input_option_find(list, 2);
+ val = input_option_get_value(opt);
+ assert(strcmp(val, v2) == 0);
+
+ list = input_option_new(list, 3, v3);
+
+ /* search, delete */
+ opt = input_option_find(list, key);
+ val = input_option_get_value(opt);
+ assert(strcmp(val, value) == 0);
+ list = input_option_free_element(list, key);
+ opt = input_option_find(list, key);
+ assert(opt == NULL);
+
+ opt = input_option_find(list, 2);
+ val = input_option_get_value(opt);
+ assert(strcmp(val, v2) == 0);
+ list = input_option_free_element(list, 2);
+ opt = input_option_find(list, 2);
+ assert(opt == NULL);
+
+ opt = input_option_find(list, 3);
+ val = input_option_get_value(opt);
+ assert(strcmp(val, v3) == 0);
+ list = input_option_free_element(list, 3);
+ opt = input_option_find(list, 3);
+ assert(opt == NULL);
I don't see any exercising of set_key/set_value here. Might make sense
to add one case where you change the key and value and check that you
got the right thing back.
+
+ /* recreate */
+ list = input_option_new(list, 3, v3);
+ list = input_option_new(list, 3, v3);
+ list = input_option_new(list, 3, v3);
+ input_option_free_list(list);
+
+ assert(list == NULL);
+}
+
+
int main(int argc, char** argv)
{
dix_input_valuator_masks();
@@ -1324,6 +1381,7 @@ int main(int argc, char** argv)
xi_unregister_handlers();
dix_valuator_alloc();
dix_get_master();
+ input_option_test();
return 0;
}
The one other major thing I saw is that you didn't change config/hal
to the new interface. I don't use the hal interface anymore, but you
might as well get the build failures handled now before Alan tries to
build this thing.
With those comments addressed,
Reviewed-by: Dan Nicholson dbn.li...@gmail.com
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 2/4] xserver: limit the kernel subsystems we look for devices in
On Mon, Jul 18, 2011 at 12:17 PM, Lennart Poettering
lenn...@poettering.net wrote:
Don't enumerate/monitor all devices of the system (since that can be
quite a few), but limit our search to devices from the input
subsystem, as well as the tty subsystem (to cover Wacom tablets).
This should make X start up a bit faster and reduce the number of
unnecessary wake-ups of the X server.
---
config/udev.c | 7 +++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/config/udev.c b/config/udev.c
index 5ac52a1..0763cc9 100644
--- a/config/udev.c
+++ b/config/udev.c
@@ -281,6 +281,9 @@ config_udev_init(void)
if (!udev_monitor)
return 0;
+ udev_monitor_filter_add_match_subsystem_devtype(udev_monitor, input,
NULL);
+ udev_monitor_filter_add_match_subsystem_devtype(udev_monitor, tty,
NULL); /* For Wacom serial devices */
+
if (udev_monitor_enable_receiving(udev_monitor)) {
ErrorF(config/udev: failed to bind the udev monitor\n);
return 0;
@@ -289,6 +292,10 @@ config_udev_init(void)
enumerate = udev_enumerate_new(udev);
if (!enumerate)
return 0;
+
+ udev_enumerate_add_match_subsystem(enumerate, input);
+ udev_enumerate_add_match_subsystem(enumerate, tty);
+
udev_enumerate_scan_devices(enumerate);
devices = udev_enumerate_get_list_entry(enumerate);
udev_list_entry_foreach(device, devices) {
Last time this came up, we were a little uneasy about limiting the
subsystems. I guess this should work for devices we care about, and
any future input devices should fall under the input subsystem (I
hope). One thing we could use help on in upstream udev is marking the
appropriate serial devices with ID_INPUT* in
60-persistent-input.rules. I'm cc'ing Thomas since he was the one who
originally requested that we not just filter to input.
--
Dan
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] xserver: add udev/systemd multi-seat support
On Mon, Jul 18, 2011 at 11:56 AM, Lennart Poettering
lenn...@poettering.net wrote:
On Mon, 18.07.11 19:29, Lennart Poettering (lenn...@poettering.net) wrote:
If this is supposed to help with the wacom driver, then this is bogus
too, as the associated rule in /lib/udev/rules.d/70-wacom.rules actually
does check for ID_INPUT.
So, I had another closer look on this, and now understand what is going
on. This rules file is actually really really borked, and really needs
to be fixed. And not only this files is borked, the matching code in X11
is borked too.
I'd never seen that wacom rules file, but I feel like I can comment on
the udev matching in xorg a little more generally. I agree it looks
pretty gross on further inspection.
If I understand this properly, then the wacom serial tablets come with
their own special ISAPNP serial port?
So here's what needs to be fixed:
1) You cannot overide ENV{SUBSYSTEM}. This env var encodes to which
kernel subsystem a device belongs to. Just by overriding this in
userspace you cannot make it move to a different piece of code! This
will break things if left in this way. In fact, after I told Kay about
this he has now added code to udev to explicitly disallow lines like
this. That means these lines will stop working with newer udev unless
they are fixed to stop doing this.
2) If you introduce your own env vars on devices, you are not allowed
not to prefix them. Unprefixed variables are private property of the
kernel. Userspace may only add prefixed variables to that. A common
prefix used is ID_, but you can choose whatever you want. This needs
fixing too. You need to find a better variable than NAME to store
your device name.
This was a sad hack that was introduced to deal with the fact that the
serial devices don't seem to have a name attribute exported from the
kernel. I don't have a serial tablet, so I don't know what the devices
look like at all. I agree that using the variable NAME is a poor
choice.
3) We try to avoid setting pretty names in udev rules, since they might
need translations later on. Hence having a property like NAME here
isn't a good idea at all -- not even if you rename it.
This is already done in the kernel, so I don't see why we're doing
anything wrong here.
$ udevadm info --attribute-walk
--name=/dev/input/by-id/usb-Silitek_Standard_USB_Keyboard-event-kbd
Udevadm info starts with the device specified by the devpath and then
walks up the chain of parent devices. It prints for every device
found, all possible attributes in the udev rules key format.
A rule to match, can be composed by the attributes of the device
and the attributes from one single parent device.
looking at device
'/devices/pci:00/:00:1d.1/usb6/6-1/6-1:1.0/input/input2/event2':
KERNEL==event2
SUBSYSTEM==input
DRIVER==
looking at parent device
'/devices/pci:00/:00:1d.1/usb6/6-1/6-1:1.0/input/input2':
KERNELS==input2
SUBSYSTEMS==input
DRIVERS==
ATTRS{name}==Silitek Standard USB Keyboard
...
I'm pretty sure hal did the same thing, which is where I looked when I
added that code. What I always thought would be much more appropriate
is if one of the udev rules standardized this name under a
ID_INPUT_NAME property or something like that. The other thing that
always kind of bugged me here is that this is the name that's matched
with MatchProduct and it's not really the product name, but that's
another story.
4) If you just check attr{id} like you do in those rules this will cause
*all* devices' id attribute to be read, and if if they happen to include
FUJ as prefix, you'll mark them as tablet. Example: an audio driver
also exposes an id property, you can even set it to whatevr you want via
a module argument. If a poor soul has a sound card with an id of FUJ
then your rule will mark it as input device for X.
To fix this: since the devices in question show up as ISAPNP devices,
this is what you need to check for. i.e. instead of this:
ATTRS{id}==FUJ*, ...
use this:
SUBSYSTEMS=pnp, ATTRS{id}==FUJ*, ...
5) You are now adding your properties to *every* child device of the pnp
device which is very much broken. You should only add it to the serial
port itself. I.e. you also need to check SUBSYSTEM=tty. (Singular, not
plural in this case).
6) Where a device shows up in the device tree is not considered ABI,
kernel devs are free and happy to add new layers to the tree where
necessary. That means in the X11 udev code you cannot use
udev_device_get_parent() to check the parent device of the wacom tablet,
assuming it was a pnp device. This is borked and will break at any
time. Use udev_device_get_parent_with_subsystem_devtype() instead, or
much better: assign the properties you are looking for to the serial
device itself, via the udev rule, by inheriting them as necessary.
This is something that affects the usb devices equally. We really need
to get to the NAME and PRODUCT properties on the
Re: [PATCH] xserver: add udev/systemd multi-seat support
On Wed, Jul 20, 2011 at 4:55 PM, Peter Hutterer
peter.hutte...@who-t.net wrote:
On Wed, Jul 20, 2011 at 08:23:15PM +0200, Lennart Poettering wrote:
On Wed, 20.07.11 06:08, Dan Nicholson (dbn.li...@gmail.com) wrote:
3) We try to avoid setting pretty names in udev rules, since they might
need translations later on. Hence having a property like NAME here
isn't a good idea at all -- not even if you rename it.
This is already done in the kernel, so I don't see why we're doing
anything wrong here.
Well, the current Wacom userspace rule adds in a pretty name in
userspace, which however is something we generally try to avoid, simply
because sooner or later people will ask that Wacom Serial Tablet shows
up as Wacom Serielles Eingabetablett on de_DE machines. Having pretty
names in such a low level database is problematic.
how is this different to assigning the name to ID_MODEL or whatever else in
userspace? Serial Wacom Tablet beats the empty string.
I'm pretty sure hal did the same thing, which is where I looked when I
added that code. What I always thought would be much more appropriate
is if one of the udev rules standardized this name under a
ID_INPUT_NAME property or something like that. The other thing that
always kind of bugged me here is that this is the name that's matched
with MatchProduct and it's not really the product name, but that's
another story.
Well, HAL is pretty good as an excercise how not do things ;-)
Most devices use ID_MODEL and ID_MODEL_FROM_DATABASE as generic model
description strings.
ok, I sat down yesterday and wrote the two small patches attached to parse
ID_MODEL before the parent's NAME. and right now, it's not usable.
NAME on the parent gives me NAME=Apple Wireless Trackpad
ID_MODEL on the device gives me ID_MODEL=BCM2045B
I know which one I'd prefer in the logs...
so unless I did something majorly wrong in the first patch (attached), I
don't think we can switch to ID_MODEL without fixing udev first and
requiring the new version.
I think the ideal thing would be ID_MODEL_FROM_DATABASE, which comes
out of the usb.ids file and could potentially be used instead of NAME.
This is, unfortunately, not wired up currently in the udev rules. Try
adding this to 60-persistent-input.rules:
SUBSYSTEMS==usb, ENV{ID_VENDOR_FROM_DATABASE}==, IMPORT{program}=usb-db %p
Now you should see something for ID_VENDOR_FROM_DATABASE and
ID_MODEL_FROM_DATABASE for a typical USB device. What you could do is
use these properties to compare to MatchVendor and MatchProduct,
respectively. The only drawback here is that for usb devices now we
get the whole name including vendor from NAME and compare that against
MatchProduct.
What comes out of usb.ids to ID_MODEL_DATABASE to MatchProduct doesn't
contain the vendor name. That could break people's configs who are
currently doing MatchProduct Logitech. Also, we'd be depending on
strings from an external database that could have different entries
from the onboard descriptor (my keyboard sadly doesn't have a device
entry in usb.ids).
--
Dan
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] xserver: Fix double free's in config file parser
On Thu, Jul 14, 2011 at 12:03 AM, Benjamin Herrenschmidt
b...@kernel.crashing.org wrote:
Feeding the parser a bad config file, I crashed the server a few times.
It looks like whoever free's val.str (yeah, val is a global .. yuck)
is also responsible for clearing the pointer or something else might try
to free it again some time later.
Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
---
diff -urN xorg-server-1.10.2.902.orig/hw/xfree86/parser/Input.c
xorg-server-1.10.2.902/hw/xfree86/parser/Input.c
--- xorg-server-1.10.2.902.orig/hw/xfree86/parser/Input.c 2011-02-25
14:27:14.0 +1100
+++ xorg-server-1.10.2.902/hw/xfree86/parser/Input.c 2011-07-14
16:57:18.912426863 +1000
@@ -106,6 +106,7 @@
if (strcmp(val.str, keyboard) == 0) {
ptr-inp_driver = strdup(kbd);
free(val.str);
+ val.str = NULL;
}
else
ptr-inp_driver = val.str;
diff -urN xorg-server-1.10.2.902.orig/hw/xfree86/parser/InputClass.c
xorg-server-1.10.2.902/hw/xfree86/parser/InputClass.c
--- xorg-server-1.10.2.902.orig/hw/xfree86/parser/InputClass.c 2011-02-25
14:27:14.0 +1100
+++ xorg-server-1.10.2.902/hw/xfree86/parser/InputClass.c 2011-07-14
16:57:28.608402699 +1000
@@ -114,6 +114,7 @@
if (strcmp(val.str, keyboard) == 0) {
ptr-driver = strdup(kbd);
free(val.str);
+ val.str = NULL;
}
else
ptr-driver = val.str;
diff -urN xorg-server-1.10.2.902.orig/hw/xfree86/parser/Screen.c
xorg-server-1.10.2.902/hw/xfree86/parser/Screen.c
--- xorg-server-1.10.2.902.orig/hw/xfree86/parser/Screen.c 2011-02-25
14:27:14.0 +1100
+++ xorg-server-1.10.2.902/hw/xfree86/parser/Screen.c 2011-07-14
16:56:38.492527593 +1000
@@ -316,6 +316,7 @@
Error (QUOTE_MSG, SubSection);
{
free(val.str);
+ val.str = NULL;
HANDLE_LIST (scrn_display_lst,
xf86parseDisplaySubSection,
XF86ConfDisplayPtr);
}
I also see a couple instances of free (val.str) in parser/Files.c
that don't set it to NULL afterward. Yay for custom parsers! With
those two instances fixed:
Reviewed-by: Dan Nicholson dbn.li...@gmail.com
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH input-synaptics 4/7] Revert build: collapse all Makefile.am files into a single non-recursive one.
On Thu, Jun 30, 2011 at 10:01 AM, Gaetan Nadon mems...@videotron.ca wrote: This reverts commit 39afe69ad7d2258d4043044d1283bd6e311e48da. Acked-by: Daniel Stone dan...@fooishbar.org Signed-off-by: Gaetan Nadon mems...@videotron.ca Since this is basically the whole reason for the series, it would be good if the commit message was expanded a bit. FWIW, these are my reasons: 1. For such a small module, the build time improvement is most likely negligible. At least, I'd like to see some timings proving it's worthiness before seeing the patch go back in. 2. This kind of change would need a thorough review. The need to operate the build from a single toplevel Makefile is a significant change. The two most noticeable issues for me are that collapsing all the Makefiles could easily cause namespacing issues with the variables, and operating on files outside the current directory can introduce subtle bugs. I feel that the non-recursive style is generally less robust than the standard recursive make scheme. 3. It's unlike all the other X.org modules. This isn't a showstopper for me, but the recursive style is well understood here and you've beaten all the modules into a consistent format that makes build bugs unique to specific modules less likely. Assuming you put your reasons into the commit message, Acked-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH input-synaptics 5/7] Revert build: sort building of tools, ensure that cross-pkg-config works.
On Thu, Jun 30, 2011 at 10:01 AM, Gaetan Nadon mems...@videotron.ca wrote: This reverts commit 4005df66072ceac175ea71427deb16176262f197. The patch introduces a couple of issues. 1) These tools are apps and conceptually do not depend on Xserver, so XORG_CFLAGS should not be used. It included pixman header files. Only drivers depend xserver This is not true here. synclient includes xserver-properties.h so it can use the input properties designated by the server. I don't see a reason to revert this patch. It seems to have good fixes in it. -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libX11 1/2] config: require fop minimum version 1.0
On Jun 23, 2011 4:21 PM, Gaetan Nadon mems...@videotron.ca wrote: On Thu, 2011-06-23 at 20:50 +0200, Julien Cristau wrote: Also, I'm not sure if looking for a version number is a very good way to check for bugs. People backport bug fixes all the time, and introduce new bugs all the time. Right. It was not my intention to test for bugs. The objective is to specify a minimum version that will correctly build the docbooks. I recall the reason for specifying no was because fop was crashing (which I verified to be the case). The reason no is the default is because building the docs for libX11 takes forever and people were getting annoyed waiting for the pdfs that they didn't want anyway. http://cgit.freedesktop.org/xorg/lib/libX11/commit/?id=1ec89689fc771f116a6165226b9e076f54254a40 -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libX11 1/2] config: require fop minimum version 1.0
On Thu, Jun 23, 2011 at 4:21 PM, Gaetan Nadon mems...@videotron.ca wrote: Help me chose the lesser of the two evils. I have no problem keeping the no. This works, but looks silly: XORG_WITH_FOP([1.0],[no]) Sorry, but why is that silly? It just says that you need version 1.0 to build but it's disabled by default. That doesn't seem unusual to me. -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libX11 3/3] config: restore building pdf with fop by default
On Fri, Jun 24, 2011 at 7:59 AM, Gaetan Nadon mems...@videotron.ca wrote: All X.Org documentation in all four formats are build by default. An expection was introduced by commit 1ec89689fc77 for libX11. All documentation can be individually disbaled using --without-docs/specs/deve-docs. The pdf/ps format can be disabled using --without-fop. There are sufficient mechanisms for an individual to optimize the build speed, if that is the concern. On the other hand, having only one module turning off pdf requires everyone who do need to build all docs in all formats to know and remember this exception and configure using --with-fop. Signed-off-by: Gaetan Nadon mems...@videotron.ca I'd prefer not. The people that know they need to build all the docs in all formats are also aware of the options and can pass --with-fop to all modules. That has the added advantage that configure would error if they didn't have the correct components instead of silently continuing. On the other hand, random builders who happen to have fop installed will have their build time massively increased for a benefit they probably don't want anyway. Honestly, unless you're building specifically to create documentation, why would you want pdfs of all the compose charts? If people want this, no big deal, but can you at least post a difference building --with-fop and --without-fop? I have old fop on my system and it's failing to build. -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PULL] Kill libxorg
On Wed, Jun 22, 2011 at 11:01 AM, Keith Packard kei...@keithp.com wrote: On Tue, 21 Jun 2011 12:16:39 -0700, Dan Nicholson dbn.li...@gmail.com wrote: Alright, git://people.freedesktop.org/~dbn/xserver.git no-libxorg has been updated. I think it's ready to pull now. I'm getting a merge conflict in test/.gitignore after Gaetan's updates to that file. If you can update your branch to deal with that change, I'll go ahead and merge it to master. It's ready to pull again. Having now played around with trying to kill libxservertest, I realized that this patchset isn't quite as robust as I thought. It turns out that getting all the Xorg convenience libraries to link without putting them into a massive convenience library isn't strictly possible. Code in libcommon uses symbols from libxorgos and vice versa for example, so there's no way to order the libraries correctly at link time to make the symbols resolve. The only reason this patch works is because of sdksyms.c. Since it references every symbol, if it's linked as an object into Xorg, it will make all subsequent symbols in the link resolve correctly. The point being that if sdksyms.c ever went away, Xorg would stop linking. I think there's another way to keep the speedup that would be more robust: keep libxorg.la but make it build all of hw/xfree86 non-recursively to avoid convenience libraries of convenience libraries. That would be really intrusive, though. Anyway, I think what I've done right now is fine for the foreseeable future. I just wanted to share my thoughts before this goes in. -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libX11 1/2] config: require fop minimum version 1.0
On Thu, Jun 23, 2011 at 5:42 AM, Gaetan Nadon mems...@videotron.ca wrote: This version fixes a bug in fop 0.95 where fop crashes in the presence of some characters like the latin capital sharp s. The XORG_WITH_FOP macro must be at version 1.15 in order to be able to specify the minimum version. Note the parameter position change. Due to limited usage and requiring 1.15, it turns out to be backward compatible. Signed-off-by: Gaetan Nadon mems...@videotron.ca --- configure.ac | 8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/configure.ac b/configure.ac index c085df3..c91ae98 100644 --- a/configure.ac +++ b/configure.ac @@ -20,14 +20,14 @@ AM_MAINTAINER_MODE # Initialize libtool AC_PROG_LIBTOOL -# Require xorg-macros minimum of 1.12 for DocBook external references +# Require xorg-macros minimum of 1.15 for fop minimum version m4_ifndef([XORG_MACROS_VERSION], - [m4_fatal([must install xorg-macros 1.12 or later before running autoconf/autogen])]) -XORG_MACROS_VERSION(1.12) + [m4_fatal([must install xorg-macros 1.15 or later before running autoconf/autogen])]) +XORG_MACROS_VERSION(1.15) XORG_DEFAULT_OPTIONS XORG_ENABLE_SPECS XORG_WITH_XMLTO(0.0.22) -XORG_WITH_FOP([no]) +XORG_WITH_FOP([1.0]) In addition to making 1.0 necessary, you've changed the default from no to auto by omitting the second parameter. Please keep the no or add a second commit explaining why auto should be used. Thanks, -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libX11 1/2] config: require fop minimum version 1.0
On Thu, Jun 23, 2011 at 10:30 AM, Gaetan Nadon mems...@videotron.ca wrote: On Thu, 2011-06-23 at 11:13 -0400, James Cloos wrote: Do any dists have any fop versions newer than 0.95? Gentoo and Debian only have 0.95. I haven't searched for other dists' package lists. I doubt so. Debian has it in the experimental branch and is a manual install. However, this is an improvement over the no for everybody. At least those who do have 1.0 can build the docs. For those who don't, they still get the no :-) Fedora has had 1.0 since last October. http://pkgs.fedoraproject.org/gitweb/?p=fop.git;a=commit;h=6d8c4d71cf0d1591095c41b0d0b7ea597ca2ed30 -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH util-macros] XORG_WITH_FOP: add minimum version checking support
On Wed, Jun 22, 2011 at 6:41 AM, Gaetan Nadon mems...@videotron.ca wrote: Adding this feature is motivated by a bug in fop 0.95 where fop crashes in the presence of some characters like the latin capital sharp s. Fop version 1.0 works correctly. This is the same feature found in XMLTO and the likes. In the macro public interface there is a shift in parameter position for the DEFAULT parameter. Doing development on older libraries (point releases on older versions) will still work the same way as the no value will be interpreted as the minimum version. It won't be found, so fop will be disabled anyway. Only libX11 has used XORG_WITH_FOP(no) and will be changed with a version bump on util-macros. Signed-off-by: Gaetan Nadon mems...@videotron.ca It's unfortunate that doing this in a backwards compatible way would break the argument order convention. It's too bad those didn't just get added from the outset. Oh well, it probably won't hurt anything significant. If you want to use the version parameter in any other modules you'll have to wait for the util-macros version bump, right? Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH v2 util-macros] Add XORG_WITH_PERL macro
On Wed, Jun 22, 2011 at 9:46 AM, Gaetan Nadon mems...@videotron.ca wrote: Following the same pattern as XMLTO and friends. Allows all modules to use the same interface, variables and options to check for perl. Perl is used in libX11 and xserver. Signed-off-by: Gaetan Nadon mems...@videotron.ca --- xorg-macros.m4.in | 56 + 1 files changed, 56 insertions(+), 0 deletions(-) diff --git a/xorg-macros.m4.in b/xorg-macros.m4.in index d3b0b7e..e8c14c3 100644 --- a/xorg-macros.m4.in +++ b/xorg-macros.m4.in @@ -494,6 +494,62 @@ m4_ifval([$1],[AC_MSG_WARN(Checking for MIN-VERSION is not implemented.)]) AM_CONDITIONAL([HAVE_XSLTPROC], [test $have_xsltproc = yes]) ]) # XORG_WITH_XSLTPROC +# XORG_WITH_PERL([MIN-VERSION], [DEFAULT]) +# +# Minimum version: 1.15.0 +# +# PERL (Practical Extraction and Report Language) is a language optimized for +# scanning arbitrary text files, extracting information from those text files, +# and printing reports based on that information. +# +# When DEFAULT is not specified, --with-perl assumes 'auto'. +# +# Interface to module: +# HAVE_PERL: used in makefiles to conditionally scan text files +# PERL: returns the path of the perl program found +# returns the path set by the user in the environment +# --with-perl: 'yes' user instructs the module to use perl +# 'no' user instructs the module not to use perl +# have_perl: returns yes if perl found in PATH or no +# +# If the user sets the value of PERL, AC_PATH_PROG skips testing the path. +# +AC_DEFUN([XORG_WITH_PERL],[ +AC_ARG_VAR([PERL], [Path to perl command]) +# Preserves the interface, should it be implemented later +m4_ifval([$1], [m4_warn([syntax], [Checking for perl MIN-VERSION is not implemented])]) +m4_define([_defopt], m4_default([$2], [auto])) +AC_ARG_WITH(perl, + AS_HELP_STRING([--with-perl], + [Use perl for extracting information from files (default: ]_defopt[)]), + [use_perl=$withval], [use_perl=]_defopt) +m4_undefine([_defopt]) + +if test x$use_perl = xauto; then + AC_PATH_PROG([PERL], [perl]) + if test x$PERL = x; then + AC_MSG_WARN([perl not found - cannot extract information and report]) + have_perl=no + else + have_perl=yes + fi +elif test x$use_perl = xyes ; then + AC_PATH_PROG([PERL], [perl]) + if test x$PERL = x; then + AC_MSG_ERROR([--with-perl=yes specified but perl not found in PATH]) + fi + have_perl=yes +elif test x$use_perl = xno ; then + if test x$PERL != x; then + AC_MSG_WARN([ignoring PERL environment variable since --with-perl=no was specified]) + fi + have_perl=no +else + AC_MSG_ERROR([--with-perl expects 'yes' or 'no']) +fi + +AM_CONDITIONAL([HAVE_PERL], [test $have_perl = yes]) +]) # XORG_WITH_PERL # XORG_WITH_ASCIIDOC([MIN-VERSION], [DEFAULT]) # Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 6/6] Test: Ensure libxservertest gets relinked when necessary
On Wed, Jun 22, 2011 at 2:17 AM, Daniel Stone dan...@fooishbar.org wrote: Similar to how we link Xorg, make sure that whenever any of the component libraries changes, we relink libxservertest and the tests. Not much use testing anything other than the actual source in your tree. Signed-off-by: Daniel Stone dan...@fooishbar.org --- test/Makefile.am | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/test/Makefile.am b/test/Makefile.am index 5574e7d..24218db 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -37,6 +37,7 @@ libxservertest_la_LIBADD = \ $(top_builddir)/mi/libmi.la \ $(top_builddir)/os/libos.la \ @XORG_LIBS@ +libxservertest_la_DEPENDENCIES = $(libxservertest_la_LIBADD) endif endif I think that's right. Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH util-macros] XORG_WITH_XSLTPROC: warn at development time rather than config time
On Wed, Jun 22, 2011 at 9:47 AM, Gaetan Nadon mems...@videotron.ca wrote: Catching up unimplemented features should be done earlier by the developer during autoconf rather than during configure. Signed-off-by: Gaetan Nadon mems...@videotron.ca --- xorg-macros.m4.in | 6 ++ 1 files changed, 2 insertions(+), 4 deletions(-) diff --git a/xorg-macros.m4.in b/xorg-macros.m4.in index e8c14c3..0527dfe 100644 --- a/xorg-macros.m4.in +++ b/xorg-macros.m4.in @@ -457,6 +457,8 @@ AM_CONDITIONAL([HAVE_XMLTO], [test $have_xmlto = yes]) # AC_DEFUN([XORG_WITH_XSLTPROC],[ AC_ARG_VAR([XSLTPROC], [Path to xsltproc command]) +# Preserves the interface, should it be implemented later +m4_ifval([$1], [m4_warn([syntax], [Checking for xsltproc MIN-VERSION is not implemented])]) m4_define([_defopt], m4_default([$2], [auto])) AC_ARG_WITH(xsltproc, AS_HELP_STRING([--with-xsltproc], @@ -487,10 +489,6 @@ else AC_MSG_ERROR([--with-xsltproc expects 'yes' or 'no']) fi -# Checking for minimum version is not implemented -# but we want to keep the interface consistent with other commands -m4_ifval([$1],[AC_MSG_WARN(Checking for MIN-VERSION is not implemented.)]) - AM_CONDITIONAL([HAVE_XSLTPROC], [test $have_xsltproc = yes]) ]) # XORG_WITH_XSLTPROC Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH modular] xorg.modules: Use the right URLs and dirs for fonts
On Wed, Jun 22, 2011 at 10:31 AM, Dirk Wallenstein hals...@t-online.de wrote: Signed-off-by: Dirk Wallenstein hals...@t-online.de --- If the URL is wrong, it should not be a problem to change the directory. Yeesh, I clearly didn't even test adding these modules in 928e1676. Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH util-macros] XORG_WITH_FOP: add minimum version checking support
On Wed, Jun 22, 2011 at 11:31 AM, Gaetan Nadon mems...@videotron.ca wrote: I am wondering if for libX11 we should not take this opportunity to use XORG_WITH_FOP([1.0]). This fixes the problem where fop crashes in the nls directory. Few people have 1.0 (not in any Debian distro yet), so it would be equivalent to the current no. As more people upgrade, it would tell us if there are any other problems. I don't recall if there were other issues. There is some time before next release... I'd do it. Generating the pdf is a pretty narrow use case, and I'd personally rather have people not crash. It looks like fop-1.0 was released almost a year ago[1]. That's plenty of time to get with the program, and maybe it would be the thing to force people to upgrade. :) [1] http://archive.apache.org/dist/xmlgraphics/fop/source/ -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 2/3 v2'] xserver: Introduce negated conditions in Match patterns
On Sat, Jun 18, 2011 at 12:07 AM, Oleh Nykyforchyn oleh@gmail.com wrote: Well, if people are so accustomed to single arguments and | as or, what about: 1) adding logical '' to logical '|'; 2) regex:|pat| = regex:/pat/ = regex:%pat% = (like sed); 3) single argument, with multiple conditions (probably regexes). This way we obtain the same (and even more) functionality. The issue isn't about single arguments, but rather adding more features and complexity when they're not necessary. You asked in another email why we weren't following the UNIX way of allowing more flexibility if it didn't interfere with the existing functionality. The reason is because adding that flexibility has more costs than the initial patch. 1. It makes the code more complex to handle the various cases. This in turn makes the code more difficult to understand and more difficult to maintain. This is why Peter's opinion is so important - he's the one that's going to be supporting this code down the road when drive-by developers like you and me aren't around. The easiest way to make code more maintainable is to remove complexity. 2. These changes become part of the Xorg interface, so they need to be considered carefully. If it becomes apparent in a year that one of these features wasn't a good idea, the removal of that feature might not be possible because now there are working setups using those features. It's not impossible, but breaking people's configurations should only be a last resort. 3. The added flexibility is more difficult for users to understand and easier for them to get wrong. The InputClass matching is already somewhat difficult to explain, so adding more features on top doesn't make things easier. Furthermore, the users have apparently been happy with the current functionality because I haven't seen any other reports that suggesting that they're too limited. You might call the above points the KISS model rather than the UNIX model. You have to ask yourself whether these features are needed rather than if they're possible. When we started this conversation a month ago, I that a useful feature was adding an optional argument where you can specify what type of match you're trying to do. Peter expressed reservations about that and suggested that we stay as close to the existing syntax as possible. I still feel like that's a useful feature, but in hindsight I can see how that's not something the typical user needs. And at the end of the day, Peter's one of the maintainers of this project, so his opinion carries more weight for me. Like I said, I think there's some useful work in these patches, but I believe the agreed upon end point was getting regex:blah supported. Instead, each iteration of these patches adds more features that push them further away from getting in. This is just my opinion, but I think it's a general strategy shared by a lot of people here. Much of the effort working on X is supporting interfaces from 20 years ago that nobody wants anymore. New interfaces and features that increase the amount of code to maintain need to be reviewed carefully. Hope that helps and doesn't discourage too much, -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PULL] Kill libxorg
On Mon, Jun 20, 2011 at 3:42 PM, Peter Hutterer peter.hutte...@who-t.net wrote: On Mon, Jun 20, 2011 at 12:25:07PM -0700, Dan Nicholson wrote: On Sun, Jun 19, 2011 at 5:25 PM, Peter Hutterer peter.hutte...@who-t.net wrote: On Fri, Jun 17, 2011 at 04:55:56PM -0700, Dan Nicholson wrote: Here's a few commits that kill off the libxorg convenience library of convenience libraries. It provides a nice speedup when you're rebuilding Xorg from only a couple local changes. Despite a distclean reconf, I cannot make this link on my machine. ./../os/os.O: In function `AddEnabledDevice': /home/whot/xorg/xserver/os/connection.c:1057: multiple definition of `AddEnabledDevice' ./../os/os.O:/home/whot/xorg/xserver/os/connection.c:1057: first defined here ./../os/os.O: In function `CreateWellKnownSockets': /home/whot/xorg/xserver/os/connection.c:371: multiple definition of `CreateWellKnownSockets' ./../os/os.O:/home/whot/xorg/xserver/os/connection.c:371: first defined here ./../os/os.O: In function `AugmentSelf': /home/whot/xorg/xserver/os/access.c:806: multiple definition of `AugmentSelf' ./../os/os.O:/home/whot/xorg/xserver/os/access.c:806: first defined here collect2: ld returned 1 exit status there are approximately 2314 more of these, but I'll skip them for brevity. Is this a problem on my side or a bug in the patchset? Patchset :) I didn't realize I could even enable dtrace for linux, but then found it in the systemtap package. Here's the issue (you might have to do this manually as gmail will most definitely whack the formatting): diff --git a/hw/xfree86/Makefile.am b/hw/xfree86/Makefile.am index a4ec1f7..e3ef14f 100644 --- a/hw/xfree86/Makefile.am +++ b/hw/xfree86/Makefile.am @@ -72,11 +72,6 @@ BUILT_SOURCES = xorg.conf.example DISTCLEANFILES = xorg.conf.example EXTRA_DIST = xorgconf.cpp -if SPECIAL_DTRACE_OBJECTS -# Re-add dtrace object code that gets lost when building static libraries -Xorg_LDADD += $(XSERVER_LIBS) -endif - if SOLARIS_ASM_INLINE # Needs to be built before any files are compiled when using Sun compilers # so in*/out* inline definitions are properly processed. I'm not really sure why that was needed when $(XSERVER_LIBS) was part of libxorg.la rather than Xorg. I guess the symbols were getting thrown out when linking os.O rather than libos.la into libxorg.la? But if you're linking everything straight into Xorg, then you're just adding the object files twice on the command line without the above fix. So, I think the diff above should fix it, but I'm not an expert on dtrace hooks. :) I'm not sure why Alan didn't hit the same thing, though since SPECIAL_DTRACE_OBJECTS is yes on Solaris, too. Can you try that out? If so, I'll roll it into PATCH 3/3. yep, that fixed it, thanks. consider this tested-by: Peter Hutterer peter.hutte...@who-t.net Alright, git://people.freedesktop.org/~dbn/xserver.git no-libxorg has been updated. I think it's ready to pull now. -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PULL] Kill libxorg
On Sun, Jun 19, 2011 at 5:25 PM, Peter Hutterer peter.hutte...@who-t.net wrote: On Fri, Jun 17, 2011 at 04:55:56PM -0700, Dan Nicholson wrote: Here's a few commits that kill off the libxorg convenience library of convenience libraries. It provides a nice speedup when you're rebuilding Xorg from only a couple local changes. Despite a distclean reconf, I cannot make this link on my machine. ./../os/os.O: In function `AddEnabledDevice': /home/whot/xorg/xserver/os/connection.c:1057: multiple definition of `AddEnabledDevice' ./../os/os.O:/home/whot/xorg/xserver/os/connection.c:1057: first defined here ./../os/os.O: In function `CreateWellKnownSockets': /home/whot/xorg/xserver/os/connection.c:371: multiple definition of `CreateWellKnownSockets' ./../os/os.O:/home/whot/xorg/xserver/os/connection.c:371: first defined here ./../os/os.O: In function `AugmentSelf': /home/whot/xorg/xserver/os/access.c:806: multiple definition of `AugmentSelf' ./../os/os.O:/home/whot/xorg/xserver/os/access.c:806: first defined here collect2: ld returned 1 exit status there are approximately 2314 more of these, but I'll skip them for brevity. Is this a problem on my side or a bug in the patchset? Patchset :) I didn't realize I could even enable dtrace for linux, but then found it in the systemtap package. Here's the issue (you might have to do this manually as gmail will most definitely whack the formatting): diff --git a/hw/xfree86/Makefile.am b/hw/xfree86/Makefile.am index a4ec1f7..e3ef14f 100644 --- a/hw/xfree86/Makefile.am +++ b/hw/xfree86/Makefile.am @@ -72,11 +72,6 @@ BUILT_SOURCES = xorg.conf.example DISTCLEANFILES = xorg.conf.example EXTRA_DIST = xorgconf.cpp -if SPECIAL_DTRACE_OBJECTS -# Re-add dtrace object code that gets lost when building static libraries -Xorg_LDADD += $(XSERVER_LIBS) -endif - if SOLARIS_ASM_INLINE # Needs to be built before any files are compiled when using Sun compilers # so in*/out* inline definitions are properly processed. I'm not really sure why that was needed when $(XSERVER_LIBS) was part of libxorg.la rather than Xorg. I guess the symbols were getting thrown out when linking os.O rather than libos.la into libxorg.la? But if you're linking everything straight into Xorg, then you're just adding the object files twice on the command line without the above fix. So, I think the diff above should fix it, but I'm not an expert on dtrace hooks. :) I'm not sure why Alan didn't hit the same thing, though since SPECIAL_DTRACE_OBJECTS is yes on Solaris, too. Can you try that out? If so, I'll roll it into PATCH 3/3. -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 2/3 v2'] xserver: Introduce negated conditions in Match patterns
On Fri, Jun 17, 2011 at 6:57 AM, Oleh Nykyforchyn oleh@gmail.com wrote:
xserver: Introduce negated conditions in Match patterns
When processing input devices, it is useful to determine not only which
attributes are to be accepted, but also which are to be rejected. Hence we
introduce
an '!' prefix, before a pattern, which means that if an attribute matches this
pattern, then the respective device is rejected by this InputClass. To allow
statements like
MatchVendor !Asus|!Logitech
we also accept the rule of last pattern: if the end of an entry is reached,
then the last pattern is decisive.
Signed-off-by: Oleh Nykyforchyn oleh.nyk at gmail.com
---
hw/xfree86/common/xf86Xinput.c | 15 ---
hw/xfree86/man/xorg.conf.man | 19 ---
hw/xfree86/parser/InputClass.c | 10 ++
hw/xfree86/parser/xf86Parser.h | 1 +
4 files changed, 35 insertions(+), 10 deletions(-)
diff --git a/hw/xfree86/common/xf86Xinput.c b/hw/xfree86/common/xf86Xinput.c
index 22e58a5..2cd1508 100644
--- a/hw/xfree86/common/xf86Xinput.c
+++ b/hw/xfree86/common/xf86Xinput.c
@@ -509,10 +509,19 @@ MatchAttrToken(const char *attr, struct list *groups)
Bool match = FALSE;
list_for_each_entry(pattern, group-patterns, entry) {
- if (match_token(attr, pattern)) {
- match = TRUE;
- break;
+ match = match_token(attr, pattern);
+ if (match) { /* success? */
+ if (pattern-is_negated) /* negated pattern matched */
+ match = FALSE; /* failure */
+ break; /* leave the entry */
+ }
+ else {
+ if (pattern-is_negated) /* at least negated pattern not */
+ match = TRUE; /* matched - partial success */
+ /* else non-negated pattern not matched */
+ /* match = FALSE; */
}
+ /* no success yet, continue */
This is really confusing. I think it says that if the pattern didn't
match the attribute and the pattern was preceded by !, then we keep
going even though it successfully matched what we asked for. Is this
the last pattern thing? Why don't we break when match = TRUE?
Assuming I've understood this correctly, I think it breaks how the
current match rules are supposed to work.
MatchProduct foo|bar
If the product name matches foo or bar, use the class.
MatchProduct foo
MatchProduct bar
If the product name matches foo and bar, use the class.
Following that logic, I would expect this:
MatchProduct !foo|!bar
If the product name does not match foo or does not match bar, use the class.
MatchProduct !foo
MatchProduct !bar
If the product name does not match foo and does not match bar, use
the class.
I think you're trying to make the former act like the latter because
that's what you'd typically want to do with negated conditions. Rather
than making the code goofy, I'd rather have people use the correct
logic. I think it could be really simple:
match = match_token(attr, pattern);
if ((match !pattern-is_negated) ||
(!match pattern-is_negated)) {
match = TRUE:
break;
} else
match = FALSE;
Or even simpler:
if (match_token(attr, pattern) == !pattern-is_negated) {
match = TRUE:
break;
}
--
Dan
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH v2 3/3] xfree86: Remove libxorg convenience library to speed up build
On Fri, Jun 17, 2011 at 8:27 AM, Daniel Stone dan...@fooishbar.org wrote: Hi, On Tue, Jun 14, 2011 at 11:02:15PM -0700, Dan Nicholson wrote: libxorg.la served to collect all the Xorg convenience libraries into one massive archive to link into Xorg. This made things easy for symbol resolution, but it tremendously slowed down the build since each change caused libxorg.la to be rebuilt. This is an extremely slow process of extracting all the objects from the sub-libraries and recombining them. Instead, the archives are linked directly into Xorg. The order of the libraries had to be tweaked a bit to make symbols resolve correctly with the lower level code moving later in the link command. My hero! This makes _such_ a difference to my builds. Seriously. Yeah, I'd never given a lot of thought why the convenience library of convenience libraries would be so painful. I think xts has a lot of this same pattern that slows things down. I noticed one other thing when I started to try removing libxservertest.la yesterday. Since sdksyms references symbols in all libraries, you pretty much need to make it an object of the program or do the conglomeration convenience library trick. I tried to make libxservertest.la be just sdksyms.c, but then I was getting undefined references back and forth with libloader. So, it seems you need to either make it an object of each test program, which kind of sucks because sdksyms.c is generated in a different directory and you need to pass a -I for almost every directory in the tree. The alternative is just to make a stub sdksyms for test that just has an empty xorg_symbols[]. What do you think? -libxorg_la_LIBADD = \ +Xorg_LDADD = \ + $(MAIN_LIB) \ $(XSERVER_LIBS) \ loader/libloader.la \ - os-support/libxorgos.la \ common/libcommon.la \ + os-support/libxorgos.la \ parser/libxf86config_internal.la \ dixmods/libdixmods.la \ modes/libxf86modes.la \ @@ -58,14 +57,11 @@ libxorg_la_LIBADD = \ ddc/libddc.la \ i2c/libi2c.la \ dixmods/libxorgxkb.la \ + $(XORG_LIBS) \ $(top_builddir)/mi/libmi.la \ $(top_builddir)/os/libos.la \ - @XORG_LIBS@ - -libxorg_la_DEPENDENCIES = $(libxorg_la_LIBADD) - -Xorg_DEPENDENCIES = libxorg.la -Xorg_LDADD = $(MAIN_LIB) libxorg.la $(XORG_SYS_LIBS) $(XSERVER_SYS_LIBS) + $(XORG_SYS_LIBS) \ + $(XSERVER_SYS_LIBS) Are you fine with the following patch, which readds the _DEPENDENCIES magic to relink Xorg whenever any of the relevant source files have changed? If so, I'll integrate that into my tree and send out a pull request for 1.11 when a couple of smaller changes have been reviewed, including a dependencies patch for test/ which makes sure it gets rebuilt as well. Sure, I should have added a comment. I looked at the automake manual and it seems that if you don't specify _DEPENDENCIES, it will use _LDADD with the external libs removed. http://www.gnu.org/software/automake/manual/automake.html#Linking Hmm, actually looking at what's generated, I think automake can't resolve through the variables and ends up using only the local .la files explicitly specified in _LDADD. Here's what it looks like for me in hw/xfree86/Makefile: am__DEPENDENCIES_1 = #am__DEPENDENCIES_2 = \ # $(am__DEPENDENCIES_1) Xorg_DEPENDENCIES = $(am__DEPENDENCIES_1) $(am__DEPENDENCIES_1) \ loader/libloader.la common/libcommon.la \ os-support/libxorgos.la parser/libxf86config_internal.la \ dixmods/libdixmods.la modes/libxf86modes.la \ ramdac/libramdac.la ddc/libddc.la i2c/libi2c.la \ dixmods/libxorgxkb.la $(am__DEPENDENCIES_1) \ $(top_builddir)/mi/libmi.la $(top_builddir)/os/libos.la \ $(am__DEPENDENCIES_1) $(am__DEPENDENCIES_1) \ $(am__DEPENDENCIES_2) So, yeah you're patch looks right. I'll add it back in and send the merge request. -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH v2 3/3] xfree86: Remove libxorg convenience library to speed up build
On Fri, Jun 17, 2011 at 8:44 AM, Gaetan Nadon mems...@videotron.ca wrote:
On Fri, 2011-06-17 at 16:27 +0100, Daniel Stone wrote:
Are you fine with the following patch, which readds the _DEPENDENCIES
magic to relink Xorg whenever any of the relevant source files have
changed? If so, I'll integrate that into my tree and send out a pull
request for 1.11 when a couple of smaller changes have been reviewed,
including a dependencies patch for test/ which makes sure it gets
rebuilt as well.
I would take this opportunity to check performance before/after the patch.
From my limited understanding, this kind of construct is partially
responsible for slowing down make. At least it would identifying which code
impacts performance.
I threw together a little test script:
#!/bin/bash
build()
{
echo Building $1
git checkout $1 /dev/null
git clean -xdf /dev/null
./autogen.sh --disable-docs --disable-devel-docs \
--disable-unit-tests $1.log
time -p make -j4 $1.log
echo After dirtying hw/xfree86/common/xf86Xinput.c in $1
touch hw/xfree86/common/xf86Xinput.c
time -p make -j4 $1.log
}
build no-libxorg build master
Here's the results:
Building no-libxorg
real 173.41
user 228.54
sys 38.06
After dirtying hw/xfree86/common/xf86Xinput.c in no-libxorg
real 3.88
user 2.60
sys 0.92
Building master
real 176.86
user 229.29
sys 39.54
After dirtying hw/xfree86/common/xf86Xinput.c in master
real 6.49
user 3.86
sys 1.91
So, it saves a few seconds for me. However, the case that Daniel has
to cope with is the dirtying case where you're just making a couple
changes but have to wait for libxorg to get rebuilt. For me, the time
is almost cut in half.
--
Dan
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 2/3 v2'] xserver: Introduce negated conditions in Match patterns
On Fri, Jun 17, 2011 at 12:48 PM, Oleh R. Nykyforchyn oleh@gmail.com wrote: On Fri, 17 Jun 2011 08:44:58 -0700 Dan Nicholson dbn.li...@gmail.com wrote: This is really confusing. I think it says that if the pattern didn't match the attribute and the pattern was preceded by !, then we keep going even though it successfully matched what we asked for. Yes, !pattern is a necessary, not a sufficient condition. Is this the last pattern thing? Last !pattern is sufficient. This exception allows us to treat MatchProduct !Mouse as all but mice. Why wouldn't that work the way I described? If my product name is Keyboard, then it will match !Mouse and the MatchProduct will be successful. Why don't we break when match = TRUE? Assuming I've understood this correctly, I think it breaks how the current match rules are supposed to work. It contradicts to treating of | as or. The only existing rule (if it can be called a rule) is any match is sufficient. I think or and any match is sufficient are equivalent. So, the | does mean or right now. MatchProduct foo|bar If the product name matches foo or bar, use the class. MatchProduct foo MatchProduct bar If the product name matches foo and bar, use the class. Correct Following that logic, I would expect this: MatchProduct !foo|!bar If the product name does not match foo or does not match bar, use the class. No, and this is one of reasons why I suggest MatchProduct !foo !bar Here '|' is just a token separator, not logical or. Hence the former is equivalent to I think that would make things more confusing for people because the behavior changes when you start using negation. MatchProduct !foo MatchProduct !bar If the product name does not match foo and does not match bar, use the class. I think you're trying to make the former act like the latter because that's what you'd typically want to do with negated conditions. Yes Rather than making the code goofy, I'd rather have people use the correct logic. I think it could be really simple: It is really simple, but less useful. I suggest !foo to filter out all attributes with foo except those that were accepted before. Thus we can demand MatchProduct good|!goo and get only good of all goo. If | is or, this is impossible. Using the existing logic, this would just be: MatchProduct good MatchProduct !goo What about that is less useful? What usefulness is added that makes changing the behavior in subtle ways worth it? -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PULL] Kill libxorg
Here's a few commits that kill off the libxorg convenience library of
convenience libraries. It provides a nice speedup when you're rebuilding
Xorg from only a couple local changes.
The following changes since commit 02d11af01211da55e9d93fe0e1851a0c6fe57472:
Merge remote-tracking branch 'kibi/master' (2011-06-07 12:31:15 -0700)
are available in the git repository at:
git://people.freedesktop.org/~dbn/xserver.git no-libxorg
Dan Nicholson (3):
Don't use empty source files
xfree86: Move sdksyms generation to ddx toplevel
xfree86: Remove libxorg convenience library to speed up build
dix/.gitignore |1 -
dix/Makefile.am|6 +---
hw/xfree86/.gitignore |4 +-
hw/xfree86/Makefile.am | 42 +++
hw/xfree86/loader/.gitignore |3 --
hw/xfree86/loader/Makefile.am | 14 +---
hw/xfree86/os-support/.gitignore |2 -
hw/xfree86/os-support/Makefile.am |7 +-
hw/xfree86/{loader = }/sdksyms.sh |0
os/.gitignore |2 -
os/Makefile.am |6 +
test/.gitignore|2 -
test/Makefile.am | 12 --
13 files changed, 35 insertions(+), 66 deletions(-)
delete mode 100644 hw/xfree86/loader/.gitignore
delete mode 100644 hw/xfree86/os-support/.gitignore
rename hw/xfree86/{loader = }/sdksyms.sh (100%)
delete mode 100644 os/.gitignore
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH v2 1/3] Don't use empty source files
When an empty _SOURCES variable is declared, automake will recognize that only linking is needed. Signed-off-by: Dan Nicholson dbn.li...@gmail.com --- dix/.gitignore|1 - dix/Makefile.am |6 ++ hw/xfree86/.gitignore |2 -- hw/xfree86/Makefile.am| 11 +++ hw/xfree86/os-support/.gitignore |2 -- hw/xfree86/os-support/Makefile.am |7 +-- os/.gitignore |2 -- os/Makefile.am|6 +- test/.gitignore |2 -- test/Makefile.am |6 +- 10 files changed, 8 insertions(+), 37 deletions(-) delete mode 100644 hw/xfree86/os-support/.gitignore delete mode 100644 os/.gitignore diff --git a/dix/.gitignore b/dix/.gitignore index 63ee767..c1b4f20 100644 --- a/dix/.gitignore +++ b/dix/.gitignore @@ -1,3 +1,2 @@ # Add Override for this directory and it's subdirectories -dix.c Xserver-dtrace.h diff --git a/dix/Makefile.am b/dix/Makefile.am index 5435546..f5af619 100644 --- a/dix/Makefile.am +++ b/dix/Makefile.am @@ -64,11 +64,9 @@ dtrace-dix.o: $(top_srcdir)/dix/Xserver.d $(am_libdix_la_OBJECTS) noinst_PROGRAMS = dix.O +dix_O_SOURCES = dix.O: dtrace-dix.o $(am_libdix_la_OBJECTS) $(AM_V_GEN)ld -r -o $@ $(am_libdix_la_OBJECTS:%.lo=.libs/%.o) endif -dix.c: - touch $@ - -CLEANFILES = dix.c Xserver-dtrace.h +CLEANFILES = Xserver-dtrace.h diff --git a/hw/xfree86/.gitignore b/hw/xfree86/.gitignore index 2ddca49..f9b3f4a 100644 --- a/hw/xfree86/.gitignore +++ b/hw/xfree86/.gitignore @@ -1,4 +1,2 @@ -libxorg.c Xorg -xorg.c xorg.conf.example diff --git a/hw/xfree86/Makefile.am b/hw/xfree86/Makefile.am index f1a759a..697571e 100644 --- a/hw/xfree86/Makefile.am +++ b/hw/xfree86/Makefile.am @@ -38,13 +38,13 @@ DIST_SUBDIRS = common ddc i2c x86emu int10 fbdevhw os-support \ utils doc man bin_PROGRAMS = Xorg -Xorg_SOURCES = xorg.c +Xorg_SOURCES = AM_CFLAGS = $(DIX_CFLAGS) @XORG_CFLAGS@ INCLUDES = @XORG_INCS@ noinst_LTLIBRARIES = libxorg.la -libxorg_la_SOURCES = libxorg.c +libxorg_la_SOURCES = libxorg_la_LIBADD = \ $(XSERVER_LIBS) \ loader/libloader.la \ @@ -63,18 +63,13 @@ libxorg_la_LIBADD = \ libxorg_la_DEPENDENCIES = $(libxorg_la_LIBADD) -libxorg.c xorg.c: - touch $@ - -DISTCLEANFILES = libxorg.c xorg.c - Xorg_DEPENDENCIES = libxorg.la Xorg_LDADD = $(MAIN_LIB) libxorg.la $(XORG_SYS_LIBS) $(XSERVER_SYS_LIBS) Xorg_LDFLAGS = $(LD_EXPORT_SYMBOLS_FLAG) BUILT_SOURCES = xorg.conf.example -DISTCLEANFILES += xorg.conf.example +DISTCLEANFILES = xorg.conf.example EXTRA_DIST = xorgconf.cpp if SPECIAL_DTRACE_OBJECTS diff --git a/hw/xfree86/os-support/.gitignore b/hw/xfree86/os-support/.gitignore deleted file mode 100644 index f2206cd..000 --- a/hw/xfree86/os-support/.gitignore +++ /dev/null @@ -1,2 +0,0 @@ -# Add Override for this directory and it's subdirectories -xorgos.c diff --git a/hw/xfree86/os-support/Makefile.am b/hw/xfree86/os-support/Makefile.am index 3af4328..348b7ff 100644 --- a/hw/xfree86/os-support/Makefile.am +++ b/hw/xfree86/os-support/Makefile.am @@ -9,18 +9,13 @@ EXTRA_DIST = int10Defines.h xf86OSpriv.h # as one library, otherwise libtool will actively defeat your attempts to # list them multiple times on the link line. noinst_LTLIBRARIES = libxorgos.la -libxorgos_la_SOURCES = xorgos.c +libxorgos_la_SOURCES = libxorgos_la_LIBADD = @XORG_OS_SUBDIR@/lib@XORG_OS_SUBDIR@.la \ bus/libbus.la \ misc/libmisc.la AM_CFLAGS = $(DIX_CFLAGS) -xorgos.c: - touch $@ - -DISTCLEANFILES = xorgos.c - # FIXME: These don't seem to be used anywhere EXTRA_DIST += \ shared/bios_devmem.c diff --git a/os/.gitignore b/os/.gitignore deleted file mode 100644 index 74b1d14..000 --- a/os/.gitignore +++ /dev/null @@ -1,2 +0,0 @@ -# Add Override for this directory and it's subdirectories -os.c diff --git a/os/Makefile.am b/os/Makefile.am index 91ca110..ef9ecdd 100644 --- a/os/Makefile.am +++ b/os/Makefile.am @@ -51,11 +51,7 @@ dtrace.o: $(top_srcdir)/dix/Xserver.d $(am_libos_la_OBJECTS) noinst_PROGRAMS = os.O +os_O_SOURCES = os.O: dtrace.o $(am_libos_la_OBJECTS) $(AM_V_GEN)ld -r -o $@ dtrace.o .libs/*.o endif - -os.c: - touch $@ - -CLEANFILES = os.c diff --git a/test/.gitignore b/test/.gitignore index 86e687f..ca0406b 100644 --- a/test/.gitignore +++ b/test/.gitignore @@ -1,5 +1,3 @@ -libxservertest.c - input list xkb diff --git a/test/Makefile.am b/test/Makefile.am index 5574e7d..29e483a 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -21,6 +21,7 @@ list_LDADD=$(TEST_LDADD) misc_LDADD=$(TEST_LDADD) fixes_LDADD=$(TEST_LDADD) +libxservertest_la_SOURCES = libxservertest_la_LIBADD = \ $(XSERVER_LIBS) \ $(top_builddir)/hw/xfree86/loader/libloader.la \ @@ -39,8 +40,3
[PATCH v2 3/3] xfree86: Remove libxorg convenience library to speed up build
libxorg.la served to collect all the Xorg convenience libraries into one massive archive to link into Xorg. This made things easy for symbol resolution, but it tremendously slowed down the build since each change caused libxorg.la to be rebuilt. This is an extremely slow process of extracting all the objects from the sub-libraries and recombining them. Instead, the archives are linked directly into Xorg. The order of the libraries had to be tweaked a bit to make symbols resolve correctly with the lower level code moving later in the link command. Signed-off-by: Dan Nicholson dbn.li...@gmail.com --- hw/xfree86/Makefile.am | 16 ++-- test/Makefile.am |1 - 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/hw/xfree86/Makefile.am b/hw/xfree86/Makefile.am index 6580e8d..67c8773 100644 --- a/hw/xfree86/Makefile.am +++ b/hw/xfree86/Makefile.am @@ -44,13 +44,12 @@ AM_CFLAGS = $(DIX_CFLAGS) @XORG_CFLAGS@ INCLUDES = $(XORG_INCS) -I$(srcdir)/parser -I$(top_srcdir)/miext/cw \ -I$(srcdir)/ddc -I$(srcdir)/i2c -I$(srcdir)/modes -I$(srcdir)/ramdac -noinst_LTLIBRARIES = libxorg.la -libxorg_la_SOURCES = -libxorg_la_LIBADD = \ +Xorg_LDADD = \ +$(MAIN_LIB) \ $(XSERVER_LIBS) \ loader/libloader.la \ -os-support/libxorgos.la \ common/libcommon.la \ +os-support/libxorgos.la \ parser/libxf86config_internal.la \ dixmods/libdixmods.la \ modes/libxf86modes.la \ @@ -58,14 +57,11 @@ libxorg_la_LIBADD = \ ddc/libddc.la \ i2c/libi2c.la \ dixmods/libxorgxkb.la \ +$(XORG_LIBS) \ $(top_builddir)/mi/libmi.la \ $(top_builddir)/os/libos.la \ -@XORG_LIBS@ - -libxorg_la_DEPENDENCIES = $(libxorg_la_LIBADD) - -Xorg_DEPENDENCIES = libxorg.la -Xorg_LDADD = $(MAIN_LIB) libxorg.la $(XORG_SYS_LIBS) $(XSERVER_SYS_LIBS) +$(XORG_SYS_LIBS) \ +$(XSERVER_SYS_LIBS) Xorg_LDFLAGS = $(LD_EXPORT_SYMBOLS_FLAG) diff --git a/test/Makefile.am b/test/Makefile.am index 04c255b..370e09a 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -37,7 +37,6 @@ libxservertest_la_LIBADD = \ $(top_builddir)/hw/xfree86/ddc/libddc.la \ $(top_builddir)/hw/xfree86/i2c/libi2c.la \ $(top_builddir)/hw/xfree86/dixmods/libxorgxkb.la \ -$(top_builddir)/hw/xfree86/libxorg.la \ $(top_builddir)/mi/libmi.la \ $(top_builddir)/os/libos.la \ @XORG_LIBS@ -- 1.7.3.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH v2 2/3] xfree86: Move sdksyms generation to ddx toplevel
The symbols in sdksyms.c cover the entire source tree. In order to make them resolve when libxorg.la goes away, move the objects from libloader to Xorg. Unfortunately, this means sdksyms needs to get built again for the test code. Signed-off-by: Dan Nicholson dbn.li...@gmail.com --- hw/xfree86/.gitignore |2 + hw/xfree86/Makefile.am| 14 ++- hw/xfree86/loader/.gitignore |3 - hw/xfree86/loader/Makefile.am | 14 +-- hw/xfree86/loader/sdksyms.sh | 425 - hw/xfree86/sdksyms.sh | 425 + test/Makefile.am |7 +- 7 files changed, 445 insertions(+), 445 deletions(-) delete mode 100644 hw/xfree86/loader/.gitignore delete mode 100755 hw/xfree86/loader/sdksyms.sh create mode 100755 hw/xfree86/sdksyms.sh diff --git a/hw/xfree86/.gitignore b/hw/xfree86/.gitignore index f9b3f4a..997a94e 100644 --- a/hw/xfree86/.gitignore +++ b/hw/xfree86/.gitignore @@ -1,2 +1,4 @@ Xorg xorg.conf.example +sdksyms.c +sdksyms.dep diff --git a/hw/xfree86/Makefile.am b/hw/xfree86/Makefile.am index 697571e..6580e8d 100644 --- a/hw/xfree86/Makefile.am +++ b/hw/xfree86/Makefile.am @@ -38,10 +38,11 @@ DIST_SUBDIRS = common ddc i2c x86emu int10 fbdevhw os-support \ utils doc man bin_PROGRAMS = Xorg -Xorg_SOURCES = +nodist_Xorg_SOURCES = sdksyms.c AM_CFLAGS = $(DIX_CFLAGS) @XORG_CFLAGS@ -INCLUDES = @XORG_INCS@ +INCLUDES = $(XORG_INCS) -I$(srcdir)/parser -I$(top_srcdir)/miext/cw \ + -I$(srcdir)/ddc -I$(srcdir)/i2c -I$(srcdir)/modes -I$(srcdir)/ramdac noinst_LTLIBRARIES = libxorg.la libxorg_la_SOURCES = @@ -111,3 +112,12 @@ xorg.conf.example: xorgconf.cpp relink: $(AM_V_at)rm -f Xorg $(MAKE) Xorg + +CLEANFILES = sdksyms.c sdksyms.dep +EXTRA_DIST += sdksyms.sh + +sdksyms.dep sdksyms.c: sdksyms.sh + CPP='$(CPP)' AWK='$(AWK)' $(srcdir)/sdksyms.sh $(top_srcdir) $(CFLAGS) $(AM_CFLAGS) $(INCLUDES) + +SDKSYMS_DEP = sdksyms.dep +include $(SDKSYMS_DEP) diff --git a/hw/xfree86/loader/.gitignore b/hw/xfree86/loader/.gitignore deleted file mode 100644 index 6b38d9e..000 --- a/hw/xfree86/loader/.gitignore +++ /dev/null @@ -1,3 +0,0 @@ -# Add Override for this directory and it's subdirectories -sdksyms.c -sdksyms.dep diff --git a/hw/xfree86/loader/Makefile.am b/hw/xfree86/loader/Makefile.am index 0e5b304..ebe0c81 100644 --- a/hw/xfree86/loader/Makefile.am +++ b/hw/xfree86/loader/Makefile.am @@ -9,11 +9,7 @@ AM_CFLAGS = $(DIX_CFLAGS) $(XORG_CFLAGS) EXTRA_DIST = \ loader.h \ - loaderProcs.h \ - sdksyms.sh - -nodist_libloader_la_SOURCES = \ - sdksyms.c + loaderProcs.h libloader_la_SOURCES = \ loader.c \ @@ -23,11 +19,3 @@ libloader_la_SOURCES = \ os.c libloader_la_LIBADD = $(DLOPEN_LIBS) - -CLEANFILES = sdksyms.c sdksyms.dep - -sdksyms.dep sdksyms.c: sdksyms.sh $(top_builddir)/include/do-not-use-config.h - CPP='$(CPP)' AWK='$(AWK)' $(srcdir)/sdksyms.sh $(top_srcdir) $(AM_CFLAGS) $(CFLAGS) $(INCLUDES) - -SDKSYMS_DEP = sdksyms.dep -include $(SDKSYMS_DEP) diff --git a/hw/xfree86/loader/sdksyms.sh b/hw/xfree86/loader/sdksyms.sh deleted file mode 100755 index 18bb735..000 --- a/hw/xfree86/loader/sdksyms.sh +++ /dev/null @@ -1,425 +0,0 @@ -#!/bin/sh - -cat sdksyms.c EOF -/* This file is automatically generated by sdksyms.sh. */ -#pragma GCC diagnostic ignored -Wdeprecated-declarations - -#ifdef HAVE_XORG_CONFIG_H -#include xorg-config.h -#endif - - -/* These must be included first */ -#include misc.h -#include miscstruct.h - - -/* render/Makefile.am */ -#include picture.h -#include mipict.h -#include glyphstr.h -#include picturestr.h - - -/* fb/Makefile.am -- module */ -/* -#include fb.h -#include fbrop.h -#include fboverlay.h -#include wfbrename.h -#include fbpict.h - */ - - -/* miext/shadow/Makefile.am -- module */ -/* -#include shadow.h - */ - - -/* miext/damage/Makefile.am */ -#include damage.h -#include damagestr.h - -/* miext/sync/Makefile.am */ -#include misync.h -#include misyncstr.h - -/* Xext/Makefile.am -- half is module, half is builtin */ -/* -#include xvdix.h -#include xvmcext.h - */ -#include geext.h -#include geint.h -#include shmint.h -#include syncsdk.h -#if XINERAMA -# include panoramiXsrv.h -# include panoramiX.h -#endif - - -/* hw/xfree86/int10/Makefile.am -- module */ -/* -#include xf86int10.h - */ - - -/* hw/xfree86/i2c/Makefile.am -- mostly modules */ -#include xf86i2c.h -/* -#include bt829.h -#include fi1236.h -#include msp3430.h -#include tda8425.h -#include tda9850.h -#include tda9885.h -#include uda1380.h -#include i2c_def.h - */ - - -/* hw/xfree86/modes/Makefile.am */ -#include xf86Crtc.h -#include xf86Modes.h -#include xf86RandR12.h -/* #include xf86Rename.h */ - - -/* hw/xfree86/ddc/Makefile.am */ -#include edid.h -#include xf86DDC.h - - -/* hw/xfree86/dri2/Makefile.am -- module */ -/* -#if DRI2 -# include dri2.h -#endif - */ - - -/* hw/xfree86/vgahw
Re: [PATCH input-synaptics 1/5] Use appropriate Autoconf statements to check for prerequisites
On Tue, Jun 14, 2011 at 1:56 AM, Diego Elio Pettenò flamee...@gmail.com wrote: Il giorno lun, 13/06/2011 alle 19.43 -0400, Gaetan Nadon ha scritto: Using m4 to check for macro defined by the xserver to ensure it is installed will work as long as the macro is not removed. The preferred way of checking for dependencies is to use PKG_CHECK_MODULE. Different moment to check for them: - PKG_CHECK_MODULE check presence when running ./configure - m4_ifndef when autoconf is executed. If you build autotools for the package on a system that has no xorg-server installed, you'll receive a broken configure script, thus why I wanted to make it explicit that you need to have it around at autoconf time. I'd have to agree here that failing when autoconf is run is better than creating a broken configure script. Better would be if xorg-server.m4 defined a version check macro like xorg-macros.m4. -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH input-synaptics 3/5] man: there is no need to create the man source directory
On Tue, Jun 14, 2011 at 1:41 AM, Diego Elio Pettenò flamee...@gmail.com wrote: Il giorno lun, 13/06/2011 alle 19.43 -0400, Gaetan Nadon ha scritto: This directory is created when the source code is extracted from git. There are 175 other man pages directories in X.Org and none are created from the makefile. You're assuming always running a build in-place. That might not be the case. Indeed Gentoo has been building Xorg modules out-of-tree for quite a while now. In a out-of-tree build, the directories are to be created: with a recursive Makefile, ./config.status would create them to convert Makefile.in to Makefile; on a non-recursive Makefile, it is the task of the target rule to create the directory. Interestingly enough, when using subdir-objects, the Makefile rules already include that for all the other targets. I think what Gaetan may not be noticing is that you changed the Makefile to be non-recursive. I think this is the only one in the xorg world. If that's the way the maintainer wants it, then this patch should be dropped. You need to create the target subdir manually, although the PHONY mandir rule is unnecessary as far as I can tell. -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH input-synaptics 1/5] Use appropriate Autoconf statements to check for prerequisites
On Tue, Jun 14, 2011 at 6:45 AM, Gaetan Nadon mems...@videotron.ca wrote: On Tue, 2011-06-14 at 10:56 +0200, Diego Elio Pettenò wrote: Il giorno lun, 13/06/2011 alle 19.43 -0400, Gaetan Nadon ha scritto: Using m4 to check for macro defined by the xserver to ensure it is installed will work as long as the macro is not removed. The preferred way of checking for dependencies is to use PKG_CHECK_MODULE. Different moment to check for them: - PKG_CHECK_MODULE check presence when running ./configure - m4_ifndef when autoconf is executed. If you build autotools for the package on a system that has no xorg-server installed, you'll receive a broken configure script, thus why I wanted to make it explicit that you need to have it around at autoconf time. What is the benefit of having: autoreconf: running: aclocal -I /home/nadon/xorg/src/inst/share/aclocal --force configure.ac:51: error: must install xorg-server development files before running autoconf/autogen over having: checking for XORG... no configure: error: Package requirements (xorg-server = 1.6) were not met: Dan is also of the opinion that it is preferable, but I just can't see why. In a distro building world perhaps? It's simple: XORG_DRIVER_CHECK_EXT is required at autoconf time whereas PKG_CHECK_MODULES doesn't operate until build time. You could generate a broken configure script if the macro isn't available. IMO, it's just good practice to catch errors at the appropriate time. I don't see a drawback to checking whether an external macro is available when it needs to be used. One drawback is the m4 test fails to fail when the server is uninstalled after synaptics have been configured as the XORG macro is cached. The following PKG_CHECK_MODULE will stop on error, however. Maybe this is the way it should work. How would it fail to fail? If xorg-server.h isn't installed anymore, then you won't get the extension. The macro is intended to check if an extension is available in the server, and it does just that. I guess you want it to fail harder if the xserver can't be found at all. -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: Synaptics: a number of issues with commit 39afe69ad7d2
On Mon, Jun 13, 2011 at 4:44 PM, Gaetan Nadon mems...@videotron.ca wrote: Commit http://cgit.freedesktop.org/xorg/driver/xf86-input-synaptics/commit/?id=39afe69ad7d2258d4043044d1283bd6e311e48da build: collapse all Makefile.am files into a single non-recursive one. With this change, the whole of the build is done non-recursively in the top-level Makefile.am. This reduces the amount of overhead due to recursing into directories only to build one file. has raised a number of issues. I have submitted patches for the easy ones, but there are some that I cannot figure out alone. I would have to agree that if this came by the mailing list I would have raised objections to it. I think the biggest benefit you'd normally get from it is reducing the number of convenience libraries of code in separate subdirectories since you have to wait for libtool to relink them all the time. That point it moot here since there's just a single module with no convenience libraries. Another typical benefit is waiting for make to fork itself repeatedly to descend into subdirectories. I'd say unless you're building on arm this is not a significant amount of build time nowadays. The other nice benefit from using non-recursive make is that when you've only dirtied a single file in your tree, you don't need to wait for make to walk the whole tree to rebuild. A single toplevel make process has all the dependency info in the toplevel Makefile. IMO, unless you're getting to a project the size of the xserver, this is a not a significant amount of time. And if you're trying to use non-recursive automake on a project as large and diverse as the xserver, it would probably be a nightmare. On the other hand, I believe non-recursive make adds to the complexity of the build rather than reducing it. With classic recursive automake, it's very easy to divide and conquer tasks into subdirectories. Since each Makefile is separate, you can tailor it specifically to what's going on in that directory (e.g. creating manpages) and you don't have to worry about variable or pathname collisions at all. To have non-recursive make work, you need to constantly think about how you're performing actions on files in sudirectories of where make is operating. There's a lot more namespacing to think about since everything's in a single file/process. Diego's own page shows some of these issues: http://www.flameeyes.eu/autotools-mythbuster/automake/nonrecursive.html Another issue I can think of is when you need a pattern rule to operate differently (e.g. use different flags) depending on the target. We do this often with the documentation. In that case, you either need to abandon the pattern rule and use a rule per-target, or use a GNU makeism to set per-target variables. Typically you can just setup the pattern rule per Makefile and not have to worry about applying the wrong settings to the wrong target. IMO, the benefits of non-recursive automake do not significantly outweigh the added complexity. -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: non-recursive make (was: [PATCH input-synaptics 2/5] Do not install ChangeLog and README with documentation)
On Tue, Jun 14, 2011 at 12:31 PM, Alan Coopersmith alan.coopersm...@oracle.com wrote: On 06/14/11 11:11 AM, Jamey Sharp wrote: On Tue, Jun 14, 2011 at 03:53:06PM +0200, Diego Elio Pettenò wrote: Having all of xorg using non-recursive make with parallel install would indeed be sweet to avoid spending so much time with the cores idle waiting for link on modules... OMG yes please, at least for xserver and perhaps libX11. I can't even keep a three year old dual-core saturated when building xserver. libX11 is already non-recursive in the nls subdir, and the src subdir has more than enough files to keep a parallel make busy - it looks like the only minor win you'd get there is in the small subdirs like modules and specs. xserver on the other hand is huge and could probably be much more parallel via a non-recursive top-level makefile, but that makefile would be scarily complex. If you honestly tried to put all the xserver build in a single Makefile, you'd want to stab yourself shortly after. One of the real advantages of recursive make is that skipping parts of the build is really easy. You just put the subdir under an AM_CONDITIONAL and you're done. The actual Makefiles in the subdirectory can stay clean because they're being skipped completely. If you wanted to put that all in a single unit, you'd end up with an insane maze of AM_CONDITIONALs that no one could understand. You know how it's nice when you get rid of #ifdefs in code? This is the opposite of that process. -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: non-recursive make (was: [PATCH input-synaptics 2/5] Do not install ChangeLog and README with documentation)
On Tue, Jun 14, 2011 at 4:49 PM, Daniel Stone dan...@fooishbar.org wrote: On Tue, Jun 14, 2011 at 04:02:45PM -0700, Dan Nicholson wrote: On Tue, Jun 14, 2011 at 12:31 PM, Alan Coopersmith alan.coopersm...@oracle.com wrote: xserver on the other hand is huge and could probably be much more parallel via a non-recursive top-level makefile, but that makefile would be scarily complex. If you honestly tried to put all the xserver build in a single Makefile, you'd want to stab yourself shortly after. One of the real advantages of recursive make is that skipping parts of the build is really easy. You just put the subdir under an AM_CONDITIONAL and you're done. The actual Makefiles in the subdirectory can stay clean because they're being skipped completely. If you wanted to put that all in a single unit, you'd end up with an insane maze of AM_CONDITIONALs that no one could understand. In fairness, you can more or less get there via includes, but then you have to be mad fascist about trying to keep a load of separate Makefile fragments from stepping on each others' toes. Right, the includes only give you the false impression that the Makefiles are independent. It might not actually help you at all because you need to be aware what's going on in 20 other files, too. You know how it's nice when you get rid of #ifdefs in code? This is the opposite of that process. Heh. So, something just occurred to me: even worse than the recursion for me is the final link into libxorg.a and libxorgtest.a, which seems to be caused by per-target flags? On my MacBook Air, all interactivity completely dies for about 20 seconds while that link goes on, which never used to happen. So if we could get rid of that, maybe things would be a lot quicker. Sadly, I don't think that's it. Because they're collections of objects, the archive needs to be rebuilt if anything underneath it changes. For example, if you change hw/xfree86/common/xf86Xinput.c (since that's where I've been hanging out :), it causes libcommon.la to be rebuilt, which then causes libxorg.la to be rebuilt, which then causes Xorg to be rebuilt. Each one is successively bigger, but each time libtool has to extract all the objects from the lower .la files and then link again. I think it's really the convenience libraries of convenience libraries that make relinking brutal. Run 'make V=1' and watch the sadness when it gets to libxorg.la. One thing I'd note is that I don't think libxorg.la is needed at all. It just collects a bunch of other convenience libraries together. You could just as easily add them directly to the Xorg link command. Especially since libtool is just turning around and doing the exact some process with the exact same objects in Xorg that it just did with libxorg.la. I started putting a patch together for that, but I'm getting undefined references on the link. I have to remember how libtool does things differently when making convenience libraries vs. programs. I do see something silly that could be corrected (although I don't think it would help any here). Xorg.c, libxorg.c and libxservertest.c are all created as empty files. This is unnecessary if you just declare an empty _SOURCES. Patch attached for the few times I found 'touch $@' in the source. -- Dan no-empty-sources.patch Description: Binary data ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: non-recursive make (was: [PATCH input-synaptics 2/5] Do not install ChangeLog and README with documentation)
On Tue, Jun 14, 2011 at 5:55 PM, Daniel Stone dan...@fooishbar.org wrote: Hi, On Tue, Jun 14, 2011 at 05:42:44PM -0700, Dan Nicholson wrote: One thing I'd note is that I don't think libxorg.la is needed at all. It just collects a bunch of other convenience libraries together. You could just as easily add them directly to the Xorg link command. Especially since libtool is just turning around and doing the exact some process with the exact same objects in Xorg that it just did with libxorg.la. I started putting a patch together for that, but I'm getting undefined references on the link. I have to remember how libtool does things differently when making convenience libraries vs. programs. When making convenience libraries, gcc/ld simply aggregate the compiled source into a single unit and leaves it at that. When doing the final program link, they actively discard unused compilation units. The process goes something like this: * load .a file * go through all referenced symbols, mark symbol as 'used', attempt to resolve symbol within same .o, then same .a, then anything linked earlier * for each .o, if no symbol in the file is marked used, or exported, discard the entire .o * rinse, repeat (That is, IIRC. ajax may have something to say on the matter.) So you have to be really careful with link order; getting it exactly right when I did the modular Xorg server took quite some time indeed. I assume half the reason libxorg.a exists is so that we don't have to worry about link order anymore: it's just a massive aggregation of the entire source tree in object form. Yeah, I think you're right. I fixed one of my undefined references by moving libcommon.la before libxorgos.la because it couldn't find xf86RemoveSIGIOHandler. That is convenient of libtool to put all the symbols in one archive for me so the linker can see them all. I'll see if I can make it work, but it would probably always be a bit fragile. -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH 0/3] Kill libxorg.la
This seems to work. I had to move sdksyms out of libloader and move the order of a couple libraries around, but everything seems to resolve now. It definitely makes a difference. Dan Nicholson (3): Don't use empty source files xfree86: Move sdksyms generation to ddx toplevel xfree86: Remove libxorg convenience library to speed up build dix/.gitignore|1 - dix/Makefile.am |6 +- hw/xfree86/.gitignore |5 +- hw/xfree86/Makefile.am| 36 ++-- hw/xfree86/loader/.gitignore |3 - hw/xfree86/loader/Makefile.am | 13 +- hw/xfree86/loader/sdksyms.sh | 424 - hw/xfree86/os-support/.gitignore |2 - hw/xfree86/os-support/Makefile.am |7 +- hw/xfree86/sdksyms.sh | 424 + os/Makefile.am|6 +- test/.gitignore |2 - test/Makefile.am |6 +- 13 files changed, 452 insertions(+), 483 deletions(-) delete mode 100644 hw/xfree86/loader/.gitignore delete mode 100755 hw/xfree86/loader/sdksyms.sh delete mode 100644 hw/xfree86/os-support/.gitignore create mode 100755 hw/xfree86/sdksyms.sh -- 1.7.3.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH 1/3] Don't use empty source files
When an empty _SOURCES variable is declared, automake will recognize that only linking is needed. Signed-off-by: Dan Nicholson dbn.li...@gmail.com --- dix/.gitignore|1 - dix/Makefile.am |6 ++ hw/xfree86/.gitignore |2 -- hw/xfree86/Makefile.am| 11 +++ hw/xfree86/os-support/.gitignore |2 -- hw/xfree86/os-support/Makefile.am |7 +-- os/Makefile.am|6 +- test/.gitignore |2 -- test/Makefile.am |6 +- 9 files changed, 8 insertions(+), 35 deletions(-) delete mode 100644 hw/xfree86/os-support/.gitignore diff --git a/dix/.gitignore b/dix/.gitignore index 63ee767..c1b4f20 100644 --- a/dix/.gitignore +++ b/dix/.gitignore @@ -1,3 +1,2 @@ # Add Override for this directory and it's subdirectories -dix.c Xserver-dtrace.h diff --git a/dix/Makefile.am b/dix/Makefile.am index 59e512b..a0eff46 100644 --- a/dix/Makefile.am +++ b/dix/Makefile.am @@ -65,11 +65,9 @@ dtrace-dix.o: $(top_srcdir)/dix/Xserver.d $(am_libdix_la_OBJECTS) noinst_PROGRAMS = dix.O +dix_O_SOURCES = dix.O: dtrace-dix.o $(am_libdix_la_OBJECTS) $(AM_V_GEN)ld -r -o $@ $(am_libdix_la_OBJECTS:%.lo=.libs/%.o) endif -dix.c: - touch $@ - -CLEANFILES = dix.c Xserver-dtrace.h +CLEANFILES = Xserver-dtrace.h diff --git a/hw/xfree86/.gitignore b/hw/xfree86/.gitignore index 2ddca49..f9b3f4a 100644 --- a/hw/xfree86/.gitignore +++ b/hw/xfree86/.gitignore @@ -1,4 +1,2 @@ -libxorg.c Xorg -xorg.c xorg.conf.example diff --git a/hw/xfree86/Makefile.am b/hw/xfree86/Makefile.am index c23b1fd..ab79873 100644 --- a/hw/xfree86/Makefile.am +++ b/hw/xfree86/Makefile.am @@ -40,13 +40,13 @@ DIST_SUBDIRS = common ddc i2c x86emu int10 fbdevhw os-support \ utils doc bin_PROGRAMS = Xorg -Xorg_SOURCES = xorg.c +Xorg_SOURCES = AM_CFLAGS = $(DIX_CFLAGS) @XORG_CFLAGS@ INCLUDES = @XORG_INCS@ noinst_LTLIBRARIES = libxorg.la -libxorg_la_SOURCES = libxorg.c +libxorg_la_SOURCES = libxorg_la_LIBADD = \ $(XSERVER_LIBS) \ loader/libloader.la \ @@ -65,18 +65,13 @@ libxorg_la_LIBADD = \ libxorg_la_DEPENDENCIES = $(libxorg_la_LIBADD) -libxorg.c xorg.c: - touch $@ - -DISTCLEANFILES = libxorg.c xorg.c - Xorg_DEPENDENCIES = libxorg.la Xorg_LDADD = $(MAIN_LIB) libxorg.la $(XORG_SYS_LIBS) $(XSERVER_SYS_LIBS) Xorg_LDFLAGS = $(LD_EXPORT_SYMBOLS_FLAG) BUILT_SOURCES = xorg.conf.example -DISTCLEANFILES += xorg.conf.example +DISTCLEANFILES = xorg.conf.example EXTRA_DIST = xorgconf.cpp if SPECIAL_DTRACE_OBJECTS diff --git a/hw/xfree86/os-support/.gitignore b/hw/xfree86/os-support/.gitignore deleted file mode 100644 index f2206cd..000 --- a/hw/xfree86/os-support/.gitignore +++ /dev/null @@ -1,2 +0,0 @@ -# Add Override for this directory and it's subdirectories -xorgos.c diff --git a/hw/xfree86/os-support/Makefile.am b/hw/xfree86/os-support/Makefile.am index 3af4328..348b7ff 100644 --- a/hw/xfree86/os-support/Makefile.am +++ b/hw/xfree86/os-support/Makefile.am @@ -9,18 +9,13 @@ EXTRA_DIST = int10Defines.h xf86OSpriv.h # as one library, otherwise libtool will actively defeat your attempts to # list them multiple times on the link line. noinst_LTLIBRARIES = libxorgos.la -libxorgos_la_SOURCES = xorgos.c +libxorgos_la_SOURCES = libxorgos_la_LIBADD = @XORG_OS_SUBDIR@/lib@XORG_OS_SUBDIR@.la \ bus/libbus.la \ misc/libmisc.la AM_CFLAGS = $(DIX_CFLAGS) -xorgos.c: - touch $@ - -DISTCLEANFILES = xorgos.c - # FIXME: These don't seem to be used anywhere EXTRA_DIST += \ shared/bios_devmem.c diff --git a/os/Makefile.am b/os/Makefile.am index 3e4f2c5..3411f0b 100644 --- a/os/Makefile.am +++ b/os/Makefile.am @@ -57,11 +57,7 @@ dtrace.o: $(top_srcdir)/dix/Xserver.d $(am_libos_la_OBJECTS) noinst_PROGRAMS = os.O +os_O_SOURCES = os.O: dtrace.o $(am_libos_la_OBJECTS) $(AM_V_GEN)ld -r -o $@ dtrace.o .libs/*.o endif - -os.c: - touch $@ - -CLEANFILES = os.c diff --git a/test/.gitignore b/test/.gitignore index db8c5f3..2c7f6d9 100644 --- a/test/.gitignore +++ b/test/.gitignore @@ -1,6 +1,4 @@ # Add Override for this directory and it's subdirectories -libxservertest.c - input xkb xtest diff --git a/test/Makefile.am b/test/Makefile.am index 456221e..4094f7b 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -17,6 +17,7 @@ xkb_LDADD=$(TEST_LDADD) input_LDADD=$(TEST_LDADD) xtest_LDADD=$(TEST_LDADD) +libxservertest_la_SOURCES = libxservertest_la_LIBADD = \ $(XSERVER_LIBS) \ $(top_builddir)/hw/xfree86/loader/libloader.la \ @@ -34,8 +35,3 @@ libxservertest_la_LIBADD = \ $(top_builddir)/os/libos.la \ @XORG_LIBS@ endif - -CLEANFILES=libxservertest.c - -libxservertest.c: - touch $@ -- 1.7.3.4 ___ xorg-devel
[PATCH 3/3] xfree86: Remove libxorg convenience library to speed up build
libxorg.la served to collect all the Xorg convenience libraries into one massive archive to link into Xorg. This made things easy for symbol resolution, but it tremendously slowed down the build since each change caused libxorg.la to be rebuilt. This is an extremely slow process of extracting all the objects from the sub-libraries and recombining them. Instead, the archives are linked directly into Xorg. The order of the libraries had to be tweaked a bit to make symbols resolve correctly with the lower level code moving later in the link command. Signed-off-by: Dan Nicholson dbn.li...@gmail.com --- hw/xfree86/Makefile.am | 16 ++-- 1 files changed, 6 insertions(+), 10 deletions(-) diff --git a/hw/xfree86/Makefile.am b/hw/xfree86/Makefile.am index b6264db..a36d997 100644 --- a/hw/xfree86/Makefile.am +++ b/hw/xfree86/Makefile.am @@ -46,13 +46,12 @@ AM_CFLAGS = $(DIX_CFLAGS) @XORG_CFLAGS@ INCLUDES = $(XORG_INCS) -I$(srcdir)/parser -I$(top_srcdir)/miext/cw \ -I$(srcdir)/ddc -I$(srcdir)/i2c -I$(srcdir)/modes -I$(srcdir)/ramdac -noinst_LTLIBRARIES = libxorg.la -libxorg_la_SOURCES = -libxorg_la_LIBADD = \ +Xorg_LDADD = \ +$(MAIN_LIB) \ $(XSERVER_LIBS) \ loader/libloader.la \ -os-support/libxorgos.la \ common/libcommon.la \ +os-support/libxorgos.la \ parser/libxf86config_internal.la \ dixmods/libdixmods.la \ modes/libxf86modes.la \ @@ -60,14 +59,11 @@ libxorg_la_LIBADD = \ ddc/libddc.la \ i2c/libi2c.la \ dixmods/libxorgxkb.la \ +$(XORG_LIBS) \ $(top_builddir)/mi/libmi.la \ $(top_builddir)/os/libos.la \ -@XORG_LIBS@ - -libxorg_la_DEPENDENCIES = $(libxorg_la_LIBADD) - -Xorg_DEPENDENCIES = libxorg.la -Xorg_LDADD = $(MAIN_LIB) libxorg.la $(XORG_SYS_LIBS) $(XSERVER_SYS_LIBS) +$(XORG_SYS_LIBS) \ +$(XSERVER_SYS_LIBS) Xorg_LDFLAGS = $(LD_EXPORT_SYMBOLS_FLAG) -- 1.7.3.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH 2/3] xfree86: Move sdksyms generation to ddx toplevel
The symbols in sdksyms.c cover the entire source tree. In order to make them resolve when libxorg.la goes away, move the objects from libloader to Xorg. Signed-off-by: Dan Nicholson dbn.li...@gmail.com --- hw/xfree86/.gitignore |3 + hw/xfree86/Makefile.am| 13 +- hw/xfree86/loader/.gitignore |3 - hw/xfree86/loader/Makefile.am | 13 +- hw/xfree86/loader/sdksyms.sh | 424 - hw/xfree86/sdksyms.sh | 424 + 6 files changed, 440 insertions(+), 440 deletions(-) delete mode 100644 hw/xfree86/loader/.gitignore delete mode 100755 hw/xfree86/loader/sdksyms.sh create mode 100755 hw/xfree86/sdksyms.sh diff --git a/hw/xfree86/.gitignore b/hw/xfree86/.gitignore index f9b3f4a..d78fdb9 100644 --- a/hw/xfree86/.gitignore +++ b/hw/xfree86/.gitignore @@ -1,2 +1,5 @@ +# Add Override for this directory and it's subdirectories Xorg xorg.conf.example +sdksyms.c +sdksyms.dep diff --git a/hw/xfree86/Makefile.am b/hw/xfree86/Makefile.am index ab79873..b6264db 100644 --- a/hw/xfree86/Makefile.am +++ b/hw/xfree86/Makefile.am @@ -40,10 +40,11 @@ DIST_SUBDIRS = common ddc i2c x86emu int10 fbdevhw os-support \ utils doc bin_PROGRAMS = Xorg -Xorg_SOURCES = +Xorg_SOURCES = sdksyms.c AM_CFLAGS = $(DIX_CFLAGS) @XORG_CFLAGS@ -INCLUDES = @XORG_INCS@ +INCLUDES = $(XORG_INCS) -I$(srcdir)/parser -I$(top_srcdir)/miext/cw \ + -I$(srcdir)/ddc -I$(srcdir)/i2c -I$(srcdir)/modes -I$(srcdir)/ramdac noinst_LTLIBRARIES = libxorg.la libxorg_la_SOURCES = @@ -113,3 +114,11 @@ xorg.conf.example: xorgconf.cpp relink: $(AM_V_at)rm -f Xorg $(MAKE) Xorg + +CLEANFILES = sdksyms.c sdksyms.dep +EXTRA_DIST += sdksyms.sh + +sdksyms.dep sdksyms.c: sdksyms.sh + CPP='$(CPP)' AWK='$(AWK)' $(srcdir)/sdksyms.sh $(top_srcdir) $(CFLAGS) $(AM_CFLAGS) $(INCLUDES) + +sinclude sdksyms.dep diff --git a/hw/xfree86/loader/.gitignore b/hw/xfree86/loader/.gitignore deleted file mode 100644 index 6b38d9e..000 --- a/hw/xfree86/loader/.gitignore +++ /dev/null @@ -1,3 +0,0 @@ -# Add Override for this directory and it's subdirectories -sdksyms.c -sdksyms.dep diff --git a/hw/xfree86/loader/Makefile.am b/hw/xfree86/loader/Makefile.am index 2bac89a..5ea4c6b 100644 --- a/hw/xfree86/loader/Makefile.am +++ b/hw/xfree86/loader/Makefile.am @@ -9,21 +9,12 @@ AM_CFLAGS = $(DIX_CFLAGS) $(XORG_CFLAGS) EXTRA_DIST = \ loader.h \ - loaderProcs.h \ - sdksyms.sh + loaderProcs.h libloader_la_SOURCES = \ loader.c \ loaderProcs.h \ loadext.c \ loadmod.c \ - os.c \ - sdksyms.c + os.c libloader_la_LIBADD = $(DLOPEN_LIBS) - -CLEANFILES = sdksyms.c sdksyms.dep - -sdksyms.dep sdksyms.c: sdksyms.sh - CPP='$(CPP)' AWK='$(AWK)' $(srcdir)/sdksyms.sh $(top_srcdir) $(AM_CFLAGS) $(CFLAGS) $(INCLUDES) - -sinclude sdksyms.dep diff --git a/hw/xfree86/loader/sdksyms.sh b/hw/xfree86/loader/sdksyms.sh deleted file mode 100755 index f60b8ed..000 --- a/hw/xfree86/loader/sdksyms.sh +++ /dev/null @@ -1,424 +0,0 @@ -#!/bin/sh - -cat sdksyms.c EOF -/* This file is automatically generated by sdksyms.sh. */ -#pragma GCC diagnostic ignored -Wdeprecated-declarations - -#ifdef HAVE_XORG_CONFIG_H -#include xorg-config.h -#endif - - -/* These must be included first */ -#include misc.h -#include miscstruct.h - - -/* render/Makefile.am */ -#include picture.h -#include mipict.h -#include glyphstr.h -#include picturestr.h - - -/* fb/Makefile.am -- module */ -/* -#include fb.h -#include fbrop.h -#include fboverlay.h -#include wfbrename.h -#include fbpict.h - */ - - -/* miext/shadow/Makefile.am -- module */ -/* -#include shadow.h - */ - - -/* miext/damage/Makefile.am */ -#include damage.h -#include damagestr.h - -/* miext/sync/Makefile.am */ -#include misync.h -#include misyncstr.h - -/* Xext/Makefile.am -- half is module, half is builtin */ -/* -#include xvdix.h -#include xvmcext.h - */ -#include geext.h -#include geint.h -#include shmint.h -#include syncsdk.h -#if XINERAMA -# include panoramiXsrv.h -# include panoramiX.h -#endif - - -/* hw/xfree86/int10/Makefile.am -- module */ -/* -#include xf86int10.h - */ - - -/* hw/xfree86/i2c/Makefile.am -- mostly modules */ -#include xf86i2c.h -/* -#include bt829.h -#include fi1236.h -#include msp3430.h -#include tda8425.h -#include tda9850.h -#include tda9885.h -#include uda1380.h -#include i2c_def.h - */ - - -/* hw/xfree86/modes/Makefile.am */ -#include xf86Crtc.h -#include xf86Modes.h -#include xf86RandR12.h -/* #include xf86Rename.h */ - - -/* hw/xfree86/ddc/Makefile.am */ -#include edid.h -#include xf86DDC.h - - -/* hw/xfree86/dri2/Makefile.am -- module */ -/* -#if DRI2 -# include dri2.h -#endif - */ - - -/* hw/xfree86/vgahw/Makefile.am -- module */ -/* -#include vgaHW.h - */ - - -/* hw/xfree86/fbdevhw/Makefile.am -- module */ -/* -#include fbdevhw.h - */ - - -/* hw/xfree86/common/Makefile.am
Re: [PATCH 0/3] Kill libxorg.la
On Tue, Jun 14, 2011 at 9:22 PM, Alan Coopersmith alan.coopersm...@oracle.com wrote: On 06/14/11 08:20 PM, Dan Nicholson wrote: This seems to work. I had to move sdksyms out of libloader and move the order of a couple libraries around, but everything seems to resolve now. It definitely makes a difference. I was going to try testing those to make sure they work with the dtrace special objects, since they modify the build of those, but I can't figure out what baseline to apply them to - they don't seem to fit against either master or server-1.10-branch. Oh, I completely forgot that my tree is way old. Looks like it's xorg-server-1.9.99.903. I'll rebase against master and resend. -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xkeyboard-config] Use XSL to generate man page from the rules XML
On Thu, Jun 9, 2011 at 10:45 PM, Peter Hutterer peter.hutte...@who-t.net wrote: Generate a man-page from the evdev.xml through the xslt/man.xsl stylesheet. Adds a requirement on the xorg util-macros and xsltproc. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- Taking this from a private thread to a public (archived!) list. Last argument of discussion was whether to distribute the man pages with the tarball or let the user build them (adding the requirement of xsltproc to build from the tarball). I don't care either way and Gaetan indicated that there are a few painful points when distributing generate man pages. So I say, screw it, let the users do it. I used to do packaging/building, so a lot of times I come with the view of not requiring extra dependencies for tarball users. Especially for documentation since you really want the users to have the docs. That said, trying to distribute generated files while handling conditional tools adds a lot more cases to handle. Instead of one (you have the tool, so you generate the file), you have four with the combination of tarball vs git and tool vs. no tool. I've thought through how that works before, so I tend to not think it's that difficult. Lately I've come around to the other side more, and these aren't difficult dependencies to fulfill. It doesn't even require a working docbook configuration, which can be a huge PITA. So, I'm OK with keeping it simple and requiring xsltproc to get the man page. The rest of the patch looks nice and even contains some more goodness in the xsl from the version I looked at the other day. The only further improvement I'd suggest is using more embedded groff to format the tables. But that can come later if someone feels like it. Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libXi 2/3] make: remove unneeded AM_V_GEN silent rule directive.
On Thu, Jun 9, 2011 at 1:03 PM, Gaetan Nadon mems...@videotron.ca wrote: It happens to be in the middle of the script statement and cause this incorrect output: rm XCloseDevice.man make GEN XOpenDevice.3 /bin/bash: line 1: @echo: command not found /bin/bash: line 2: @echo: command not found [...] Signed-off-by: Gaetan Nadon mems...@videotron.ca --- man/Makefile.am | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/man/Makefile.am b/man/Makefile.am index c8db111..1b29b72 100644 --- a/man/Makefile.am +++ b/man/Makefile.am @@ -171,8 +171,8 @@ SUFFIXES += .txt .xml # Invoke asciidoc/xmlto main man page generation for shadow pages $(libman_shadows): @if test ! -f $(@:.man=.libmansuffix); then \ - $(AM_V_GEN)rm -f $; \ - $(AM_V_GEN)$(MAKE) $(AM_MAKEFLAGS) $ || exit 1; \ + rm -f $; \ + $(MAKE) $(AM_MAKEFLAGS) $ || exit 1; \ fi $(AM_V_GEN)mv -f $(@:.man=.libmansuffix) $@ endif Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libXi 3/3] make: use AM_V_at rather than AM_V_GEN to prefix the mv command
On Thu, Jun 9, 2011 at 1:03 PM, Gaetan Nadon mems...@videotron.ca wrote: This will prevent outputting a GEN prefix. Moving and removing files is not generating anything. Signed-off-by: Gaetan Nadon mems...@videotron.ca --- man/Makefile.am | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/man/Makefile.am b/man/Makefile.am index 1b29b72..2181ea9 100644 --- a/man/Makefile.am +++ b/man/Makefile.am @@ -166,7 +166,7 @@ SUFFIXES += .txt .xml $(AM_V_GEN)$(ASCIIDOC) -b docbook -d manpage -o $@ $ .xml.man: $(AM_V_GEN)$(XMLTO) man $ - $(AM_V_GEN)mv -f $(@:.man=.libmansuffix) $@ + $(AM_V_at)mv -f $(@:.man=.libmansuffix) $@ # Invoke asciidoc/xmlto main man page generation for shadow pages $(libman_shadows): @@ -174,5 +174,5 @@ $(libman_shadows): rm -f $; \ $(MAKE) $(AM_MAKEFLAGS) $ || exit 1; \ fi - $(AM_V_GEN)mv -f $(@:.man=.libmansuffix) $@ + $(AM_V_at)mv -f $(@:.man=.libmansuffix) $@ endif AM_V_at silences the output unless you pass V=1, right? I don't have the code in front of my. Assuming yes, Reviewed-by: Dan Nicholson dbn.li...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 1/4] xserver: Use lists and constants for matching modes in Match entries
On Sat, Jun 4, 2011 at 10:48 PM, Oleh Nykyforchyn oleh@gmail.com wrote:
Use lists and constants for matching modes in Match entries
You're leaving out some critical information here. Particularly that
each Match entry would take multiple arguments. I'm going to ignore
those details for now since I don't think we've agreed on it.
Signed-off-by: Oleh Nykyforchyn oleh@gmail.com
---
hw/xfree86/common/xf86Xinput.c | 117 +++---
hw/xfree86/parser/InputClass.c | 336
+---
hw/xfree86/parser/xf86Parser.h | 23 +++-
3 files changed, 292 insertions(+), 184 deletions(-)
diff --git a/hw/xfree86/common/xf86Xinput.c b/hw/xfree86/common/xf86Xinput.c
index 26051ad..246108e 100644
--- a/hw/xfree86/common/xf86Xinput.c
+++ b/hw/xfree86/common/xf86Xinput.c
@@ -443,77 +443,84 @@ HostOS(void)
#endif
}
+/*
+ * Match an attribute against a pattern. Matching mode is
+ * determined by pattern-mode member.
+ */
static int
-match_substring(const char *attr, const char *pattern)
+multi_match(const char *attr, xf86MatchPattern *pattern)
I like this function a lot now with the switch, but multi_match may
not be the best name. Maybe match_token? I'm also pretty sure pattern
could be const.
{
- return (strstr(attr, pattern)) ? 0 : -1;
-}
+ if (!pattern) return 0;
I'm not sure others feel strongly about this, but I much prefer
expressions to be on a separate line from the conditional.
+ switch (pattern-mode)
+ {
+ case MATCH_IS_INVALID:
+ return 0;
+ case MATCH_IS_STRCMP:
+ return (strcmp(attr, pattern-str)) ? 0 : -1;
+ case MATCH_IS_STRCASECMP:
+ return (strcasecmp(attr, pattern-str)) ? 0 : -1;
+ /*
+ * If no Layout section is found, xf86ServerLayout.id becomes
(implicit)
+ * It is convenient that in patterns means no explicit layout
+ */
+ case MATCH_IS_STRIMPLICIT:
+ if (*(pattern-str))
+ return (strcmp(attr, pattern-str)) ? 0 : -1;
+ else
+ return (strcmp(attr, (implicit))) ? 0 : -1;
+ case MATCH_IS_STRSTR:
+ return (strstr(attr, pattern-str)) ? -1 : 0;
+ case MATCH_IS_STRCASESTR:
+ return (strcasestr(attr, pattern-str)) ? -1 : 0;
#ifdef HAVE_FNMATCH_H
-static int
-match_pattern(const char *attr, const char *pattern)
-{
- return fnmatch(pattern, attr, 0);
-}
+ case MATCH_IS_FILENAME:
+ return (fnmatch(pattern-str, attr, 0)) ? 0 : -1;
+ case MATCH_IS_PATHNAME:
+ return (fnmatch(pattern-str, attr, FNM_PATHNAME)) ? 0 : -1;
#else
-#define match_pattern match_substring
+ case MATCH_IS_FILENAME:
+ return (strstr(attr, pattern-str)) ? -1 : 0;
+ case MATCH_IS_PATHNAME:
+ return (strstr(attr, pattern-str)) ? -1 : 0;
#endif
-
-#ifdef HAVE_FNMATCH_H
-static int
-match_path_pattern(const char *attr, const char *pattern)
-{
- return fnmatch(pattern, attr, FNM_PATHNAME);
-}
-#else
-#define match_path_pattern match_substring
-#endif
-
-/*
- * If no Layout section is found, xf86ServerLayout.id becomes (implicit)
- * It is convenient that in patterns means no explicit layout
- */
-static int
-match_string_implicit(const char *attr, const char *pattern)
-{
- if (strlen(pattern)) {
- return strcmp(attr, pattern);
- } else {
- return strcmp(attr,(implicit));
+ default:
+ /* Impossible */
}
}
/*
- * Match an attribute against a list of NULL terminated arrays of patterns.
- * If a pattern in each list entry is matched, return TRUE.
+ * Match an attribute against a list of xf86MatchGroup's.
+ * Return TRUE only if each list entry is successful.
*/
static Bool
-MatchAttrToken(const char *attr, struct list *patterns,
- int (*compare)(const char *attr, const char *pattern))
+MatchAttrToken(const char *attr, struct list *groups)
{
- const xf86MatchGroup *group;
+ xf86MatchGroup *group;
+ xf86MatchPattern *pattern;
- /* If there are no patterns, accept the match */
- if (list_is_empty(patterns))
+ /* If there are no groups, accept the match */
+ if (list_is_empty(groups))
return TRUE;
- /* If there are patterns but no attribute, reject the match */
+ /* If there are groups but no attribute, reject the match */
if (!attr)
return FALSE;
/*
- * Otherwise, iterate the list of patterns ensuring each entry has a
- * match. Each list entry is a separate Match line of the same type.
+ * Otherwise, iterate the list of groups ensuring each entry has a
+ * match. Each list entry is a list of patterns obtained from
+ * a separate Match line.
*/
- list_for_each_entry(group, patterns, entry) {
- char * const *cur;
+ list_for_each_entry(group, groups, entry) {
Re: Build fixes (was Anything more for xproto 7.0.22?)
On Jun 5, 2011 4:48 AM, Julien Cristau jcris...@debian.org wrote: On Sat, Jun 4, 2011 at 09:25:07 -0400, Gaetan Nadon wrote: The DocBook/XML olink databases (this is what those files are) are installed in the location provided by the specs/Makefile variable sgmldbsdir which is set to $(XORG_SGML_PATH)/X11/dbs. The value of XORG_SGML_PATH is obtained from the macro XORG_CHECK_SGML_DOCTOOLS (see config.log). The macro obtains the value from pkg-config --variable=sgmlrootdir xorg-sgml-doctools. Perhaps the xorg-sgml-doctools was installed in /usr/share. That needs to be overridden for distcheck. One of the requirements of distcheck is that everything is installed under $prefix (which is different from xorg-sgml-doctools' prefix). So there needs to be a --with-sgml-path=foo override for the detected path. Yeah, see evdev for instance overriding the server's header location during distcheck. Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] Multiple matching modes (incl. regex) selection support in Match* statements
On Fri, Jun 3, 2011 at 1:30 AM, Peter Hutterer peter.hutte...@who-t.net wrote: On Thu, Jun 02, 2011 at 05:53:14PM +0300, Oleh Nykyforchyn wrote: Please consider a final version of the patch. --- Multiple matching modes selection (incl. regex) support in Match* statements Any number of patterns can be written in one line either as MatchProduct foo|!bar something|re:^else$ you !regex:n..d or as MatchProduct foo !bar|something regex:^else$ you|!re:n..d etc. A regex ends at the end of the respective piece, irrespective of '|'s. Regexes are prefixed by regex: of re:, prefixes are also added for all other matching modes in two versions each: short and real name of the used function. Negated conditions are prefixed by !, which can be combined with each other prefix. Different modes, as well as usual and negated patterns, can be mixed in one line. whoa, this was a bit scary to look at at first :) a few high-level comments, the patch itself looks sane on a first glance. - let's just use regex:, not either of regex: or re:. one is enough. - I'm not a big fan of allowing a mix of multiple arguments and |. We could just require escaping | in a regex so that the result is a MatchProduct foo|!bar|regex:^blah$|regex:a\|b imo this is much easier to read than a multi-argument mix like your two examples above. Even though I've argued repeatedly that having an additional argument is the way to go, I would agree that an arbitrary amount of arguments doesn't make this easier to understand. However, I think this will get much more complicated if you want to get into the escaping game. Then you're building a full out parser instead of just strtok and strncmp. I think the only simple (to code and to understand by the user) is just to say that you can either have a simple match separated by '|' or a regex match. If you need both, you need a new InputClass section. - I don't think we should expose the multi-match modes. As I said in a previous email - if we have true regex support I think we can emulate all of them with a regex. fnmatch etc. will still need to be used internally for the native matching mode. I would strongly suggest that we not try to emulate simple matches with regex. Are you going to try to escape all the special characters before passing to regcomp? That again puts you in the land of building your a parser. I'd argue wrapping strcmp/strstr/etc to make the return codes match is far simpler than trying to get regex to match their behavior. And if you're arguing that keeping fnmatch is necessary, then you already have to separate regex from non-regex. - man page additions please Agree completely here. This is getting pretty complex, and I'm really curious to see it explained in a manual for people coming in fresh. Sorry to be harsh, but I'm having a hard time seeing how these proposals will improve the situation and not break current setups. -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: macros: Changes to 'master'
On Wed, Jun 1, 2011 at 5:34 AM, Mark Kettenis mark.kette...@xs4all.nl wrote: From: Gaetan Nadon mems...@videotron.ca Date: Wed, 01 Jun 2011 07:42:32 -0400 AC_LANG_DEFINES_PROVIDED seems to be only in newer, GPLv3, versions of au= toconf ... is there a way around this? I looked at it yesterday, using AC_LANG_SOURCE should be the right thing to do (some examples in the server). I can submit a patch later today if no one else beats me to it. You can reset macros a couple of commits back to get the build going. Not related to this problem. but can you elaborate on the issue of GPLv3 of autoconf? Many people object to the version 3 of the GPL. This includes companies like Apple, projects like FreeBSD and OpenBSD, the primary author of the Linux kernel, etc. etc, each for their own reason. My personal reason is that I don't understand the license anymore. GPLv2 is written in normal language; GPLv3 is full of lawyer speak. This is just in the license of the build infrastructure, and doesn't extend to the code it's being used with. Do you object to installing newer GPLv3 versions of autoconf or are you worried it will infest the code? -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] regular expression support (was Re: [PATCH xfree86] Signed-off-by: Oleh Nykyforchyn)
On Thu, May 26, 2011 at 10:56 PM, Peter Hutterer
peter.hutte...@who-t.net wrote:
On Wed, May 25, 2011 at 03:08:32PM +0300, Oleh Nykyforchyn wrote:
On Wed, 25 May 2011 16:14:42 +1000
Peter Hutterer peter.hutte...@who-t.net wrote:
I believe | is used frequently in regular expressions, so using it as
decision over regex or simple is dangerous. we can't use slash either,
there
are likely a few configurations out there already.
My suggestion is simply prefixing with re:. I guess there's very few
configurations out there that need exactly that match and it makes it very
clear that this match is a regular expression
MatchProduct re:[^ a-z]{4}|(x|y)
Nice, short and intuitive. I modified my patch to use this idea, and the code
is simpler and clearer now. A piece of Xorg.0.log and the result is below:
[ 2959.284] (II) config/udev: Adding input device Power Button
(/dev/input/event2)
[ 2959.285] (**) Applying regex .implicit. to attribute Radeon
[ 2959.285] (**) Applying regex ^R.d[a-z]*$ to attribute Radeon
[ 2959.285] (**) Applying regex .implicit. to attribute Radeon
[ 2959.285] (**) Applying regex ^R.d[a-z]*$ to attribute Radeon
[ 2959.285] (**) Power Button: Applying InputClass Keyboard0
Statements were:
MatchLayout re:.implicit.
MatchLayout re:^R.d[a-z]*$
To tell the truth, I have no idea why each match is repeated twice, but it
works.
The patch itself:
---
Add option to use extended regex expresssions in Match* statements
If a pattern list is of the form re:str, then str is treated
as a single extended regular expression
Signed-off-by: Oleh Nykyforchyn oleh@gmail.com
---
hw/xfree86/common/xf86Xinput.c | 25 +++
hw/xfree86/parser/InputClass.c | 53
+---
2 files changed, 69 insertions(+), 9 deletions(-)
diff --git a/hw/xfree86/common/xf86Xinput.c b/hw/xfree86/common/xf86Xinput.c
index 88cf292..0b90b5b 100644
--- a/hw/xfree86/common/xf86Xinput.c
+++ b/hw/xfree86/common/xf86Xinput.c
@@ -85,6 +85,9 @@
#include ptrveloc.h /* dix pointer acceleration */
#include xserver-properties.h
+#include sys/types.h /* MatchAttrToken */
+#include regex.h
+
#ifdef XFreeXDGA
#include dgaproc.h
#endif
@@ -509,6 +512,28 @@ MatchAttrToken(const char *attr, struct list *patterns,
char * const *cur;
Bool match = FALSE;
+ cur = group-values;
+ if ((cur[0]) (!*cur[0]) /* cur[0] == , can never come out
of strtok() */
+ (cur[1]) /* cur[1] == regex */
+ (cur[2] == NULL)) {
urgh, this seems a bit hacked on. any chance we can have a define, enum,
special return value so we don't rely on magic value combinations?
patterns is a xf86MatchGroup struct, we could extend this with a enum type {
MATCH_TYPE_NORMAL, MATCH_TYPE_REGEX } or somesuch.
+ /* Treat this statement as a regex match */
+ regex_t buf;
+ int r;
+ r = regcomp(buf, cur[1], REG_EXTENDED | REG_NOSUB);
+ if (r) { /* bad regex */
+ regfree(buf);
+ xf86Msg(X_ERROR, Wrong regex: \%s\\n, cur[1]);
+ return FALSE;
+ }
+ xf86Msg(X_CONFIG, Applying regex \%s\ to attribute
\%s\\n, cur[1], attr);
+ r = regexec(buf, attr, 0, NULL, 0);
+ regfree(buf);
+ if (r)
+ return FALSE;
+ else
+ return TRUE;
+ }
+
for (cur = group-values; *cur; cur++) {
if (**cur == '!') {
/*
diff --git a/hw/xfree86/parser/InputClass.c b/hw/xfree86/parser/InputClass.c
index 3f80170..78a9446 100644
--- a/hw/xfree86/parser/InputClass.c
+++ b/hw/xfree86/parser/InputClass.c
@@ -66,6 +66,39 @@ xf86ConfigSymTabRec InputClassTab[] =
#define TOKEN_SEP |
+/*
+ * Tokenize a string into a NULL terminated array of strings. The same that
+ * xstrtokenize unless the string starts with re:, then the rest of
+ * the string is treated as a single extended regex, and
+ * {, regex, NULL} is returned.
+ */
+static char**
+rstrtokenize(const char *str, const char *separators)
+{
+ if (!str)
+ return NULL;
+ if (!strncmp(str,re:,3)){
+ char **list;
+
+ list = calloc(3, sizeof(*list));
+ if (!list)
+ return NULL;
+ list[0] = strdup();
+ list[1] = calloc(strlen(str)-2, sizeof(char));
+ if (!list[0] || !list[1])
+ goto error;
+ strcpy(list[1], str+3);
list[1] = xasprintf(%s, str + 3)
would be simpler here.
Dan, any comments?
I think at this point I've made myself pretty clear what I think about
this idea. I don't think I need to repeat myself again.
For the actual code, I would agree that extending the xf86MatchGroup
with a type member would be better than strangely manipulating the
Re: [PATCH xfree86] Signed-off-by: Oleh Nykyforchyn oleh....@gmail.com
On Tue, May 24, 2011 at 8:42 AM, Oleh Nykyforchyn oleh@gmail.com wrote: On Tue, 24 May 2011 05:54:33 -0700 Dan Nicholson dbn.li...@gmail.com wrote: The man page does say what type of matching each uses, but it's still not that discoverable. You are right, it's my fault that I missed this info. What do You think about such an approach? Frankly, I'm not crazy about it for two reasons. First, it's another magic sequence to parse out and handle and attempt to explain to people. The |regex| syntax is something new that no one would know about. The only regex syntax I can think of that applies and people are familiar with is awk's /regex/, and the /s would conflict with other valid uses of the argument. We can point everybody to 'man 7 regex'. No, it's not the syntax of the regex I'm worried about. If you know what regex means, I don't need to tell you how to build one. I'm talking about the syntax that tells the server here's a regex and not an innocent string you've been accepting for a while, really. That's the part where you guys are proposing putting magic characters in the string and I'm saying that's silly because we can just have another argument that says regex and be completely unambiguous about it. Second, what if I as a user have the reasonable thought that I want to have multiple regexs to match against like the non-regex matching? MatchProduct foo|bar works, but why not MatchProduct |foo|bar|? foo|bar is a pretty legal regex, hence the both variants are equivalent. Except the first is not supposed to be a regex! It's perfectly legal right now to put either of those in a configuration file. What they mean is that the product name should have either foo or bar in it. Not to mention that you can currently have |foo| and the code will just treat it as a single normal argument (run that through strtok with '|' as the delimiter). You are right again. But my suggestion is to remove outmost '|' _before_ running strtok(). Yes, that's how you would parse it. Unfortunately, someone might have |/foo| in their working configuration right now and suddenly it will mean something completely different. -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xfree86] Signed-off-by: Oleh Nykyforchyn oleh....@gmail.com
On Mon, May 23, 2011 at 7:26 AM, Oleh Nykyforchyn oleh@gmail.com wrote: On Mon, 23 May 2011 05:54:55 -0700 Dan Nicholson dbn.li...@gmail.com wrote: The whole reason why the current setup is weird is that it's completely implicit what type of matching happens in each statement. MatchProduct is string comparison (case sensitive) while MatchDevicePath is a path pattern. I agree that this is a problem. Nobody who has not read the code can not know that for Product and Vendor we look for a substring, for DevicePath, PnPID and USBID a path pattern is matched etc, although these choices probably are optimal. My suggestion is the following: 1) preserve the existing behaviour, but clarify it in docs, including man page; The man page does say what type of matching each uses, but it's still not that discoverable. In your suggestion, how is the regex specified? If you say another sentinel character, how is that less complex than adding an argument to specify what the other string means? We're again making up our own syntax. 2) [already posted, but repeated] BTW, if we adopt two modes: simple |-separated sequence and regex, we don't even need an optional argument. Consider e.g. a string of the form |regex|. It is useless in the simple mode, hence regex is recognized and two |-s are dropped. Syntax for regex inside, e.g., can be compatible with regex library, so we do not invent anything. People who do not use new regex syntax simply do not notice anything. Of course, there is also nothing bad with optional regex keyword, I am just not sure that strcmp, strstr etc are necessary. BTW, somebody can also want to mix strstr and strcmp in one pattern sequence, then an optional argument also becomes a sequence, and so on. What do You think about such an approach? Frankly, I'm not crazy about it for two reasons. First, it's another magic sequence to parse out and handle and attempt to explain to people. The |regex| syntax is something new that no one would know about. The only regex syntax I can think of that applies and people are familiar with is awk's /regex/, and the /s would conflict with other valid uses of the argument. Second, what if I as a user have the reasonable thought that I want to have multiple regexs to match against like the non-regex matching? MatchProduct foo|bar works, but why not MatchProduct |foo|bar|? Not to mention that you can currently have |foo| and the code will just treat it as a single normal argument (run that through strtok with '|' as the delimiter). To put it in programming terms, which of the following two APIs would you want to use? 1. Single overloaded argument which changes behavior based on certain values 2. Optional second argument that specifies the desired behavior I'm pretty sure I'm going with #2. I can't see a reason why put in magic characters to get a regex is better than add an argument specifying a regex. What am I missing? -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xfree86] Signed-off-by: Oleh Nykyforchyn oleh....@gmail.com
On Sun, May 22, 2011 at 11:24 PM, Oleh Nykyforchyn oleh@gmail.com wrote: On Mon, 23 May 2011 12:55:20 +1000 Peter Hutterer peter.hutte...@who-t.net wrote: right, I remember now. my main grief was that it would have been another rather specific syntax instead of the quite common s/foo/bar. i don't have a problem with regex per-se as long as we can get the syntax sorted out. It seems to me that supporting multiple comparison modes like strcmp, strcasecmp, fnmatch, regex etc through an optional argument would add more complexity than will be used even in rather weird setups. What about two modes: simple |-separated sequence, with hardcoded comparison functions, as it is now, and single regex per statement, which covers all special cases? The whole reason why the current setup is weird is that it's completely implicit what type of matching happens in each statement. MatchProduct is string comparison (case sensitive) while MatchDevicePath is a path pattern. In your suggestion, how is the regex specified? If you say another sentinel character, how is that less complex than adding an argument to specify what the other string means? We're again making up our own syntax. -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] config/udev: add input subsystem filter for udev events
On Sun, May 22, 2011 at 9:00 AM, Daniel Kurtz djku...@google.com wrote: Thanks for the pointers. On Fri, May 20, 2011 at 11:55 AM, Peter Hutterer peter.hutte...@who-t.net wrote: On Thu, May 19, 2011 at 09:58:59PM +0200, Julien Cristau wrote: On Thu, May 19, 2011 at 16:22:22 +0800, Daniel Kurtz wrote: libudev allows adding an efficient in-kernel filter on udev events. Since we manually filter for ID_INPUT anyway, it seems reasonable to ask the kernel to send only events for the input subsystem. This filtering is performed both for initial device enumeration, and for subsequent udev events. Signed-off-by: Daniel Kurtz djku...@google.com --- config/udev.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) Pretty sure we had that initially and it was removed. We did. thread here http://lists.x.org/archives/xorg-devel/2010-January/004596.html commit 84905007702da2c05a4f7446b3fc5ff52be49655 Author: Thomas Jaeger Date: Mon Jan 4 15:00:49 2010 -0500 udev: Don't filter subsystem input This allows serial wacom devices to work, whose subsystem is tty. The following looks like it was submitted by Thomas, and adds tty filter as was recommended in that thread: http://lists.x.org/archives/xorg-devel/2010-February/005614.html But, AFAICT, it was lost. I don't see any review comments, nor do I see it in the tree. Does anybody remember why this was not accepted? Right, that was before he sent the don't filter on input patch. It was decided that instead of trying to filter on subsystems, we'd just let the filtering happen on ID_INPUT* being set in udev rules. No one was really sure what all the input device providing subsystems were. Are you seeing issues with this? -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xfree86] Signed-off-by: Oleh Nykyforchyn oleh....@gmail.com
On Mon, May 16, 2011 at 11:14 PM, Peter Hutterer peter.hutte...@who-t.net wrote: On Mon, May 16, 2011 at 09:15:51PM +0300, Oleh R. Nykyforchyn wrote: xfree86: allow negative conditions in Match* statements Match statement syntax is extended to allow strings like: aaa,!a,bbb,!b,ccc,!c Match succeedes if an attribute matches aaa, bbb, or ccc, or does not match neither a, b, or c. Signed-off-by: Oleh Nykyforchyn oleh@gmail.com this needs an entry in the man page. I also wonder how long it will take us until we need full regex support... We had this conversation before. I had a patch where you specified an optional third argument stating what type of match you wanted. It was then a trivial patch to add a regex match type. You weren't a huge fan, though. :) http://lists.x.org/archives/xorg-devel/2010-December/016369.html I was reworking it so that it accepted programmer names like strcmp in addition to string, but life has gotten in the way quite a bit. I still think that would be a useful addition. These changes that Oleh is proposing seem good to me at a glance. You already merged them, but here's a belated: Acked-by: Dan Nicholson dbn.li...@gmail.com -- Dan ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
