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

Fix sys.path manipulation #674

Merged
merged 1 commit into from
Dec 19, 2014
Merged

Conversation

mgedmin
Copy link

@mgedmin mgedmin commented Dec 3, 2014

Fixes #671, #673. Supersedes #672.

base64 should be imported after sys.path manipulation -- not doing
this causes #671.

sys.path is fixed up at the top level (since commit dde220e),
doing it again in copy_required_modules() breaks things -- this is #673.
This makes the try/finally to restore the old sys.path unnecessary,
which produces the gnarly diff :/

Fixes pypa#671, pypa#673.

base64 should be imported *after* sys.path manipulation -- not doing
this causes pypa#671.

sys.path is fixed up at the top level (since commit dde220e),
doing it again in copy_required_modules() breaks things -- this is pypa#673.
This makes the try/finally to restore the old sys.path unnecessary,
which produces the gnarly diff :/
dstufft added a commit that referenced this pull request Dec 19, 2014
@dstufft dstufft merged commit 920d7c7 into pypa:develop Dec 19, 2014
dstufft added a commit to dstufft/virtualenv that referenced this pull request Dec 24, 2014
@dstufft
Copy link
Member

dstufft commented Dec 24, 2014

I had to revert this, because 12.0 was severely broken and nothing I could do seemed to fix it. Details - https://etherpad.openstack.org/p/pydistutils-issues.

@mgedmin
Copy link
Author

mgedmin commented Dec 24, 2014

Quoting that etherpad:

Reverting this commit appears to fix it: dde220e

Yes, that commit was buggy and was the reason why I filed #673. #674 was supposed to fix it! And it worked fine for me when I tested on my Ubuntu 14.10 laptop.

#672 was a different PR that was also supposed to fix #673 (by actually reverting dde220e and replacing it with a different fix for the original bug, which was #625) as well as #671. Have you tried it instead of #674?

Currently virtualenv 12.0.4 is broken:

mg@platonas: ~ $ virtualenv -p /usr/bin/python3 /tmp/py3
Running virtualenv with interpreter /usr/bin/python3
Traceback (most recent call last):
  File "/home/mg/.venv/local/lib/python2.7/site-packages/virtualenv.py", line 8, in <module>
    import base64
  File "/usr/lib/python3.4/base64.py", line 9, in <module>
    import re
  File "/usr/lib/python3.4/re.py", line 324, in <module>
    import copyreg
  File "/home/mg/.venv/lib/python2.7/site-packages/copyreg.py", line 3, in <module>
    from copy_reg import *
ImportError: No module named 'copy_reg'
Error in sys.excepthook:
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/apport_python_hook.py", line 53, in apport_excepthook
    if not enabled():
  File "/usr/lib/python3/dist-packages/apport_python_hook.py", line 24, in enabled
    import re
  File "/usr/lib/python3.4/re.py", line 324, in <module>
    import copyreg
  File "/home/mg/.venv/lib/python2.7/site-packages/copyreg.py", line 3, in <module>
    from copy_reg import *
ImportError: No module named 'copy_reg'

Original exception was:
Traceback (most recent call last):
  File "/home/mg/.venv/local/lib/python2.7/site-packages/virtualenv.py", line 8, in <module>
    import base64
  File "/usr/lib/python3.4/base64.py", line 9, in <module>
    import re
  File "/usr/lib/python3.4/re.py", line 324, in <module>
    import copyreg
  File "/home/mg/.venv/lib/python2.7/site-packages/copyreg.py", line 3, in <module>
    from copy_reg import *
ImportError: No module named 'copy_reg'

[exited with 1]

which is bug #671.

@dstufft
Copy link
Member

dstufft commented Dec 24, 2014

I went with existing broken is better then new broken. When I couldn't understand why either one was broken :/

@dstufft dstufft mentioned this pull request Jan 5, 2015
5 tasks
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

Successfully merging this pull request may close these issues.

2 participants