From ddeb564841e6dc4dcee3718c932a2cbe2d2861fc Mon Sep 17 00:00:00 2001 From: Adam Sampson Date: Tue, 20 Sep 2016 19:22:24 +0100 Subject: [PATCH] Dereference symlinks more thoroughly in copyfile 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 #268). Make it follow symlinks until it finds an actual file to copy. --- docs/changes.rst | 1 + src/virtualenv.py | 8 ++++---- tests/test_virtualenv.py | 28 ++++++++++++++++++++++++++++ 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index dee3aa622..4bedcd6e5 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -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) ------------------- diff --git a/src/virtualenv.py b/src/virtualenv.py index e4b9897d1..dd352d8fa 100755 --- a/src/virtualenv.py +++ b/src/virtualenv.py @@ -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: diff --git a/tests/test_virtualenv.py b/tests/test_virtualenv.py index fa1f74d65..badfeaa2f 100644 --- a/tests/test_virtualenv.py +++ b/tests/test_virtualenv.py @@ -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.