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

Supposed typo: https://github.com/pypa/virtualenv/issues/565#issuecom… #1010

Closed
wants to merge 1 commit into from

Conversation

Neurocinetics
Copy link

Supposed typo: #565 (comment) On CentOS 7 building certain projects fails without this "fix". Also relevant, spotify/dh-virtualenv#150 and numpy/numpy#7570.

@brycepg
Copy link

brycepg commented Feb 18, 2017

This commit fixed an error for me using the --always-copy flag on Centos 7.3 with virtualenv version 15.1.0

Here's the traceback which no longer occurs:

[user]~/% virtualenv -p $(which python3) dist --always-copy
Running virtualenv with interpreter /usr/bin/python3
Using base prefix '/usr'
Cannot find file lib (bad symlink)
New python executable in /home/user/dist/bin/python3
Also creating executable in /home/user/dist/bin/python
Installing setuptools, pip, wheel...
  Complete output from command /home/user/dist/bin/python3 - setuptools pip wheel:
  Traceback (most recent call last):
  File "<stdin>", line 4, in <module>
  File "/home/user/dist/lib64/python3.4/tempfile.py", line 34, in <module>
    import shutil as _shutil
  File "/home/user/dist/lib64/python3.4/shutil.py", line 14, in <module>
    import tarfile
  File "/home/user/dist/lib64/python3.4/tarfile.py", line 47, in <module>
    import time
ImportError: No module named 'time'

Hopefully it gets merged. Thank you.

@spacefito
Copy link

This patch also fixes this issue: #985
please merge is right.

@leondutoit
Copy link

+1 for merging this - I need this fix on Centos 7. Good job on finding the root cause!

@pfmoore
Copy link
Member

pfmoore commented Mar 6, 2017

The comment in #565 (comment) seems to be saying that this fix will cause issues on some other systems (CentOS 6, AIUI). Any fix that gets merged should work everywhere, not just trade a failure on one system for a failure on another.

@aelnaggar
Copy link

PLEASE MERGE IT

@pfmoore
Copy link
Member

pfmoore commented Jun 30, 2017

@aelnaggar Shouting at the developers is neither polite nor likely to be productive. We're aware of the potential benefit, but haven't a clear picture of the potential issues on other systems yet. It's extremely hard to maintain virtualenv precisely because of the various distribution idiosyncracies we have to cater for, and the virtualenv developers (who are all volunteers) have little enough time as it is.

If this issue is important enough to you, you're perfectly welcome to maintain this change as a local patch to your copy of virtualenv. Otherwise please be patient and respect the fact that you are getting virtualenv for free, as a result of others' volunteer efforts.

@Neurocinetics
Copy link
Author

@pfmoore Should I/we change the pull request, so that the updated pull request can distinguish a Debian system from a RHEL system? That would be a more appropriate fix right?

@joesanford
Copy link

If any updates are required to get this PR merged in, please let me know and I will gladly take over dev responsibility for this PR. If what is mentioned in comment #565 is what is needed, please let me know and I can ensure this only is run on affected distros.

@pfmoore
Copy link
Member

pfmoore commented Sep 20, 2018

I don't honestly know, myself. I'm not familiar with the differences between the various Linux distros, and how this change would need to cover that. Someone needs to do the due dilligence on how things vary between the various platforms (RHEL/CentOS, Fedora, Debian/Ubuntu, SuSE, Gentoo, Arch, ...?), and confirm that. Then I guess one of the virtualenv maintainers who does work with Linux should review it and take it from there.

(The PR is described as a "typo fix" - it may well be that to someone who knows the platforms, it's "obviously right", but I can't say on that).

@joesanford
Copy link

joesanford commented Sep 20, 2018

From what I can see in the suggested change, it's not really a "typo fix" and moreso branching behavior based on which distro is currently being used. If that's the case, I could add in the logic around platform.dist() (https://docs.python.org/3.7/library/platform.html#platform.dist) to only have this run on RHEL/CentOS/SuSe.

Thoughts?

@gaborbernat gaborbernat added this to the 16.1 milestone Oct 31, 2018
@gaborbernat gaborbernat self-assigned this Oct 31, 2018
@gaborbernat
Copy link
Contributor

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.

None yet

8 participants