From 6bdec7e907cc2295292c12f2b8c61dc8b992d423 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 22 Aug 2026 12:26:24 -0500 Subject: [PATCH] Warn only for source-like suffix drops, batch ar argv, name non-iterable dependencies The unmapped-suffix warning fired for every header-only library (the default +<*> filter matches headers), so ArduinoJson would have warned on every ESP8266 build. It now names exactly the source-like files (.CPP, .ino and case-variants of the map) the case-sensitive suffix map rejects, partial drops included, and stays quiet for headers and metadata. The ar shim batches the expanded object list by argv length (rc then q appends), keeping the command line under the Windows 32767-char limit the rspfile existed to avoid. A non-iterable dependencies value in a manifest now warns by library name instead of raising a bare TypeError. --- esphome/arduino/library.py | 41 +++++++++-------- esphome/build_gen/build_tool.py | 20 +++++++-- esphome/platformio/library.py | 7 +++ tests/unit_tests/build_gen/test_build_tool.py | 45 +++++++++++++++++++ tests/unit_tests/test_arduino_library.py | 27 +++++++++-- tests/unit_tests/test_platformio_library.py | 3 ++ 6 files changed, 118 insertions(+), 25 deletions(-) diff --git a/esphome/arduino/library.py b/esphome/arduino/library.py index e706f66ff0..8260f17c52 100644 --- a/esphome/arduino/library.py +++ b/esphome/arduino/library.py @@ -180,25 +180,30 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: for f in matched if (path := Path(f)).suffix in SRC_FILE_EXTENSIONS ) - if skipped := [f for f in matched if Path(f).suffix not in SRC_FILE_EXTENSIONS]: - _LOGGER.debug( - "Library %s: %d matched files are not sources", name, len(skipped) + # A source-like suffix the case-sensitive map rejects (.CPP, .ino) is a + # dropped compilation unit that surfaces as undefined symbols at link; + # headers and metadata files fall through silently (header-only + # libraries are routine) + source_like = {s.lower() for s in SRC_FILE_EXTENSIONS} | {".ino"} + if dropped := [ + Path(f).name + for f in matched + if Path(f).suffix not in SRC_FILE_EXTENSIONS + and Path(f).suffix.lower() in source_like + ]: + _LOGGER.warning( + "Library %s: %d file(s) with unmapped source suffixes are not compiled: %s", + name, + len(dropped), + ", ".join(sorted(dropped)), + ) + if not lib.sources and not matched and ("srcFilter" in build or "srcDir" in build): + # A default probe finding nothing is a header-only library; a + # declared filter matching nothing is a manifest/tree problem. + _LOGGER.warning( + "Library %s declares srcFilter/srcDir but no source files matched", + name, ) - if not lib.sources: - if matched: - # Every matched file fell through the suffix map: an empty - # archive would fail far away at link - _LOGGER.warning( - "Library %s: no matched file has a recognized source suffix", - name, - ) - elif "srcFilter" in build or "srcDir" in build: - # A default probe finding nothing is a header-only library; a - # declared filter matching nothing is a manifest/tree problem. - _LOGGER.warning( - "Library %s declares srcFilter/srcDir but no source files matched", - name, - ) return lib diff --git a/esphome/build_gen/build_tool.py b/esphome/build_gen/build_tool.py index 5dcadc9671..c19c4dc220 100644 --- a/esphome/build_gen/build_tool.py +++ b/esphome/build_gen/build_tool.py @@ -40,9 +40,23 @@ def main() -> int: # An empty archive would "succeed" here and fail far away at link print(f"ar: no objects listed in {rspfile} for {archive}", file=sys.stderr) return 1 - return subprocess.run( - [ar, "rc", archive, *objects], check=False, close_fds=False - ).returncode + # Batch by argv length: expanding the rspfile gives back the Windows + # 32767-char command-line limit it existed to avoid. "rc" creates, + # "q" appends the remainder. + op = "rc" + while objects: + batch = [objects.pop(0)] + batch_len = len(batch[0]) + while objects and batch_len + len(objects[0]) < 25000: + batch_len += len(objects[0]) + 1 + batch.append(objects.pop(0)) + rc = subprocess.run( + [ar, op, archive, *batch], check=False, close_fds=False + ).returncode + if rc != 0: + return rc + op = "q" + return 0 if mode == "copy": src, dst = sys.argv[2:4] shutil.copyfile(src, dst) diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index 8a21e131a1..6c0b0d09cf 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -699,6 +699,13 @@ def normalize_dependencies( continue normalized.append(entry) return normalized + if not isinstance(dependencies, (list, tuple)): + _LOGGER.warning( + "Ignoring unrecognized dependencies %r of %s", + dependencies, + manifest_name, + ) + return [] normalized = [] for entry in dependencies: if isinstance(entry, dict): diff --git a/tests/unit_tests/build_gen/test_build_tool.py b/tests/unit_tests/build_gen/test_build_tool.py index 7f56abe9cb..0e9d069b4a 100644 --- a/tests/unit_tests/build_gen/test_build_tool.py +++ b/tests/unit_tests/build_gen/test_build_tool.py @@ -124,3 +124,48 @@ def test_ar_empty_object_list_fails( rc = build_tool.main() assert rc == 1 assert "no objects listed" in capsys.readouterr().err + + +def test_ar_batches_long_object_lists(tmp_path: Path) -> None: + """The expanded argv must stay under the Windows 32767-char limit: a + long object list creates with rc, then appends with q.""" + archive = tmp_path / "lib.a" + rsp = tmp_path / "lib.a.rsp" + objects = [f"dir/{'x' * 120}_{i}.o" for i in range(400)] + rsp.write_text("\n".join(objects) + "\n") + with ( + patch.object( + build_tool.sys, + "argv", + ["build_tool", "ar", "ar-bin", str(archive), str(rsp)], + ), + patch.object( + build_tool.subprocess, "run", return_value=MagicMock(returncode=0) + ) as mock_run, + ): + assert build_tool.main() == 0 + calls = [c[0][0] for c in mock_run.call_args_list] + assert len(calls) > 1 + assert calls[0][1] == "rc" + assert all(c[1] == "q" for c in calls[1:]) + assert [o for c in calls for o in c[3:]] == objects + assert all(sum(len(a) + 1 for a in c) < 32000 for c in calls) + + +def test_ar_batch_failure_stops(tmp_path: Path) -> None: + """A failing batch propagates its exit code without running the rest.""" + archive = tmp_path / "lib.a" + rsp = tmp_path / "lib.a.rsp" + rsp.write_text("\n".join(f"{'y' * 200}_{i}.o" for i in range(300)) + "\n") + with ( + patch.object( + build_tool.sys, + "argv", + ["build_tool", "ar", "ar-bin", str(archive), str(rsp)], + ), + patch.object( + build_tool.subprocess, "run", return_value=MagicMock(returncode=3) + ) as mock_run, + ): + assert build_tool.main() == 3 + assert mock_run.call_count == 1 diff --git a/tests/unit_tests/test_arduino_library.py b/tests/unit_tests/test_arduino_library.py index 4deb842763..ef73c62314 100644 --- a/tests/unit_tests/test_arduino_library.py +++ b/tests/unit_tests/test_arduino_library.py @@ -477,13 +477,32 @@ def test_library_info_dropped_link_fields_warn( def test_library_info_unmapped_sources_warn( tmp_path: Path, caplog: pytest.LogCaptureFixture ) -> None: - """Matched files that all fall through the suffix map are visible; an - empty archive would fail far away at link.""" + """Source-like files the case-sensitive suffix map rejects are named, + even when other sources compiled (a partial drop links with undefined + symbols far from the cause).""" read_path = tmp_path / "lib" (read_path / "src").mkdir(parents=True) (read_path / "src" / "impl.CPP").write_text("") - component._library_info("x", read_path, {"build": {}}) - assert "no matched file has a recognized source suffix" in caplog.text + (read_path / "src" / "sketch.ino").write_text("") + (read_path / "src" / "ok.cpp").write_text("") + lib = component._library_info("x", read_path, {"build": {}}) + assert [s.name for s in lib.sources] == ["ok.cpp"] + assert "not compiled: impl.CPP, sketch.ino" in caplog.text + + +def test_library_info_header_only_src_stays_quiet( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """A header-only library (real headers in src/) is routine, not a + warning (the default +<*> filter matches the headers too).""" + read_path = tmp_path / "lib" + (read_path / "src").mkdir(parents=True) + (read_path / "src" / "ArduinoJson.h").write_text("") + (read_path / "keywords.txt").write_text("") + lib = component._library_info("x", read_path, {"build": {}}) + assert lib.sources == [] + assert "not compiled" not in caplog.text + assert "srcFilter" not in caplog.text def test_library_info_lib_archive_malformed_raises(tmp_path: Path) -> None: diff --git a/tests/unit_tests/test_platformio_library.py b/tests/unit_tests/test_platformio_library.py index 7f497d0344..f46f4e4fa2 100644 --- a/tests/unit_tests/test_platformio_library.py +++ b/tests/unit_tests/test_platformio_library.py @@ -645,6 +645,9 @@ def test_normalize_dependencies_forms(caplog) -> None: {"name": "SPI"}, ] assert normalize_dependencies("Wire") == [{"name": "Wire"}] + # A non-iterable value fails by manifest name, never a bare TypeError + assert normalize_dependencies(5, "libx") == [] + assert "Ignoring unrecognized dependencies 5 of libx" in caplog.text # The dict-shorthand form validates names like the list form: an empty # key and a spec overriding name with a non-string both warn and drop assert normalize_dependencies(