Skip to content

Widen extra_scripts MockEnv shim (Choice C from #553): tiered real-world fixtures + tests #554

Description

@zackees

Plan: Widen the PlatformIO extra_scripts compatibility shim (Choice C)

Decision context: issue #553 evaluated four options for extra_scripts parity. This plan
executes Choice C — keep the existing MockEnv Python-subprocess shim (no real SCons),
and widen it to cover more deterministic / read-only APIs, while keeping effectful SCons
behavior on the --platformio fallback. Driven by a real-world survey of 7 scripts at three
complexity tiers.

Touch points:

  • crates/fbuild-build/src/script_runtime_harness.py (the MockEnv mock + script driver)
  • crates/fbuild-build/src/script_runtime.rs (Rust translation of scope state → flags; tests)
  • crates/fbuild-build/src/flag_overlay.rs (scope-state structs / values_to_args)
  • Fixtures: crates/fbuild-build/tests/fixtures/extra_scripts/{simple,medium,complex}/
  • Docs: crates/fbuild-build/README.md, docs/reference/platformio-ini.md

Survey results — what real scripts actually use

# Repo / file Tier Verdict
1 espmanager/git_rev_macro.py simple build_flags = !python ... stdout -D pattern — separate ingestion path, out of scope here
2 Marlin/common-cxxflags.py simple Shimmable once GetBuildType() + BUILD_FLAGS + live-list mutation land
3 m5panel/littlefsbuilder.py medium env.get() + Replace(MKSPIFFSTOOL=...) — needs known-non-flag-scope handling
4 namf/platformio_script.py medium Replace(LINKFLAGS) token removal + AddPostAction (safe no-op)
5 LDAT/8kHz_polling.py medium/cplx DefaultEnvironment import; pure file-patching → negative fixture (no flags, must not crash/hang)
6 amsreader/generate_includes.py complex codegen + Execute(pip) + git → negative fixture (zero flags)
7 Marlin/common-dependencies.py + marlin.py complex SConscript, AddMethod, GetProjectConfig().set, import platformio.*must bail clearly

Reconciled against current code (agent claims vs. reality)

  • PioPlatform().get_package_dir(name)already implemented (MockPioPlatform), no work needed.
  • AddPostAction / AddPreAction / Alias / Depends / AlwaysBuildalready no-op (NOOP_METHODS).
  • env.get(key) — exists but only reads _vars; does not fall through to project_options (partial gap).
  • GetProjectOption (singular) — exists. GetProjectOptions (plural) + GetBuildTypemissing.
  • DefaultEnvironment — injected as a script global, but from SCons.Script import DefaultEnvironment fails (no fake module). 4/7 scripts use this idiom — biggest single unlock.
  • In-place scope mutation (env["CPPDEFINES"].append(...), env["LINKFLAGS"][i]=...) — works for supported scopes (live list returned), but tuple-shaped CPPDEFINES entries appended in place are exported as raw JSON arrays that cppdefines_to_flags rejects. BUILD_FLAGS is not a scope → KeyError.

Documented structural limitations of Choice C (must be written into docs)

  1. Mock scopes start empty. They capture only what a script adds, not the platform's full
    effective flag set. Read-transform scripts (namf removing -u _printf_float from LINKFLAGS)
    operate on an empty list — the shim collects their additions, and real removals route through
    fbuild's own build_unflags later, not the script. This is acceptable but must be stated.
  2. No real SCons. SConscript, AddMethod/custom builders, GetProjectConfig().set, and any
    import platformio.* are structurally impossible → fail fast with --platformio recommendation.
  3. Effectful codegen / Execute is a no-op. Scripts that generate sources/headers or shell out
    contribute zero flags; they either degrade gracefully or must bail.
  4. build_flags = !python script.py (stdout -D) is a different feature (build_flags parsing,
    not the script runtime) — tracked separately, not in this plan.

TDD execution plan (RED → GREEN per slice)

Phase 0 — Fixtures + harness test helper

  • Add trimmed, verbatim-derived fixtures under
    crates/fbuild-build/tests/fixtures/extra_scripts/{simple,medium,complex}/ for scripts 2–7.
  • Add a test helper that writes a platformio.ini (atmelavr/uno/arduino) + a fixture
    script into a tempdir and runs resolve_extra_script_overlay, gated by find_python().is_some()
    (matches existing tests in script_runtime.rs).

Phase 1 — SIMPLE tier (mostly green today; small additions)

  • RED: pure-append fixture asserting CPPDEFINES/CXXFLAGS collected (sanity — likely passes).
  • Add MockEnv.GetBuildType() → returns canned build_type from project_options (default release).
  • Add BUILD_FLAGS as a recognized common-flags list: env["BUILD_FLAGS"] returns a live list,
    its contents fold into the global compile overlay on export.
  • Normalize tuple/2-element CPPDEFINES entries at export time so in-place
    .append((name,value)) survives the JSON round-trip (or extend cppdefines_to_flags to accept
    a 2-element array). Add a Rust unit test for the array→-Dname=value mapping.
  • GREEN: Marlin/common-cxxflags.py fixture → assert -Wno-register in CXXFLAGS, GetBuildType
    returns release, AddPostAction ignored, no crash.

Phase 2 — MEDIUM tier

  • Install a fake SCons.Script module into the harness sys.modules exposing
    DefaultEnvironment, Import, ARGUMENTS, COMMAND_LINE_TARGETS so
    from SCons.Script import DefaultEnvironment resolves to the same mock env.
    RED with a fixture using that idiom; GREEN after the shim.
  • Extend env.get(key, default) to fall through _varsproject_optionsdefault.
  • Known-non-flag-scope policy: curated set (MKSPIFFSTOOL, BUILD_DIR, PROGNAME,
    UPLOAD_PROTOCOL, …) accepted on Append/Replace/__setitem__ as a recorded note
    (no flag effect); genuinely unknown scopes still fail fast (preserve fail-fast for real
    divergence). RED: m5panel/littlefsbuilder.py currently fails on MKSPIFFSTOOL; GREEN: it
    runs, records a note, yields no flag mutations.
  • GREEN: namf/platformio_script.py → runs, Replace(LINKFLAGS=...) collected (empty-input
    caveat documented), AddPostAction ignored.
  • LDAT/8kHz_polling.py negative fixture → assert graceful failure (file-patch on a
    nonexistent framework dir) with an actionable error / --platformio recommendation, and that
    it never hangs.

Phase 3 — COMPLEX tier (negative / bail fixtures)

  • amsreader/generate_includes.py (trimmed to the env surface) → assert zero flag mutations
    and that the no-op Execute(pip) path does not hang or error the run.
  • Marlin/common-dependencies.py → assert the shim reports unsupported (via SConscript /
    AddMethod / import platformio.*) with a clear message, instead of producing wrong flags.
  • Assert error messages name the offending construct and recommend --platformio.

Phase 4 — Docs

  • Update crates/fbuild-build/README.md "Native extra_scripts Boundary" with the newly
    supported APIs (GetBuildType, GetProjectOptions, DefaultEnvironment import, BUILD_FLAGS,
    tuple CPPDEFINES, recorded non-flag scopes) and the four structural limitations above.
  • Cross-link from docs/reference/platformio-ini.md.

Acceptance criteria

  • New Rust unit tests in script_runtime.rs cover ≥2 simple, ≥2 medium, ≥2 complex fixtures,
    all gated on Python availability (consistent with existing tests).
  • Simple + medium fixtures GREEN (collect expected flags or documented no-op).
  • Complex fixtures prove graceful refusal (clear error + --platformio), never wrong flags, never hang.
  • soldr cargo clippy --workspace --all-targets -- -D warnings and uv run test -p fbuild-build clean.
  • Docs updated; structural limitations written down.

Explicit non-goals (stay on --platformio)

  • Real SConscript recursion, custom builders/middleware, AddMethod, GetProjectConfig().set.
  • Hosting/importing real PlatformIO Core packages.
  • The build_flags = !python ... stdout--D pattern (separate follow-up).

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    Status
    Triage

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions