From ea85b851243d64733788c11886e2360e26366e13 Mon Sep 17 00:00:00 2001 From: Gyeongjae Choi Date: Mon, 25 Nov 2024 19:30:33 +0900 Subject: [PATCH] Validate Python version when initializing xbuildenv (#62) This adds a check when initializing xbuildenv (== `init_environment`) to make sure the local Python version used to install the xbuildenv was not changed. This prevents the following scenario: 1. One has a local Python version (3.11.3) and installs Pyodide xbuildenv (0.25.1). 2. Then, one changes the local Python version to (3.12.1) using pyenv. 3. Pyodide xbuildenv 0.25.1 is no longer compatible to the local Python version, so it should fail. Related issue: https://github.com/pyodide/pyodide-build/issues/44 --- CHANGELOG.md | 4 +++ pyodide_build/build_env.py | 2 ++ pyodide_build/cli/xbuildenv.py | 4 +-- pyodide_build/tests/test_xbuildenv.py | 43 +++++++++++++++++++++++++++ pyodide_build/xbuildenv.py | 40 ++++++++++++++++++++++++- 5 files changed, 90 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca59cba..b53a709 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 variable to skip emscripten version check. [#53](https://github.com/pyodide/pyodide-build/pull/53) +- The `pyodide build` command will now raise an error if the local Python version has been changed, + after the cross-build environment has been set up. + [#62](https://github.com/pyodide/pyodide-build/pull/62) + ## [0.29.0] - 2024/09/19 ### Added diff --git a/pyodide_build/build_env.py b/pyodide_build/build_env.py index 929453d..c0d598b 100644 --- a/pyodide_build/build_env.py +++ b/pyodide_build/build_env.py @@ -83,6 +83,8 @@ def _init_xbuild_env(*, quiet: bool = False) -> Path: if manager.current_version is None: manager.install() + manager.check_version_marker() + return manager.pyodide_root diff --git a/pyodide_build/cli/xbuildenv.py b/pyodide_build/cli/xbuildenv.py index e6a5f93..98f708c 100644 --- a/pyodide_build/cli/xbuildenv.py +++ b/pyodide_build/cli/xbuildenv.py @@ -117,8 +117,8 @@ def _uninstall( """ check_xbuildenv_root(path) manager = CrossBuildEnvManager(path) - manager.uninstall_version(version) - typer.echo(f"Pyodide cross-build environment {version} uninstalled") + v = manager.uninstall_version(version) + typer.echo(f"Pyodide cross-build environment {v} uninstalled") @app.command("use") diff --git a/pyodide_build/tests/test_xbuildenv.py b/pyodide_build/tests/test_xbuildenv.py index 40933d6..3cb5934 100644 --- a/pyodide_build/tests/test_xbuildenv.py +++ b/pyodide_build/tests/test_xbuildenv.py @@ -1,3 +1,5 @@ +import sys + import pytest from pyodide_build.xbuildenv import CrossBuildEnvManager, _url_to_version @@ -156,6 +158,11 @@ def test_install_version( ).exists() assert (manager.symlink_dir / "xbuildenv" / "site-packages-extras").exists() + assert (manager.symlink_dir / ".build-python-version").exists() + assert ( + manager.symlink_dir / ".build-python-version" + ).read_text() == f"{sys.version_info.major}.{sys.version_info.minor}" + # installing the same version again should be a no-op manager.install(version) @@ -180,6 +187,11 @@ def test_install_url( ).exists() assert (manager.symlink_dir / "xbuildenv" / "site-packages-extras").exists() + assert (manager.symlink_dir / ".build-python-version").exists() + assert ( + manager.symlink_dir / ".build-python-version" + ).read_text() == f"{sys.version_info.major}.{sys.version_info.minor}" + def test_install_force( self, tmp_path, @@ -278,6 +290,37 @@ def test_uninstall_version(self, tmp_path): assert set(manager.list_versions()) == set(versions) - {"0.25.0", "0.25.1"} + def test_version_marker( + self, + tmp_path, + dummy_xbuildenv_url, + monkeypatch, + monkeypatch_subprocess_run_pip, + fake_xbuildenv_releases_compatible, + ): + manager = CrossBuildEnvManager( + tmp_path, str(fake_xbuildenv_releases_compatible) + ) + version = "0.1.0" + + manager.install(version) + + assert (manager.symlink_dir / ".build-python-version").exists() + assert ( + manager.symlink_dir / ".build-python-version" + ).read_text() == f"{sys.version_info.major}.{sys.version_info.minor}" + + # No error + assert manager.check_version_marker() is None + + (manager.symlink_dir / ".build-python-version").write_text("2.7.10") + + with pytest.raises( + ValueError, + match="does not match the Python version", + ): + manager.check_version_marker() + @pytest.mark.parametrize( "url, version", diff --git a/pyodide_build/xbuildenv.py b/pyodide_build/xbuildenv.py index 8ca01a6..3559b7d 100644 --- a/pyodide_build/xbuildenv.py +++ b/pyodide_build/xbuildenv.py @@ -18,6 +18,7 @@ ) CDN_BASE = "https://cdn.jsdelivr.net/pyodide/v{version}/full/" +PYTHON_VERSION_MARKER_FILE = ".build-python-version" class CrossBuildEnvManager: @@ -215,6 +216,7 @@ def install( install_marker.touch() self.use_version(version) + self._add_version_marker() except Exception as e: # if the installation failed, remove the downloaded directory shutil.rmtree(download_path) @@ -366,7 +368,7 @@ def _create_package_index(self, xbuildenv_pyodide_root: Path, version: str) -> N lockfile = PyodideLockSpec(**json.loads(lockfile_path.read_bytes())) create_package_index(lockfile.packages, xbuildenv_pyodide_root, cdn_base) - def uninstall_version(self, version: str) -> None: + def uninstall_version(self, version: str | None) -> str: """ Uninstall the installed xbuildenv version. @@ -375,6 +377,12 @@ def uninstall_version(self, version: str) -> None: version The version of xbuildenv to uninstall. """ + if version is None: + version = self.current_version + + if version is None: + raise ValueError("No xbuildenv version is currently in use") + version_path = self._path_for_version(version) # if the target version is the current version, remove the symlink @@ -389,6 +397,36 @@ def uninstall_version(self, version: str) -> None: f"Cannot find cross-build environment version {version}, available versions: {self.list_versions()}" ) + return version + + def _add_version_marker(self) -> None: + """ + Store the Python version in the xbuildenv directory, so we can check compatibility later. + """ + if not self.symlink_dir.is_dir(): + raise ValueError("cross-build env directory does not exist") + + version_file = self.symlink_dir / PYTHON_VERSION_MARKER_FILE + version_file.write_text(build_env.local_versions()["python"]) + + def check_version_marker(self): + if not self.symlink_dir.is_dir(): + raise ValueError("cross-build env directory does not exist") + + version_file = self.symlink_dir / PYTHON_VERSION_MARKER_FILE + if not version_file.exists(): + raise ValueError("Python version marker file not found") + + version_local = build_env.local_versions()["python"] + version_on_install = version_file.read_text().strip() + if version_on_install != version_local: + raise ValueError( + f"local Python version ({version_local}) does not match the Python version ({version_on_install}) " + "used to create the Pyodide cross-build environment. " + "Please switch back to the original Python version, " + "or reinstall the xbuildenv, by running `pyodide xbuildenv uninstall` and then `pyodide xbuildenv install`" + ) + def _url_to_version(url: str) -> str: return url.replace("://", "_").replace(".", "_").replace("/", "_")