From 5c47e165658ddc482e6a5241e2266a0b6913ae0c Mon Sep 17 00:00:00 2001
From: Brad Barnett <127794626+bdbarnett@users.noreply.github.com>
Date: Fri, 25 Sep 2026 09:50:50 -0500
Subject: [PATCH] firmware: keep mpy-cross out of a --build-dir build (#46)

BUILD= on make's command line travels to every sub-make through MAKEFLAGS,
including the mpy-cross build the esp32 port starts from py/mkrules.cmake.
That build then wrote its objects and qstr fragments into the firmware's
build directory, and the link failed. With --build-dir, mpftp now names the
mpy-cross it already builds (in mpy-cross/build, BUILD pinned) in
MICROPY_MPYCROSS, so the port never starts that sub-make.

Proved with ESP32_GENERIC_S3 into a fresh directory: main failed with
MP_QSTR_* undeclared and mpy-cross objects in the build dir; the branch
builds firmware.bin with none there.
---
 CHANGELOG.md                       |  8 ++++++++
 cli/src/mpftp/firmware.py          | 15 ++++++++++++--
 cli/tests/test_firmware_modules.py | 32 +++++++++++++++++++++++++++++-
 docs/firmware-modules.md           |  6 +++++-
 4 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 247280f..dcd8edd 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,11 @@
+## Unreleased
+
+- `firmware build --build-dir` works on esp32. `BUILD=` reached the mpy-cross
+  build the port starts for itself, which then wrote its objects into the same
+  folder and broke the firmware's link. mpftp now hands the port a ready
+  mpy-cross through `MICROPY_MPYCROSS`, so that sub-make never runs
+  (mpftp#46).
+
 ## v0.0.9 (2026-09-25)
 
 - `mpftp --version` prints the version (mpftp#52).
diff --git a/cli/src/mpftp/firmware.py b/cli/src/mpftp/firmware.py
index 2a033df..b8024b7 100644
--- a/cli/src/mpftp/firmware.py
+++ b/cli/src/mpftp/firmware.py
@@ -1317,6 +1317,16 @@ def do_build(ns: argparse.Namespace) -> None:
         build_override = str(Path(build_override).expanduser().resolve())
         make_args.append(f"BUILD={build_override}")
         emit_log(f"[mpftp] BUILD={build_override}")
+        # BUILD= on make's command line reaches every sub-make through
+        # MAKEFLAGS, including the mpy-cross build the port starts for itself
+        # (py/mkrules.cmake, py/mkrules.mk). That one would then build into
+        # this directory too, and its qstr and module fragments end up in the
+        # firmware (undefined mp_module_string, missing MP_QSTR_*; mpftp#46,
+        # micropython#19667). Naming the mpy-cross built below in
+        # MICROPY_MPYCROSS means the port never starts that sub-make.
+        if not env.get("MICROPY_MPYCROSS"):
+            env["MICROPY_MPYCROSS"] = str(mp / "mpy-cross" / "build" / "mpy-cross")
+        emit_log(f"[mpftp] MICROPY_MPYCROSS={env['MICROPY_MPYCROSS']}")
 
     # Windows is a cross-compile from Linux/WSL: force the MinGW-w64 toolchain so
     # make doesn't fall back to host gcc (which fails on <windows.h>). The MinGW
@@ -1371,9 +1381,10 @@ def do_build(ns: argparse.Namespace) -> None:
 
     q_args = " ".join(_shq(a) for a in make_args)
 
-    # Host mpy-cross with cleared user modules (parity with build_mp.sh).
+    # Host mpy-cross with cleared user modules (parity with build_mp.sh), in
+    # its own build directory whatever BUILD the environment carries.
     script_lines.append(
-        f'make -C "{mp}/mpy-cross" USER_C_MODULES= FROZEN_MANIFEST='
+        f'make -C "{mp}/mpy-cross" BUILD=build USER_C_MODULES= FROZEN_MANIFEST='
     )
 
     submodules = f'make {" ".join(j_arg)} submodules {q_args}'
diff --git a/cli/tests/test_firmware_modules.py b/cli/tests/test_firmware_modules.py
index 14c3d60..74f232a 100644
--- a/cli/tests/test_firmware_modules.py
+++ b/cli/tests/test_firmware_modules.py
@@ -6,6 +6,7 @@
 import argparse
 import io
 import json
+import os
 import tempfile
 import unittest
 from contextlib import redirect_stdout
@@ -243,6 +244,10 @@ class TestBuildArgs(ModulesTestCase):
     """What do_build hands to make, with the shell and toolchains stubbed out."""
 
     def build(self, **kw) -> tuple[list[str], list[dict]]:
+        lines, records, _envs = self.build_env(**kw)
+        return lines, records
+
+    def build_env(self, **kw) -> tuple[list[str], list[dict], list[dict]]:
         ns = argparse.Namespace(
             mp=str(self.mp), port="esp32", board="", variant="", board_dir="",
             variant_dir="", build_dir="", module_roots=None, preset="", modules="",
@@ -251,9 +256,11 @@ def build(self, **kw) -> tuple[list[str], list[dict]]:
         for k, v in kw.items():
             setattr(ns, k, v)
         scripts: list[list[str]] = []
+        envs: list[dict] = []
 
         def fake_shell(lines, cwd, env):
             scripts.append(list(lines))
+            envs.append(dict(env))
             return 0, ""
 
         out = io.StringIO()
@@ -267,7 +274,7 @@ def fake_shell(lines, cwd, env):
                 redirect_stdout(out):
             firmware.do_build(ns)
         records = [json.loads(ln) for ln in out.getvalue().splitlines() if ln.strip()]
-        return [ln for s in scripts for ln in s], records
+        return [ln for s in scripts for ln in s], records, envs
 
     def test_p4_kitchen_sink_plus_earful(self) -> None:
         lines, records = self.build(
@@ -296,6 +303,29 @@ def test_build_dir_override(self) -> None:
         make_all = next(ln for ln in lines if " all " in ln)
         self.assertIn(f"BUILD={bdir}", make_all)
 
+    def test_build_dir_keeps_mpy_cross_out_of_it(self) -> None:
+        # mpftp#46: BUILD= reaches the port's own mpy-cross sub-make through
+        # MAKEFLAGS. With MICROPY_MPYCROSS set, the port never starts one.
+        bdir = self.base / "scratch-build"
+        with mock.patch.dict(os.environ, {}, clear=False):
+            os.environ.pop("MICROPY_MPYCROSS", None)
+            os.environ["BUILD"] = "leaked-from-the-shell"
+            lines, _, envs = self.build_env(board="ESP32_GENERIC", build_dir=str(bdir))
+        mpy_cross = str(self.mp / "mpy-cross" / "build" / "mpy-cross")
+        self.assertTrue(envs)
+        for env in envs:
+            self.assertEqual(env.get("MICROPY_MPYCROSS"), mpy_cross)
+        prebuild = next(ln for ln in lines if "/mpy-cross\"" in ln)
+        self.assertIn("BUILD=build ", prebuild)
+
+    def test_default_build_dir_leaves_mpy_cross_to_the_port(self) -> None:
+        with mock.patch.dict(os.environ, {}, clear=False):
+            os.environ.pop("MICROPY_MPYCROSS", None)
+            _, _, envs = self.build_env(board="ESP32_GENERIC")
+        self.assertTrue(envs)
+        for env in envs:
+            self.assertNotIn("MICROPY_MPYCROSS", env)
+
     def test_unknown_names_fail_before_make(self) -> None:
         for kw, word in (({"modules": "nope"}, "Unknown module"),
                          ({"preset": "nope"}, "Unknown preset"),
diff --git a/docs/firmware-modules.md b/docs/firmware-modules.md
index 04fbb67..ec5bb84 100644
--- a/docs/firmware-modules.md
+++ b/docs/firmware-modules.md
@@ -65,7 +65,11 @@ which is itself a preset), and then includes each ticked module's
 `manifest.py`. Read it to see exactly what went into a build.
 
 `--build-dir` builds somewhere other than the port's `build-<target>`
-folder, which helps when several people share one checkout.
+folder, which helps when several people share one checkout. It works on every
+port, esp32 included: mpftp builds mpy-cross in its own folder first and names
+it in `MICROPY_MPYCROSS`, so the port's mpy-cross sub-make, which would
+otherwise inherit `BUILD=` and write its objects into your build folder, never
+runs (mpftp#46).
 
 The older sibling-folder aggregators (`micropython.cmake` and
 `manifest-micropython.py` at the workspace root) are no longer read.