OpenLP 2.0 uses uno.py to access LibreOffice. This worked fine for the 3.x series but for the 4.x this is crashing the OpenLP code. The problem is that uno.py overrides the default import method and then triggers errors within the normal python code. A local hack has been found to work but this is not a production fix. In the _uno_import method comment out the exception raise! RuntimeException = pyuno.getClass( "com.sun.star.uno.RuntimeException" ) for x in fromlist: if x not in d: if x.startswith( "typeOf" ): try: d[x] = pyuno.getTypeByName( name + "." + x[6:len(x)] ) except RuntimeException as e: #raise ImportError( "type " + name + "." + x[6:len(x)] +" is unknown" ) pass This was causing code (not related to the libreoffice integration to fail.
Removing MAB - MAB MUST be confirmed and must be done independent of the original reporter unless your a developer on the project.
I was requested to add the mad4.1 by Michael Meeks
Indeed the code hasn't been changed for the moment, see http://opengrok.libreoffice.org/xref/core/pyuno/source/module/uno.py#281 Michael: do you confirm the patch? I could push the change but I don't know what could be the sideeffects.
The patch was a hack to see if we could get the code working. I would not confirm that it is correct or what the side effects would be. All I can confirm is there is now a breakage in functionality!
(In reply to comment #4) > The patch was a hack to see if we could get the code working. > I would not confirm that it is correct or what the side effects would be. > > All I can confirm is there is now a breakage in functionality! Thank you for your feedback Tim. Indeed the question was for Michael. Could you reproduce the crash by adding some traces before this line? d[x] = pyuno.getTypeByName( name + "." + x[6:len(x)] ) It could be very helpful to know the value of: -name -x at the moment of the crash.
Created attachment 81214 [details] Run Log file This log file shows the exceptions being called and the back trace.
Created attachment 81215 [details] The value of name and x before the exception was raised. httpserver is part of Cherrypy openlp classes are all our code.
Tim: I know almost nothing in Python but perhaps you should start with these first errors in the logs: 2013-06-22 14:28:04,108 openlp.core.lib.pluginmanager DEBUG Importing songsplugin from /home/tim/Projects/openlp/python3-productive/openlp/plugins/songs. Depth 10 2013-06-22 14:28:04,114 openlp.core.lib.pluginmanager ERROR Failed to import module songsplugin on path /home/tim/Projects/openlp/python3-productive/openlp/plugins/songs/songsplugin.py: type openlp.core.lib.db.Manager is unknown Traceback (most recent call last): File "/usr/lib64/python3.3/site-packages/uno.py", line 298, in _uno_import d[x] = pyuno.getClass( name + "." + x ) uno.com.sun.star.uno.RuntimeException: pyuno.getClass: uno exception openlp.core.lib.db.Manager is unknown Anyway, you must know that LO has included Python 3.3 since LO 4.0 version. Is openlp code compliant with Python 3.3? If not perhaps this thread may help: http://nabble.documentfoundation.org/ANNOUNCE-Python-3-3-is-now-bundled-with-LibreOffice-4-0-td4021038.html, I quote: "however for a transition period of a few releases it will still be possible to use a Python 2 version 2.6 or later, by explicitly setting PYTHON, PYTHON_CFLAGS, PYTHON_LIBS variables. "
Just thought there could be a link with fdo#66031 put in see also.
Ah - in the future please put that in comments so QA knows, else we assume that you're just a user trying to push their own bug without following workflow. Orders from Michael definitely valid enough to put on MAB but the bug should still be confirmed first
(In reply to comment #8) > Tim: I know almost nothing in Python but perhaps you should start with these > first errors in the logs: > 2013-06-22 14:28:04,108 openlp.core.lib.pluginmanager > DEBUG Importing songsplugin from > /home/tim/Projects/openlp/python3-productive/openlp/plugins/songs. Depth 10 > 2013-06-22 14:28:04,114 openlp.core.lib.pluginmanager > ERROR Failed to import module songsplugin on path > /home/tim/Projects/openlp/python3-productive/openlp/plugins/songs/ > songsplugin.py: type openlp.core.lib.db.Manager is unknown > Traceback (most recent call last): > File "/usr/lib64/python3.3/site-packages/uno.py", line 298, in _uno_import > d[x] = pyuno.getClass( name + "." + x ) > uno.com.sun.star.uno.RuntimeException: pyuno.getClass: uno exception > openlp.core.lib.db.Manager is unknown > > Anyway, you must know that LO has included Python 3.3 since LO 4.0 version. > Is openlp code compliant with Python 3.3? If not perhaps this thread may > help: > http://nabble.documentfoundation.org/ANNOUNCE-Python-3-3-is-now-bundled-with- > LibreOffice-4-0-td4021038.html, I quote: > "however for a transition period of a few releases it will still be > possible to use a Python 2 version 2.6 or later, by explicitly setting > PYTHON, PYTHON_CFLAGS, PYTHON_LIBS variables. > " Fedora only packages the 3.x version and we are trying to upgrade the OpenLP programme from python 2 to python 3. The code has been working fine with LO 3.5/6 but these errors started when we swapped to python 3.
Tim: I think you need to keep on the Python3 migration since LO uses it + Fedora doesn't seem to offer Python 2 (according what you said). I noticed you use pylint in this page http://wiki.openlp.org/Development:Cleanup pep8 might help you too. If you need specific help for the interface with LO, don't hesitate to email the dev mailing list (see http://nabble.documentfoundation.org/Dev-f1639786.html) Michael/Joel: At this point, I don't know if this tracker shouldn't be considered as "NOTOURBUG". What do you think?
Hi guys, Yes - I did ask for this to be a MAB, but I guess we need more details here. http://docs.python.org/3/reference/import.html#replacing-the-standard-import-system Seems to suggest that it is correct for an __import__ hook to raise ImportError to allow processing of other hooks to continue - but I'm no expert here - I CC David Bolen who is though :-) Tim - the values of name and 'x' - don't seem to start with 'typeOf': if x.startswith( "typeOf" ): try: d[x] = pyuno.getTypeByName( name + "." + x[6:len(x)] ) except RuntimeException as e: raise ImportError( "type " + name + "." + x[6:len(x)] +" is unknown" ) So I'm a tad confused by the first fragment. I guess that it comes from: except RuntimeException as e3: # no known uno type ! raise ImportError( "type "+ name + "." +x + " is unknown" ) ie. this is not an UNO type: which is fine of course - and AFAICS we should throw that exception. That is presumably creates noise on the console - but ... it's unclear to me how to do better :-) [ but I'm no python guy ].
Well, expert is a bit strong, but I do at least have some Python/LibO overlap... Michael: The doc reference you made to raising ImportError to *prevent* further processing, but that paragraph also only applies to individual loaders on the meta path, whereas uno is doing a wholesale import replacement (the second paragraph in that section). If it was being implemented now, an import loader as opposed to wholesale __import__ replacement would be cleaner (and avoid uno having to be concerned with default import behavior), but such a change now would require some caution to ensure all existing behavior is maintained, so the cost/benefit tradeoffs are unclear. Tim: At first glance, I'm thinking the issue is not necessarily inside uno per se, but at the very least it's masking the original problem. I'm guessing there is something causing a regular ImportError outside of LibreOffice in your environment for these imports. While uno does do a wholesale import override, the first thing it always does is use the default Python import loader to try to process the import and only does extra processing if that fails. I agree with Michael that your tracebacks (and name/x debug output) don't appear to match the location in your workaround patch. But I don't think that's critical, since from what I can see, it appears uno.py can only reach any of those exceptions if the regular Python import machinery (via the _g_delegatee call to the original __import__ at the top of _uno_import) itself has raised an ImportError. In such a case, the issue is then that any usage of the form "from xxx import yyy" must match a pre-existing imported module, or is assumed to be uno related, with corresponding exceptions. For example: > /opt/libreoffice4.1/program/python Python 3.3.0 (default, Jun 18 2013, 17:24:23) [GCC 4.1.2 20080704 (Red Hat 4.1.2-54)] on linux Type "help", "copyright", "credits" or "license" for more information. (... first without uno ...) >>> import foo Traceback (most recent call last): File "<stdin>", line 1, in <module> ImportError: No module named 'foo' >>> from foo import bar Traceback (most recent call last): File "<stdin>", line 1, in <module> ImportError: No module named 'foo' (... next, with uno import override ...) >>> import uno >>> import foo Traceback (most recent call last): File "<stdin>", line 1, in <module> File "./uno.py", line 265, in _uno_import return _g_delegatee( name, *optargs, **kwargs ) ImportError: No module named 'foo' (... fails, but within the uno import function ...) >>> from foo import bar Traceback (most recent call last): File "./uno.py", line 292, in _uno_import d[x] = pyuno.getClass( name + "." + x ) uno.RuntimeException: pyuno.getClass: uno exception foo.bar is unknown During handling of the above exception, another exception occurred: Traceback (most recent call last): File "./uno.py", line 296, in _uno_import d[x] = Enum( name , x ) File "./uno.py", line 123, in __init__ pyuno.checkEnum( self ) uno.RuntimeException: enum foo is unknown During handling of the above exception, another exception occurred: Traceback (most recent call last): File "./uno.py", line 300, in _uno_import d[x] = getConstantByName( name + "." + x ) File "./uno.py", line 48, in getConstantByName return pyuno.getConstantByName( constant ) uno.RuntimeException: foo.bar During handling of the above exception, another exception occurred: Traceback (most recent call last): File "<stdin>", line 1, in <module> File "./uno.py", line 303, in _uno_import raise ImportError( "type "+ name + "." +x + " is unknown" ) ImportError: type foo.bar is unknown >>> Without knowing the original exception, it's hard to say whether uno is involved in the issue. If you were to dump the ImportError exception from the original Python import call, you might get a hint as to the underlying failure, e.g.: *** uno.py.orig 2013-06-18 18:53:44.000000000 -0400 --- uno.py 2013-06-27 16:32:03.000000000 -0400 *************** *** 269,273 **** --- 269,278 ---- if not fromlist: raise + print("--- Python import error --") + import traceback + traceback.print_exc() + print("--- Python import error --") + modnames = name.split( "." ) mod = None d = sys.modules (Tweak as needed to get output in a way you can see it in your environment) Alternatively, making uno's custom import processing more selective could help prevent masking non-uno imports, as in: --- uno.py.orig 2013-06-18 18:53:44.000000000 -0400 +++ uno.py 2013-06-27 17:08:26.000000000 -0400 @@ -266,7 +266,7 @@ except ImportError: # process optargs globals, locals, fromlist = list(optargs)[:3] + [kwargs.get('globals',{}), kwargs.get('locals',{}), kwargs.get('fromlist',[])][len(optargs):] - if not fromlist: + if not (name.startswith('com.sun.star.') and fromlist): raise modnames = name.split( "." ) mod = None I'm reasonably confident all uno imports will start with com.sun.star - certainly enough for this level of testing. There may still be a dependency on some uno behavior that triggers or contributes to the issue, but seeing the underlying ImportError is probably a good next step towards figuring out where to look.
I add Michael Stahl into CC who did the migration to python 3.3 in LO. He might have some ideas. Anyway, the problem is specific to an extension. The initial comment suggests that it happens already with LO 4.0 but nobody reported it earlier. Such bugs can't block the release => lowering the severity a bit.
@ David Bolen: I applied your changes to uno.py. *** uno.py.orig 2013-06-18 18:53:44.000000000 -0400 --- uno.py 2013-06-27 16:32:03.000000000 -0400 *************** *** 269,273 **** --- 269,278 ---- if not fromlist: raise + print("--- Python import error --") + import traceback + traceback.print_exc() + print("--- Python import error --") + modnames = name.split( "." ) mod = None d = sys.modules The output: http://pastebin.com/T6mEB519 And for completeness our debug log: http://pastebin.com/9zYJp6cR Currently I have the second changed applied to be able to continue our python 2/3 migration.
Tim: Ok, so at first glance I'm guessing that somewhere in your code base there's something like "from pygments.lexers.web import HtmlLexer" which is either (a) new, (b) used to handle the case of a missing pygments module but isn't under LO4+ or (c) the port to Py3 introduced a bug in handling the missing module case that interacts badly with existing LO behavior. While LO (after an uno import) clearly affects the exact exception instance raised, in testing each of 3.6.6, 4.0.4 and 4.1.0, I don't really see any material difference in LO's behavior in this regard. In each case the final exception remains an ImportError. That is, code such as: try: from foo import bar except ImportError: pass seems to work fine in plain Python, and each LO release with or without uno imported. It is true that the text message in the exception differs from a regular Python ImportError, and also that with the shift to Py3 the exception has a "msg" attribute rather than "message". But I wouldn't normally expect any parent code trying to trap an optional module import to depend on that (or if it did, it would be fragile itself across Python versions). Can you perhaps locate the code attempting the import to see how it might be trying to trap the import failure (either itself or through a parent handler up the call chain)? I've downloaded the OpenLP source for 2.0.1, the latest 2.2.2 and a python3-productive branch I found on launchpad, and can't seem to find any reference to pygments anywhere in the source tree? Others: It does feel wrong that simply importing uno changes future import errors (even if just subtly) from statements of the form of "from xxx import yyy". I did do some experiments switch to an import loader instead, but realized that the semantics differ enough as to potentially be an compatibility issue. That is, the current implementation has characteristics of (a) working without needing intermediate "com", "com.sun", and "com.sun.star" modules and (b) working even if some other modules of those names are already imported. The second case is particularly hard to do with a loader. So sticking with the global import hook replacement seems safer. However, I think my earlier patch at the end of comment 14 (which Tim is using) to at least constrain the namespace under which the uno-specific code runs seems like a plausible change, unless anyone is aware of anything accessed via uno not being under the "com.sun.star" namespace.
We / I have never used pygments. I have reinstalled libreoffice-pyuno to get a clean version and added the traceback code. At that point OpenLP creashed looking for pygments so I needed to install that. The it asked for PIL. the last dump I got on a clean install running python 3 is 21:02:53,275 openlp.core.lib.pluginmanager INFO bootstrap_initialise 2013-07-18 21:02:53,276 openlp.core.lib.pluginmanager INFO Finding plugins 2013-07-18 21:02:53,276 openlp.core.lib.pluginmanager DEBUG finding plugins in /home/tim/Projects/openlp/python3-run/openlp/plugins at depth 8 2013-07-18 21:02:53,276 openlp.core.lib.pluginmanager DEBUG Importing remoteplugin from /home/tim/Projects/openlp/python3-run/openlp/plugins/remotes. Depth 10 2013-07-18 21:02:53,304 openlp.core.lib.pluginmanager ERROR Failed to import module remoteplugin on path /home/tim/Projects/openlp/python3-run/openlp/plugins/remotes/remoteplugin.py: type openlp.plugins.remotes.lib.RemoteTab is unknown Traceback (most recent call last): File "/usr/lib64/python3.3/site-packages/uno.py", line 295, in _uno_import d[x] = pyuno.getClass( name + "." + x ) uno.com.sun.star.uno.RuntimeException: pyuno.getClass: uno exception openlp.plugins.remotes.lib.RemoteTab is unknown During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/usr/lib64/python3.3/site-packages/uno.py", line 299, in _uno_import d[x] = Enum( name , x ) File "/usr/lib64/python3.3/site-packages/uno.py", line 126, in __init__ pyuno.checkEnum( self ) uno.com.sun.star.uno.RuntimeException: enum openlp.plugins.remotes.lib is unknown Now the remotes plugin is a wrapper for CherryPy and has nothing to do with uno so how it is needing uno is a mystery at present by I am continuing to hunt. We have in the pas had issues with imports of uno.py see our pluginmanager.py file for the comments.
The markuptrace looks like this Traceback (most recent call last): File "/usr/lib64/python3.3/site-packages/uno.py", line 268, in _uno_import return _g_delegatee( name, *optargs, **kwargs ) ImportError: No module named 'markupsafe._speedups' During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/home/tim/Projects/openlp/python3-run/openlp.py", line 45, in <module> main() File "/home/tim/Projects/openlp/python3-run/openlp/core/__init__.py", line 326, in main sys.exit(application.run(qt_args)) File "/home/tim/Projects/openlp/python3-run/openlp/core/__init__.py", line 137, in run Registry().execute('bootstrap_initialise') File "/home/tim/Projects/openlp/python3-run/openlp/core/lib/registry.py", line 159, in execute result = function(*args, **kwargs) File "/home/tim/Projects/openlp/python3-run/openlp/core/lib/pluginmanager.py", line 68, in bootstrap_initialise self.find_plugins() File "/home/tim/Projects/openlp/python3-run/openlp/core/lib/pluginmanager.py", line 118, in find_plugins imp.load_module(module_name, fp, path_name, description) File "/usr/lib64/python3.3/imp.py", line 175, in load_module return load_source(name, filename, file) File "/usr/lib64/python3.3/imp.py", line 114, in load_source _LoadSourceCompatibility(name, pathname, file).load_module(name) File "<frozen importlib._bootstrap>", line 586, in _check_name_wrapper File "<frozen importlib._bootstrap>", line 1024, in load_module File "<frozen importlib._bootstrap>", line 1005, in load_module File "<frozen importlib._bootstrap>", line 562, in module_for_loader_wrapper File "<frozen importlib._bootstrap>", line 870, in _load_module File "<frozen importlib._bootstrap>", line 313, in _call_with_frames_removed File "/home/tim/Projects/openlp/python3-run/openlp/plugins/remotes/remoteplugin.py", line 35, in <module> from openlp.plugins.remotes.lib import RemoteTab, HttpServer File "/usr/lib64/python3.3/site-packages/uno.py", line 268, in _uno_import return _g_delegatee( name, *optargs, **kwargs ) File "/home/tim/Projects/openlp/python3-run/openlp/plugins/remotes/lib/__init__.py", line 31, in <module> from .httpserver import HttpServer File "/usr/lib64/python3.3/site-packages/uno.py", line 268, in _uno_import return _g_delegatee( name, *optargs, **kwargs ) File "/home/tim/Projects/openlp/python3-run/openlp/plugins/remotes/lib/httpserver.py", line 124, in <module> from mako.template import Template File "/usr/lib64/python3.3/site-packages/uno.py", line 268, in _uno_import return _g_delegatee( name, *optargs, **kwargs ) File "/usr/lib/python3.3/site-packages/mako/template.py", line 10, in <module> from mako.lexer import Lexer File "/usr/lib64/python3.3/site-packages/uno.py", line 268, in _uno_import return _g_delegatee( name, *optargs, **kwargs ) File "/usr/lib/python3.3/site-packages/mako/lexer.py", line 10, in <module> from mako import parsetree, exceptions, util File "/usr/lib64/python3.3/site-packages/uno.py", line 268, in _uno_import return _g_delegatee( name, *optargs, **kwargs ) File "/usr/lib64/python3.3/site-packages/uno.py", line 268, in _uno_import return _g_delegatee( name, *optargs, **kwargs ) File "/usr/lib/python3.3/site-packages/mako/parsetree.py", line 9, in <module> from mako import exceptions, ast, util, filters File "/usr/lib64/python3.3/site-packages/uno.py", line 268, in _uno_import return _g_delegatee( name, *optargs, **kwargs ) File "/usr/lib64/python3.3/site-packages/uno.py", line 268, in _uno_import return _g_delegatee( name, *optargs, **kwargs ) File "/usr/lib/python3.3/site-packages/mako/filters.py", line 31, in <module> import markupsafe File "/usr/lib64/python3.3/site-packages/uno.py", line 268, in _uno_import return _g_delegatee( name, *optargs, **kwargs ) File "/usr/lib64/python3.3/site-packages/markupsafe/__init__.py", line 227, in <module> from markupsafe._speedups import escape, escape_silent, soft_unicode File "/usr/lib64/python3.3/site-packages/uno.py", line 276, in _uno_import traceback.print.exc() AttributeError: 'module' object has no attribute 'print'
Created attachment 82636 [details] trace file Here is the trace file from uno.py
These two statements seem contradictory: "We / I have never used pygments." "At that point OpenLP creashed looking for pygments so I needed to install that." So it sounds like you're saying OpenLP does use (or conditionally support) pygments, right? It's probably not critical though since that was just once instance of the issue, so we can pick another instead. Would it be possible to provide me (here or offline) with some specific instructions on how to set up an environment similar to yours and run exactly the same scenario you are? Can I just start with the bzr python3-productive branch and run setup.py? Are you using the LO python to run setup or a system version? "Now the remotes plugin is a wrapper for CherryPy and has nothing to do with uno so how it is needing uno is a mystery at present by I am continuing to hunt." It doesn't really, but once uno.py is imported any import statement runs through that that code since it intercepts the global import hook. What still has me confused though is that even though that code runs and you'll see uno stuff in the exception traceback, you only get into the uno code if Python already rejected the import and the final result for a bad import should still be an ImportError with or without uno. For example, in your first case: ----------------------------- DEBUG Importing remoteplugin from /home/tim/Projects/openlp/python3-run/openlp/plugins/remotes. Depth 10 2013-07-18 21:02:53,304 openlp.core.lib.pluginmanager ERROR Failed to import module remoteplugin on path /home/tim/Projects/openlp/python3-run/openlp/plugins/remotes/remoteplugin.py: type openlp.plugins.remotes.lib.RemoteTab is unknown Traceback (most recent call last): File "/usr/lib64/python3.3/site-packages/uno.py", line 295, in _uno_import d[x] = pyuno.getClass( name + "." + x ) uno.com.sun.star.uno.RuntimeException: pyuno.getClass: uno exception openlp.plugins.remotes.lib.RemoteTab is unknown ----------------------------- The ERROR line is from pluginmanager.py in the "except ImportError" clause, so that indicates that the attempt to import remoteplugin failed with an ImportError. The rest of the traceback is from when the uno codes runs. It appears to have been triggered by a failure on the line: from openlp.plugins.remotes.lib import RemoteTab, HttpServer Now, it's flowing through the uno code, but that can only happen if treating it as a regular Python import already failed (and it was a from xxx import yyy syntax). But there's no import failure protection here, so I'd expect to see some sort of failure even without uno imported - so that's perplexing. The comments in pluginmanager (where it uses imp directly to get around a PyUNO issue) references tests. So are you seeing errors during tests, or in actual operation? The pluginmanager code avoids the uno path for the actual module load, but not for internal imports within the module itself (e.g., the above import in remoteplugin.py itself). I may just be missing something obvious though, so being able to run this locally might help. BTW, in the other traceback the missing print attribute bit is because it should be print_exc not print.exc
Tim, I've gotten a (sorta, kinda, mostly) working copy of the OpenLP python3-productive branch under Python 3.3.2 with LibreOffice 4.1.0.2. I had to hack a bunch of stuff in terms of missing Python 3'isms, but it gets through the first time wizard and to the main screen, and all but 20 or so of the self-tests run (and they're mostly 3'isms related to the test resource files). I will say that so far I don't see any issue that would point to an error in the uno module. I will agree that given the import changes needed to the OpenLP code base for Python 3 (lots of current failures) the way uno interferes with import errors is definitely a bit of a pain, so would suggest doing the porting work without uno as long as possible, just introducing it when needed at the end. Or, keep my patch in place. But I don't see it actually breaking anything that isn't already broken. In terms of running, I will note that the branch has a top level openlp.py script which hides the openlp package, so I couldn't even start it without renaming that. In terms of testing, the single biggest issue creating failed imports is probably relative module references (closely followed by exception syntax), which I'm guessing is what you've been working on cleaning up. For example, in plugins/remotes/lib/__init__ the imports won't work as is (you need to add a "." prefix to let it search relative in its own package). Without this fix, in turn the higher level remoteplugin import line fails just as you're seeing. It fails with or without uno, but with uno imported, you get precisely the sort of tracebacks you are seeing. Even though uno code is shown in the traceback, the root issue is still inside of OpenLP, at least for the cases I've been able to recreate.
@ David Bolen: bzr checkout lp:~openlp-core/openlp/python3-productive cd python3-productive 2to3 --write --nobackups --no-diffs -x next print * sh scripts/generate_resources.sh Also run python3 scripts/check_dependencies.py to check if you have all dependencies installed. Then you should have a python3 code base. Start with python3 openlp.py -l d -d
@Andreas: Thanks. Doing that works fine for me with or without uno, so either the python3-productive branch is more up to date, or that's not quite how Tim has been running it. Maybe somehow some of the plugins didn't get run through 2to3, since that does generate errors such as he's seen. For those reading later, I also recommend removing openlp/core/resources.py before running 2to3 on the tree - it took a huge amount of time (cpu-bound) to process that file for me (at least in a VM), and generate_resources.sh re-generates a Py3 compatible file anyway. I also think there should be another "-x" before print in the 2to3 command though whether I ran that filter or not didn't seem to change the final behavior.
@David: Suddenly it works. The only thing I remember doing is to reinstall LO (to ensure that uno.py is up to date). Unfortunately I do not have a backup of the old (?) uno.py because I did not expect that things work :D Anyway, I might check my pacman log for any updates, but a brief look did not show that any of the relevant packages were updated lately- What distro did you test this on BTW?
Note, it was a reinstall, not an update (http://pastebin.com/8vhvArgR) I also tried older revisions of python3-productive, which also worked, so it was not our branch which fixed it.
@Andreas: Yeah, I hate it when an issue just disappears like that. Not quite sure what to say. I'm still unable to see a way for the uno.py code to create import errors that aren't already there, though it clearly masks their origin, making the original error harder to detect, so I'm guessing the original attribution of the source of the issue in this ticket may have been misplaced. Hard to be completely confident without some way to reproduce though. Any chance you somehow had an OpenLP tree that maybe was only partially processed with 2to3? That was the only way I was able to create failures such as reported earlier in the bug. It would appear with any source issue resulting in an ImportError anywhere along the way. For my part I've been testing under Ubuntu 12.04. LibreOffice was installed from debs from the web site, not distribution packages. Python and PyQt were local builds from source, using distribution packages for C/C++ dependencies. Python module dependencies were all installed directly from pypi. OpenLP was from its repository. -- David
(In reply to comment #17) > However, I think my earlier patch at the end of comment 14 (which Tim is > using) to at least constrain the namespace under which the uno-specific code > runs seems like a plausible change, unless anyone is aware of anything > accessed via uno not being under the "com.sun.star" namespace. Not everything "accessed via uno [is] under the 'com.sun.star' namespace." Most of the UNOIDL entities coming with LO are, but: * at least the stuff coming with LO from offapi/org/freedesktop/PackageKit/ is not; * anything coming with 3rd-party extensions should not be.
(In reply to comment #29) > * at least the stuff coming with LO from offapi/org/freedesktop/PackageKit/ > is not; > * anything coming with 3rd-party extensions should not be. Thanks, good to know. So assuming those other namespaces are also accessible from the uno import code, clearly that means hardcoded namespace checks can't work. It would still be a nice improvement if there was some way to avoid uno losing the original python import error and traceback for non-uno stuff, but offhand it sounds like there's no safe way to distinguish uno vs. non-uno in the failure case. I wonder if it might be better to prefer re-raising the original python import error if present if the default uno processing fails. The uno failure exception is pretty generic anyway, so the original Python exception should work about as well, while in the non-uno case, the original Python exception (and more critically, traceback) is far superior to the current uno exception.
(In reply to comment #30) > I wonder if it might be better to prefer re-raising the original python > import error if present if the default uno processing fails. The uno > failure exception is pretty generic anyway, so the original Python exception > should work about as well, while in the non-uno case, the original Python > exception (and more critically, traceback) is far superior to the current > uno exception. Sounds reasonable to me, feel free to provide a patch. (Note that an original python ImportError will always be present for re-raising in the two places that currently raise a fresh ImportError in _uno_import.)
Created attachment 82883 [details] Import handler patch Ok, so in looking a little deeper just using the python exception has its own problems. In that case, with Python 3, the python exception for uno failures will just declare an import error of the top level namespace (e.g, "com") since uno.py never actually installs anything in sys.modules. But changing that would break the same semantics I ran into problems with in comment 17 trying the new Python 3 import handler model. So I've opted to try to combine some aspects of both. I re-raise a new exception (so I can include the full name to cover uno issues), but I model the exception text after a regular module import error, and include the original python traceback to better satisfy cases where the issue is outside of LO. I've attached a suggested patch (uno.py.patch), a small test script (exctest.py) and some test output (exctest.txt) reflecting running the script under plain python 2.7.5 and 3.3.2 (no uno), and LibreOffice 3.6.6.2, 4.0.4.2 and 4.1.0.3, both unpatched and patched. I'm attaching them to this ticket since it seems reasonable to treat them as a response to the issue reported here (which I still believe was likely due to a non-LO module error, but obscured by the uno traceback). As the output shows, it's really the LO 4.x and Python 3.x where the unpatched traceback is particularly rough in a python import failure case. LO 3.x isn't all that bad, so while the patch is compatible, I'm not sure it needs back-porting.
Created attachment 82884 [details] Patch test script
Created attachment 82885 [details] Patch test script output
David Bolen committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=7fd81244c21ad54a8b9766902fd7c34e8055b165 fdo#66025: Improve ImportError raised from _uno_import The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Created attachment 82905 [details] Patch correction Hmm, this is embarrassing. In seeing the patch committed, I suddenly realized I left one bit of test code in it, in the first exception handler (which has no need to assign the exception to 'e'). This patch just removes that small bit of code, and applies on top of the prior patch. The original patch technically works fine as-is, and I considered not even bothering with a fix, but some future reviewer may wonder about any relationship between that variable and the final exception (there isn't any) due to their naming, so it feels worth cleaning up. My apologies. I don't know how I missed it originally.
(In reply to comment #36) > Created attachment 82905 [details] [review] > Patch correction > > Hmm, this is embarrassing. In seeing the patch committed, I suddenly > realized I left one bit of test code in it, in the first exception handler > (which has no need to assign the exception to 'e'). This patch just removes > that small bit of code, and applies on top of the prior patch. > > The original patch technically works fine as-is, and I considered not even > bothering with a fix, but some future reviewer may wonder about any > relationship between that variable and the final exception (there isn't any) > due to their naming, so it feels worth cleaning up. > > My apologies. I don't know how I missed it originally. No problem at all, will push that additional patch in an instant. (In the future, it would be extra helpful if you could produce patches with "git format-patch", which are directly applicable via "git am", or even push them directly to LO's gerrit, see <https://wiki.documentfoundation.org/Development/gerrit>.) Funnily enough, I had wondered about that spurious "as e" when looking at the patch, but decided that it is harmless enough and/or my non-existent Python knowledge plays tricks on me. Another thing that made me wonder is: I assume that _uno_import will never be called by multiple threads in parallel, anyway, or else the "d[x] = unknown" and "del d[x]" might introduce a new race there.
Hi David - thanks -so- much for digging into this one so thoroughly :-) that is nothing to be ashamed of ! you should see the number of corrective fixes to my patches ;-> Should we back-port this to the -4-1 branch ? if so, can we close this bug ? It's turned into rather a monster sprawl; are you happy with the result Tim and/or is OpenLP setup ? :-)
David Bolen committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=329125abb63061214897e4f215d41cfa4b13b4b0 fdo#66025: Minor clean-up of previous patch The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Created attachment 82928 [details] Patch without use of sentinel (In reply to comment #37) > Another thing that made me wonder is: I assume that _uno_import will never > be called by multiple threads in parallel, anyway, or else the "d[x] = > unknown" and "del d[x]" might introduce a new race there. It should be safe - the Python import lock prevents two threads from importing simultaneously (and possibly seeing partial modules). In retrospect, it appears more complicated than necessary, and your uncertainty could be shared by future viewers. A simple flag set at the bottom of each exception path works just as well. This patch (via git format-patch) uses that approach. It's off of the current master and incorporates the patch2 change since that wasn't there yet). I'll look into gerrit for the future when I have a chance.
David Bolen committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=fbe28de6fbfdce41544e4e93168d32661add8285 fdo#66025: Simplify new ImportError logic The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Many thanks for the updates and the hard work. Running the test on our code the errors are cleaner but we still seem to have a problem with uno.py. In the original OpenLP code we had an exception window which imported uno outside the class (when the file was read). This triggered of all the import checking. At the same time we had a bug (due to the python3 conversion in our HTTPServer class). Uno.py just reported that HttpServer was missing and not why. I recoded our exception form to only import uno when it was needed and the traces showed what the error was in HttpServer. This is now being fixed. Whist this has cleaned things up and we have learnt by our mistakes the loss of the original exception has not helped. If someone knows how to throw the original exception there would seem not much more we can do with this issue. A back port to 4.1 would be nice though.
(In reply to comment #42) > I recoded our exception form to only import uno when it was needed and the > traces showed what the error was in HttpServer. This is now being fixed. (...) > If someone knows how to throw the original exception there would seem not > much more we can do with this issue. A back port to 4.1 would be nice > though. It's a bit of a catch-22. If uno re-throws the exact original exception directly, while best for regular import failures, it's worse on LO import failures since the exception will only refer to a top level com module. And the uno code can't know for sure which case it is in. But it's still an ImportError, and the traceback should help. I'm wondering if a trade-off I made in the current patch favors uno imports too much. It skips the lowest frame from the _g_delegatee failure at a given level, which works well for uno failures since it avoids many extra g_delegatee frames while the "fake" uno module hierarchy is traversed, but is probably hiding needed frames in non-uno nested import failures. Would it be possible for you to reintroduce the httpserver error, then try the patched version of uno.py but with the following change: --- uno.py.patch 2013-07-24 06:54:38.703357947 -0400 +++ uno.py 2013-07-24 16:26:29.436937908 -0400 @@ -266,2 +266,3 @@ except ImportError as e: + python_tb = sys.exc_info()[2] # process optargs @@ -318,3 +319,3 @@ (name, name, x)) - e.__traceback__ = sys.exc_info()[2] + e.__traceback__ = python_tb and let me know if this produces a traceback that would have worked for your issue with httpserver (without needing to remove the uno import)? It still won't be exactly the same as the original Python exception and traceback, but should have all the needed references to the OpenLP code.
That does the trick. The original error is present in the stack trace.
Created attachment 83056 [details] Final patch Ok, let's see if we can't close this out. The attached is a (hopefully) final patch. I will also be attaching some updated samples of before and after tracebacks. This patch incorporates a full traceback, with an ImportError always of the form "ImportError: XXX (or YYY)" where XXX is the original Python exception text, and YYY is the name uno tried to resolve after the Python exception. I've run this by Tim independently, and I believe it now fully satisfies his original problem case where his import issue was completely obscured by uno (my other attachment will include tracebacks from his testing). On the back-port question, I don't know if there are any guidelines for what qualifies or not. Technically this behavior (obscuring the underlying failure) has been around for a really long time (including in 3.x) so it's not a new bug or regression. With that said, I think the new behavior is a real improvement for problem diagnosis in the nested import case. As written it is compatible with any 4.x series, and can even apply to 3.x with minor changes, though I suspect it's not worth the hassle to go back further than 4.1. A few other details, mostly for the bug record: The patch provides inclusion of the full traceback (so you'll always see the actual line of the failure) and for the exception text, combines the original Python text with the first uno lookup error. The "first" part is important since the uno code will perform additional lookups higher up the import nesting chain on an error and the uno failure would be out of sync with the traceback if the failure was in a nested module. As this is still a mash-up of two possibilities (a python import issue versus an uno lookup problem) the exception is of necessity more ambiguous, and sub-optimal in some cases (which I can't see any way to resolve without an unambiguous way to identify uno modules/namespaces): 1. The full traceback includes all uno_import frames, so there will be a number of _g_delegatee entries that are potentially confusing and cannot be avoided. But on balance you get to see the actual point of failure, even for uno issues occurring in a nested module, where previously all you saw was the highest level "from x import y" that uno processed. 2. For a python exception, the uno reference is going to be unnecessary and could be potentially confusing. No real way to avoid this however, since the original python exception is often useless for uno diagnostics (see next point). 3. For an uno issue, the import processing walks the module hierarchy so for example "from com.sun.star.style import foo" will attempt to import com.sun.star.style, com.sun.star, com.sun and com. This isn't an issue if foo is found, but if not, in Python 3 the exception will have _g_delegatee traceback frames for each nested (missing) module, and the failure cause will be the uppermost com module. Point (3) means that most usual uno issues now have an exception of the form "ImportError: No module named 'com' (or YYY)" where YYY is the real problem. (This is a change from Python 2 was where it would show the full com.sun.star.style in such an exception). So the com reference may be confusing as it never actually exists. I debated flipping XXX and YYY in the exception message for this reason, since arguably importing uno could be an implicit acceptance of wanting to see uno-related errors primarily. But I feel that it is more defensible to keep the actual Python exception as the base and augment with uno, especially since importing uno is going to affect all import errors across a code base (at least using from x import y) - OpenLP being a good example. I also debated doing special processing in the "com" case to adjust the message (since even if not the exclusive namespace for uno, seems likely to be the most heavily used), but opted against it as not being worth the additional logic. It does mean you'll probably need to always ignore the first part of the exception message in an uno issue, but the YYY is the same as that currently shown (or even more accurate, in a nested case) so I don't think it's much of a hardship.
Created attachment 83058 [details] Updated test output for final patch This is a sample of test imports (both nested and non-nested) showing the final patch compared to a stock LO 4.1.0.4. It also includes a before/after traceback sample from Tim Bentley for OpenLP. The most notable improvements are in the nested cases (which includes OpenLP).
David Bolen committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=948b6ea02ea9de7fb4e1e2baf95ecae3ba1cd54e fdo#66025: Improve accuracy of ImportError traceback and message The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
David Bolen committed a patch related to this issue. It has been pushed to "libreoffice-4-1": http://cgit.freedesktop.org/libreoffice/core/commit/?id=52a533cc31f630ad482fe0fde8d925b459c787bf&h=libreoffice-4-1 fdo#66025: Improve accuracy of ImportError traceback and message It will be available in LibreOffice 4.1.1. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
David Bolen committed a patch related to this issue. It has been pushed to "libreoffice-4-0": http://cgit.freedesktop.org/libreoffice/core/commit/?id=acc7484b0fd1d5df2a709399bcdf03ec81943e71&h=libreoffice-4-0 fdo#66025: Improve accuracy of ImportError traceback and message It will be available in LibreOffice 4.0.5. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
So I take it that the accumulated patches by David (and thanks again a lot for them) fix the original problem. I backported them up to LO 4.0 now and mark this issue as fixed.