Skip to content

Commit

Permalink
Dereference symlinks more thoroughly in copyfile
Browse files Browse the repository at this point in the history
copyfile was broken for symlinks that pointed to relative paths, and for
symlinks that pointed to further symlinks (which meant that virtualenv
failed to work in some cases when the Python executable it found was
a symlink, e.g. when Python had been installed using stow or homebrew;
issue pypa#268).

Make it follow symlinks until it finds an actual file to copy.
  • Loading branch information
atsampson committed Dec 17, 2018
1 parent 17bb29d commit ddeb564
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 4 deletions.
1 change: 1 addition & 0 deletions docs/changes.rst
Expand Up @@ -8,6 +8,7 @@ Release History
* pip wheels with removed certifi's ``cacert.pem`` are now supported :pull:`1252`
* upgrade setuptools from ``40.5.0`` to ``40.6.3``
* upgrade wheel from ``0.32.2`` to ``0.32.3``
* bug fix: ``copyfile`` now handles relative symlinks and symlinks to symlinks, avoiding problems when Python was installed using stow or homebrew :issue:`268`

16.1.0 (2018-10-31)
-------------------
Expand Down
8 changes: 4 additions & 4 deletions src/virtualenv.py
Expand Up @@ -398,10 +398,10 @@ def copyfile(src, dest, symlink=True):
if not os.path.exists(os.path.dirname(dest)):
logger.info("Creating parent directories for %s", os.path.dirname(dest))
os.makedirs(os.path.dirname(dest))
if not os.path.islink(src):
srcpath = os.path.abspath(src)
else:
srcpath = os.readlink(src)
srcpath = src
while os.path.islink(srcpath):
srcpath = os.path.join(os.path.dirname(srcpath), os.readlink(srcpath))
srcpath = os.path.abspath(srcpath)
if symlink and hasattr(os, "symlink") and not is_win:
logger.info("Symlinking %s", dest)
try:
Expand Down
28 changes: 28 additions & 0 deletions tests/test_virtualenv.py
Expand Up @@ -337,6 +337,34 @@ def test_relative_symlink(tmpdir):
assert os.path.exists(lib64)


@pytest.mark.skipif(not hasattr(os, "symlink"), reason="requires working symlink implementation")
def test_copyfile_from_symlink(tmp_path):
"""Test that copyfile works correctly when the source is a symlink with a
relative target, and a symlink to a symlink. (This can occur when creating
an environment if Python was installed using stow or homebrew.)"""

# Set up src/link2 -> ../src/link1 -> file.
src_dir = tmp_path / "src"
src_dir.mkdir()
with open(str(src_dir / "file"), "w") as f:
f.write("contents")
os.symlink("file", str(src_dir / "link1"))
os.symlink(str(Path("..") / "src" / "link1"), str(src_dir / "link2"))

# Check that copyfile works on link2.
# This may produce a symlink or a regular file depending on the platform --
# which doesn't matter as long as it has the right contents.
dest_dir = tmp_path / "dest"
dest_dir.mkdir()
copy_path = dest_dir / "copy"
virtualenv.copyfile(str(src_dir / "link2"), str(copy_path))
with open(str(copy_path), "r") as f:
assert f.read() == "contents"

shutil.rmtree(str(src_dir))
shutil.rmtree(str(dest_dir))


def test_missing_certifi_pem(tmp_path):
"""Make sure that we can still create virtual environment if pip is
patched to not use certifi's cacert.pem and the file is removed.
Expand Down

0 comments on commit ddeb564

Please sign in to comment.