Compare commits

..
Author SHA1 Message Date
J. Nick Koston 421c5e5d32 Merge branch 'dev' into platformio-prefetch-git-clones 2026-09-02 05:16:40 -04:00
J. Nick Koston ab59822c1f Tighten comments 2026-08-31 17:41:13 -04:00
J. Nick Koston fa5180bd51 Keep local .git paths with pio run 2026-08-31 16:49:39 -04:00
J. Nick Koston 8582bf194a Classify .git URLs before the scheme exclusion, restore the name fallback 2026-08-31 16:31:28 -04:00
J. Nick Koston 328e83ad64 Simplify: one custom-name rule in _uri_jobs, ordering owned by the pool 2026-08-31 16:08:57 -04:00
J. Nick Koston 6877513195 Classify VCS URIs positively and pin the clone floor 2026-08-31 15:39:01 -04:00
J. Nick Koston 03104b894b Gate derived-name lib clones out of the pool, widen it for clones, pin the wiring 2026-08-31 14:19:16 -05:00
J. Nick Koston 26a308a82d Address review: name the clones-first ordering, harden the name fallback 2026-08-31 13:54:18 -05:00
J. Nick Koston 938f598709 Merge branch 'dev' into platformio-prefetch-git-clones
# Conflicts:
#	esphome/platformio/prefetch.py
2026-08-31 13:23:59 -05:00
J. Nick Koston 2b4a196cf8 Merge branch 'dev' into platformio-prefetch-git-clones 2026-08-28 14:26:22 -05:00
J. Nick Koston 3f65f5c12c Use the computed name for download candidates too 2026-08-27 15:26:29 -05:00
J. Nick Koston 7cc892ea14 Pin PackageSpec git URL normalization in the contract test 2026-08-27 15:23:11 -05:00
J. Nick Koston 9e4bec7e49 [core] Clone git PlatformIO packages in parallel in the prefetch 2026-08-27 15:09:03 -05:00
5 changed files with 199 additions and 398 deletions
+2 -96
View File
@@ -44,16 +44,6 @@ This document provides essential context for AI models interacting with this pro
## 4. Coding Conventions & Style Guide
**Read the developer documentation before writing a component.** https://developers.esphome.io covers the
component lifecycle, the main loop, and the reasoning behind the rules below in far more depth than this
file does, and it is the authority when they disagree. The most useful starting points:
* https://developers.esphome.io/architecture/components/ - component lifecycle, `setup()`, `loop()`,
setup priorities, and how a component is registered.
* https://developers.esphome.io/architecture/components/advanced/ - choosing between `loop()`,
`set_interval`, `set_timeout` and `defer`; waking the loop from another thread; the RAM cost of each.
* https://developers.esphome.io/contributing/code/ - contribution rules, public API and breaking changes.
* **Formatting:**
* **Python:** Uses `ruff` and `flake8` for linting and formatting. Configuration is in `pyproject.toml`.
* **C++:** Uses `clang-format` for formatting. Configuration is in `.clang-format`.
@@ -152,47 +142,6 @@ file does, and it is the authority when they disagree. The most useful starting
* **Indentation:** Use spaces (two per indentation level), not tabs
* **Type aliases:** Prefer `using type_t = int;` over `typedef int type_t;`
* **Line length:** Wrap lines at no more than 120 characters
* **Timing in `loop()`:** Never call `millis()` in a `loop()` body. The current tick's timestamp is
already cached - use `App.get_loop_component_start_time()` (from `esphome/core/application.h`).
Only reach for `millis()` when you genuinely need sub-tick resolution inside a long operation.
* **The main loop runs every 16 ms.** A rate-limit gate shorter than that does nothing: the check
passes on essentially every pass of the loop, so it costs a comparison and buys nothing. Pick an
interval comfortably coarser than 16 ms, or drop the gate entirely and accept running every loop.
```cpp
// Bad - a 10ms gate against a 16ms loop never holds anything back
static constexpr uint32_t POLL_INTERVAL_MS = 10;
const uint32_t now = millis();
if (now - this->last_poll_ < POLL_INTERVAL_MS)
return;
this->last_poll_ = now;
```
```cpp
// Good - an interval that actually rate limits, off the cached timestamp
static constexpr uint32_t POLL_INTERVAL_MS = 100;
const uint32_t now = App.get_loop_component_start_time();
if (now - this->last_poll_ < POLL_INTERVAL_MS)
return;
this->last_poll_ = now;
```
Pick the primitive by cadence: under 250 ms use a gated `loop()`; 500 ms and above use
`set_interval`. Full reasoning, including why `set_interval` costs more below 500 ms:
https://developers.esphome.io/architecture/components/advanced/#quick-rule-of-thumb
* **Don't override a default with the same value:** if a base class method already returns what you
want, do not override it. `Component::get_setup_priority()` returns `setup_priority::DATA`, so a
component that wants `DATA` should simply leave it alone.
```cpp
// Bad - this is exactly what the base class already does
float get_setup_priority() const override { return setup_priority::DATA; }
```
* **Logging string literals:** wrap literals passed as `%s` arguments in `LOG_STR_LITERAL()` so they
can be stored in flash rather than RAM.
```cpp
// Bad
ESP_LOGV(TAG, "Key %u %s", key, pressed ? "pressed" : "released");
// Good
ESP_LOGV(TAG, "Key %u %s", key, pressed ? LOG_STR_LITERAL("pressed") : LOG_STR_LITERAL("released"));
```
* **Constructor parameters vs setters:** Component properties that are both **required** and **invariant**
(never change after construction) should be constructor parameters rather than set via setter methods.
This makes the dependency explicit and prevents use of the object in an incompletely-initialized state.
@@ -613,33 +562,6 @@ file does, and it is the authority when they disagree. The most useful starting
Use `cg.add_define("MAX_SERVICES", count)` to set the size from Python configuration.
Like `std::array` but with vector-like API (`push_back()`, `size()`) and no STL reallocation code.
**Listener and child-entity registration lists are the most common case, and the most commonly
missed.** A `register_*()` method called once per child at code generation time has a count that
is known at compile time, so it should never be a `std::vector`. Use `cg.slot_counter()`: it
returns a function that each consumer calls once per slot it will occupy, and after every
`to_code` has run it emits the define with the final count. When nothing registers, no define is
emitted and the storage plus its registration method compile out entirely.
```python
# hub component's __init__.py
_request_listener_slot = cg.slot_counter("MY_COMPONENT_LISTENER_COUNT")
async def register_listener(hub: MockObj, var: MockObj) -> None:
_request_listener_slot()
cg.add(hub.register_listener(var))
```
```cpp
#ifdef MY_COMPONENT_LISTENER_COUNT
void register_listener(MyComponentListener *listener);
#endif
protected:
#ifdef MY_COMPONENT_LISTENER_COUNT
StaticVector<MyComponentListener *, MY_COMPONENT_LISTENER_COUNT> listeners_;
#endif
```
Request slots from `to_code`, not from a job that runs after `CoroPriority.FINAL` - a late
request raises rather than silently undercounting.
3. **Runtime-known sizes:** Use `FixedVector` from `esphome/core/helpers.h` when the size is only known at runtime initialization.
```cpp
// Bad - generates STL realloc code (_M_realloc_insert)
@@ -677,25 +599,9 @@ file does, and it is the authority when they disagree. The most useful starting
```
Linear search on small datasets (1-16 elements) is often faster than hashing/tree overhead, but this depends on lookup frequency and access patterns. For frequent lookups in hot code paths, the O(1) vs O(n) complexity difference may still matter even for small datasets. `std::vector` with simple structs is usually fine—it's the heavy containers (`map`, `set`, `unordered_map`) that should be avoided for small datasets unless profiling shows otherwise.
5. **Strings set once from configuration:** Use `StringRef` (`esphome/core/string_ref.h`) rather than
`std::string`. Code generation passes a string literal that lives in flash for the life of the
program, so storing a `std::string` copies it onto the heap for nothing. `StringRef` is a
non-owning pointer plus length; it does not copy, and it must only ever refer to storage that
outlives it (a string literal, or a buffer owned elsewhere).
```cpp
// Bad - heap copy of a literal that is already in flash
void set_keys(std::string keys) { this->keys_ = std::move(keys); }
std::string keys_;
```
```cpp
// Good - no allocation
void set_keys(const char *keys) { this->keys_ = StringRef(keys); }
StringRef keys_;
```
5. **Avoid `std::deque`:** It allocates in 512-byte blocks regardless of element size, guaranteeing at least 512 bytes of RAM usage immediately. This is a major source of crashes on memory-constrained devices.
6. **Avoid `std::deque`:** It allocates in 512-byte blocks regardless of element size, guaranteeing at least 512 bytes of RAM usage immediately. This is a major source of crashes on memory-constrained devices.
7. **Detection:** Look for these patterns in compiler output:
6. **Detection:** Look for these patterns in compiler output:
- Large code sections with STL symbols (vector, map, set)
- `alloc`, `realloc`, `dealloc` in symbol names
- `_M_realloc_insert`, `_M_default_append` (vector reallocation)
+66 -18
View File
@@ -16,7 +16,7 @@ name and promote with an atomic rename.
from __future__ import annotations
from collections.abc import Iterator
from collections.abc import Iterable, Iterator
from concurrent.futures import ThreadPoolExecutor
from contextlib import contextmanager, suppress
import hashlib
@@ -55,8 +55,8 @@ def _preserved_sys_path() -> Iterator[None]:
sys.path[:] = saved
# Concurrent registry resolutions / HEAD probes (each is network-bound)
_RESOLVE_WORKERS = 8
# Cap for network-bound work: resolutions, HEAD probes, clone floor
_NETWORK_WORKERS = 8
# A hung child must not block the build; downloads resume on the next run
_PREFETCH_TIMEOUT = 20 * 60
@@ -341,7 +341,7 @@ def _registry_jobs(
if not pending:
return [], 0, []
# Serial resolutions (registry GET + mirror HEAD each) dominate
with ThreadPoolExecutor(max_workers=min(_RESOLVE_WORKERS, len(pending))) as ex:
with ThreadPoolExecutor(max_workers=min(_NETWORK_WORKERS, len(pending))) as ex:
results = list(ex.map(_resolve, pending))
jobs: list[tuple[str, int, Any]] = []
installable: list[tuple[str, Any]] = []
@@ -374,14 +374,47 @@ def _registry_jobs(
return jobs, failed, installable
# The schemes pio's VCSClientFactory dispatches on (Git/Hg/SvnClient)
_VCS_URI_PREFIXES = ("git+", "hg+", "svn+", "git://", "hg://", "svn://")
def _is_vcs_spec_uri(url: str | None) -> bool:
"""Whether pio's ``install_from_uri`` would clone this URI (PackageSpec
normalizes git URLs to ``git+``). The .git check runs first so an
un-normalized repo URL fails as a clone, not as an archive download."""
if not url or url.startswith(("file://", "symlink://")):
return False
if url.split("#", 1)[0].endswith(".git"):
return True
if url.startswith(("http://", "https://")):
return False
return url.startswith(_VCS_URI_PREFIXES)
# (name, spec) from wave 1, (name, spec, compatibility) from dep waves
_Entry = tuple[str, Any] | tuple[str, Any, Any]
def _entry_is_vcs(entry: _Entry) -> bool:
"""Whether this pre-install entry is cloned rather than unpacked."""
return _is_vcs_spec_uri(entry[1].uri)
def _clones_first(entries: Iterable[_Entry]) -> list[_Entry]:
"""Clones first: they wait on the network, so they must not queue
behind CPU-bound archive extractions in the pre-install pool."""
return sorted(entries, key=lambda entry: not _entry_is_vcs(entry))
def _uri_jobs(
manager: Any, specs: list[Any], seen: set[str]
) -> tuple[list[tuple[str, int, Any]], int, list[tuple[str, Any]]]:
"""Jobs for direct-URL specs; a HEAD sizes each for the combined bar.
Also returns how many HEAD probes errored (an absent length is not an
error) and the ``(name, spec)`` pairs whose archives will be
installable.
error) and the ``(name, spec)`` pairs to pre-install: downloaded
archives, plus VCS specs, which have no archive -- the pre-install
itself clones them, in parallel instead of one at a time in pio run.
"""
from esphome.net_retry import fetch_with_retry, http_request
@@ -389,13 +422,25 @@ def _uri_jobs(
installable: list[tuple[str, Any]] = []
for spec in specs:
url = spec.uri
if not url or not url.startswith(("http://", "https://")):
continue # git+/file specs are cloned/copied, not downloaded
if url.split("#", 1)[0].endswith(".git"):
continue # bare-URL VCS spec; PlatformIO clones it
if not url:
continue
is_vcs = _is_vcs_spec_uri(url)
if not is_vcs and not url.startswith(("http://", "https://")):
if not url.startswith(("file://", "symlink://")):
_LOGGER.debug(
"Unrecognized package URI, leaving it to pio run: %s", url
)
continue # file/symlink specs are copied in place by pio run
if manager.get_package(spec):
continue
name = spec.name or url.rsplit("/", 1)[-1]
if is_vcs:
# The pre-install clones it, gated like the cached-archive
# branch below: only a custom name is the destination dir.
# Platform tool specs always parse as custom-named
if spec.has_custom_name():
installable.append((name, spec))
continue
# PlatformIO downloads URL specs with no checksum
dl_path = Path(manager.compute_download_path(url, ""))
if dl_path.is_file():
@@ -409,7 +454,7 @@ def _uri_jobs(
if str(dl_path) in seen:
continue # another spec already claimed this .part
seen.add(str(dl_path))
candidates.append((spec.name, url, dl_path, spec))
candidates.append((name, url, dl_path, spec))
errors: list[str] = []
@@ -435,7 +480,7 @@ def _uri_jobs(
if not candidates:
return [], 0, installable
with ThreadPoolExecutor(max_workers=min(_RESOLVE_WORKERS, len(candidates))) as ex:
with ThreadPoolExecutor(max_workers=min(_NETWORK_WORKERS, len(candidates))) as ex:
sizes = list(ex.map(_head_size, [url for _, url, _, _ in candidates]))
jobs: list[tuple[str, int, Any]] = []
failed = 0
@@ -583,10 +628,6 @@ def _uri_fetch_job(manager: Any, url: str, dl_path: Path, size: int) -> Any:
return run
# (name, spec) from wave 1, (name, spec, compatibility) from dep waves
_Entry = tuple[str, Any] | tuple[str, Any, Any]
def _dependency_entries(
manager: Any, entries: list[_Entry], seen_names: set[str]
) -> list[_Entry]:
@@ -701,7 +742,14 @@ def _preinstall(
would hang, not fail). Waves skip dependencies; the installed
manifests feed the next wave. Any failure falls back to pio run.
"""
workers = min(get_usable_cpu_count(), len(entries))
entries = _clones_first(entries)
clones = sum(1 for entry in entries if _entry_is_vcs(entry))
# Network-bound clones run wide even on small-core runners; capped
# since each worker builds a sibling manager and may run a
# postinstall, and a mixed wave's extractions inherit the width
workers = min(
max(get_usable_cpu_count(), min(clones, _NETWORK_WORKERS)), len(entries)
)
# One manager per worker (_install mutates instance state); built
# serially because construction rewires the shared manager logger
managers: SimpleQueue = SimpleQueue()
@@ -733,7 +781,7 @@ def _preinstall(
raise
_LOGGER.info(
"Installing %d PlatformIO package(s) with %d extraction worker(s): %s",
"Installing %d PlatformIO package(s) with %d worker(s): %s",
len(entries),
workers,
", ".join(name for name, *_ in entries),
+6 -140
View File
@@ -3,7 +3,6 @@
import argparse
import codecs
import collections
from collections.abc import Iterator
import fnmatch
import functools
import os.path
@@ -1121,56 +1120,7 @@ def lint_no_std_bind(fname, match):
)
LOG_CALL_START_RE = re.compile(r"ESP_LOG\w+\s*\(")
# Comments, raw/plain string literals and single char literals are consumed whole so ; ( ) ? :
# inside them are never seen. A char literal is exactly one (escaped) char so a digit separator
# like 1'000'000 cannot open one.
CPP_COMMENT_RE = r"//[^\n]*|/\*.*?\*/"
CPP_SKIP_RE = (
CPP_COMMENT_RE
+ r'|R"(?P<raw_delim>[^(\s]*)\(.*?\)(?P=raw_delim)"|"(?:[^"\\]|\\.)*"|\'(?:[^\'\\\n]|\\.)\''
)
LOG_CALL_TOKEN_RE = re.compile(CPP_SKIP_RE + r"|[()]", re.DOTALL)
# The last alternative matches a ? or : followed (after spaces or comments) by an opening quote,
# i.e. a string literal used as a ternary branch.
LOG_TERNARY_LITERAL_RE = re.compile(
CPP_SKIP_RE + r"|[?:](?:\s|" + CPP_COMMENT_RE + r')*(?=")', re.DOTALL
)
# A bare NOLINT; a clang-tidy NOLINT(check-name) is aimed at a different tool.
NOLINT_RE = re.compile(r"\bNOLINT\b(?!\()")
def _line_col(content: str, pos: int) -> tuple[int, int]:
"""1-based line and column of an offset in content."""
return content.count("\n", 0, pos) + 1, pos - content.rfind("\n", 0, pos)
def _iter_log_calls(content: str) -> Iterator[tuple[int, str | None]]:
"""Yield (start, text) for every ESP_LOG*(...) call, text running to the matching close paren.
text is None when no matching paren exists so callers can report the call instead of skipping it."""
for head in LOG_CALL_START_RE.finditer(content):
depth = 1
for tok in LOG_CALL_TOKEN_RE.finditer(content, head.end()):
if tok.group(0) == "(":
depth += 1
elif tok.group(0) == ")":
depth -= 1
if depth == 0:
yield head.start(), content[head.start() : tok.end()]
break
else:
yield head.start(), None
def _unbalanced_log_call_error(content: str, pos: int) -> tuple[int, int, str]:
lineno, col = _line_col(content, pos)
return (
lineno,
col,
"ESP_LOG call has no matching closing parenthesis, so it cannot be checked.",
)
LOG_MULTILINE_RE = re.compile(r"ESP_LOG\w+\s*\(.*?;", re.DOTALL)
LOG_BAD_CONTINUATION_RE = re.compile(r'\\n(?:[^ \\"\r\n\t]|"\s*\n\s*"[^ \\])')
LOG_PERCENT_S_CONTINUATION_RE = re.compile(r'\\n(?:%s|"\s*\n\s*"%s)')
@@ -1178,16 +1128,16 @@ LOG_PERCENT_S_CONTINUATION_RE = re.compile(r'\\n(?:%s|"\s*\n\s*"%s)')
@lint_content_check(include=cpp_include)
def lint_log_multiline_continuation(fname, content):
errs = []
for log_start, log_text in _iter_log_calls(content):
if log_text is None:
errs.append(_unbalanced_log_call_error(content, log_start))
continue
for log_match in LOG_MULTILINE_RE.finditer(content):
log_text = log_match.group(0)
for bad_match in LOG_BAD_CONTINUATION_RE.finditer(log_text):
# %s may expand to a whitespace prefix at runtime, skip those
if LOG_PERCENT_S_CONTINUATION_RE.match(log_text, bad_match.start()):
continue
# Calculate line number from position in full content
lineno, col = _line_col(content, log_start + bad_match.start())
abs_pos = log_match.start() + bad_match.start()
lineno = content.count("\n", 0, abs_pos) + 1
col = abs_pos - content.rfind("\n", 0, abs_pos)
errs.append(
(
lineno,
@@ -1205,90 +1155,6 @@ def lint_log_multiline_continuation(fname, content):
return errs
def _find_ternary_literals(text: str) -> Iterator[tuple[int, str]]:
"""Yield (offset, literal) for every string literal used as a ternary branch."""
branch = False
for m in LOG_TERNARY_LITERAL_RE.finditer(text):
tok = m.group(0)
# An empty literal is merged with every other string's terminator, so it costs no RAM,
# while a PSTR("") would add its own flash array; leave it alone.
if branch and tok[0] == '"' and tok != '""':
yield m.start(), tok
branch = tok[0] in "?:"
# LOG_STR_LITERAL is a no op everywhere except ESP8266, so code that never builds there is skipped
# to avoid churn: platform specific sources and components for ESP32, LibreTiny, RP2 and Zephyr only.
# A component belongs here only if it has no tests/components/<name>/test.esp8266-ard.yaml.
LOG_LITERAL_LINT_EXCLUDE = [
"*_esp32.cpp",
"*_esp32_*.cpp",
"*_esp_idf.cpp",
"*_rmt.cpp",
"*_zephyr.cpp",
"*_bk72xx.cpp",
"*_libretiny.cpp",
"*_pico_w.cpp",
"*_host.cpp",
"esphome/components/esp32*/*",
"esphome/components/bk72xx*/*",
"esphome/components/ln882h*/*",
"esphome/components/ln882x*/*",
"esphome/components/rp2*/*",
"esphome/components/zephyr*/*",
"esphome/components/host/*",
"esphome/components/libretiny*/*",
"esphome/components/bluetooth_proxy/*",
"esphome/components/bluetooth_connection/*",
"esphome/components/ble_client/*",
"esphome/components/bedjet/*",
"esphome/components/anova/*",
"esphome/components/xiaomi_ble/*",
"esphome/components/bthome_mithermometer/*",
"esphome/components/usb_host/*",
"esphome/components/zigbee/*",
"esphome/components/lvgl/*",
# Test fixtures and host only unit tests - not production embedded code
"tests/integration/fixtures/*",
"tests/components/*",
]
@lint_content_check(include=cpp_include, exclude=LOG_LITERAL_LINT_EXCLUDE)
def lint_log_no_bare_literal_ternary(
fname: Path, content: str
) -> list[tuple[int, int, str]]:
errs = []
for log_start, log_text in _iter_log_calls(content):
if log_text is None:
continue # reported by lint_log_multiline_continuation, which sees every file
# A NOLINT anywhere on the lines the call spans silences every branch in it
first_line = content.rfind("\n", 0, log_start) + 1
last_line = content.find("\n", log_start + len(log_text))
if NOLINT_RE.search(
content[first_line : last_line if last_line != -1 else None]
):
continue
for offset, literal in _find_ternary_literals(log_text):
lineno, col = _line_col(content, log_start + offset)
errs.append(
(
lineno,
col,
(
"String literal used as a ternary branch in a log call. On ESP8266 the "
"log macro moves the format string to flash, but bare literal arguments "
"stay in RAM. Wrap each branch passed straight to the log call in "
f"{highlight('LOG_STR_LITERAL(...)')}:\n"
f" Before: {highlight(literal)}\n"
f" After: {highlight(f'LOG_STR_LITERAL({literal})')}\n"
f"(If strictly necessary, add `{highlight('// NOLINT')}` to the end of the line)"
),
)
)
return errs
@lint_content_find_check(
"ESP_LOG",
include=["*.h", "*.tcc"],
-126
View File
@@ -4,16 +4,12 @@ The rule flags an if/else/for/while whose only body is an unbraced ESP_LOG*() ca
empty statement -- and a -Wempty-body warning -- once the log level compiles the macro out). These
tests pin the comment/string/raw-string masker, the accepted control-statement shapes, and the
NOLINT escape hatch at both placements a contributor would try.
Also covers the ESP_LOG call scanner (_iter_log_calls) and the bare-literal-ternary lint.
"""
import importlib.util
from pathlib import Path
import sys
import pytest
SCRIPT_DIR = (Path(__file__).parent / ".." / ".." / "script").resolve()
sys.path.insert(0, str(SCRIPT_DIR))
_spec = importlib.util.spec_from_file_location("ci_custom", SCRIPT_DIR / "ci-custom.py")
@@ -149,125 +145,3 @@ def test_nolint_at_end_of_log_line_suppresses() -> None:
def test_nolint_on_control_line_suppresses() -> None:
assert not _lint("if (x) // NOLINT\n ESP_LOGD(t);\n")
# --- ESP_LOG call scanner and bare-literal-ternary lint ---
def _calls(content: str) -> list[str | None]:
return [text for _, text in ci_custom._iter_log_calls(content)]
def _ternary_errors(content: str) -> list[tuple[int, int]]:
errs = ci_custom.lint_log_no_bare_literal_ternary(Path("x.cpp"), content)
return [(line, col) for line, col, _ in errs]
@pytest.mark.parametrize(
"content",
[
'ESP_LOGD(TAG, "a ) b ( c; d")',
'ESP_LOGD(TAG, "quote \\" inside")',
"ESP_LOGD(TAG, \"%s\", format_hex_pretty(x, '-', false).c_str())",
"ESP_LOGD(TAG, \"%c%c\", '(', ')')",
"ESP_LOGD(TAG, \"%d\", 1'000'000)",
'ESP_LOGD(TAG, // it\'s a comment with ) and (\n "x")',
'ESP_LOGD(TAG, /* :) */ "x")',
'ESP_LOGD(TAG, "%s", R"(say "hi" :) )")',
'ESP_LOGD(TAG, "%s", R"x(a)"b)x")',
],
)
def test_iter_log_calls_spans_whole_call(content: str) -> None:
calls = _calls(content + ";\nint other = (1);")
assert calls == [content]
def test_iter_log_calls_reports_unbalanced_call_once() -> None:
content = 'ESP_LOGD(TAG, "x";\nvoid f();'
assert _calls(content) == [None]
errs = ci_custom.lint_log_multiline_continuation(Path("x.cpp"), content)
assert len(errs) == 1
assert errs[0][:2] == (1, 1)
assert "no matching closing parenthesis" in errs[0][2]
assert _ternary_errors(content) == []
@pytest.mark.parametrize(
("content", "expected"),
[
# A ; inside the format string no longer cuts the call short
('ESP_LOGD(TAG, "a; b\\nc %s", x);', [(1, 20)]),
# A \n%s continuation is exempt since %s may expand to leading whitespace
('ESP_LOGD(TAG, "a\\n%s", x);', []),
('ESP_LOGD(TAG, "a\\n b");', []),
],
)
def test_multiline_continuation_detection(
content: str, expected: list[tuple[int, int]]
) -> None:
errs = ci_custom.lint_log_multiline_continuation(Path("x.cpp"), content)
assert [(line, col) for line, col, _ in errs] == expected
def test_exclusion_list_only_names_components_without_esp8266_tests() -> None:
root = Path(__file__).parent / ".." / ".."
for pattern in ci_custom.LOG_LITERAL_LINT_EXCLUDE:
if not pattern.startswith("esphome/components/"):
continue
prefix = pattern.removeprefix("esphome/components/").split("/")[0]
comps = list((root / "esphome" / "components").glob(prefix))
assert comps, f"{pattern!r} matches no component"
for comp in comps:
test = root / "tests" / "components" / comp.name / "test.esp8266-ard.yaml"
assert not test.exists(), (
f"{comp.name} builds for ESP8266, drop {pattern!r}"
)
def test_unbalanced_calls_are_reported_by_a_check_that_sees_every_file() -> None:
# lint_log_no_bare_literal_ternary skips unbalanced calls and relies on this
checks = {c["func"].__name__: c for c in ci_custom.LINT_CONTENT_CHECKS}
continuation = checks["lint_log_multiline_continuation"]
ternary = checks["lint_log_no_bare_literal_ternary"]
assert continuation["exclude"] == []
assert continuation["include"] == ternary["include"]
@pytest.mark.parametrize(
("content", "expected"),
[
('ESP_LOGD(TAG, "%s", x ? "on" : "off");', [(1, 25), (1, 32)]),
(
'ESP_LOGD(TAG, "%s", x ? LOG_STR_LITERAL("on") : LOG_STR_LITERAL("off"));',
[],
),
('ESP_LOGD(TAG, "%s", x ? LOG_STR_LITERAL("on") : "off");', [(1, 49)]),
('ESP_LOGD(TAG, "%s", x ? "on" : "");', [(1, 25)]),
(
'ESP_LOGD(TAG, "%s",\n x ? "yes"\n : "no");',
[(2, 14), (3, 14)],
),
("ESP_LOGD(TAG, \"%c\", x ? '1' : '0');", []),
('ESP_LOGD(TAG, "a ? b : c %s", x ? "on" : "off");', [(1, 35), (1, 42)]),
('ESP_LOGD(TAG, "x:" "y %s", p);', []),
('ESP_LOGD(TAG, "%s", x ? "on" : "off"); // NOLINT', []),
('ESP_LOGD(TAG, "%s",\n x ? "yes"\n : "no"); // NOLINT', []),
('ESP_LOGD(TAG, "%s", x ? /* c */ "on" : "off");', [(1, 33), (1, 40)]),
(
'ESP_LOGD(TAG, "%s",\n x ? "on" // NOLINT(some-clang-check)\n : "off");',
[(2, 14), (3, 14)],
),
],
)
def test_ternary_literal_detection(
content: str, expected: list[tuple[int, int]]
) -> None:
assert _ternary_errors(content) == expected
def test_ternary_error_message_names_the_literal() -> None:
errs = ci_custom.lint_log_no_bare_literal_ternary(
Path("x.cpp"), 'ESP_LOGD(TAG, "%s", x ? "enabled" : LOG_STR_LITERAL("off"));'
)
assert len(errs) == 1
assert 'LOG_STR_LITERAL("enabled")' in errs[0][2]
+125 -18
View File
@@ -588,7 +588,8 @@ def test_registry_jobs_one_bad_spec_keeps_the_rest(tmp_path: Path) -> None:
def test_uri_jobs_head_sizes_the_bar(tmp_path: Path) -> None:
"""HEAD sizes direct-URL specs; git and unreachable URLs are skipped."""
"""HEAD sizes direct-URL specs; VCS specs skip the download but are
still installable (the pre-install clones them in parallel)."""
m = _fake_manager(tmp_path)
resp = MagicMock()
resp.headers = {"content-length": "2222"}
@@ -597,15 +598,14 @@ def test_uri_jobs_head_sizes_the_bar(tmp_path: Path) -> None:
m,
[
_FakeSpec(uri="https://x/big.zip", name="big", custom_name=True),
_FakeSpec(uri="git+https://x/repo.git", name="repo"),
_FakeSpec(uri="https://x/repo.git#v1", name="barevcs"),
_FakeSpec(uri="git+https://x/repo.git", name="repo", custom_name=True),
_FakeSpec(name="registry"),
],
set(),
)
assert failed == 0
assert [(n, s) for n, s, _ in jobs] == [("big", 2222)]
assert [n for n, _ in installable] == ["big"]
assert [n for n, _ in installable] == ["repo", "big"]
# a successful HEAD with no Content-Length is a clean skip
resp.headers = {}
with patch("esphome.net_retry.http_request", return_value=resp):
@@ -614,6 +614,67 @@ def test_uri_jobs_head_sizes_the_bar(tmp_path: Path) -> None:
) == ([], 0, [])
def test_uri_jobs_vcs_specs_installable_without_probe(tmp_path: Path) -> None:
"""VCS specs never probe the network; custom-named uninstalled ones
pre-install, everything else is left to pio run."""
m = _fake_manager(tmp_path)
with patch("esphome.net_retry.http_request") as mock_head:
jobs, failed, installable = pf._uri_jobs(
m,
[
_FakeSpec(
uri="git+https://x/tool.git#1.0", name="tool", custom_name=True
),
_FakeSpec(uri="hg+https://x/old", name="mercurial", custom_name=True),
_FakeSpec(uri="git+https://x/derived.git", name="derived"),
# An un-normalized repo URL classifies as VCS (never as a
# downloadable archive), then drops here as derived-name
_FakeSpec(uri="https://x/unnorm.git", name="unnorm"),
_FakeSpec(uri="file:///local/dir", name="local"),
# A local .git path is copied in place, never cloned
_FakeSpec(uri="file:///local/repo.git", name="localgit"),
_FakeSpec(uri="symlink:///local/dir", name="link"),
],
set(),
)
mock_head.assert_not_called()
assert (jobs, failed) == ([], 0)
assert [n for n, _ in installable] == ["tool", "mercurial"]
# Positive classification: an unknown scheme is left to pio run
with patch("esphome.net_retry.http_request") as mock_head:
assert pf._uri_jobs(
m, [_FakeSpec(uri="weird://x/pkg", name="weird")], set()
) == ([], 0, [])
mock_head.assert_not_called()
m.get_package.return_value = object() # already installed: warm and silent
with patch("esphome.net_retry.http_request"):
assert pf._uri_jobs(
m,
[
_FakeSpec(
uri="git+https://x/tool.git#1.0", name="tool", custom_name=True
)
],
set(),
) == ([], 0, [])
def test_clones_first_orders_vcs_before_archives() -> None:
"""The pre-install pool receives clones first: they wait on the
network and must not queue behind CPU-bound archive extractions."""
archive = ("zip", _FakeSpec(uri="https://x/a.zip", name="zip"))
registry = ("reg", _FakeSpec(uri=None, name="reg"))
clone = ("repo", _FakeSpec(uri="git+https://x/repo.git", name="repo"))
ordered = pf._clones_first([archive, registry, clone])
assert ordered[0] == clone
# Stable partition: non-clone relative order is preserved
assert ordered[1:] == [archive, registry]
assert pf._entry_is_vcs(clone)
assert not pf._entry_is_vcs(archive)
assert not pf._entry_is_vcs(registry)
def test_uri_jobs_head_failure_counts_as_unresolved(
tmp_path: Path, caplog: pytest.LogCaptureFixture
) -> None:
@@ -1600,12 +1661,9 @@ def test_preinstall_dependency_wave_skips_seen_names(tmp_path: Path) -> None:
assert installed == ["noise-c"]
def test_preinstall_uses_distinct_managers_in_parallel(tmp_path: Path) -> None:
"""Each worker thread gets its own pre-built manager and installs
genuinely overlap (the barrier deadlocks a serial pool). The worker
count is pinned so a 1-CPU host cannot serialize the pool."""
barrier = threading.Barrier(2, timeout=5)
used: set = set()
def _wave_manager(tmp_path, on_install):
"""A minimal pio-manager stand-in for _preinstall pool tests;
``on_install(manager, spec)`` observes each _install call."""
class _WaveManager:
package_dir = str(tmp_path)
@@ -1636,20 +1694,59 @@ def test_preinstall_uses_distinct_managers_in_parallel(tmp_path: Path) -> None:
return None
def _install(self, spec, skip_dependencies, compatibility=None) -> None:
used.add(id(self))
barrier.wait()
on_install(self, spec)
seed = _WaveManager(str(tmp_path))
with patch.object(pf, "get_usable_cpu_count", return_value=2):
return _WaveManager
@pytest.mark.parametrize(
("cpu_count", "entries"),
[
(2, [("a@1", _FakeSpec(name="a")), ("b@1", _FakeSpec(name="b"))]),
# The clone floor: network-bound clones run wide on a 1-CPU host
(
1,
[
(f"r{i}", _FakeSpec(uri=f"git+https://x/r{i}.git", name=f"r{i}"))
for i in range(4)
],
),
],
ids=("cpu-sized", "clone-floor"),
)
def test_preinstall_pool_width(tmp_path: Path, cpu_count: int, entries: list) -> None:
"""The barrier deadlocks unless every entry gets its own manager
and runs concurrently."""
barrier = threading.Barrier(len(entries), timeout=5)
used: set = set()
def on_install(mgr, spec) -> None:
used.add(id(mgr))
barrier.wait()
cls = _wave_manager(tmp_path, on_install)
seed = cls(str(tmp_path))
with patch.object(pf, "get_usable_cpu_count", return_value=cpu_count):
pf._preinstall(seed, entries)
assert len(used) == len(entries)
assert id(seed) not in used
def test_preinstall_orders_clones_before_extractions(tmp_path: Path) -> None:
"""With one worker, the clone installs before the archive
regardless of caller order."""
order: list[str] = []
cls = _wave_manager(tmp_path, lambda mgr, spec: order.append(spec.name))
seed = cls(str(tmp_path))
with patch.object(pf, "get_usable_cpu_count", return_value=1):
pf._preinstall(
seed,
[
("a@1", _FakeSpec(name="a")),
("b@1", _FakeSpec(name="b")),
("zip", _FakeSpec(uri="https://x/a.zip", name="zip")),
("repo", _FakeSpec(uri="git+https://x/repo.git", name="repo")),
],
)
assert len(used) == 2
assert id(seed) not in used
assert order == ["repo", "zip"]
def test_sibling_manager_and_sigterm() -> None:
@@ -1803,3 +1900,13 @@ def test_platformio_private_api_contract() -> None:
derived = PackageSpec("https://x/y/archive/master.zip")
assert derived.name and not derived.has_custom_name()
assert PackageSpec("Foo=https://x/y/archive/master.zip").has_custom_name()
# _is_vcs_spec_uri relies on bare .git URLs normalizing to git+, on
# both parse paths (raw string, and requirements= for platform tools)
assert PackageSpec("https://github.com/x/y.git#v1").uri.startswith("git+")
platform_tool = PackageSpec(
owner="o", name="tool-x", requirements="https://github.com/x/y.git"
)
assert platform_tool.uri.startswith("git+")
# A URL requirement re-parses as name=url, marking the name custom;
# this is what keeps platform tool clones in the parallel pre-install
assert platform_tool.has_custom_name()