From 132f494195869750e7ccf3ad2a66f58c0234da21 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 20 Aug 2026 06:59:57 -0500 Subject: [PATCH] [nrf52] Rebuild the Python env when its interpreter symlink dangles (#18540) --- esphome/components/nrf52/framework.py | 26 ++++--- tests/unit_tests/test_nrf52_framework.py | 90 +++++++++++++++++++++++- 2 files changed, 107 insertions(+), 9 deletions(-) diff --git a/esphome/components/nrf52/framework.py b/esphome/components/nrf52/framework.py index 6b32fe1fea..d487820440 100644 --- a/esphome/components/nrf52/framework.py +++ b/esphome/components/nrf52/framework.py @@ -62,6 +62,22 @@ def get_sdk_nrf_tools_path() -> Path: return path.resolve() +def _needs_venv_rebuild( + env_python_path: Path, sentinel: Path, requirements_hash: str +) -> bool: + """True when a penv must be (re)built. + + Rebuild when the interpreter is not a regular file, which covers a + dangling symlink (a cached venv outliving a host interpreter upgrade) + and a corrupt restore, or when the sentinel is missing or stale. + """ + return ( + not env_python_path.is_file() + or not sentinel.exists() + or sentinel.read_text(encoding="utf-8") != requirements_hash + ) + + def _get_python_env_path(version: str) -> Path: return get_sdk_nrf_tools_path() / "penvs" / version @@ -198,10 +214,7 @@ def setup_platformio_python_env() -> None: + "\n".join(_PLATFORMIO_PENV_REQUIREMENTS).encode() + f"python{sys.version_info.major}.{sys.version_info.minor}".encode() ).hexdigest() - if ( - not sentinel.exists() - or sentinel.read_text(encoding="utf-8") != requirements_hash - ): + if _needs_venv_rebuild(env_python_path, sentinel, requirements_hash): rmdir(penv_path, msg="Clean up PlatformIO toolchain Python environment") create_venv(penv_path, msg="PlatformIO toolchain") @@ -250,10 +263,7 @@ def check_and_install() -> None: env_python_path = get_python_env_executable_path(python_env_path, "python") sentinel = python_env_path / ".ready" requirements_hash = hashlib.sha256(_REQUIREMENTS.read_bytes()).hexdigest() - install_venv = ( - not sentinel.exists() - or sentinel.read_text(encoding="utf-8") != requirements_hash - ) + install_venv = _needs_venv_rebuild(env_python_path, sentinel, requirements_hash) if install_venv: rmdir(python_env_path, msg=f"Clean up {version} Python environment") diff --git a/tests/unit_tests/test_nrf52_framework.py b/tests/unit_tests/test_nrf52_framework.py index 0a6bddc280..c2ee0c2a75 100644 --- a/tests/unit_tests/test_nrf52_framework.py +++ b/tests/unit_tests/test_nrf52_framework.py @@ -16,6 +16,7 @@ from esphome.components.nrf52.framework import ( _get_penv_site_packages, _get_platformio_penv_path, _get_toolchain_platform_info, + _needs_venv_rebuild, check_and_install, get_build_env, get_sdk_nrf_tools_path, @@ -123,10 +124,19 @@ def mock_nrf52_ops(): # --------------------------------------------------------------------------- +def _touch_penv_python(penv: Path) -> None: + """Create the interpreter file so the rebuild gate sees a live venv.""" + python = get_python_env_executable_path(penv, "python") + python.parent.mkdir(parents=True, exist_ok=True) + python.touch() + + def _mark_venv_ready(python_env: Path) -> None: - """Write the venv sentinel with the current requirements hash.""" + """Write the venv sentinel with the current requirements hash and a + present interpreter so the rebuild gate passes.""" requirements_hash = hashlib.sha256(_REQUIREMENTS.read_bytes()).hexdigest() (python_env / ".ready").write_text(requirements_hash, encoding="utf-8") + _touch_penv_python(python_env) class TestCheckAndInstall: @@ -148,6 +158,23 @@ class TestCheckAndInstall: mock_nrf52_ops.download_from_mirrors.assert_not_called() mock_nrf52_ops.archive_extract_all.assert_not_called() + def test_missing_interpreter_rebuilds_venv( + self, + nrf52_dirs: SimpleNamespace, + mock_nrf52_ops: SimpleNamespace, + ) -> None: + """A valid sentinel must not mask a missing interpreter (a cached venv + restored after a host interpreter upgrade).""" + requirements_hash = hashlib.sha256(_REQUIREMENTS.read_bytes()).hexdigest() + (nrf52_dirs.python_env / ".ready").write_text( + requirements_hash, encoding="utf-8" + ) + # no interpreter on disk + + check_and_install() + + mock_nrf52_ops.create_venv.assert_called_once() + def test_fresh_install_runs_all_steps( self, nrf52_dirs: SimpleNamespace, @@ -348,6 +375,7 @@ class TestSetupPlatformioPythonEnv: (platformio_penv_dir / ".ready").write_text( _platformio_requirements_hash(), encoding="utf-8" ) + _touch_penv_python(platformio_penv_dir) with patch.dict(os.environ): setup_platformio_python_env() @@ -392,6 +420,22 @@ class TestSetupPlatformioPythonEnv: assert not (platformio_penv_dir / ".ready").exists() + def test_missing_interpreter_reinstalls( + self, + platformio_penv_dir: Path, + mock_nrf52_ops: SimpleNamespace, + ) -> None: + """A valid sentinel must not mask a missing interpreter.""" + (platformio_penv_dir / ".ready").write_text( + _platformio_requirements_hash(), encoding="utf-8" + ) + # no interpreter on disk + + with patch.dict(os.environ): + setup_platformio_python_env() + + mock_nrf52_ops.create_venv.assert_called_once() + def test_repeated_calls_do_not_duplicate_env_entries( self, platformio_penv_dir: Path, @@ -401,6 +445,7 @@ class TestSetupPlatformioPythonEnv: (platformio_penv_dir / ".ready").write_text( _platformio_requirements_hash(), encoding="utf-8" ) + _touch_penv_python(platformio_penv_dir) site_packages = str(_get_penv_site_packages(platformio_penv_dir)) bin_dir = str( get_python_env_executable_path(platformio_penv_dir, "python").parent @@ -422,6 +467,7 @@ class TestSetupPlatformioPythonEnv: (platformio_penv_dir / ".ready").write_text( _platformio_requirements_hash(), encoding="utf-8" ) + _touch_penv_python(platformio_penv_dir) site_packages = str(_get_penv_site_packages(platformio_penv_dir)) with patch.dict(os.environ, {"PYTHONPATH": "/existing/path"}): @@ -531,3 +577,45 @@ def testget_tools_path_default_is_global_cache( Path(platformdirs.user_cache_dir("esphome", appauthor=False)) / "sdk-nrf" ).resolve() assert get_sdk_nrf_tools_path() == expected + + +def test_needs_venv_rebuild_gates(tmp_path: Path) -> None: + """The shared penv gate rebuilds on any missing or stale piece.""" + penv = tmp_path / "penv" + penv.mkdir() + python = penv / "python" + sentinel = penv / ".ready" + good_hash = "abc123" + + # Nothing in place yet + assert _needs_venv_rebuild(python, sentinel, good_hash) + + python.write_text("") + # Interpreter present but no sentinel + assert _needs_venv_rebuild(python, sentinel, good_hash) + + sentinel.write_text(good_hash, encoding="utf-8") + # Everything in place + assert not _needs_venv_rebuild(python, sentinel, good_hash) + + # Stale requirements hash + assert _needs_venv_rebuild(python, sentinel, "otherhash") + + +@pytest.mark.skipif( + sys.platform == "win32", reason="symlink creation needs privileges on Windows" +) +def test_needs_venv_rebuild_on_dangling_interpreter_symlink(tmp_path: Path) -> None: + """A cached venv restored after a host interpreter upgrade has a + bin/python symlink whose target is gone; the valid sentinel must not + mask it.""" + penv = tmp_path / "penv" + penv.mkdir() + python = penv / "python" + sentinel = penv / ".ready" + sentinel.write_text("abc123", encoding="utf-8") + python.symlink_to(tmp_path / "hostedtoolcache" / "3.12.14" / "python3") + assert python.is_symlink() + assert not python.exists() + + assert _needs_venv_rebuild(python, sentinel, "abc123")