Better pip download failure handling (#1814)

docs/changelog/1809.bugfix.rst

@@ -0,0 +1 @@
+Fix download fails with python 3.4 - by :user:`gaborbernat`.

docs/changelog/1813.bugfix.rst

@@ -0,0 +1 @@
+Fix download is ``True`` by default - by :user:`gaborbernat`.

docs/changelog/1814.bugfix.rst

@@ -0,0 +1 @@
+Fail ``app-data`` seed operation when wheel download fails and better error message - by :user:`gaborbernat`.

src/virtualenv/seed/embed/base_embed.py

@@ -45,13 +45,6 @@ def package_version(self):
     @classmethod
     def add_parser_arguments(cls, parser, interpreter, app_data):
         group = parser.add_mutually_exclusive_group()
-        group.add_argument(
-            "--download",
-            dest="download",
-            action="store_true",
-            help="pass to enable download of the latest {} from PyPI".format("/".join(cls.packages)),
-            default=False,
-        )
         group.add_argument(
             "--no-download",
             "--never-download",
@@ -60,6 +53,13 @@ def add_parser_arguments(cls, parser, interpreter, app_data):
             help="pass to disable download of the latest {} from PyPI".format("/".join(cls.packages)),
             default=True,
         )
+        group.add_argument(
+            "--download",
+            dest="download",
+            action="store_true",
+            help="pass to enable download of the latest {} from PyPI".format("/".join(cls.packages)),
+            default=False,
+        )
         parser.add_argument(
             "--extra-search-dir",
             metavar="d",

src/virtualenv/seed/embed/wheels/acquire.py

@@ -21,7 +21,16 @@
 BUNDLE_FOLDER = Path(os.path.abspath(__file__)).parent
 
 
-def get_wheels(for_py_version, wheel_cache_dir, extra_search_dir, download, packages, app_data):
+class WheelDownloadFail(ValueError):
+    def __init__(self, packages, for_py_version, exit_code, out, err):
+        self.packages = packages
+        self.for_py_version = for_py_version
+        self.exit_code = exit_code
+        self.out = out.strip()
+        self.err = err.strip()
+
+
+def get_wheels(for_py_version, wheel_cache_dir, extra_search_dir, packages, app_data, download):
     # not all wheels are compatible with all python versions, so we need to py version qualify it
     processed = copy(packages)
     # 1. acquire from bundle
@@ -147,11 +156,11 @@ def download_wheel(packages, for_py_version, to_folder, app_data):
     cmd.extend(to_download)
     # pip has no interface in python - must be a new sub-process
 
-    with pip_wheel_env_run("{}{}".format(*sys.version_info[0:2]), app_data) as env:
-        process = Popen(cmd, env=env, stdout=subprocess.PIPE)
-        process.communicate()
+    with pip_wheel_env_run("{}.{}".format(*sys.version_info[0:2]), app_data) as env:
+        process = Popen(cmd, env=env, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
+        out, err = process.communicate()
         if process.returncode != 0:
-            raise RuntimeError("failed to download wheels")
+            raise WheelDownloadFail(packages, for_py_version, process.returncode, out, err)
 
 
 @contextmanager

src/virtualenv/seed/via_app_data/via_app_data.py

@@ -3,11 +3,12 @@
 
 import logging
 from contextlib import contextmanager
+from functools import partial
 from threading import Lock, Thread
 
 from virtualenv.info import fs_supports_symlink
 from virtualenv.seed.embed.base_embed import BaseEmbed
-from virtualenv.seed.embed.wheels.acquire import get_wheels
+from virtualenv.seed.embed.wheels.acquire import WheelDownloadFail, get_wheels
 from virtualenv.util.path import safe_delete
 
 from .pip_install.copy import CopyPipInstall
@@ -63,26 +64,54 @@ def _get_seed_wheels(self, creator, base_cache):
             if wheels_to.exists():
                 safe_delete(wheels_to)
             wheels_to.mkdir(parents=True, exist_ok=True)
-            name_to_whl, lock = {}, Lock()
+            name_to_whl, lock, fail = {}, Lock(), {}
 
             def _get(package, version):
-                result = get_wheels(
+                wheel_loader = partial(
+                    get_wheels,
                     creator.interpreter.version_release_str,
                     wheels_to,
                     self.extra_search_dir,
-                    self.download,
                     {package: version},
                     self.app_data,
                 )
-                with lock:
-                    name_to_whl.update(result)
+                failure, result = None, None
+                # fallback to download in case the exact version is not available
+                for download in [True] if self.download else [False, True]:
+                    failure = None
+                    try:
+                        result = wheel_loader(download)
+                        if result:
+                            break
+                    except Exception as exception:
+                        failure = exception
+                if failure:
+                    if isinstance(failure, WheelDownloadFail):
+                        msg = "failed to download {}".format(package)
+                        if version is not None:
+                            msg += " version {}".format(version)
+                        msg += ", pip download exit code {}".format(failure.exit_code)
+                        output = failure.out + failure.err
+                        if output:
+                            msg += "\n"
+                            msg += output
+                    else:
+                        msg = repr(failure)
+                    logging.error(msg)
+                    with lock:
+                        fail[package] = version
+                else:
+                    with lock:
+                        name_to_whl.update(result)
 
-            threads = list(Thread(target=_get, args=(pkg, v)) for pkg, v in self.package_version().items())
+            package_versions = self.package_version()
+            threads = list(Thread(target=_get, args=(pkg, v)) for pkg, v in package_versions.items())
             for thread in threads:
                 thread.start()
             for thread in threads:
                 thread.join()
-
+            if fail:
+                raise RuntimeError("seed failed due to failing to download wheels {}".format(", ".join(fail.keys())))
             yield name_to_whl
 
     def installer_class(self, pip_version):

tests/unit/seed/test_base_embed.py

@@ -0,0 +1,11 @@
+import pytest
+
+from virtualenv.run import session_via_cli
+
+
+@pytest.mark.parametrize(
+    "args, download", [([], False), (["--no-download"], False), (["--never-download"], False), (["--download"], True)]
+)
+def test_download_cli_flag(args, download, tmp_path):
+    session = session_via_cli(args + [str(tmp_path)])
+    assert session.seeder.download is download