Repository: pypa/virtualenv

Issue:
virtualenv 20: is the symlink hack really worth it?
I did some timing and it seems like the trouble it causes is not really worth it -- at the very least I'd like an option which copies instead of symlinks

Here's some timing I did to try and guage the differences -- since there's no options I could find I toggled this line to `if False` to get my "copy" data: https://github.com/pypa/virtualenv/blob/8c2985c2946e767bb6f74a7e22f51add17b38987/src/virtualenv/seed/via_app_data/via_app_data.py#L92

### with symlinks

my platform for this example is relatively low powered, a 2015 MBP

```console
$ rm -rf vvv; time virtualenv vvv

real	0m0.128s
user	0m0.107s
sys	0m0.023s
$ rm -rf vvv; time virtualenv vvv

real	0m0.128s
user	0m0.118s
sys	0m0.012s
$ rm -rf vvv; time virtualenv vvv

real	0m0.123s
user	0m0.121s
sys	0m0.004s
$ rm -rf vvv; time virtualenv vvv

real	0m0.119s
user	0m0.117s
sys	0m0.004s
$ rm -rf vvv; time virtualenv vvv

real	0m0.127s
user	0m0.109s
sys	0m0.020s
```

disk usage:

```console
$ du -hs vvv
128K	vvv
```

problems this can cause:

```console
$ # copied to same path on other machine
$ ./vvv/bin/python -c 'import setuptools'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ModuleNotFoundError: No module named 'setuptools'
$ ./vvv/bin/pip --help
Traceback (most recent call last):
  File "./vvv/bin/pip", line 6, in <module>
    from pip._internal.cli.main import main
ModuleNotFoundError: No module named 'pip'
```

### with copies

```console
$ rm -rf vvv; time virtualenv vvv

real	0m0.179s
user	0m0.155s
sys	0m0.050s
$ rm -rf vvv; time virtualenv vvv

real	0m0.185s
user	0m0.158s
sys	0m0.050s
$ rm -rf vvv; time virtualenv vvv

real	0m0.183s
user	0m0.160s
sys	0m0.048s
$ rm -rf vvv; time virtualenv vvv

real	0m0.172s
user	0m0.162s
sys	0m0.035s
$ rm -rf vvv; time virtualenv vvv

real	0m0.181s
user	0m0.142s
sys	0m0.065s
```

```console
$ du -hs vvv
7.5M	vvv
```

### trade off

so we're looking at ~60ms of time overhead -- which (imo) isn't that much -- the disk usage is another concern but we're still taking that usage one way or another

### other considerations

hardlinks would be another consideration -- it would alleviate the problems I have with symlinks (caches, using virtualenv as a deployment mechanism, etc.) -- I'd have to do some implementation work to verify that case

Discussion:

Patch:
diff --git a/docs/changelog/1557.feature.rst b/docs/changelog/1557.feature.rst
index 1e39ff13..c02d66b7 100644
--- a/docs/changelog/1557.feature.rst
+++ b/docs/changelog/1557.feature.rst
@@ -1,2 +1,2 @@
 Print out a one line message about the created virtual environment when no :option:`verbose` is set, this can now be
-sileneced to get back the original behaviour via the :option:`quiet` flag - by :user:`pradyunsg`.
+silenced to get back the original behaviour via the :option:`quiet` flag - by :user:`pradyunsg`.
diff --git a/docs/changelog/1561.bugfix.rst b/docs/changelog/1561.bugfix.rst
new file mode 100644
index 00000000..c50c0c4c
--- /dev/null
+++ b/docs/changelog/1561.bugfix.rst
@@ -0,0 +1,3 @@
+When copying (either files or trees) do not copy the permission bits, last access time, last modification time, and
+flags as access to these might be forbidden (for example in case of the macOs Framework Python) and these are not needed
+for the user to use the virtual environment - by :user:`gaborbernat`.
diff --git a/docs/changelog/1575.bugfix.rst b/docs/changelog/1575.bugfix.rst
index 2dd2d72e..1c6bff95 100644
--- a/docs/changelog/1575.bugfix.rst
+++ b/docs/changelog/1575.bugfix.rst
@@ -1,1 +1,3 @@
-Respect the :option:`copies` flag inside the ``app-data`` :option:`seeder` by :user:`gaborbernat`.
+The ``app-data`` :option:`seeder` no longer symlinks the packages on UNIX and copies on Windows. Instead by default
+always copies, however now has the :option:`symlink-app-data` flag allowing users to request this less robust but faster
+method - by :user:`gaborbernat`.
diff --git a/src/virtualenv/seed/via_app_data/pip_install/base.py b/src/virtualenv/seed/via_app_data/pip_install/base.py
index 7082cf67..2878a65a 100644
--- a/src/virtualenv/seed/via_app_data/pip_install/base.py
+++ b/src/virtualenv/seed/via_app_data/pip_install/base.py
@@ -36,7 +36,6 @@ class PipInstall(object):
         # sync image
         for filename in self._image_dir.iterdir():
             into = self._creator.purelib / filename.name
-            logging.debug("%s %s from %s", self.__class__.__name__, into, filename)
             if into.exists():
                 if into.is_dir() and not into.is_symlink():
                     shutil.rmtree(str(into))
diff --git a/src/virtualenv/seed/via_app_data/pip_install/copy.py b/src/virtualenv/seed/via_app_data/pip_install/copy.py
index 5a3424e4..1b76edb6 100644
--- a/src/virtualenv/seed/via_app_data/pip_install/copy.py
+++ b/src/virtualenv/seed/via_app_data/pip_install/copy.py
@@ -1,23 +1,17 @@
 from __future__ import absolute_import, unicode_literals
 
 import os
-import shutil
 
 import six
 
-from virtualenv.util.path import Path
+from virtualenv.util.path import Path, copy
 
 from .base import PipInstall
 
 
 class CopyPipInstall(PipInstall):
     def _sync(self, src, dst):
-        src_str = six.ensure_text(str(src))
-        dest_str = six.ensure_text(str(dst))
-        if src.is_dir():
-            shutil.copytree(src_str, dest_str)
-        else:
-            shutil.copy(src_str, dest_str)
+        copy(src, dst)
 
     def _generate_new_files(self):
         # create the pyc files
diff --git a/src/virtualenv/seed/via_app_data/via_app_data.py b/src/virtualenv/seed/via_app_data/via_app_data.py
index 9bfd9d12..62bcb28b 100644
--- a/src/virtualenv/seed/via_app_data/via_app_data.py
+++ b/src/virtualenv/seed/via_app_data/via_app_data.py
@@ -9,6 +9,7 @@ from threading import Lock, Thread
 import six
 
 from virtualenv.dirs import default_data_dir
+from virtualenv.info import fs_supports_symlink
 from virtualenv.seed.embed.base_embed import BaseEmbed
 from virtualenv.seed.embed.wheels.acquire import get_wheels
 
@@ -21,7 +22,7 @@ class FromAppData(BaseEmbed):
         super(FromAppData, self).__init__(options)
         self.clear = options.clear_app_data
         self.app_data_dir = default_data_dir() / "seed-v1"
-        self.symlinks = getattr(options, "copies", False) is False
+        self.symlinks = options.symlink_app_data
 
     @classmethod
     def add_parser_arguments(cls, parser, interpreter):
@@ -33,6 +34,16 @@ class FromAppData(BaseEmbed):
             help="clear the app data folder of seed images ({})".format((default_data_dir() / "seed-v1").path),
             default=False,
         )
+        can_symlink = fs_supports_symlink()
+        parser.add_argument(
+            "--symlink-app-data",
+            dest="symlink_app_data",
+            action="store_true" if can_symlink else "store_false",
+            help="{} symlink the python packages from the app-data folder (requires seed pip>=19.3)".format(
+                "" if can_symlink else "not supported - "
+            ),
+            default=False,
+        )
 
     def run(self, creator):
         if not self.enabled:
@@ -95,4 +106,6 @@ class FromAppData(BaseEmbed):
         return CopyPipInstall
 
     def __unicode__(self):
-        return super(FromAppData, self).__unicode__() + " app_data_dir={}".format(self.app_data_dir.path)
+        return super(FromAppData, self).__unicode__() + " app_data_dir={} via={}".format(
+            self.app_data_dir.path, "symlink" if self.symlinks else "copy"
+        )
diff --git a/src/virtualenv/util/path/__init__.py b/src/virtualenv/util/path/__init__.py
index 6cae2657..acc65f14 100644
--- a/src/virtualenv/util/path/__init__.py
+++ b/src/virtualenv/util/path/__init__.py
@@ -2,6 +2,14 @@ from __future__ import absolute_import, unicode_literals
 
 from ._pathlib import Path
 from ._permission import make_exe
-from ._sync import copy, ensure_dir, link, symlink
+from ._sync import copy, copytree, ensure_dir, link, symlink
 
-__all__ = ("ensure_dir", "link", "symlink", "copy", "Path", "make_exe")
+__all__ = (
+    "ensure_dir",
+    "link",
+    "symlink",
+    "copy",
+    "copytree",
+    "Path",
+    "make_exe",
+)
diff --git a/src/virtualenv/util/path/_sync.py b/src/virtualenv/util/path/_sync.py
index 9bb20676..dcf040fd 100644
--- a/src/virtualenv/util/path/_sync.py
+++ b/src/virtualenv/util/path/_sync.py
@@ -45,11 +45,22 @@ def symlink(src, dest):
 def copy(src, dest):
     ensure_safe_to_do(src, dest)
     is_dir = src.is_dir()
-    method = shutil.copytree if is_dir else shutil.copy2
+    method = copytree if is_dir else shutil.copy
     logging.debug("copy %s", _Debug(src, dest))
     method(norm(src), norm(dest))
 
 
+def copytree(src, dest):
+    for root, _, files in os.walk(src):
+        dest_dir = os.path.join(dest, os.path.relpath(root, src))
+        if not os.path.exists(dest_dir):
+            os.makedirs(dest_dir)
+        for name in files:
+            src_f = os.path.join(root, name)
+            dest_f = os.path.join(dest_dir, name)
+            shutil.copy(src_f, dest_f)
+
+
 def link(src, dest):
     ensure_safe_to_do(src, dest)
     logging.debug("hard link %s", _Debug(src, dest.name))
@@ -67,4 +78,12 @@ class _Debug(object):
         )
 
 
-__all__ = ("ensure_dir", "symlink", "copy", "link", "symlink", "link")
+__all__ = (
+    "ensure_dir",
+    "symlink",
+    "copy",
+    "link",
+    "symlink",
+    "link",
+    "copytree",
+)


Test patch:
diff --git a/tests/integration/test_zipapp.py b/tests/integration/test_zipapp.py
index 84dd0fb5..ed4f6ef4 100644
--- a/tests/integration/test_zipapp.py
+++ b/tests/integration/test_zipapp.py
@@ -29,7 +29,7 @@ def zipapp_build_env(tmp_path_factory):
                 try:
                     # create a virtual environment which is also guaranteed to contain a recent enough pip (bundled)
                     session = run_via_cli(
-                        ["-v", "-p", "{}3.{}".format(impl, version), "--activators", "", str(create_env_path)]
+                        ["-vvv", "-p", "{}3.{}".format(impl, version), "--activators", "", str(create_env_path)]
                     )
                     exe = str(session.creator.exe)
                     found = True
diff --git a/tests/unit/seed/test_boostrap_link_via_app_data.py b/tests/unit/seed/test_boostrap_link_via_app_data.py
index ca900e44..65620bd7 100644
--- a/tests/unit/seed/test_boostrap_link_via_app_data.py
+++ b/tests/unit/seed/test_boostrap_link_via_app_data.py
@@ -7,6 +7,7 @@ import pytest
 import six
 
 from virtualenv.discovery.py_info import PythonInfo
+from virtualenv.info import fs_supports_symlink
 from virtualenv.run import run_via_cli
 from virtualenv.seed.embed.wheels import BUNDLE_SUPPORT
 from virtualenv.seed.embed.wheels.acquire import BUNDLE_FOLDER
@@ -14,7 +15,8 @@ from virtualenv.util.subprocess import Popen
 
 
 @pytest.mark.slow
-def test_base_bootstrap_link_via_app_data(tmp_path, coverage_env, current_fastest):
+@pytest.mark.parametrize("copies", [True, False] if fs_supports_symlink() else [True])
+def test_base_bootstrap_link_via_app_data(tmp_path, coverage_env, current_fastest, copies):
     current = PythonInfo.current_system()
     bundle_ver = BUNDLE_SUPPORT[current.version_release_str]
     create_cmd = [
@@ -33,6 +35,8 @@ def test_base_bootstrap_link_via_app_data(tmp_path, coverage_env, current_fastes
         current_fastest,
         "-vv",
     ]
+    if not copies:
+        create_cmd.append("--symlink-app-data")
     result = run_via_cli(create_cmd)
     coverage_env()
     assert result