Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Installer compiles bytecode for wrong python version #172

Open
radoering opened this issue Mar 14, 2023 · 5 comments · May be fixed by #175
Open

Installer compiles bytecode for wrong python version #172

radoering opened this issue Mar 14, 2023 · 5 comments · May be fixed by #175
Labels
component: core Related to core installation logic type: bug A confirmed bug or unintended behavior

Comments

@radoering
Copy link

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
Copy link
Member

FFY00 commented Mar 15, 2023

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:

for level in self.bytecode_optimization_levels:
compileall.compile_file(
target_path, optimize=level, quiet=1, ddir=dir_path_to_embed
)

@dimbleby
Copy link
Contributor

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
Copy link
Member

FFY00 commented Mar 18, 2023

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

@BlueGlassBlock BlueGlassBlock linked a pull request Mar 23, 2023 that will close this issue
@pradyunsg
Copy link
Member

pradyunsg commented May 29, 2023

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: core Related to core installation logic type: bug A confirmed bug or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants