Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

4.0.1 + mastet: pytest is failing because incorrect pycryptodome module name #302

Open
kloczek opened this issue Apr 8, 2022 · 5 comments

Comments

@kloczek
Copy link

kloczek commented Apr 8, 2022

I'm trying to package your module as an rpm package. So I'm using the typical PEP517 based build, install and test cycle used on building packages from non-root account.

  • python3 -sBm build -w --no-isolation
  • because I'm calling build with --no-isolation I'm using during all processes only locally installed modules
  • install .whl file in </install/prefix>
  • run pytest with PYTHONPATH pointing to sitearch and sitelib inside </install/prefix>

I've tested 4.0.1 + master commits and found that pytest is failing with:

+ PYTHONPATH=/home/tkloczko/rpmbuild/BUILDROOT/python-pykeepass-4.0.1-4.fc35.x86_64/usr/lib64/python3.8/site-packages:/home/tkloczko/rpmbuild/BUILDROOT/python-pykeepass-4.0.1-4.fc35.x86_64/usr/lib/python3.8/site-packages
+ /usr/bin/pytest -ra tests/tests.py
=========================================================================== test session starts ============================================================================
platform linux -- Python 3.8.13, pytest-7.1.1, pluggy-1.0.0
rootdir: /home/tkloczko/rpmbuild/BUILD/pykeepass-4.0.1
collected 0 items / 1 error

================================================================================== ERRORS ==================================================================================
_____________________________________________________________________ ERROR collecting tests/tests.py ______________________________________________________________________
ImportError while importing test module '/home/tkloczko/rpmbuild/BUILD/pykeepass-4.0.1/tests/tests.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib64/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/tests.py:18: in <module>
    from pykeepass import PyKeePass, icons
pykeepass/__init__.py:2: in <module>
    from .pykeepass import PyKeePass, create_database
pykeepass/pykeepass.py:27: in <module>
    from pykeepass.kdbx_parsing.kdbx import KDBX
pykeepass/kdbx_parsing/__init__.py:1: in <module>
    from .kdbx import KDBX
pykeepass/kdbx_parsing/kdbx.py:2: in <module>
    from .kdbx3 import DynamicHeader as DynamicHeader3
pykeepass/kdbx_parsing/kdbx3.py:10: in <module>
    from .common import (
pykeepass/kdbx_parsing/common.py:1: in <module>
    from Cryptodome.Cipher import AES, ChaCha20, Salsa20
E   ModuleNotFoundError: No module named 'Cryptodome'
========================================================================= short test summary info ==========================================================================
ERROR tests/tests.py
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
============================================================================= 1 error in 0.23s =============================================================================

In requirements.txt is listed pycryptodomex==3.14.1 however looks like that module effectively provides not Cryptodome but Crypto module.
After apply below patch

--- a/pykeepass/kdbx_parsing/common.py~ 2022-04-08 19:14:02.000000000 +0000
+++ b/pykeepass/kdbx_parsing/common.py  2022-04-08 19:15:54.291996033 +0000
@@ -1,6 +1,6 @@
-from Cryptodome.Cipher import AES, ChaCha20, Salsa20
+from Crypto.Cipher import AES, ChaCha20, Salsa20
 from .twofish import Twofish
-from Cryptodome.Util import Padding as CryptoPadding
+from Crypto.Util import Padding as CryptoPadding
 import hashlib
 from construct import (
     Adapter, BitStruct, BitsSwapped, Container, Flag, Padding, ListContainer, Mapping, GreedyBytes, Int32ul, Switch
--- a/pykeepass/kdbx_parsing/twofish.py~        2021-05-22 07:07:02.000000000 +0000
+++ b/pykeepass/kdbx_parsing/twofish.py 2022-04-08 19:15:48.077999169 +0000
@@ -25,8 +25,8 @@
 __all__ = ['Twofish']

 from . import pytwofish
-from Cryptodome.Util.strxor import strxor
-from Cryptodome.Util.Padding import pad
+from Crypto.Util.strxor import strxor
+from Crypto.Util.Padding import pad

 MODE_ECB = 1
 MODE_CBC = 2

I was able to pass cleanly test suite

+ PYTHONPATH=/home/tkloczko/rpmbuild/BUILDROOT/python-pykeepass-4.0.1-4.fc35.x86_64/usr/lib64/python3.8/site-packages:/home/tkloczko/rpmbuild/BUILDROOT/python-pykeepass-4.0.1-4.fc35.x86_64/usr/lib/python3.8/site-packages
+ /usr/bin/pytest -ra tests/tests.py
=========================================================================== test session starts ============================================================================
platform linux -- Python 3.8.13, pytest-7.1.1, pluggy-1.0.0
rootdir: /home/tkloczko/rpmbuild/BUILD/pykeepass-4.0.1
collected 98 items

tests/tests.py ..................................................................................................                                                    [100%]

=========================================================================== 98 passed in 25.11s ============================================================================

Please let me know do you want that pach as PR.

BTW: fo you have any plans to make new reelease?
Looks all master patches are needed to be able at least build pykeepass with latest versions of all required modules so IMO it would be good to make new rel;ease 😄
To be honet it would be good to add proper pytest support to not force pass tests/tests.py as param (without that pytest is not able to find units).

@Evidlo
Copy link
Member

Evidlo commented Apr 11, 2022

pycryptodomex is identical to pycryptodome except that the module is importable as Cryptodome, instead of Crypto.

This is desirable for us because we don't want to accidentally import the pycrypto package, which is no longer being maintained and is missing some algorithms that we need.

There was a related discussion for Gentoo: #232

@kloczek
Copy link
Author

kloczek commented Apr 11, 2022

Hmm .. however https://pypi.org/project/pycryptodomex/ "Source" link points to https://github.com/Legrandin/pycryptodome/
I'm puzzled a bit 😖

@Evidlo
Copy link
Member

Evidlo commented Apr 12, 2022

It's the same package, it's just that the author toggles a flag in setup.py when building to change the module name.

@Evidlo
Copy link
Member

Evidlo commented Jun 5, 2022

What distro is this, by the way?

edit: nevermind. Seems to be Fedora 35

@kloczek
Copy link
Author

kloczek commented Jun 5, 2022

No I'm using my own distro which ia relatively close to fedora rawhide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants