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

Link or copy PyPy headers instead of include directory as a whole #1302

Merged
merged 7 commits into from Jan 31, 2019

Conversation

Projects
None yet
3 participants
@daa
Copy link
Contributor

daa commented Jan 30, 2019

This pull request changes method of virtualenv's include directory population - instead of linking Python include directory (for example /usr/include/python3.7m) it links or copies distinct header files from that directory. The reason for this change is to make virtualenv'` include directory not to be symlink in case of PyPy.

Closes #1198

  • wrote descriptive pull request text
  • added/updated test(s)
  • updated/extended the documentation
  • added news fragment in docs/changelog folder

daa added some commits Jan 30, 2019

@gaborbernat

This comment has been minimized.

Copy link
Contributor

gaborbernat commented Jan 30, 2019

@daa is there any reason why this should not be a PyPy only feature? What are the performance penalties we take for this? Is it needed for CPython and Jython?

@daa

This comment has been minimized.

Copy link
Contributor Author

daa commented Jan 30, 2019

First I wanted it to be not only PyPy feature to avoid its special handling. For CPython it is not needed, for Jython it does not make sense because Jython doesn't have include headers. Regarding performance implications - I haven't thought of it, here are some superficial timings:

# without patch
$ rm -rf ~/e; time python3.6 virtualenv.py ~/e --no-pip --no-setuptools --no-wheel
Using base prefix '/usr'
New python executable in /home/daa/e/bin/python3.6
Also creating executable in /home/daa/e/bin/python
Please make sure you remove any previous custom paths from your /home/daa/.pydistutils.cfg file.

real	0m0,116s
user	0m0,101s
sys	0m0,015s

# with patch
$ rm -rf ~/e; time python3.6 virtualenv.py ~/e --no-pip --no-setuptools --no-wheel
Using base prefix '/usr'
New python executable in /home/daa/e/bin/python3.6
Also creating executable in /home/daa/e/bin/python
Please make sure you remove any previous custom paths from your /home/daa/.pydistutils.cfg file.

real	0m0,138s
user	0m0,122s
sys	0m0,016s

So it has some performance impact and if you'd like to enable this feature only when strictly required I can change it.

@gaborbernat

This comment has been minimized.

Copy link
Contributor

gaborbernat commented Jan 30, 2019

Given this, I would prefer to make it a pypy only feature. We already have tons of extra edge cases we handle for various platforms, just add an explanation of why this issue arises for pypy only. @asottile any opinions on this?

@asottile

This comment has been minimized.

Copy link
Contributor

asottile commented Jan 30, 2019

only enabling this for pypy makes sense to me (probably in the same place where pypy creates the include symlink?)

Since include isn't created for the other platforms, things which install to venv/include already work

daa added some commits Jan 30, 2019

@daa daa changed the title Link Python include files instead of include directory as a whole Link or copy PyPy headers instead of include directory as a whole Jan 30, 2019

@gaborbernat gaborbernat merged commit d53a3c4 into pypa:master Jan 31, 2019

1 check passed

pypa.virtualenv #pypa.virtualenv_20190130.12 succeeded
Details
@gaborbernat

This comment has been minimized.

Copy link
Contributor

gaborbernat commented Jan 31, 2019

thanks

clrpackages pushed a commit to clearlinux-pkgs/virtualenv that referenced this pull request Feb 15, 2019

virtualenv: Autospec creation for update from version 16.3.0 to versi…
…on 16.4.0

Anthony Sottile (2):
      Use importlib in python3 (#1293)
      virtualenv does not *always* have a runtime dependency on setuptools (#1300)

Bernat Gabor (2):
      release 16.4.0
      release 16.4.0

Bernát Gábor (2):
      Update virtualenv.py
      upgrade setuptools and pip (#1312)

daa (2):
      Do not check pip config if such command is not available (#1303)
      Link or copy PyPy headers instead of include directory as a whole (#1302)

rofl0r (1):
      fix handling of relative symlinks (#1309)

v16.4.0 (2019-02-09)
--------------------

Bugfixes
^^^^^^^^

- fixes the scenario where the python base install is symlinked with relative symlinks (`#490 <https://github.com/pypa/virtualenv/issues/490>`_)
- Use ``importlib`` over ``imp`` in ``virtualenv.py`` for ``python >= 3.4`` - by Anthony Sottile (`#1293 <https://github.com/pypa/virtualenv/issues/1293>`_)
- Copy or link PyPy header files instead of include directory itself (`#1302 <https://github.com/pypa/virtualenv/issues/1302>`_)
- Allow virtualenv creation with older pip not having ``config`` command
  correspondingly disabling configuration related features (such as pip cert
  setting) in this case. (`#1303 <https://github.com/pypa/virtualenv/issues/1303>`_)

Features

(NEWS truncated at 15 lines)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment