From 3e1a9e8a3aa313e7a44b5c4638a9a47ed7010b0b Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 19 Jul 2026 19:34:56 -1000 Subject: [PATCH] [platformio] Re-download library when cached copy is missing its manifest (#17691) Co-authored-by: Jesse Hills <3060199+jesserockz@users.noreply.github.com> --- esphome/platformio/library.py | 20 ++++- tests/unit_tests/test_platformio_library.py | 84 ++++++++++++++++++--- 2 files changed, 90 insertions(+), 14 deletions(-) diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index 72a50b795b..b3fd24c2b7 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -669,9 +669,25 @@ def convert_libraries( library_json_path = component.path / "library.json" library_properties_path = component.path / "library.properties" - if library_json_path.is_file(): + has_json = library_json_path.is_file() + has_properties = library_properties_path.is_file() + if not has_json and not has_properties: + # The shared cache can hold a broken copy (e.g. a clone or an + # extraction interrupted by a killed process). Force one + # re-download so a bad cache entry self-heals instead of failing + # every build until the user runs a full clean. + _LOGGER.warning( + "Library %s at %s is missing library.json and library.properties; " + "re-downloading", + key, + component.path, + ) + component.download(force=True, salt=salt, namespace=backend.cache_key) + has_json = library_json_path.is_file() + has_properties = library_properties_path.is_file() + if has_json: component.data = _parse_library_json(library_json_path) - elif library_properties_path.is_file(): + elif has_properties: component.data = _parse_library_properties(library_properties_path) else: raise RuntimeError( diff --git a/tests/unit_tests/test_platformio_library.py b/tests/unit_tests/test_platformio_library.py index 6a4c057469..d2ca71bad6 100644 --- a/tests/unit_tests/test_platformio_library.py +++ b/tests/unit_tests/test_platformio_library.py @@ -133,18 +133,8 @@ def test_resolve_registry_version_raises_without_pkg_file(monkeypatch): _resolve_registry_version("owner", "pkg", set()) -def _patch_download_with_manifests(monkeypatch, tmp_path, manifests, *, properties=()): - """Fake ConvertedLibrary.download to materialize canned manifests on disk.""" - - def fake_download(self, force=False, salt="", namespace=""): - self.path = tmp_path / self.get_sanitized_name().replace("/", "__") - self.path.mkdir(parents=True, exist_ok=True) - if self.name in properties: - (self.path / "library.properties").write_text(manifests[self.name]) - else: - (self.path / "library.json").write_text(json.dumps(manifests[self.name])) - - monkeypatch.setattr(ConvertedLibrary, "download", fake_download) +def _patch_registry_resolve(monkeypatch: pytest.MonkeyPatch) -> None: + """Stub the registry lookup so tests never touch the network.""" monkeypatch.setattr( lib, "_resolve_registry_version", @@ -157,6 +147,21 @@ def _patch_download_with_manifests(monkeypatch, tmp_path, manifests, *, properti ) +def _patch_download_with_manifests(monkeypatch, tmp_path, manifests, *, properties=()): + """Fake ConvertedLibrary.download to materialize canned manifests on disk.""" + + def fake_download(self, force=False, salt="", namespace=""): + self.path = tmp_path / self.get_require_name() + self.path.mkdir(parents=True, exist_ok=True) + if self.name in properties: + (self.path / "library.properties").write_text(manifests[self.name]) + else: + (self.path / "library.json").write_text(json.dumps(manifests[self.name])) + + monkeypatch.setattr(ConvertedLibrary, "download", fake_download) + _patch_registry_resolve(monkeypatch) + + def test_convert_libraries_parses_library_properties(tmp_path, monkeypatch): # A manifest provided as library.properties (Arduino style) instead of # library.json must still be parsed and converted. @@ -212,6 +217,61 @@ def test_convert_libraries_handles_unparsable_dependency_version(tmp_path, monke assert [d.name for d in top[0].dependencies] == ["C"] +def _patch_download_without_manifest( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, *, manifest_on_force: bool +) -> list[bool]: + """Fake ConvertedLibrary.download that leaves the manifest missing. + + When ``manifest_on_force`` is set, a forced re-download writes a valid + library.json, simulating a broken cache entry that heals on retry. + Returns the list of ``force`` values download was called with. + """ + calls: list[bool] = [] + + def fake_download( + self: ConvertedLibrary, force: bool = False, salt: str = "", namespace: str = "" + ) -> None: + calls.append(force) + self.path = tmp_path / self.get_require_name() + self.path.mkdir(parents=True, exist_ok=True) + if force and manifest_on_force: + (self.path / "library.json").write_text(json.dumps({"name": "A"})) + + monkeypatch.setattr(ConvertedLibrary, "download", fake_download) + _patch_registry_resolve(monkeypatch) + return calls + + +def test_convert_libraries_redownloads_when_manifest_missing( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + # A cached copy without any manifest (e.g. an interrupted clone or + # extraction) triggers exactly one forced re-download and then succeeds. + calls = _patch_download_without_manifest( + monkeypatch, tmp_path, manifest_on_force=True + ) + + top = convert_libraries([Library("esphome/A", "1.0.0", None)], _backend()) + + assert calls == [False, True] + assert top[0].data["name"] == "A" + + +def test_convert_libraries_raises_when_manifest_missing_after_retry( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + # If the forced re-download still yields no manifest, the error is raised + # after exactly one retry (no retry loop). + calls = _patch_download_without_manifest( + monkeypatch, tmp_path, manifest_on_force=False + ) + + with pytest.raises(RuntimeError, match="Invalid PIO library"): + convert_libraries([Library("esphome/A", "1.0.0", None)], _backend()) + + assert calls == [False, True] + + @pytest.mark.parametrize( ("value", "expected"), [