GNOME Bugzilla – Bug 670685
Configure finds python modules which aren't installed
Last modified: 2012-02-26 14:23:30 UTC
Created attachment 208269 [details] [review] Trivial patch to fix incorrect python module checks (false positives) In brief, s/Gtk/GObject/ in acinclude.m4 Now for the lengthy explanation. configure uses AM_CHECK_PYMOD to check for essential modules such as dbus, json, cairo etc. AM_CHECK_PYMOD is defined in acinclude.m4 The principle is simple: from within python try to load the module, and if an ImportError exception is thrown, decide that the module isn't available. At some stage, "check we are using py-gobject 3 with introspection rather than pygtk or other old py-gobject" was added to the test, which now reads something like import sys try: from gi.repository import Gtk <-- line to check for py-gobject with introspection XXX import json except ImportError: sys.exit(1) except: sys.exit(0) sys.exit(0) For the purposes of the py-gobject+introspection test, any module could have been chosen. The problem with choosing Gtk, rather than e.g., the much simpler GObject, is that the Gtk module will attempt to find an X display. Try it for yourself: - ssh to some remote machine - su root (this is common if you have a remote build machine on which you want to build orca) - xterm (this should fail - it shouldn't be able to open on your display) - run python - from gi.repository import GObject (should work if you have everything installed) - from gi.repository import Gtk (should fail with something like a RuntimeError Gtk couldn't be initialised, or sometimes a Protocol not specified from the underlying XLib) So, the import of Gtk fails because you don't have X setup to the build machine. This means that an exception is thrown, and the test says "I didn't get the ImportError exception => the json module must exist" (or whichever module) A load of the module (eg json) wasn't even tried in this scenario.
I'm pleased to see that pyatspi2 doesn't suffer from this disease... I'll commit this patch in a few days...
> I'll commit this patch in a few days... If I said the same thing about dasher, what would be your reaction? Having said that, go ahead.
I would be happy that someone actually worked on dasher other than just me! (And if you look in the dasher patch queue, the patch which is stagnating there looks like a s/old function name/new function/g - none of the analysis and proof presented here.) m4 / configure problems tend not to be developers' speciality. Maybe I could have carte blanche to commit such patches? (Not orca changes, just build changes - as I do for at-spi2-* and pyatspi2) Applied in http://git.gnome.org/browse/orca/commit/?id=0c0be16f1ab5ce6a48070bbc3fb78ad9df7fcdaf
(In reply to comment #3) > m4 / configure problems tend not to be developers' speciality. Maybe I could > have carte blanche to commit such patches? I'd prefer you check rather than surprise me. Also, since I'd like to be a better developer.... Why is your check not redundant? There is already a line in configure.ac which checks for pygojbect: PKG_CHECK_MODULES([PYGOBJECT], [pygobject-3.0 >= pygobject_required_version]) Please explain why the real fix is not to remove it completely from acinclude.m4.
I agree that the check is redundant. I suppose that the philosophy when that line was added was that the check for an introspection-capable pygobject would occur whether or not the line you quote was added to the configure.ac. I think that even better would be to use the m4/python.m4 file straight from pygobject, rather than use a modified copy from acinclude.m4. As you already use MACRO_DIR, copying it into m4 would work. That would make getting future changes from pygobject easier.
(In reply to comment #5) > I agree that the check is redundant. Shall we remove it then? > I suppose that the philosophy when that > line was added was that the check for an introspection-capable pygobject would > occur whether or not the line you quote was added to the configure.ac. Actually, to suggest there is associated philosophy would be too much credit. ;) The line was added in the pygtk days, converted to pygi, and then promptly forgotten about. The configure.ac line was added much more recently to ensure users had the version we expected. > I think that even better would be to use the m4/python.m4 file straight from > pygobject, rather than use a modified copy from acinclude.m4. As you already > use MACRO_DIR, copying it into m4 would work. That would make getting future > changes from pygobject easier. That sounds promising. Thanks for the tip! I can look into it once I get caught up on other things. Or if you have a patch to propose, I would be happy to see it.