pypa / installer

A low-level library for installing from a Python wheel distribution.
https://installer.readthedocs.io/
MIT License
126 stars 52 forks source link

Installer compiles bytecode for wrong python version #172

Open radoering opened 1 year ago

radoering commented 1 year ago

I don't know if this is a valid use case or out of scope for this project but since it's possible to install in another environment one might expect that it is also possible to compile bytecode for the target environment.

In the following example installer lives in a Python 3.9 environment and installs tomli into a Python 3.11 environment. So far so good. However, it compiles bytecode for its own (Python 3.9) environment instead of the target (Python 3.11) environment. (Probably, it's not even possible to compile for the target environment without creating a subprocess to run the target interpreter?)

This issue was originally posted as a side note in python-poetry/poetry#7639. Before implementing our own solution, I just wanted to know if this will probably remain a known limitation of installer or if anyone has an idea if/how it could be supported by installer itself.

$ virtualenv -p 3.11 .venv11
$ virtualenv -p 3.9 .venv9
$ .venv9/bin/python -m pip install installer
$ .venv9/bin/python install_and_compile.py
$ ls .venv11/lib/python3.11/site-packages/tomli/__pycache__
__init__.cpython-39.pyc  _parser.cpython-39.pyc  _re.cpython-39.pyc  _types.cpython-39.pyc
install_and_compile.py ``` import json import subprocess from installer import install from installer.destinations import SchemeDictionaryDestination from installer.sources import WheelFile scheme_dict_oneliner = ( "import sysconfig; import json; print(json.dumps(sysconfig.get_paths()))" ) scheme_dict = json.loads( subprocess.check_output([".venv11/bin/python", "-c", scheme_dict_oneliner]).decode() ) destination = SchemeDictionaryDestination( scheme_dict, interpreter=".venv11/bin/python", script_kind="posix", bytecode_optimization_levels=(0,) ) with WheelFile.open("tomli-2.0.1-py3-none-any.whl") as source: install( source=source, destination=destination, additional_metadata={ "INSTALLER": b"amazing-installer 0.1.0", }, ) ```
FFY00 commented 1 year ago

Yeah, I'd probably note this down as a bug, because SchemeDictionaryDestination does receive an interpreter argument.

This can be fixed by using python -m compileall instead in:

https://github.com/pypa/installer/blob/5d1514ca53bf7394d52e92ab8948c89120d7eda6/src/installer/destinations.py#L250-L253

dimbleby commented 1 year ago

I dabbled with switching in subprocess.run(...) in that code, but it's a performance disaster to do that once for every file in a large wheel.

So probably this needs some slightly more invasive refactoring.

FFY00 commented 1 year ago

You can just save the files that need compiling and do a single subprocess call in finalize_installation.

pradyunsg commented 1 year ago

I don't think it's reasonable to expect a mismatched interepreter version to behave correctly -- the current logic uses information from sysconfig of the currently running interpreter by default, and overriding that means we're breaking that assumption. This is a case that we didn't consider when implementing bytecode compilation though.

I think it'd be cleaner to disallow creating a SchemeDictionaryDestination for an interpreter that isn't sys.executable and not disabling bytecode_optimization_levels.

eli-schwartz commented 1 year ago

If SchemeDictionaryDestination knows the interpreter and it isn't sys.executable it seems trivial to simply allow it via subprocess.run -- batching the files is simply a performance optimization. I'd expect that anyone passing in a bytecode_optimization_levels would rather it work slowly than raise an error.