#2873 closed defect (fixed)

remove obsolete dependency on "pycrypto"

Reported by: warner Owned by: daira
Priority: normal Milestone: 1.13.0
Component: packaging Version: 1.12.1
Keywords: Cc:
Launchpad Bug:

Description

Tahoe currently declares a dependency on pycrypto, despite not using it directly (the only reference is a from Crypto import Util in test_sftp.py, used to check whether we think SFTP is available). We should remove this.

(note: this ticket is not about removing pycryptopp, the minimal python wrapper that zooko initially wrote for the high-quality C++ library known as Crypto++. #2322 is about doing that)

We added this dependency years ago when twisted.conch (which powers our SFTP frontend) needed it for various crypto purposes. At that time, I considered the SFTP frontend to be "optional", and didn't want to impose additional dependencies on users who didn't want it. So I made the SFTP unit tests conditional on whether we could import Crypto or not, thinking that it was the one thing that might not be present (the rest of twisted.conch shows up "for free" as part of Twisted).

Later, we (maybe Daira, who did a lot of work on SFTP) decided to simplify the set of possibilities, and make all dependencies mandatory.

Since then, twisted.conch has switched from pycrypto to (pyca) cryptography.

The right way to express this dependency is by depending upon twisted[conch] (probably on twisted[tls,conch] if we need both).

Change History (5)

comment:1 Changed at 2017-04-12T02:13:40Z by warner

(also see #2766, about removing other unnecessary dependencies)

I'm playing with this on a branch.. it's a 3-line fix.

diff --git a/src/allmydata/_auto_deps.py b/src/allmydata/_auto_deps.py
index bec052fa0..9eccf7f8e 100644
--- a/src/allmydata/_auto_deps.py
+++ b/src/allmydata/_auto_deps.py
@@ -42,11 +42,6 @@ install_requires = [
     # * foolscap >= 0.12.6 has an i2p.sam_endpoint() that takes kwargs
     "foolscap >= 0.12.6",
 
-    # Needed for SFTP.
-    # pycrypto 2.2 doesn't work due to <https://bugs.launchpad.net/pycrypto/+bug/620253>
-    # pycrypto 2.4 doesn't work due to <https://bugs.launchpad.net/pycrypto/+bug/881130>
-    "pycrypto >= 2.1.0, != 2.2, != 2.4",
-
     # pycryptopp-0.6.0 includes ed25519
     "pycryptopp >= 0.6.0",
 
@@ -73,7 +68,7 @@ install_requires = [
     # * Twisted-16.1.0 fixes https://twistedmatrix.com/trac/ticket/8223,
     #   which otherwise causes test_system to fail (DirtyReactorError, due to
     #   leftover timers)
-    "Twisted[tls] >= 16.1.0",
+    "Twisted[tls,conch] >= 16.1.0",
 
     # We need Nevow >= 0.11.1 which can be installed using pip.
     "Nevow >= 0.11.1",
@@ -107,7 +102,6 @@ package_imports = [
     ('platform',         None),
     ('pyOpenSSL',        'OpenSSL'),
     ('OpenSSL',          None),
-    ('pycrypto',         'Crypto'),
     ('pyasn1',           'pyasn1'),
     ('service-identity', 'service_identity'),
     ('characteristic',   'characteristic'),

But switching to twisted[tls,conch] causes the check_all_requirements() in our _auto_deps.py to break, I think because it doesn't know how to parse the comma-separated list of extras:

py27 runtests: commands[1] | tahoe --version
Traceback (most recent call last):
  File "/Users/warner/stuff/tahoe/tahoe/.tox/py27/bin/tahoe", line 11, in <module>
    load_entry_point('tahoe-lafs', 'console_scripts', 'tahoe')()
  File "/Users/warner/stuff/tahoe/tahoe/.tox/py27/lib/python2.7/site-packages/pkg_resources/__init__.py", line 560, in load_entry_point
    return get_distribution(dist).load_entry_point(group, name)
  File "/Users/warner/stuff/tahoe/tahoe/.tox/py27/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2648, in load_entry_point
    return ep.load()
  File "/Users/warner/stuff/tahoe/tahoe/.tox/py27/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2302, in load
    return self.resolve()
  File "/Users/warner/stuff/tahoe/tahoe/.tox/py27/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2308, in resolve
    module = __import__(self.module_name, fromlist=['__name__'], level=0)
  File "/Users/warner/stuff/tahoe/tahoe/src/allmydata/__init__.py", line 452, in <module>
    check_all_requirements()
  File "/Users/warner/stuff/tahoe/tahoe/src/allmydata/__init__.py", line 450, in check_all_requirements
    raise PackagingError(get_error_string(fatal_errors + _cross_check_errors, debug=True))
allmydata.PackagingError:
PackagingError: no version info or could not understand requirement 'Twisted[tls,conch] >= 16.1.0'

For debugging purposes, the PYTHONPATH was
  None
install_requires was
  ['setuptools >= 11.3', 'zfec >= 1.1.0', 'zope.interface >= 3.6.0, != 3.6.3, != 3.6.4', 'foolscap >= 0.12.6', 'pycryptopp >= 0.6.0', 'service-identity', 'characteristic >= 14.0.0', 'pyasn1 >= 0.1.8', 'pyasn1-modules >= 0.0.5', 'Twisted[tls,conch] >= 16.1.0', 'Nevow >= 0.11.1', 'pyOpenSSL >= 0.14', 'PyYAML >= 3.11', 'six >= 1.10.0']
sys.path after importing pkg_resources was
  /Users/warner/stuff/tahoe/tahoe/.tox/py27/bin:
  /Users/warner/stuff/tahoe/tahoe/.tox/py27/lib/python27.zip:
  /Users/warner/stuff/tahoe/tahoe/.tox/py27/lib/python2.7:
  /Users/warner/stuff/tahoe/tahoe/.tox/py27/lib/python2.7/plat-darwin:
  /Users/warner/stuff/tahoe/tahoe/.tox/py27/lib/python2.7/plat-mac:
  /Users/warner/stuff/tahoe/tahoe/.tox/py27/lib/python2.7/plat-mac/lib-scriptpackages:
  /Users/warner/stuff/tahoe/tahoe/.tox/py27/lib/python2.7/lib-tk:
  /Users/warner/stuff/tahoe/tahoe/.tox/py27/lib/python2.7/lib-old:
  /Users/warner/stuff/tahoe/tahoe/.tox/py27/lib/python2.7/lib-dynload:
  /usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/lib/python2.7:
  /usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/lib/python2.7/plat-darwin:
  /usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/lib/python2.7/lib-tk:
  /usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/lib/python2.7/plat-mac:
  /usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/lib/python2.7/plat-mac/lib-scriptpackages:
  /Users/warner/stuff/tahoe/tahoe/.tox/py27/lib/python2.7/site-packages:
  /Users/warner/stuff/tahoe/tahoe/src

I'm glad that the error was at least fairly straightforward ("could not understand requirement"), but again (#2749, #1872) I want to delete that code. We should not be duplicating the work of setuptools, especially when our function cannot do the job as well as setuptools.

comment:2 Changed at 2018-07-19T18:47:43Z by exarkun

But switching to twisted[tls,conch] causes the check_all_requirements() in our _auto_deps.py to break, I think because it doesn't know how to parse the comma-separated list of extras:

This is possibly sad but maybe irrelevant because https://tahoe-lafs.org/trac/tahoe-lafs/ticket/2740 means we should not actually declare a dependency on twisted[conch], apparently.

Leaving out that part of the diff, I think this change is uncontroversial and I'm going to put it up for CI to play with.

comment:3 Changed at 2018-07-19T18:59:43Z by exarkun

FWIW, Twisted 16.6.0 removes the gmpy dependency entirely. So bumping the Twisted minimum version to that would make it okay to declare twisted[conch] as a dependency. Then someone just has to deal with the problem warner encountered above - ideally, I would hope, by largely removing _auto_deps.py and relying on pip to figure out what the install requires. I have not yet internalized all of the purpose/functionality of _auto_deps.py so I'm not ready to do that myself.

comment:4 Changed at 2018-07-19T20:43:16Z by warner

As y'all know, I'm inclined to delete _auto_deps.py and the entire contents of __init__.py, and move all the dependency data into setup.py. Back before setuptools became good, we had a lot of defensive code to double-check that all the dependencies we actually installed, and those checks needed to know the full list of dependencies (from "inside" the codebase), so they were stored in _auto_deps.py so they could be shared with "outside" the codebase (where setup.py could use them).

I think Daira was the last defender of this arrangement, and I don't know if she still has an opinion about it. I'd just delete it.

comment:5 Changed at 2018-07-26T15:10:47Z by exarkun

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.