[nrf52] Rebuild the Python env when its interpreter symlink dangles (#18540)

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 (
+    if _needs_venv_rebuild(env_python_path, sentinel, requirements_hash):
-        not sentinel.exists()
-        or sentinel.read_text(encoding="utf-8") != 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 = (
+    install_venv = _needs_venv_rebuild(env_python_path, sentinel, requirements_hash)
-        not sentinel.exists()
-        or sentinel.read_text(encoding="utf-8") != requirements_hash
-    )
     if install_venv:
         rmdir(python_env_path, msg=f"Clean up {version} Python environment")
 

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")