exception from parsing requirements #2242

New Issue

zooko · 2014-06-10T17:54:51Z

zooko commented

2014-06-10 17:54:51 +00:00

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.

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](https://tahoe-lafs.org/trac/tahoe-lafs/browser/trunk/setuptools-0.6c16dev4.egg/pkg_resources.py?annotate=blame&rev=5ab27f58cc35bc2abc87822798460021b9785e36#L527) in our splinter of setuptools tried to parse each character of that string.

zooko added the

labels 2014-06-10 17:54:51 +00:00

zooko added this to the soon milestone 2014-06-10 17:54:51 +00:00

daira commented

2014-06-12 15:44:57 +00:00

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

zooko commented

2015-02-24 18:22:54 +00:00

e9e63c5e72619f4e5811cd1eed31e344db6236c8 will reveal more information in this case.

warner commented

2015-02-25 21:16:03 +00:00

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

zooko commented

2015-03-17 16:35:11 +00:00

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': '='})

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 commented

2015-03-17 16:49:07 +00:00

+1

zooko commented

2015-03-17 16:50:01 +00:00

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:95103 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

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:95103](/tahoe-lafs/trac-2024-07-25/issues/2242#issuecomment-95103) 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 commented

2015-03-17 17:57:58 +00:00

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

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 commented

2015-03-19 23:32:06 +00:00

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.

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.

tahoe-lafs modified the milestone from soon to 1.10.1

2015-04-13 12:54:41 +00:00

warner commented

2015-04-28 19:20:25 +00:00

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

warner added the

fixed

label 2015-04-28 19:20:25 +00:00