-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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 Tcl inside a virtualenv on Windows (issue #93) #627
Conversation
@@ -1191,6 +1191,9 @@ def install_python(home_dir, lib_dir, inc_dir, bin_dir, site_packages, clear, sy | |||
site_filename_dst = change_prefix(site_filename, home_dir) | |||
site_dir = os.path.dirname(site_filename_dst) | |||
writefile(site_filename_dst, SITE_PY) | |||
if is_win: | |||
fixtk_filename = join(site_dir, 'FixTk.py') | |||
writefile(fixtk_filename, FIXTK_PY) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to write a new file, but never use it. How is this functionality activated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, wait. I see there's a Lib\lib-tk\FixTk.py in Python 2.7. So this fix doesn't work on Python 3 (where there's no lib-tk directory). Can it be updated to cover that as well?
Also, if this is a copy/paste and edit of the core FixTk.py, won't any changes core Python makes in later versions be lost? That doesn't sound good - I'd prefer a fix that properly monkeypatched whatever was in the core distribution, if we're going down this route at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, if this is a copy/paste and edit of the core FixTk.py, won't any changes core Python makes in later versions be lost?
That's actually already the case for virtualenv's site.py
that it has vendored / modified - it's missing functions that were introduced in 2.7 See #355
But since this is for 2.X only and we already know there's no 2.8, I can't imagine that is a worry for this particular file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it meant to be for 2.x only? If so, the file should only be written on 2.x. And if it's not, the point remains for the equivalent file on 3.x - copying the file has risks. (OTOH, it may be that the 3.x version fixes this issue as it presumably supports venv...)
But yeah, it's a relatively minor point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, wait. I see there's a Lib\lib-tk\FixTk.py in Python 2.7. So this fix doesn't work on Python 3 (where there's no lib-tk directory). Can it be updated to cover that as well?
The fix file also exist in Python 3, here: "C:\Python34\Lib\tkinter_fix.py"
it's simmilar content to "C:\Python27\Lib\lib-tk\FixTk.py"
I would explicitly limit the scope to 2.x. I doubt one can fix this for both 2.x and 3.x at the same time, and all traffic on issue #93 (and its parent https://bugs.launchpad.net/virtualenv/+bug/449537) is from python 2.x users. Yes - FixTk.py should only be written to 2.x virtual envs. I will look into excluding it from 3.x virtual envs. What is the consensus about including the edited FixTk.py the way I did, vs. the risk of usptream python changes, considering this will be a Python 2.x thing only? |
I think it should be fine provided it's inserted into 2.x virtualenvs only. |
I agree with @Ivoz. Maybe add a comment explaining where FixTk.py came from (i.e., that it's a copy of the file from the Python 2.7 sources). Also, if someone can test that this is not an issue for Python 3.x that would be good. I'd rather we didn't end up adding a barrier to people upgrading to Python 3.x "because matplotlib stops working in my virtualenvs". |
Just trying to find out if this is going to be merged to master anytime soon? |
I'd also like this to be merged at some point in future - any update? |
d33e617
to
1682ed6
Compare
Same here. |
Hello! As part of an effort to ease the contribution process and adopt a more standard workflow virtualenv has switched to doing development on the If you do nothing, this Pull Request will be automatically migrated by @BrownTruck for you. If you would like to retain control over this pull request then you should resubmit it against the If you choose to migrate this Pull Request yourself, here is an example message that you can copy and paste:
If this pull request is no longer needed, please feel free to close it. |
This Pull Request has been automatically migrated to #911 to reparent it to the |
See issue #93 for details and alternative solutions. One suggestion was to bring code from Lib\lib-tk\FixTk.py into site.py, but leaving site.py alone and copying a minimally modified FixTk.py to the Lib directory when the virtualenv is created seems cleaner to me. The only changes to FixTk.py were to replace sys.prefix with sys.real_prefix at lines 49 and 52.
Fixing this is really important because it affects matplotlib.
All nosetests pass and matplotlib and Tkinter work on python 2.6 and 2.7. Issue #93 referred to Python 2.6 so this would close it. A quick test importing tkinter and calling tkinter._test() in a Python 3.4 virtualenv shows the problem is unresolved there, but it probably doesn't matter anyway since Python 3.3 and venv.