diff --git a/docs/changelog/1809.bugfix.rst b/docs/changelog/1809.bugfix.rst new file mode 100644 index 000000000..cbd1b063e --- /dev/null +++ b/docs/changelog/1809.bugfix.rst @@ -0,0 +1 @@ +Fix download fails with python 3.4 - by :user:`gaborbernat`. diff --git a/docs/changelog/1813.bugfix.rst b/docs/changelog/1813.bugfix.rst new file mode 100644 index 000000000..7a894bb43 --- /dev/null +++ b/docs/changelog/1813.bugfix.rst @@ -0,0 +1 @@ +Fix download is ``True`` by default - by :user:`gaborbernat`. diff --git a/docs/changelog/1814.bugfix.rst b/docs/changelog/1814.bugfix.rst new file mode 100644 index 000000000..f4195c4a3 --- /dev/null +++ b/docs/changelog/1814.bugfix.rst @@ -0,0 +1 @@ +Fail ``app-data`` seed operation when wheel download fails and better error message - by :user:`gaborbernat`. diff --git a/src/virtualenv/seed/embed/base_embed.py b/src/virtualenv/seed/embed/base_embed.py index 1067a7b70..e98a72d1c 100644 --- a/src/virtualenv/seed/embed/base_embed.py +++ b/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", diff --git a/src/virtualenv/seed/embed/wheels/acquire.py b/src/virtualenv/seed/embed/wheels/acquire.py index 72ab4a626..28ac6a770 100644 --- a/src/virtualenv/seed/embed/wheels/acquire.py +++ b/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 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 0e3fb9936..3d4b7c720 100644 --- a/src/virtualenv/seed/via_app_data/via_app_data.py +++ b/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): diff --git a/tests/unit/seed/test_base_embed.py b/tests/unit/seed/test_base_embed.py new file mode 100644 index 000000000..379211c6e --- /dev/null +++ b/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