Merge branch 'esp8266-native-build-surgery' into esp8266-native-toolchain-plumbing

esphome/espidf/framework.py

@@ -730,9 +730,15 @@ def _prefetch_idf_tool_archives(
             return
         dist_path = get_idf_tools_path() / "dist"
         entries = []
+        seen_dests: set[str] = set()
         for entry in json.loads(stdout):
             if (dist_path / entry["dest"]).is_file():
                 continue
+            if entry["dest"] in seen_dests:
+                # Two workers on one .part file would interleave
+                # seek/truncate writes; mirror the library prefetch's dedupe
+                continue
+            seen_dests.add(entry["dest"])
             # tools.json always carries sha256 and size; an entry missing
             # either must not be downloaded unverified here, so leave it to
             # the installer (which fails loudly on a bad archive).

esphome/framework_helpers.py

@@ -756,7 +756,7 @@ def run_batch_downloads(
     bar is done, so the caller's warnings never land on the bar's row; a
     failed job credits its tracker 0 so the bar can still complete. Ctrl-C
     drops queued jobs instead of downloading them all before the process
-    can exit; in-flight ones still finish.
+    can exit; in-flight ones still finish. ``jobs`` must be non-empty.
     """
     failures: list[tuple[str, Exception]] = []
 

esphome/platformio/extra_script.py

@@ -229,8 +229,18 @@ def captured_as_build_flags(
     generated build files stay portable.
     """
     flags: list[str] = []
+
+    def _strs(bucket: list, kind: str) -> list[str]:
+        # Third-party scripts legally append SCons nodes, ints, or dicts;
+        # stringifying those into flags would hand the compiler garbage
+        good = [entry for entry in bucket if isinstance(entry, str)]
+        for entry in bucket:
+            if not isinstance(entry, str):
+                _LOGGER.warning("Ignoring unsupported %s entry %r", kind, entry)
+        return good
+
     library_root = library_dir.resolve()
-    for path in result.libpath:
+    for path in _strs(result.libpath, "LIBPATH"):
         # Anchor relative paths to library_dir; the script's CWD has been
         # restored by now
         resolved = (library_dir / path).resolve()
@@ -238,7 +248,7 @@ def captured_as_build_flags(
             flags.append(f"-L{resolved.relative_to(library_root)}")
         except ValueError:
             flags.append(f"-L{resolved}")
-    flags.extend(f"-l{lib}" for lib in result.libs)
+    flags.extend(f"-l{lib}" for lib in _strs(result.libs, "LIBS"))
     for define in result.cppdefines:
         # SCons also accepts dict/list CPPDEFINES; formatting those blind
         # would hand the compiler garbage like -D{'FOO': '1'}
@@ -248,6 +258,6 @@ def captured_as_build_flags(
             flags.append(f"-D{define}")
         else:
             _LOGGER.warning("Ignoring unsupported CPPDEFINES entry %r", define)
-    flags.extend(result.linkflags)
-    flags.extend(result.cppflags)
+    flags.extend(_strs(result.linkflags, "LINKFLAGS"))
+    flags.extend(_strs(result.cppflags, "CPPFLAGS"))
     return flags

esphome/platformio/library.py

@@ -57,6 +57,9 @@ DEFAULT_BUILD_INCLUDE_DIR = "include"
 DEFAULT_BUILD_FLAGS = []
 # Suffix -> compiler kind (PlatformIO's CSUFFIXES/CXXSUFFIXES/ASSUFFIXES).
 # "asm" merges SCons's AS and ASPP sets: all compile as assembler-with-cpp.
+# The kind values drive the ESP8266 native ninja rules (later in this
+# chain); existing backends consume only the keys. Note .C/.C++ join the
+# suffix set here, matching PlatformIO's CXXSUFFIXES.
 SOURCE_KIND_FOR_SUFFIX: dict[str, str] = {
     ".c": "c",
     ".cpp": "cxx",
@@ -649,6 +652,9 @@ def raise_on_empty_arg_flags(tokens: list[str], owner: str) -> None:
     """Reject bare ``-I``/``-D``/``-L``/``-l`` tokens left by an empty glued
     argument (``-D ""``).
 
+    Consumed by the ESP8266 native build generator (later in this chain)
+    for user build_flags; library manifests deliberately stay warn-and-drop.
+
     Lives next to ``join_flag_args`` because the bare token is its
     postcondition: a trailing bare flag is warned and dropped there, so a
     surviving one always means an empty argument. gcc would eat the next
@@ -683,7 +689,9 @@ def warn_properties_depends(name: str, data: object) -> None:
     ``library.properties`` spelling would otherwise drop silently.
     """
     if isinstance(data, dict) and not data.get("dependencies") and data.get("depends"):
-        _LOGGER.warning(
+        # INFO: common and unactionable for transitive libraries; a WARNING
+        # on every build would train users to ignore the stream
+        _LOGGER.info(
             "Library %s declares dependencies via library.properties "
             "depends=, which are not resolved automatically; add them with "
             "add_library() if needed",
@@ -965,18 +973,14 @@ def _prefetch_wave(
         components.append(component)
     if len(components) < 2:
         return
-    _LOGGER.info(
-        "Downloading %d libraries: %s",
-        len(components),
-        ", ".join(c.name for c in components),
-    )
     # One combined bar over the batch, sized by HEAD requests. An unknown
     # size would mean a silent multi-MB download; fall back to sequential
     # downloads with their per-file bars instead.
     sizes = _content_lengths([c.source.url for c in components])
     if not all(sizes):
-        # Name the culprits so the fallback is distinguishable from a hang
-        _LOGGER.debug(
+        # Announced before the sequential per-file downloads take over, so
+        # the fallback is distinguishable from a hang
+        _LOGGER.info(
             "No Content-Length for %s; downloading sequentially",
             ", ".join(
                 c.source.url
@@ -985,6 +989,11 @@ def _prefetch_wave(
             ),
         )
         return
+    _LOGGER.info(
+        "Downloading %d libraries: %s",
+        len(components),
+        ", ".join(c.name for c in components),
+    )
 
     def _fetch(component: ConvertedLibrary):
         return lambda tracker: component.download(
@@ -1126,6 +1135,12 @@ def convert_libraries(
         _prefetch_wave(wave, salt, backend.cache_key)
         for key, component in wave:
             node = nodes[key]
+            if frozenset(node.requirements) != resolved_requirements[key]:
+                # An earlier wave entry grew this node's requirements after
+                # the drain resolved it; downloading the superseded version
+                # would be wasted work, and the next wave re-resolves it
+                worklist.append(key)
+                continue
             component.download(salt=salt, namespace=backend.cache_key)
 
             source_dir = component.source_dir

tests/unit_tests/test_espidf_framework.py

@@ -940,6 +940,26 @@ def test_prefetch_all_entries_unverifiable_is_a_noop(tmp_path: Path) -> None:
     download.assert_not_called()
 
 
+def test_prefetch_dedupes_entries_by_dest(tmp_path: Path) -> None:
+    """Two entries resolving to one dest would interleave writes into the
+    same .part file; only the first downloads."""
+    entries = json.loads(_PREFETCH_JSON)
+    dup = dict(entries[0]) | {"name": "cmake-alias@3.30.2"}
+    entries.append(dup)
+    with (
+        patch(
+            "esphome.espidf.framework.run_command",
+            return_value=(True, json.dumps(entries), ""),
+        ),
+        patch("esphome.espidf.framework.download_with_resume") as download,
+        patch("esphome.espidf.framework.get_system_python_path", return_value="python"),
+        patch("esphome.espidf.framework.BatchDownloadProgress"),
+    ):
+        _prefetch_idf_tool_archives(tmp_path, "esp32", ["required"], None)
+    dests = [call[0][1].name for call in download.call_args_list]
+    assert dests.count("cmake-3.30.2.tar.gz") == 1
+
+
 def test_prefetch_downloads_each_archive_with_resume(tmp_path: Path) -> None:
     with (
         patch(

tests/unit_tests/test_platformio_extra_script.py

@@ -169,6 +169,30 @@ def test_apply_extra_script_callable_target_and_str_flags(tmp_path) -> None:
     assert c.data["build"]["flags"] == ["-DBASE=1", "-lesp8266"]
 
 
+def test_captured_nonstring_buckets_warn_and_skip(tmp_path, caplog) -> None:
+    """Non-string LIBS/LINKFLAGS/CPPFLAGS/LIBPATH entries (legal SCons
+    nodes) are skipped by name instead of stringified into garbage flags."""
+    (tmp_path / "src").mkdir()
+    script = tmp_path / "extra.py"
+    script.write_text(
+        "env.Append(LIBS=['m', 42], LINKFLAGS=['-Wl,-x', {'no': 1}], "
+        "CPPFLAGS=['-Os', 3.5], LIBPATH=['libs', 7])\n"
+    )
+    (tmp_path / "libs").mkdir()
+
+    c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy"))
+    c.path = tmp_path
+    c.data = {"build": {"extraScript": "extra.py"}}
+
+    apply_extra_script(c, board_mcu=lambda: "esp8266", pio_platform="espressif8266")
+
+    flags = c.data["build"]["flags"]
+    assert "-lm" in flags and "-Wl,-x" in flags and "-Os" in flags
+    assert not any("42" in f or "no" in f or "3.5" in f for f in flags)
+    assert "Ignoring unsupported LIBS entry 42" in caplog.text
+    assert "Ignoring unsupported LIBPATH entry 7" in caplog.text
+
+
 def test_captured_dict_cppdefines_warn_and_skip(tmp_path, caplog) -> None:
     """A dict CPPDEFINES entry (legal SCons) must warn and skip; formatting
     it blind would hand the compiler -D{'FOO': '1'} garbage."""

tests/unit_tests/test_platformio_library.py

@@ -255,6 +255,37 @@ def _patch_download_with_manifests(monkeypatch, tmp_path, manifests, *, properti
     _patch_registry_resolve(monkeypatch)
 
 
+def test_wave_requirement_growth_defers_the_superseded_download(tmp_path, monkeypatch):
+    """A's manifest constrains B while B sits in the same wave: B's
+    drain-time resolution is superseded, so its download defers to the
+    next wave instead of fetching a version that is immediately replaced."""
+    download_names: list[str] = []
+    manifests = {
+        "esphome/A": {
+            "name": "A",
+            "build": {},
+            "dependencies": {"esphome/B": ">=1.0"},
+        },
+        "esphome/B": {"name": "B", "build": {}},
+    }
+
+    def fake_download(self, force=False, salt="", namespace="", progress=None):
+        download_names.append(self.name)
+        self.path = tmp_path / self.get_require_name()
+        self.path.mkdir(parents=True, exist_ok=True)
+        (self.path / "library.json").write_text(json.dumps(manifests[self.name]))
+
+    monkeypatch.setattr(ConvertedLibrary, "download", fake_download)
+    _patch_registry_resolve(monkeypatch)
+    top = convert_libraries(
+        [Library("esphome/A", "1.0.0", None), Library("esphome/B", None, None)],
+        _backend(),
+    )
+    assert sorted(c.name for c in top) == ["esphome/A", "esphome/B"]
+    # B downloads exactly once, after its requirement set stabilized
+    assert download_names.count("esphome/B") == 1
+
+
 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.
@@ -814,6 +845,7 @@ def test_walk_warns_for_properties_only_depends(
         {"esphome/A": "name=A\nversion=1.0\ndepends=Wire, SPI\n"},
         properties=("esphome/A",),
     )
+    caplog.set_level("INFO")
     convert_libraries([Library("esphome/A", "1.0.0", None)], _backend())
     assert "declares dependencies via library.properties" in caplog.text