search

Sigil-Ebook / Sigil

Sigil is a multi-platform EPUB ebook editor
GNU General Public License v3.0
5.99k stars 578 forks source link

Catch AttributeError in addition to OSError in pluginhunspell.py #742

Closed mihailim closed 9 months ago

mihailim commented 9 months ago

Catch AttributeError in addition to OSError in pluginhunspell.py to work around Python 3.12.0a4 ctypes.cdll behavior change introduced in this commit as a response to this issue: In 3.12.0a4 or newer, a key lookup when treating cdll as a dict no longer raises an OSError, but instead raises an AttributeError.

Fixes #741 .

This change is backwards compatible with any Python version.

Alternatively, instead of cdll[hunspell_dllpath] we could use cdll.LoadLibrary(hunspell_dllpath) there, but:

so let's be cautious and just catch the new exception, since this way we won't need to rely on any dynloader implementation subtleties.

kevinhendricks commented 9 months ago

Has this python3 change appeared in any production release of Python yet (non-alpha, non-beta)? Why would they make a backwards incompatible change in a point release without any deprecation warnings? What python bug was that commit meant to actually fix?

Unfortunately your pull request is incomplete. Your change is needed in other places in that file, and in the ml (multiple languages) version of the hunspell plugin in multiple places as well sigilgumbo library and possibly others.

So please revise your pull request to at least make all the changes where needed in just the file you targeted. I will merge it and I will track down all of the other places where cdll is used in our python code.

Thank you for your bug report and PR!

The python3 crowd should stop making backwards incompatible changes in minor point releases. It is a very bad practice.

mihailim commented 9 months ago

Yup, it appeared in 3.12.0a4 and has been part of it ever since, including 3.12 stable, 3.12.1 and the latest 3.12.2. I'm using Ubuntu 24.04 alpha (which is basically still Debian unstable at this point, since it's not yet frozen), which ships Sigil 2.0.1. 3.12 became the default system-wide Python a few days ago, and I noticed it the first time I ran a plugin on an EPUB :)

Yeah, I'm pretty annoyed at this as well, it took the better part of two hours to find out what was going on (especially since I hadn't touched ctypes interop before today.) This was basically an undocumented API breakage :/

Didn't realize it needed changes in some other parts, this fixed it in my scenario :) I'll track down the others and push the rest. Thanks!

mihailim commented 9 months ago

Oh, as it turns out, the patch for this file at least is complete. The other cdll reference, i.e. self.hunspell = cdll.LoadLibrary(sys_hunspell_location), doesn't suffer from the same problem: the behavior when trying to dynload a non-existent library via LoadLibrary() hasn't changed, still throws OSError so it's handled correctly. They only changed the behavior when trying to access an attribute of cdll, to make it behave like a "true" dict would behave (i.e. they only changed the __getattr__ behavior.) To be fair, that's more uniform indeed, but this should absolutely have been documented.

pluginhunspellml.py is the only other place where we use cdll as a dict, also just on line 64 with self.hunspell = cdll[hunspell_dllpath]. sigil_gumboc.py does the LoadLibrary() dance so it's not affected.

kevinhendricks commented 9 months ago

Great detective work. Thank you.

dougmassay commented 9 months ago

As an Arch user, I'm amazed (and a little thankful) that they've shown as much restraint as they have with Python 3.12. They're taking their time (which is usually against their nature) to make sure that packages that will be affected (the removal of the imp module in 3.12 being another major point) are updated first.

But it looks like they're going to move on it fairly soon.

@.***/thread/J56DVVEVTSPPV3LNXVEQ6AEZUQFZLHMI/

On Wed, Feb 28, 2024, 11:34 AM Mihai Limbășan @.***> wrote:

Yup, it appeared in 3.12.0a4 and has been part of it ever since, including 3.12 stable, 3.12.1 and the latest 3.12.2. I'm using Ubuntu 24.04 alpha (which is basically still Debian unstable at this point, since it's not yet frozen), which ships Sigil 2.0.1. 3.12 became the default system-wide Python a few days ago, and I noticed it the first time I ran a plugin on an EPUB :)

Yeah, I'm pretty annoyed at this as well, it took the better part of two hours to find out what was going on (especially since I hadn't touched ctypes interop before today.) This was basically an undocumented API breakage :/

Didn't realize it needed changes in some other parts, this fixed it in my scenario :) I'll track down the others and push the rest. Thanks!

— Reply to this email directly, view it on GitHub https://github.com/Sigil-Ebook/Sigil/pull/742#issuecomment-1969379145, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACG3CXQPK4HTV7YLD6S4KYDYV5MCDAVCNFSM6AAAAABD5RULX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRZGM3TSMJUGU . You are receiving this because you are subscribed to this thread.Message ID: @.***>

mihailim commented 9 months ago

BTW, for our peace of mind :) This is how it behaves:

LoadLibrary on 3.11, throws OSError:

user@host:~$ python3.11
Python 3.11.8 (main, Feb  7 2024, 21:52:08) [GCC 13.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from ctypes import cdll
>>> cdll.LoadLibrary('libc.so.6')
<CDLL 'libc.so.6', handle 7b4de4645110 at 0x7b4de37a9410>
>>> cdll.LoadLibrary('libc.so.ZZZZZZZZZ')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.11/ctypes/__init__.py", line 454, in LoadLibrary
    return self._dlltype(name)
           ^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/ctypes/__init__.py", line 376, in __init__
    self._handle = _dlopen(self._name, mode)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^
OSError: libc.so.ZZZZZZZZZ: cannot open shared object file: No such file or directory

LoadLibrary() on 3.12.2 -- fortunately unchanged, throws OSError:

user@host:~$ python3.12
Python 3.12.2 (main, Feb  7 2024, 20:47:03) [GCC 13.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from ctypes import cdll
>>> cdll.LoadLibrary('libc.so.6')
<CDLL 'libc.so.6', handle 70bc105ca110 at 0x70bc0f752300>
>>> cdll.LoadLibrary('libc.so.ZZZZZZZZZ')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.12/ctypes/__init__.py", line 460, in LoadLibrary
    return self._dlltype(name)
           ^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/ctypes/__init__.py", line 379, in __init__
    self._handle = _dlopen(self._name, mode)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^
OSError: libc.so.ZZZZZZZZZ: cannot open shared object file: No such file or directory

Attribute on 3.11.8, throws OSError:

user@host:~$ python3.11
Python 3.11.8 (main, Feb  7 2024, 21:52:08) [GCC 13.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from ctypes import cdll
>>> cdll['libc.so.6']
<CDLL 'libc.so.6', handle 77d1445c0110 at 0x77d143797690>
>>> cdll['libc.so.ZZZZZZZZZ']
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.11/ctypes/__init__.py", line 451, in __getitem__
    return getattr(self, name)
           ^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/ctypes/__init__.py", line 446, in __getattr__
    dll = self._dlltype(name)
          ^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/ctypes/__init__.py", line 376, in __init__
    self._handle = _dlopen(self._name, mode)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^
OSError: libc.so.ZZZZZZZZZ: cannot open shared object file: No such file or directory

Attribute on 3.12.2, surprise AttributeError:

user@host:~$ python3.12
Python 3.12.2 (main, Feb  7 2024, 20:47:03) [GCC 13.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from ctypes import cdll
>>> cdll['libc.so.6']
<CDLL 'libc.so.6', handle 77808c055110 at 0x77808b1515e0>
>>> cdll['libc.so.ZZZZZZZZZ']
Traceback (most recent call last):
  File "/usr/lib/python3.12/ctypes/__init__.py", line 450, in __getattr__
    dll = self._dlltype(name)
          ^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/ctypes/__init__.py", line 379, in __init__
    self._handle = _dlopen(self._name, mode)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^
OSError: libc.so.ZZZZZZZZZ: cannot open shared object file: No such file or directory

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.12/ctypes/__init__.py", line 457, in __getitem__
    return getattr(self, name)
           ^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/ctypes/__init__.py", line 452, in __getattr__
    raise AttributeError(name)
AttributeError: libc.so.ZZZZZZZZZ
mihailim commented 9 months ago

BTW #2, still for our peace of mind and taking advantage of the opportunity: I've messed around with my usual workflows and have not come across any other Sigil problem with Python 3.12, so I think we should be free from major, uh, unexpected improvements :) Of course, individual plugins may or may not be affected, but the Sigil core plugin infra looks good for 3.12. Small mercies.

mihailim commented 9 months ago

Oh, and I'll take care of alerting the Debian Sigil maintainer of the issue and maybe trying to upstream 2.1 once it's out.

kevinhendricks commented 9 months ago

Thank you. Feel free to point to your commits for cherry picking into Debian 2.0.2 as 2.1.0 will not be officially out until just after the end of March.