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

Copy only required files from exec_prefix and only if it is really different from prefix path #1282

Merged
merged 7 commits into from Jan 25, 2019

Conversation

Projects
None yet
5 participants
@daa
Copy link
Contributor

daa commented Jan 14, 2019

When sys.exec_prefix contains relative parts but actually points to the same directory as sys.base_prefix or sys.prefix virtualenv unnecessarily copies files from it and can't set up virtual environment. This happens for example when distribution uses python-exec such as Gentoo, another manifestation is described in #1270.

Closes #1270.

Thanks for contributing a pull request, see checklist all is good!

  • 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 14, 2019

@daa

This comment has been minimized.

Copy link
Contributor Author

daa commented Jan 14, 2019

I'm not sure how to test this change, would like any advice.

@gaborbernat

This comment has been minimized.

Copy link
Contributor

gaborbernat commented Jan 14, 2019

@daa should a symlink be enough to test, not?

@fungi

This comment has been minimized.

Copy link

fungi commented Jan 14, 2019

A symlink should be enough to test as long as it's to a relative rather than absolute path.

@daa

This comment has been minimized.

Copy link
Contributor Author

daa commented Jan 14, 2019

@gaborbernat @fungi Symlink will suffice, thank you for suggestion.

home_dir, lib_dir, inc_dir, bin_dir = virtualenv.path_locations(venvdir)
assert not os.path.islink(os.path.join(lib_dir, "distutils"))
finally:
os.environ["PATH"] = old_path

This comment has been minimized.

@gaborbernat

gaborbernat Jan 14, 2019

Contributor

note literals are unicode and windows requires str for os.environ (python 2)

This comment has been minimized.

@daa

daa Jan 14, 2019

Author Contributor

Thank you, seen that recently but forgot.

daa added some commits Jan 14, 2019

@daa

This comment has been minimized.

Copy link
Contributor Author

daa commented Jan 14, 2019

While working on the test for this case I've noticed that creation of virtual environment using Python from another virtual environment results in distutils in new environment being symlink to distuils from source environment. Not sure if it should be fixed because it makes no harm besides new virtual environment being dependent on the older and the fix may be complicated.
Reason is the same - copying files when exec_prefix does not match prefix.

@daa

This comment has been minimized.

Copy link
Contributor Author

daa commented Jan 16, 2019

As mentioned in the related ticket linking distutils when creating virtual environment from virtual environment actually makes harm, to cure it I've limited copying from Python library directory inside sys.exec_prefix to required files only.

@daa daa changed the title Copy files from exec_prefix only if it is really different from prefix path Copy only required files from exec_prefix and only if it is really different from prefix path Jan 16, 2019

python_dir.mkdir()
path_key = "PATH"
if six.PY2:
path_key = path_key.encode()

This comment has been minimized.

@asottile

asottile Jan 20, 2019

Contributor

a neat trick for "native string literal" is path_key = str("PATH")

This comment has been minimized.

@daa

daa Jan 21, 2019

Author Contributor

Thank you.

@gaborbernat
Copy link
Contributor

gaborbernat left a comment

I approve.

seal of appro

Thanks!

@gaborbernat gaborbernat merged commit a393048 into pypa:master Jan 25, 2019

1 check passed

pypa.virtualenv #pypa.virtualenv_20190121.01 succeeded
Details
@daa

This comment has been minimized.

Copy link
Contributor Author

daa commented Jan 25, 2019

Thank you!

@daa daa deleted the daa:copy-exec-prefix-bugfix branch Jan 25, 2019

@andreineculau

This comment has been minimized.

Copy link

andreineculau commented Jan 25, 2019

👏

any chance of a patch release soon to get this out?

@gaborbernat

This comment has been minimized.

Copy link
Contributor

gaborbernat commented Jan 25, 2019

Released as 16.3.0.
thanks

andreineculau added a commit to andreineculau/homebrew-core that referenced this pull request Jan 25, 2019

Upgrade pipenv's virtualenv dependency to 16.3.0
This will pick up a fix that makes pipenv 100% unusable on some systems.
Ref: pypa/virtualenv#1282

andreineculau added a commit to andreineculau/homebrew-core that referenced this pull request Jan 25, 2019

pipenv: upgrade virtualenv dependency to 16.3.0
This will pick up a fix that makes pipenv 100% unusable on some systems.
Ref: pypa/virtualenv#1282

@andreineculau andreineculau referenced this pull request Jan 25, 2019

Closed

pipenv: upgrade virtualenv dependency to 16.3.0 #36395

5 of 5 tasks complete

BrewTestBot added a commit to BrewTestBot/homebrew-core that referenced this pull request Jan 25, 2019

pipenv: upgrade virtualenv dependency to 16.3.0
This will pick up a fix that makes pipenv 100% unusable on some systems.
Ref: pypa/virtualenv#1282

lembacon added a commit to lembacon/homebrew-core that referenced this pull request Jan 26, 2019

pipenv: virtualenv 16.3.0
This will pick up a fix that makes pipenv 100% unusable on some systems.
Ref: pypa/virtualenv#1282

Closes Homebrew#36395.

Signed-off-by: Chongyu Zhu <i@lembacon.com>

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

[update to 16.3.0] Copy only required files from exec_prefix and only…
… if it is really different from prefix path (#1282)

Adam Sampson (2):
      Dereference symlinks more thoroughly in copyfile (#956)
      Use os.path.realpath to resolve the symlink. (#1259)

Anthony Scopatz (1):
      Xonsh Support (#1206)

Anthony Sottile (3):
      Update outdated docs referring to bin/rebuild-script.py (#1286)
      Use importlib over imp in python3.x (#1289)
      produce a virtualenv zipapp (#1287)

Antti Haapala (1):
      Allow sourcing activate.sh in shells "exit on error" mode

Bernat Gabor (19):
      fix master not embeded
      pass through proxy and HOME variables
      add ISSUE/PR template, include pyproject.toml, clean not needed news
      Add news documenation for #1244
      clarify project
      add changelog
      Move to a declarative build script via setup.cfg, improve package metadata
      add pytest xdist to speed up tests
      add release Azure task
      fix releae
      release 16.2.0
      fish 3 compatbility
      also deploy sdist
      fix linting
      add stale issue
      try to fix brew CI break
      fix documentation, alter faulty examples
      avoid packaging non source code
      release 16.3.0

Bernát Gábor (11):
      enable test report for forks (#1243)
      bump embeded wheels (#1257)
      Shell fixes (#1258)
      code improvements (#1262)
      Jython enable for coverage report. (#1263)
      revert to a non src layout (#1264)
      Fix boostrap script generation (#1265)
      Support the embed script on Windows (#1266)
      make assertions on packaging, recommend tox for all testings (#1267)
      as powershell is now universaly available, do not enforce Windows line endings (#1268)
      bump vendored pip to 19.0.1 (#1291)

BrownTruck (1):
      solved problem when using -u in a non interactive shell (#922)

George Lund (1):
      Link to Python 3 venv

Jurko Gospodnetić (1):
      update testing notes, and test space path support

Kyle Altendorf (1):
      Discourage installation as root, such as with sudo (#1283)

Mickaël Schoentgen (1):
      Fix ResourceWarning: unclosed file (#1277)

Microdog (1):
      Pre-import some built-in modules in PyPy (#1281)

Miro Hrončok (2):
      Add test for missing pip/certifi's cacert.pem
      Fix error with missing pip/certifi's cacert.pem

Sviatoslav Sydorenko (1):
      Fix testenv:upgrade deps in tox config (#1276)

Wouter De Borger (1):
      made lib64 symlink relative again (#1249)

Zachary Ware (1):
      Update default license URL to match upstream (#1247)

anatoly techtonik (1):
      Document when --always-copy was introduced (in PR #409)

daa (2):
      Do not override certificate bundle if it is defined in pip config (#1273)
      Copy only required files from exec_prefix and only if it is really different from prefix path (#1282)

drevicko (1):
      added instructions for changing --system-site-packages in existing envir... (#738)

nonylene (1):
      Set VIRTUAL_ENV enviroment variable in activate_this.py (#1057)

ptaneli (1):
      Move to news files for changelog via towncrier. (#1234)

ykdojo (1):
      Fixed a typo in index.rst (#1269)

Éric Araujo (1):
      fix reporting in site module (#1245)

v16.3.0 (2019-01-25)
--------------------

Bugfixes
^^^^^^^^

- Use ``importlib`` over deprecated ``imp` in ``distutils/__init__.py`` for python 3 - by Anthony Sottile (`#955 <https://github.com/pypa/virtualenv/issues/955>`_)
- Preserve ``cert`` option defined in ``pip.conf`` or environment variable. (`#1273 <https://github.com/pypa/virtualenv/issues/1273>`_)
- fixed a ``ResourceWarning: unclosed file`` in ``call_subprocess()`` - by Mickaël Schoentgen (`#1277 <https://github.com/pypa/virtualenv/issues/1277>`_)
- pre-import some built-in modules in ``site.py`` on PyPy according to PyPy's ``site.py`` - by microdog (`#1281 <https://github.com/pypa/virtualenv/issues/1281>`_)
- Copy files from ``sys.exec_prefix`` only if it is really different path than
  used prefix, bugfix for #1270 (`#1282 <https://github.com/pypa/virtualenv/issues/1282>`_)

Features

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