exception from parsing requirements

[zooko]

as reported by ambimorph on IRC:

amber@abhyasa:~/tahoe-lafs$ bin/tahoe @python-coverage run --branch --include='src/allmydata/*' @tahoe debug trial allmydata.test.test_cli
Traceback (most recent call last):
  File "/usr/bin/python-coverage", line 5, in <module>
    from pkg_resources import load_entry_point
  File "/home/amber/tahoe-lafs/support/lib/python2.7/site-packages/setuptools-0.6c16dev4.egg/pkg_resources.py", line 2638, in <module>
    working_set.add(dist)
  File "/home/amber/tahoe-lafs/support/lib/python2.7/site-packages/setuptools-0.6c16dev4.egg/pkg_resources.py", line 532, in add
    for thisreq in parse_requirements(thisreqstr):
  File "/home/amber/tahoe-lafs/support/lib/python2.7/site-packages/setuptools-0.6c16dev4.egg/pkg_resources.py", line 2451, in parse_requirements
    raise ValueError("Missing distribution spec", line)
ValueError: ('Missing distribution spec', '=', {'thisreqstr': '='})

Investigation showed the the __requires___ variable held a string ("coverage==3.4") instead of an iterable containing strings, therefore ​this code in our splinter of setuptools tried to parse each character of that string.

[daira]

Do we want to fix this specific bug in zetuptoolz, or concentrate on migrating to a later version of setuptools (#2044)?

[zooko]

[e9e63c5e72619f4e5811cd1eed31e344db6236c8] will reveal more information in this case.

[warner]

This came up in #2378, causing one of the buildslaves to fail the "test-from-egg" step.

[zooko]

Would it be okay if I pushed this patch into trunk? I tested it manually and it worked:

diff --git a/setuptools-0.6c16dev5.egg/pkg_resources.py b/setuptools-0.6c16dev5.egg/pkg_resources.py
index a94d89f..bde9087 100644
--- a/setuptools-0.6c16dev5.egg/pkg_resources.py
+++ b/setuptools-0.6c16dev5.egg/pkg_resources.py
@@ -536,7 +536,7 @@ class WorkingSet(object):
                             if dist not in thisreq:
                                 return
                 except ValueError, e:
-                    e.args = tuple(e.args + ({'thisreqstr': thisreqstr},))
+                    e.args = tuple(e.args + ({'thisreqstr': thisreqstr, '__requires__': __requires__},))
                     raise
 
         self.by_key[dist.key] = dist

Here's the result of running locally a manual test with this patch applied, which does indeed helpfully show me what requirements are triggering the problem:

Traceback (most recent call last):
  File "/usr/local/bin/verinfo", line 5, in <module>
    from pkg_resources import load_entry_point
  File "/home/zooko/playground/tahoe/tahoe-lafs/support/lib/python2.7/site-packages/setuptools-0.6c16dev5.egg/pkg_resources.py", line 2650, in <module>
    working_set.add(dist)
  File "/home/zooko/playground/tahoe/tahoe-lafs/support/lib/python2.7/site-packages/setuptools-0.6c16dev5.egg/pkg_resources.py", line 534, in add
    for thisreq in parse_requirements(thisreqstr):
  File "/home/zooko/playground/tahoe/tahoe-lafs/support/lib/python2.7/site-packages/setuptools-0.6c16dev5.egg/pkg_resources.py", line 2463, in parse_requirements
    raise ValueError("Missing distribution spec", line, strs)
ValueError: ('Missing distribution spec', '=', '=', {'__requires__': 'pyutil==unknown', 'thisreqstr': '='})

[warner]

+1

[zooko]

Here is a patch that is intended to fix this by checking whether __requires__ is a string or a list, and if it is a string then treat it as a list of one element:

diff --git a/setuptools-0.6c16dev5.egg/pkg_resources.py b/setuptools-0.6c16dev5.egg/pkg_resources.py
index a94d89f..a026b1b 100644
--- a/setuptools-0.6c16dev5.egg/pkg_resources.py
+++ b/setuptools-0.6c16dev5.egg/pkg_resources.py
@@ -529,14 +529,20 @@ class WorkingSet(object):
         # If we have a __requires__ then we can already tell if this
         # dist is unsatisfactory, in which case we won't add it.
         if __requires__ is not None:
-            for thisreqstr in __requires__:
+            assert isinstance(__requires__, (basestring, list)), repr(__requires__)
+            if isinstance(__requires__, basestring):
+                array_of__requires__ = [__requires__]
+            else:
+                array_of__requires__ = __requires__
+
+            for thisreqstr in array_of__requires__:
                 try:
                     for thisreq in parse_requirements(thisreqstr):
                         if thisreq.key == dist.key:
                             if dist not in thisreq:
                                 return
                 except ValueError, e:
-                    e.args = tuple(e.args + ({'thisreqstr': thisreqstr},))
+                    e.args = tuple(e.args + ({'thisreqstr': thisreqstr, '__requires__': __requires__},))
                     raise
 
         self.by_key[dist.key] = dist

I tested this patch manually, and it changed the error shown in comment:4 to this result, which I think means it worked:

pkg_resources.require('Twisted') =>  [Twisted 15.0.0 (/usr/local/lib/python2.7/dist-packages/Twisted-15.0.0-py2.7-linux-x86_64.egg), zope.interface 4.0.5 (/usr/lib/python2.7/dist-packages)]
import Twisted;print Twisted => 
Traceback (most recent call last):
  File "/usr/local/bin/verinfo", line 9, in <module>
    load_entry_point('pyutil==unknown', 'console_scripts', 'verinfo')()
  File "/usr/local/lib/python2.7/dist-packages/pyutil-unknown-py2.7.egg/pyutil/scripts/verinfo.py", line 22, in main
    x = __import__(PACKNAME)
ImportError: No module named Twisted

[zooko]

review from IRC:

<dstufft> zooko: that patch looks OK to me, th eonly comment I have is the
	  assert means you'll fail on other iterables other than lists like
	  tuples... but I don't know if it's likely you'll ever see a tuple

[daira]

I would suggest using that code without the assert. The for will raise an exception in any case if array_of__requires__ is not iterable.

[warner]

Fixed in [575d7a2] (without the assert).