From 9742921eb2b2cecde0acfa876e704fcd28e13d6b Mon Sep 17 00:00:00 2001 From: David Tucker Date: Wed, 11 Mar 2020 10:55:21 -0700 Subject: [PATCH 1/4] Allow missing .py files if .pyc is present --- docs/changelog/1714.bugfix.rst | 1 + .../via_global_ref/builtin/python2/python2.py | 8 +++-- tests/unit/create/test_creator.py | 29 +++++++++++++++++++ 3 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 docs/changelog/1714.bugfix.rst diff --git a/docs/changelog/1714.bugfix.rst b/docs/changelog/1714.bugfix.rst new file mode 100644 index 000000000..8a9bcaafa --- /dev/null +++ b/docs/changelog/1714.bugfix.rst @@ -0,0 +1 @@ +Allow missing ``.py`` files if a compiled ``.pyc`` version is available - by :user:`tucked`. diff --git a/src/virtualenv/create/via_global_ref/builtin/python2/python2.py b/src/virtualenv/create/via_global_ref/builtin/python2/python2.py index 7c8acf3f7..7813ea92e 100644 --- a/src/virtualenv/create/via_global_ref/builtin/python2/python2.py +++ b/src/virtualenv/create/via_global_ref/builtin/python2/python2.py @@ -58,9 +58,11 @@ def sources(cls, interpreter): # install files needed to run site.py for req in cls.modules(): stdlib_path = interpreter.stdlib_path("{}.py".format(req)) - yield PathRefToDest(stdlib_path, dest=cls.to_stdlib) - comp = stdlib_path.parent / "{}.pyc".format(req) - if comp.exists(): + comp = Path(stdlib_path.stem + ".pyc") + comp_exists = comp.exists() + if stdlib_path.exists() or not comp_exists: + yield PathRefToDest(stdlib_path, dest=cls.to_stdlib) + if comp_exists: yield PathRefToDest(comp, dest=cls.to_stdlib) def to_stdlib(self, src): diff --git a/tests/unit/create/test_creator.py b/tests/unit/create/test_creator.py index 70e0e6a84..dcaab90f3 100644 --- a/tests/unit/create/test_creator.py +++ b/tests/unit/create/test_creator.py @@ -19,6 +19,7 @@ from virtualenv.__main__ import run, run_with_catch from virtualenv.create.creator import DEBUG_SCRIPT, Creator, get_env_debug_info +from virtualenv.create.via_global_ref.builtin.cpython.cpython2 import CPython2 from virtualenv.discovery.builtin import get_interpreter from virtualenv.discovery.py_info import PythonInfo from virtualenv.info import IS_PYPY, IS_WIN, PY3, fs_is_case_sensitive, fs_supports_symlink @@ -448,3 +449,31 @@ def _get_sys_path(flag=None): assert non_python_path == [i for i in base if i not in extra_as_python_path] else: assert base == extra_all + + +@pytest.fixture +def creator_with_pyc_only_cp2_modules(tmp_path, monkeypatch, current_creators): + """ + Get a current_creator in a context where CPython2.modules only + references modules that have .pyc files (and no corresponding .py file). + """ + monkeypatch.chdir(tmp_path) + creator = current_creators[2] + module_name = os.path.relpath("CPython2_patch_modules_module", CURRENT.system_stdlib) + module_path = Path(CURRENT.system_stdlib) / "{}.pyc".format(module_name) + with module_path.open(mode="w"): + pass # just a touch + + @classmethod + def modules(cls): + return [module_name] + + monkeypatch.setattr(CPython2, "modules", modules) + yield creator + module_path.unlink() + + +@pytest.mark.skipif(PY3, reason=".py files are only checked for in py2.") +def test_pyc_only(creator_with_pyc_only_cp2_modules): + """Ensure that creation can succeed if os.pyc exists (even if os.py has been deleted).""" + assert creator_with_pyc_only_cp2_modules.can_create(CURRENT) is not None From 05dc1ad02237e234de626de1495c37de2ed40676 Mon Sep 17 00:00:00 2001 From: Bernat Gabor Date: Fri, 13 Mar 2020 12:15:43 +0000 Subject: [PATCH 2/4] Document packaging types we support Signed-off-by: Bernat Gabor --- docs/changelog/1714.doc.rst | 2 ++ docs/installation.rst | 39 +++++++++++++++++++++++++++++++++---- 2 files changed, 37 insertions(+), 4 deletions(-) create mode 100644 docs/changelog/1714.doc.rst diff --git a/docs/changelog/1714.doc.rst b/docs/changelog/1714.doc.rst new file mode 100644 index 000000000..e74174486 --- /dev/null +++ b/docs/changelog/1714.doc.rst @@ -0,0 +1,2 @@ +:ref:`supports ` details now explicitly what Python installations we support +- by :user:`gaborbernat`. diff --git a/docs/installation.rst b/docs/installation.rst index 2611fcaa7..92a82b47a 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -89,8 +89,39 @@ virtualenv works with the following Python interpreter implementations: - `PyPy `_ 2.7 and 3.4+. This means virtualenv works on the latest patch version of each of these minor versions. Previous patch versions are -supported on a best effort approach. virtualenv works on the following platforms: +supported on a best effort approach. -- Unix/Linux, -- macOS, -- Windows. +CPython is shipped in multiple forms, and each OS repackages it, often applying some customization along the way. +Therefore we cannot say universally that we support all platforms, but rather specify some we test against. In case +of ones not specified here the support is unknown, though likely will work. If you find some cases please open a feature +request on our issue tracker. + +Linux +~~~~~ +- installations from `python.org `_ +- Ubuntu 16.04+ (both upstream and `deasnakes `_ builds) +- Fedora +- RHEL and CentOS +- OpenSuse +- Arch Linux + +macOS +~~~~~ +In case of macOs we support: +- installations from `python.org `_ +- python versions installed via `brew `_ (both older python2.7 and python3) +- Python 3 part of XCode (Python framework - ``/Library/Frameworks/Python3.framework/``) +- Python 2 part of the OS (``/System/Library/Frameworks/Python.framework/Versions/``) + +Windows +~~~~~~~ +- Installations from `python.org `_ +- Windows Store Python - note only `version 3.8+ `_ (``3.7`` + was marked experimental and contains many bugs that would make it very hard for us to support it) + +Packaging variants +~~~~~~~~~~~~~~~~~~ +- Normal variant (file structure as comes from `python.org `_). +- We support system installations that do not contain the python files for the standard library if the respective + compiled files are present (e.g. only ``os.pyc``, not ``os.py``) - relevant mostly for ``Python 2.7``. This can be + used by custom systems may want to maximize available storage or obfuscate source code by removing ``.py`` files. From 4e8bb356b7395761b1d8582273cb03d522120bdd Mon Sep 17 00:00:00 2001 From: Bernat Gabor Date: Fri, 13 Mar 2020 15:06:21 +0000 Subject: [PATCH 3/4] improve test Signed-off-by: Bernat Gabor --- .../via_global_ref/builtin/python2/python2.py | 8 +-- tests/unit/create/test_creator.py | 51 ++++++++++--------- 2 files changed, 31 insertions(+), 28 deletions(-) diff --git a/src/virtualenv/create/via_global_ref/builtin/python2/python2.py b/src/virtualenv/create/via_global_ref/builtin/python2/python2.py index 7813ea92e..fb4325af3 100644 --- a/src/virtualenv/create/via_global_ref/builtin/python2/python2.py +++ b/src/virtualenv/create/via_global_ref/builtin/python2/python2.py @@ -57,13 +57,13 @@ def sources(cls, interpreter): yield src # install files needed to run site.py for req in cls.modules(): - stdlib_path = interpreter.stdlib_path("{}.py".format(req)) - comp = Path(stdlib_path.stem + ".pyc") + comp = interpreter.stdlib_path("{}.pyc".format(req)) comp_exists = comp.exists() - if stdlib_path.exists() or not comp_exists: - yield PathRefToDest(stdlib_path, dest=cls.to_stdlib) if comp_exists: yield PathRefToDest(comp, dest=cls.to_stdlib) + stdlib_path = interpreter.stdlib_path("{}.py".format(req)) + if stdlib_path.exists() or not comp_exists: + yield PathRefToDest(stdlib_path, dest=cls.to_stdlib) def to_stdlib(self, src): return self.stdlib / src.name diff --git a/tests/unit/create/test_creator.py b/tests/unit/create/test_creator.py index dcaab90f3..585419c16 100644 --- a/tests/unit/create/test_creator.py +++ b/tests/unit/create/test_creator.py @@ -19,10 +19,9 @@ from virtualenv.__main__ import run, run_with_catch from virtualenv.create.creator import DEBUG_SCRIPT, Creator, get_env_debug_info -from virtualenv.create.via_global_ref.builtin.cpython.cpython2 import CPython2 from virtualenv.discovery.builtin import get_interpreter from virtualenv.discovery.py_info import PythonInfo -from virtualenv.info import IS_PYPY, IS_WIN, PY3, fs_is_case_sensitive, fs_supports_symlink +from virtualenv.info import IS_PYPY, IS_WIN, PY2, PY3, fs_is_case_sensitive, fs_supports_symlink from virtualenv.pyenv_cfg import PyEnvCfg from virtualenv.run import cli_run, session_via_cli from virtualenv.util.path import Path @@ -451,29 +450,33 @@ def _get_sys_path(flag=None): assert base == extra_all -@pytest.fixture -def creator_with_pyc_only_cp2_modules(tmp_path, monkeypatch, current_creators): - """ - Get a current_creator in a context where CPython2.modules only - references modules that have .pyc files (and no corresponding .py file). - """ - monkeypatch.chdir(tmp_path) - creator = current_creators[2] - module_name = os.path.relpath("CPython2_patch_modules_module", CURRENT.system_stdlib) - module_path = Path(CURRENT.system_stdlib) / "{}.pyc".format(module_name) - with module_path.open(mode="w"): - pass # just a touch +@pytest.mark.skipif( + not PY2 or not ("builtin" in CURRENT.creators().key_to_class), reason="stdlib python files only needed for Python 2" +) +def test_pyc_only(tmp_path, mocker, session_app_data): + """Ensure that creation can succeed if os.pyc exists (even if os.py has been deleted)""" + interpreter = PythonInfo.from_exe(sys.executable, session_app_data) + host_pyc = interpreter.stdlib_path("os.pyc") + if not host_pyc.exists(): + pytest.skip("missing system os.pyc at {}".format(host_pyc)) + previous = interpreter.stdlib_path + + def stdlib_path(name): + path = previous(name) + if name.endswith(".py"): - @classmethod - def modules(cls): - return [module_name] + class _Path(type(path)): + @staticmethod + def exists(): + return False - monkeypatch.setattr(CPython2, "modules", modules) - yield creator - module_path.unlink() + return _Path(path) + return path + mocker.patch.object(interpreter, "stdlib_path", side_effect=stdlib_path) + + result = cli_run([ensure_text(str(tmp_path)), "--without-pip", "--activators", ""]) -@pytest.mark.skipif(PY3, reason=".py files are only checked for in py2.") -def test_pyc_only(creator_with_pyc_only_cp2_modules): - """Ensure that creation can succeed if os.pyc exists (even if os.py has been deleted).""" - assert creator_with_pyc_only_cp2_modules.can_create(CURRENT) is not None + assert not (result.creator.stdlib / "os.py").exists() + assert (result.creator.stdlib / "os.pyc").exists() + assert "os.pyc" in result.creator.debug["os"] From b5deb1c68fbec3ae1568d0b2fc4045bbe920aadd Mon Sep 17 00:00:00 2001 From: Bernat Gabor Date: Fri, 13 Mar 2020 16:56:52 +0000 Subject: [PATCH 4/4] PyPy requires the standard library source files Signed-off-by: Bernat Gabor --- docs/installation.rst | 6 ++-- .../builtin/cpython/cpython2.py | 4 +++ .../via_global_ref/builtin/pypy/pypy2.py | 4 +++ .../via_global_ref/builtin/python2/python2.py | 28 ++++++++++++++----- tests/unit/create/test_creator.py | 3 +- 5 files changed, 34 insertions(+), 11 deletions(-) diff --git a/docs/installation.rst b/docs/installation.rst index 92a82b47a..dffd3f5fa 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -122,6 +122,6 @@ Windows Packaging variants ~~~~~~~~~~~~~~~~~~ - Normal variant (file structure as comes from `python.org `_). -- We support system installations that do not contain the python files for the standard library if the respective - compiled files are present (e.g. only ``os.pyc``, not ``os.py``) - relevant mostly for ``Python 2.7``. This can be - used by custom systems may want to maximize available storage or obfuscate source code by removing ``.py`` files. +- We support CPython 2 system installations that do not contain the python files for the standard library if the + respective compiled files are present (e.g. only ``os.pyc``, not ``os.py``). This can be used by custom systems may + want to maximize available storage or obfuscate source code by removing ``.py`` files. diff --git a/src/virtualenv/create/via_global_ref/builtin/cpython/cpython2.py b/src/virtualenv/create/via_global_ref/builtin/cpython/cpython2.py index 4e20f4aef..e4c754968 100644 --- a/src/virtualenv/create/via_global_ref/builtin/cpython/cpython2.py +++ b/src/virtualenv/create/via_global_ref/builtin/cpython/cpython2.py @@ -25,6 +25,10 @@ def sources(cls, interpreter): if host_include_marker.exists(): yield PathRefToDest(host_include_marker.parent, dest=lambda self, _: self.include) + @classmethod + def needs_stdlib_py_module(cls): + return False + @classmethod def host_include_marker(cls, interpreter): return Path(interpreter.system_include) / "Python.h" diff --git a/src/virtualenv/create/via_global_ref/builtin/pypy/pypy2.py b/src/virtualenv/create/via_global_ref/builtin/pypy/pypy2.py index c7288da08..020000b34 100644 --- a/src/virtualenv/create/via_global_ref/builtin/pypy/pypy2.py +++ b/src/virtualenv/create/via_global_ref/builtin/pypy/pypy2.py @@ -31,6 +31,10 @@ def sources(cls, interpreter): if host_include_marker.exists(): yield PathRefToDest(host_include_marker.parent, dest=lambda self, _: self.include) + @classmethod + def needs_stdlib_py_module(cls): + return True + @classmethod def host_include_marker(cls, interpreter): return Path(interpreter.system_include) / "PyPy.h" diff --git a/src/virtualenv/create/via_global_ref/builtin/python2/python2.py b/src/virtualenv/create/via_global_ref/builtin/python2/python2.py index fb4325af3..22d1da009 100644 --- a/src/virtualenv/create/via_global_ref/builtin/python2/python2.py +++ b/src/virtualenv/create/via_global_ref/builtin/python2/python2.py @@ -57,13 +57,27 @@ def sources(cls, interpreter): yield src # install files needed to run site.py for req in cls.modules(): - comp = interpreter.stdlib_path("{}.pyc".format(req)) - comp_exists = comp.exists() - if comp_exists: - yield PathRefToDest(comp, dest=cls.to_stdlib) - stdlib_path = interpreter.stdlib_path("{}.py".format(req)) - if stdlib_path.exists() or not comp_exists: - yield PathRefToDest(stdlib_path, dest=cls.to_stdlib) + + # the compiled path is optional, but refer to it if exists + module_compiled_path = interpreter.stdlib_path("{}.pyc".format(req)) + has_compile = module_compiled_path.exists() + if has_compile: + yield PathRefToDest(module_compiled_path, dest=cls.to_stdlib) + + # stdlib module src may be missing if the interpreter allows it by falling back to the compiled + module_path = interpreter.stdlib_path("{}.py".format(req)) + add_py_module = cls.needs_stdlib_py_module() + if add_py_module is False: + if module_path.exists(): # if present add it + add_py_module = True + else: + add_py_module = not has_compile # otherwise only add it if the pyc is not present + if add_py_module: + yield PathRefToDest(module_path, dest=cls.to_stdlib) + + @classmethod + def needs_stdlib_py_module(cls): + raise NotImplementedError def to_stdlib(self, src): return self.stdlib / src.name diff --git a/tests/unit/create/test_creator.py b/tests/unit/create/test_creator.py index 585419c16..8c256875a 100644 --- a/tests/unit/create/test_creator.py +++ b/tests/unit/create/test_creator.py @@ -451,7 +451,8 @@ def _get_sys_path(flag=None): @pytest.mark.skipif( - not PY2 or not ("builtin" in CURRENT.creators().key_to_class), reason="stdlib python files only needed for Python 2" + not (CURRENT.implementation == "CPython" and PY2), + reason="stdlib components without py files only possible on CPython2", ) def test_pyc_only(tmp_path, mocker, session_app_data): """Ensure that creation can succeed if os.pyc exists (even if os.py has been deleted)"""