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

setuptools.depends.get_module_constant doesn't work in Python 3.6? #866

Closed
Preston-Landers opened this Issue Dec 3, 2016 · 8 comments

Comments

Projects
None yet
4 participants
@Preston-Landers
Contributor

Preston-Landers commented Dec 3, 2016

Python 3.6.0b4 (default, Nov 22 2016, 05:30:12) [MSC v.1900 64 bit (AMD64)] on win32 
Type "help", "copyright", "credits" or "license" for more information.               
>>> import setuptools             
>>> print(setuptools.__version__) 
30.1.0                            
>>> from setuptools.depends import get_module_constant                                                                   
>>> print(get_module_constant("setuptools", "__version__"))                                                              
Traceback (most recent call last):                                                                                       
  File "<stdin>", line 1, in <module>                                                                                    
  File "C:\dev\timesheet\dist\Windows_NT\python\lib\site-packages\setuptools\depends.py", line 164, in get_module_constant                                                                                                                        
    return extract_constant(code, symbol, default)                                                                       
  File "C:\dev\timesheet\dist\Windows_NT\python\lib\site-packages\setuptools\depends.py", line 195, in extract_constant  
    const = code.co_consts[arg]                                                                                          
IndexError: tuple index out of range                                                                                                                                                                       

I ask because I'm trying to use python-memcached which uses this get_module_constant and fails like that. Thanks.

@jaraco

This comment has been minimized.

Member

jaraco commented Dec 25, 2016

I've confirmed this is indeed the case, and there's no test case in setuptools to even capture this usage. Would you be willing to look into the underlying cause and recommend a fix?

@carl-ellis

This comment has been minimized.

carl-ellis commented Dec 28, 2016

Just having a little debug, and it seems that the byte format may have changed.

For example, currently python-memcached is broken due to the use of get_module_constant("memcache", "__version__") in setup.py

The line in question is (https://github.com/pypa/setuptools/blob/master/setuptools/depends.py#L99)
arg = bytes[ptr + 1] + bytes[ptr + 2] * 256 + extended_arg

In python3.6 the line returns 23040 and the bytes object is array('b', [100, 0, 90, 0, 100, 1, ...]) . This fails as is used to index the code.co_consts tuple that has 29 elements

In python3.5 the line returns 0 and the bytes object is array('b', [100, 0, 0, 90, 0, 0, 100, ...]) and manages to index the code.co_consts tuple correctly.

Not sure where the underlying issue really lies, but it seems that the arg value is 8-bit in this particular use case in 3.6 and 16-bit in 3.5

Preston-Landers added a commit to Preston-Landers/setuptools that referenced this issue Dec 28, 2016

Attempt to fix issue pypa#866 by iterating over code with `dis.Byteco…
…de` instead of the internal `_iter_code`.

The `dis` module was already used in `_iter_code` so I figured it was safe to use `Bytecode` from it. Not sure how this assumption holds up across all supported Python releases. I can only assume `Bytecode` wasn't there before when `_iter_code` was originally written?

Note that `_iter_code` doesn't appear to be called anywhere in light of this change so I removed it.

I should also note that `get_module_constant` has never worked with `setuptools.__version__` (returns -1) because it's not a string literal; it gets that attribute from another module.  But this change does work in cases where a string literal is requested.

pypa#866
@Preston-Landers

This comment has been minimized.

Contributor

Preston-Landers commented Dec 28, 2016

I can't really speak to how the bytecode arrangement may have changed, that's a bit above my pay grade.

I did notice that setuptools.depends._iter_code uses the dis builtin module, which contains a dis.Bytecode iterator that appears to perform the same function. Replacing _iter_code with dis.Bytecode seems to account for this difference and works as expected.

I should point out that in the original case I posted:

print(get_module_constant("setuptools", "__version__"))   

Cases like setuptools.__version__ never worked (return -1) because it's not set as a string literal at that point, it gets the attribute from another internal module. But the fix referenced above does fix cases that DO try to get directly defined string literals, including python-memcached.

Overall I'm not 100% sure why packages like python-memcached are using this compiler/AST based function get_module_constant - I suppose to avoid actually importing the module in case it has side effects?

@jaraco

This comment has been minimized.

Member

jaraco commented Dec 29, 2016

Thanks to both of you for this analysis. I have some tests that should prove @Preston-Landers fix.

jaraco added a commit that referenced this issue Dec 29, 2016

Add two more tests for _iter_code per #866, capturing the apparent ex…
…pectation in the byte-code processing that's now failing on Python 3.6.

jaraco added a commit that referenced this issue Dec 29, 2016

@jaraco

This comment has been minimized.

Member

jaraco commented Dec 29, 2016

I've pushed the commits up in the [issue-866 branch](/pypa/setuptools/tree/issue-866], but unfortunately, the new test added in 6a960cb is failing on pypy (only after the use of dis.Bytecode).

Aha. Looking at the docs for dis, Bytecode was added in Python 3.4. That also explains the other failures in that build.

jaraco added a commit that referenced this issue Dec 29, 2016

@jaraco jaraco closed this in a29c9a4 Dec 29, 2016

jaraco added a commit that referenced this issue Dec 29, 2016

@jaraco

This comment has been minimized.

Member

jaraco commented Dec 29, 2016

Fix rolling out as v32.3.1.

nijel added a commit to WeblateOrg/weblate that referenced this issue Jan 30, 2017

Update Travis test matrix
- remove Python 3.6 for now as setuptools is broken there, see
  pypa/setuptools#866
- add Django 1.11 pre release, this will fail at least until fix for
  python-social-auth/social-app-django#24 is
  released

Signed-off-by: Michal Čihař <michal@cihar.com>
@joristork

This comment has been minimized.

joristork commented Aug 18, 2017

I get a similar crash under setuptools v36.0.1, python v3.6.2rc1, pip 9.0.1:

Collecting detectlanguage==1.2.1 (from -r requirements.txt (line 15))
  Using cached detectlanguage-1.2.1.tar.gz
    Complete output from command python setup.py egg_info:
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/pip-build-c87dvbaa/detectlanguage/setup.py", line 9, in <module>
        version = get_module_constant('detectlanguage', '__version__'),
      File "/srv/chatbot/mentat/venv/lib/python3.6/site-packages/setuptools/depends.py", line 164, in get_module_constant
        return extract_constant(code, symbol, default)
      File "/srv/chatbot/mentat/venv/lib/python3.6/site-packages/setuptools/depends.py", line 195, in extract_constant
        const = code.co_consts[arg]
    IndexError: tuple index out of range
@joristork

This comment has been minimized.

joristork commented Aug 18, 2017

upgrading setuptools from v36.0.1 to v36.2.7 fixed it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment