Leseratte10 / acsm-calibre-plugin

Calibre plugin for ACSM->EPUB and ACSM->PDF conversion.
https://www.mobileread.com/forums/showthread.php?t=341975
GNU General Public License v3.0
600 stars 23 forks source link

Use pycryptodome instead of pycrypto #4

Closed fperrin closed 2 years ago

fperrin commented 2 years ago

Hi,

Module libadobe uses module Crypto. That module comes from pycrypto, which is dead: the project's last commit is [https://github.com/pycrypto/pycrypto/commits/master](from 2014). Debian stable no longer ships it in system packages (was called pyton-crypto). There is a issue on pycrypto that advises to stop using it.

The path forwards would be to use pycryptodome, which [https://www.pycryptodome.org/en/latest/src/changelog.html](is an active project). It is API compatible, requiring only changing all import Crypto... into import Cryptodome....

Leseratte10 commented 2 years ago

Thanks for this PR. If I remember correctly I had some issue with Cryptodome that required me to use the old Crypto instead. I don't remember anymore what that issue was, so I will check that again and see if I can switch to Cryptodome.

Leseratte10 commented 2 years ago

Okay, when I apply that PR I get errors like "No module named Cryptodome", so if I want to switch to Cryptodome I would need to bundle it / include it in the plugin itself as it seems to not be included with Calibre.

fperrin commented 2 years ago

It seems calibre does indeed a dependency on Crypto: https://github.com/kovidgoyal/calibre/blob/master/src/calibre/test_build.py#L35 Well in fact, from the name, it has a dependency on pycryptodome.

The situation is all-around messy. As I understand it:

In the beginning was pycrypto, which installed its modules in the Crypto namespace. Then the author lost interest or something, the project stopped receiving updates, so was forked by a new project pycryptodome. pycryptodome can either install itself under the same namespace Crypto (when installed as the PIP package pycryptodome) or under the namespace Cryptodome (when installed as the PIP package pycryptodomex). That means, with the first option, if you install pycryptodome, all your code that was written with pycrypto in mind would keep working, you don't even need to rewrite the import statements like I did here, and life is good.

However! Debian does not provide pycryptodome under the Crypto namespace. There is a bug #886291 about it, and I'm not sure why it didn't get fixed in time for buster, but here we are. (It means that the Debian package python3-pycryptodome is, in reality, python3-pycryptodomex.)

Presumably you use a different Linux distribution that either still ships pycrypto, or that ships pycryptodome in its API compatible mode.

It means the issue is, really, with how Debian packages pycryptodome.

One thing that can be done is to try to import first Crypto.Cipher.AES, then, if that fails, Cryptodome.Cipher.AES (and give up if that fails too. I'll update this PR.

Leseratte10 commented 2 years ago

With that change there's still from Cryptodome import Random at the beginning of the file, did you miss that? I'll try that and see if that works.

I knew the Crypto / Cryptodome situation was a bit messy, but I didn't know that that also applied to Calibre. I thought Calibre was just always bundled with Crypto.

fperrin commented 2 years ago

No that was an oversight, I did a second push.

fperrin commented 2 years ago

I knew the Crypto / Cryptodome situation was a bit messy, but I didn't know that that also applied to Calibre. I thought Calibre was just always bundled with Crypto.

In the source dependencies, it requires pycryptodome, ie. the compatible-with-pycrypto version.

https://github.com/kovidgoyal/calibre/blob/master/bypy/sources.json#L718

So really, the problem is Debian's packaging, because the package python3-pycryptodome really installs pycryptodomex.

However, it seems that none of the official plugins actually use Crypto (or Cryptodome), otherwise they would have had the same issue.

Leseratte10 commented 2 years ago

However, it seems that none of the official plugins actually use Crypto (or Cryptodome), otherwise they would have had the same issue.

I remember reading about this when I started making my plugin. I believe Kovid said that Calibre used to use Crypto in the past but no longer uses it right now, but he chose to leave it included in the build system / in Calibre because there's a lot of plugins that started relying on Crypto being available and he didn't want to break them all.

fperrin commented 2 years ago

Thanks @Leseratte10 !