fix(compile): lock the build DIRECTORY, not the FQBN — concurrent compiles swapped binaries
`_prepare_persistent_project_dir` keys the persistent build dir on (idf_target, variant), but the lock was keyed on the FQBN. Several FQBNs resolve to one variant — arduino-esp32's boards.txt gives both `esp32` and `esp32cam` build.variant=esp32 — so those two were free to build in the SAME directory at the same time. Measured against staging, compiling both concurrently: esp32 285504 bytes sha f65ca909… contains its own marker esp32cam 285504 bytes sha f65ca909… contains the OTHER sketch's marker Byte-identical firmware, and the esp32cam caller got the esp32 sketch's binary. In the product that is two gallery tabs open at once: the first tab prints the second example's serial log and does nothing itself. It made esp32-pwm-led-rgb look broken (dark LED, every SPICE node 0 V) when the example is fine — it was running someone else's program. Key the lock on the same identity as the directory. Boards with genuinely separate build dirs still compile in parallel up to the semaphore cap; an unresolvable FQBN falls back to the default pair, which is the safe direction (serialising more only costs throughput). Also in this commit: battery-voltage-monitor ships the 9 V battery its own description promises. The divider's top end was wired to nothing, so A0 read 0 V and the sketch printed "Battery: 0.0 V / WARNING: Battery low!" forever.
This commit is contained in:
parent
76e656ea8d
commit
5545089389
|
|
@ -44,14 +44,44 @@ _COMPILE_SEMAPHORE = asyncio.Semaphore(2)
|
|||
_TARGET_LOCKS: dict[str, asyncio.Lock] = {}
|
||||
|
||||
|
||||
def _build_identity(board_fqbn: str) -> str:
|
||||
"""The identity of the persistent BUILD DIRECTORY this FQBN compiles in.
|
||||
|
||||
The lock below has to be keyed on the shared resource, and that resource is
|
||||
the build dir — which `_prepare_persistent_project_dir` keys on
|
||||
(idf_target, variant), NOT on the FQBN. Several FQBNs map to one variant:
|
||||
arduino-esp32's boards.txt gives both `esp32` and `esp32cam`
|
||||
`build.variant=esp32`, so they land in the SAME directory.
|
||||
|
||||
Keying the lock on the FQBN therefore left the two of them free to run at
|
||||
once inside one build dir. Measured: compiling `esp32:esp32:esp32` and
|
||||
`esp32:esp32:esp32cam` concurrently returned BYTE-IDENTICAL firmware (same
|
||||
sha256, 285504 bytes) and the esp32cam request got the other sketch's
|
||||
binary — its serial printed the other example's output. In the app that
|
||||
looked like an example running someone else's program: two gallery tabs
|
||||
open, and the second one shows the first one's log and does nothing.
|
||||
|
||||
Falls back to the raw FQBN if the compiler cannot resolve the pair, which
|
||||
is the previous behaviour and never less strict than it needs to be for a
|
||||
board whose variant we could not read.
|
||||
"""
|
||||
try:
|
||||
target = espidf_compiler._idf_target(board_fqbn)
|
||||
variant = espidf_compiler._arduino_variant(board_fqbn, target)
|
||||
return f"{target}::{variant}"
|
||||
except Exception: # noqa: BLE001 - identity is best-effort; FQBN is a safe fallback
|
||||
return board_fqbn
|
||||
|
||||
|
||||
def _target_lock(board_fqbn: str) -> asyncio.Lock:
|
||||
"""Lazy-initialised per-target lock so concurrent compiles to the same
|
||||
board serialise. Different boards still run in parallel up to the
|
||||
semaphore cap."""
|
||||
lock = _TARGET_LOCKS.get(board_fqbn)
|
||||
"""Lazy-initialised lock so concurrent compiles that SHARE A BUILD DIR
|
||||
serialise. Boards with genuinely different build dirs still run in
|
||||
parallel up to the semaphore cap."""
|
||||
key = _build_identity(board_fqbn)
|
||||
lock = _TARGET_LOCKS.get(key)
|
||||
if lock is None:
|
||||
lock = asyncio.Lock()
|
||||
_TARGET_LOCKS[board_fqbn] = lock
|
||||
_TARGET_LOCKS[key] = lock
|
||||
return lock
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -1449,9 +1449,13 @@ D_led1 led1_sense_mid 0 LED_RED
|
|||
|
||||
exports[`netlist snapshot — circuits (41 examples) > battery-voltage-monitor 1`] = `
|
||||
"* Velxio circuit
|
||||
R_r2 n0 n1 10000
|
||||
V_bat n0 bat_int DC 9
|
||||
R_bat_esr bat_int n2 1.5
|
||||
R_r1 n0 n1 20000
|
||||
R_r2 n1 n2 10000
|
||||
R_autopull_n0 n0 0 100Meg
|
||||
R_autopull_n1 n1 0 100Meg
|
||||
R_autopull_n2 n2 0 100Meg
|
||||
.op
|
||||
.end"
|
||||
`;
|
||||
|
|
|
|||
|
|
@ -1424,13 +1424,22 @@ void loop() {
|
|||
}`,
|
||||
components: [
|
||||
UNO,
|
||||
// The battery the description promises. Without it the divider's top end
|
||||
// hung in the air, so A0 sat at 0 V and the sketch printed
|
||||
// "Battery: 0.0 V / WARNING: Battery low!" forever — the one thing this
|
||||
// example exists to show never happened.
|
||||
{ type: 'wokwi-battery-9v', id: 'bat', x: 250, y: 120, properties: {} },
|
||||
{ type: 'wokwi-resistor', id: 'r1', x: 350, y: 80, properties: { value: '20000' } },
|
||||
{ type: 'wokwi-resistor', id: 'r2', x: 350, y: 200, properties: { value: '10000' } },
|
||||
],
|
||||
wires: [
|
||||
w('w0', ['bat', '+'], ['r1', '1'], '#ff0000'),
|
||||
w('w1', ['r1', '2'], ['r2', '1']),
|
||||
w('w2', ['r2', '2'], ['arduino-uno', 'GND'], '#000000'),
|
||||
w('w3', ['r1', '2'], ['arduino-uno', 'A0'], '#ffaa00'),
|
||||
// Battery negative shares the Arduino's ground — without a common
|
||||
// reference the ADC has nothing to measure against.
|
||||
w('w4', ['bat', '−'], ['arduino-uno', 'GND'], '#000000'),
|
||||
],
|
||||
},
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,80 @@
|
|||
"""
|
||||
The compile lock must protect the BUILD DIRECTORY, not the FQBN.
|
||||
|
||||
`_prepare_persistent_project_dir` keys the persistent build dir on
|
||||
(idf_target, variant). arduino-esp32's boards.txt gives several boards the same
|
||||
variant — `esp32` and `esp32cam` are both `build.variant=esp32` — so those FQBNs
|
||||
share one directory. The lock used to be keyed on the FQBN, which left them free
|
||||
to build in it at the same time.
|
||||
|
||||
That is not a theoretical race. Measured against staging: compiling
|
||||
`esp32:esp32:esp32` and `esp32:esp32:esp32cam` concurrently returned
|
||||
BYTE-IDENTICAL firmware (same sha256, 285504 bytes) and the esp32cam caller got
|
||||
the other sketch's binary — verified by embedding a unique marker string in each
|
||||
sketch and finding the wrong one in the returned image. In the product that
|
||||
surfaced as a gallery example running someone else's program: open two ESP32
|
||||
examples in two tabs, and the first tab shows the second's serial log and does
|
||||
nothing itself.
|
||||
|
||||
Run from the repo root:
|
||||
python -m pytest test/backend/unit/test_compile_build_lock.py -v
|
||||
"""
|
||||
|
||||
import sys
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).parent.parent.parent.parent / 'backend'))
|
||||
|
||||
from app.api.routes import compile as compile_module
|
||||
|
||||
|
||||
class BuildIdentityTests(unittest.TestCase):
|
||||
def test_boards_sharing_a_variant_share_one_lock(self):
|
||||
"""esp32 and esp32cam build in the same dir, so they must serialise."""
|
||||
plain = compile_module._target_lock('esp32:esp32:esp32')
|
||||
cam = compile_module._target_lock('esp32:esp32:esp32cam')
|
||||
self.assertIs(
|
||||
plain,
|
||||
cam,
|
||||
'esp32 and esp32cam share one persistent build dir (both are '
|
||||
'build.variant=esp32); handing them different locks lets them '
|
||||
'corrupt each other and swap binaries.',
|
||||
)
|
||||
|
||||
def test_the_same_board_still_serialises(self):
|
||||
self.assertIs(
|
||||
compile_module._target_lock('esp32:esp32:esp32'),
|
||||
compile_module._target_lock('esp32:esp32:esp32'),
|
||||
)
|
||||
|
||||
def test_different_chips_still_run_in_parallel(self):
|
||||
"""The lock must not become a global one — that would halve throughput."""
|
||||
xtensa = compile_module._target_lock('esp32:esp32:esp32')
|
||||
riscv = compile_module._target_lock('esp32:esp32:esp32c3')
|
||||
self.assertIsNot(
|
||||
xtensa,
|
||||
riscv,
|
||||
'different IDF targets have separate build dirs and should still '
|
||||
'compile concurrently up to the semaphore cap.',
|
||||
)
|
||||
|
||||
def test_unknown_boards_share_the_default_pair(self):
|
||||
"""An unrecognised FQBN resolves to the default (esp32, esp32) pair.
|
||||
|
||||
That is not a hole: those compiles genuinely land in the same build dir,
|
||||
because the compiler derives the dir from the same two functions. Sharing
|
||||
a lock here is the safe direction — serialising more than necessary only
|
||||
costs throughput, while serialising less corrupts a build.
|
||||
"""
|
||||
a = compile_module._build_identity('definitely:not:a:board')
|
||||
b = compile_module._build_identity('also:not:a:board')
|
||||
self.assertEqual(a, b)
|
||||
self.assertIs(
|
||||
compile_module._target_lock('definitely:not:a:board'),
|
||||
compile_module._target_lock('also:not:a:board'),
|
||||
)
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
unittest.main()
|
||||
Loading…
Reference in New Issue