From 9e874917b49f82e84effaf26e91bc43401b43bc5 Mon Sep 17 00:00:00 2001 From: mikecarper Date: Wed, 2 Sep 2026 11:41:24 -0700 Subject: [PATCH] Fix Windows Full firmware builds --- build.sh | 9 + scripts/deduplicate_full_build_flags.py | 242 +++++++++++++++++++ test/test_full_build_flag_dedup.py | 293 ++++++++++++++++++++++++ test/test_indicator_render_profile.py | 31 ++- 4 files changed, 571 insertions(+), 4 deletions(-) create mode 100644 scripts/deduplicate_full_build_flags.py create mode 100644 test/test_full_build_flag_dedup.py diff --git a/build.sh b/build.sh index 19bcdaea..4019726a 100755 --- a/build.sh +++ b/build.sh @@ -3980,6 +3980,15 @@ build_firmware() { "scripts/esp32_full_partition.py"; then append_platformio_extra_script "pre:scripts/esp32_full_partition.py" fi + # PlatformIO appends PLATFORMIO_BUILD_FLAGS while resolving every extends + # layer. Deep ESP32 Full profiles can therefore exceed Windows' process + # command-line limit before GCC can launch cc1plus. Compact repeated macro + # flags and Windows include paths after all profiles are resolved. + if ! pio_env_option_contains "$pio_env_name" extra_scripts \ + "scripts/deduplicate_full_build_flags.py"; then + append_platformio_extra_script \ + "pre:scripts/deduplicate_full_build_flags.py" + fi else unset MESHCORE_ESP32_FULL_BUILD unset MESHCORE_COMPANION_RADIO_FULL diff --git a/scripts/deduplicate_full_build_flags.py b/scripts/deduplicate_full_build_flags.py new file mode 100644 index 00000000..f99967c5 --- /dev/null +++ b/scripts/deduplicate_full_build_flags.py @@ -0,0 +1,242 @@ +"""Collapse repeated external build-flag overlays in ESP32 Full builds. + +PlatformIO appends ``PLATFORMIO_BUILD_FLAGS`` while resolving every +``extends`` layer. A deep Full environment can therefore put the same large +release overlay on the compiler command line several times. On Windows that +can exceed the command-line limit used when GCC starts ``cc1plus``. + +Only exact logical ``-D``/``-U`` flags from the external overlay are +collapsed. The final overlay's multiplicity and order win, matching +preprocessor handling of ordered overrides, while search paths and arbitrary +compiler/linker inputs remain untouched. On Windows, existing include +directories are also rewritten to shorter 8.3 aliases without changing their +order. This keeps GCC's expanded ``cc1plus`` command below the CreateProcess +limit. +""" + +Import("env") # noqa: F821 + +import os +import shlex +from collections import Counter +from functools import lru_cache + + +# Keep GNU-style option/argument pairs indivisible so de-duplication can never +# strand (for example) a bare ``-D`` or detach it from its value. +OPTIONS_WITH_ARGUMENT = { + "-D", + "-U", + "-I", + "-L", + "-l", + "-F", + "-arch", + "-dylib_file", + "-framework", + "-idirafter", + "-imacros", + "-include", + "-iquote", + "-isystem", + "-isysroot", + "--param", +} + + +# An object marker keeps a raw ``!command`` distinct from an ordinary quoted +# token whose value merely starts with ``!``. +COMMAND_FLAG = object() +SHORTENING_REPORTED = False + + +def safely_deduplicated(flag): + """Return whether removing exact repeats cannot alter input ordering.""" + if flag[0] is COMMAND_FLAG: + return False + if len(flag) == 2: + return flag[0] in {"-D", "-U"} + return flag[0].startswith(("-D", "-U")) + + +def logical_flags(values): + """Return shell tokens, grouping options that consume one argument.""" + if not isinstance(values, (list, tuple)): + values = [values] + + result = [] + for raw in values: + text = str(raw) + if not text.strip(): + continue + if text.startswith("!"): + # ParseFlags executes a leading-! item and its output is not known + # yet. Keep it opaque rather than changing command semantics. + result.append((COMMAND_FLAG, text)) + continue + + tokens = shlex.split(text) + index = 0 + while index < len(tokens): + token = tokens[index] + if token in OPTIONS_WITH_ARGUMENT and index + 1 < len(tokens): + result.append((token, tokens[index + 1])) + index += 2 + else: + result.append((token,)) + index += 1 + return result + + +def serialize_flags(flags): + """Serialize logical flags for a second, equivalent ParseFlags pass.""" + return [ + flag[1] if flag[0] is COMMAND_FLAG + else shlex.join(flag) + for flag in flags + ] + + +def deduplicate_overlay(build_flags, overlay): + """Retain the final overlay's exact multiplicity of safe macro flags.""" + try: + flags = logical_flags(build_flags) + overlay_counts = Counter( + flag + for flag in logical_flags(overlay) + if safely_deduplicated(flag) + ) + except ValueError: + # Preserve PlatformIO's original input and error behavior when shell + # quoting is malformed; this compactor must not reinterpret it. + return build_flags, 0 + if not overlay_counts: + return serialize_flags(flags), 0 + + seen_from_right = Counter() + retained_reversed = [] + for flag in reversed(flags): + if flag in overlay_counts: + seen_from_right[flag] += 1 + if seen_from_right[flag] > overlay_counts[flag]: + continue + retained_reversed.append(flag) + unique = list(reversed(retained_reversed)) + return serialize_flags(unique), len(flags) - len(unique) + + +def resolved_path(value, project_dir, expander=None): + """Resolve a SCons path node/string without requiring SCons in tests.""" + if hasattr(value, "get_abspath"): + return value.get_abspath() + text = str(value) + if expander is not None: + text = expander(text) + if not os.path.isabs(text): + text = os.path.join(project_dir, text) + return os.path.abspath(text) + + +def shorten_cpppaths(values, project_dir, shortener, expander=None): + """Use a shorter alias for each path when one exists; retain list order.""" + result = [] + characters_saved = 0 + for value in values: + original = str(value) + candidate = shortener( + resolved_path(value, project_dir, expander=expander) + ) + if candidate: + candidate = candidate.replace("\\", "/") + if candidate and len(candidate) < len(original): + result.append(candidate) + characters_saved += len(original) - len(candidate) + else: + result.append(value) + return result, characters_saved + + +@lru_cache(maxsize=None) +def windows_short_path(path): + """Return an existing path's Win32 8.3 alias, or None when unavailable.""" + if not os.path.exists(path): + return None + + import ctypes + from ctypes import wintypes + + get_short_path = ctypes.WinDLL( + "kernel32", use_last_error=True + ).GetShortPathNameW + get_short_path.argtypes = ( + wintypes.LPCWSTR, + wintypes.LPWSTR, + wintypes.DWORD, + ) + get_short_path.restype = wintypes.DWORD + + required = get_short_path(path, None, 0) + if not required: + return None + buffer = ctypes.create_unicode_buffer(required) + written = get_short_path(path, buffer, required) + if not written or written >= required: + return None + return buffer.value + + +def shorten_full_build_cpppaths(build_env, node): + """PlatformIO build middleware applied after all include paths exist.""" + global SHORTENING_REPORTED + if build_env.get("_MESHCORE_FULL_CPPPATHS_SHORTENED", False): + return node + cpppaths = build_env.get("CPPPATH", []) + if not cpppaths: + return node + build_env.Replace(_MESHCORE_FULL_CPPPATHS_SHORTENED=True) + project_dir = build_env.subst("$PROJECT_DIR") + shortened, saved = shorten_cpppaths( + cpppaths, + project_dir, + windows_short_path, + expander=build_env.subst, + ) + if saved: + build_env.Replace(CPPPATH=shortened) + if not SHORTENING_REPORTED: + print( + "ESP32 FULL build: shortened Windows include paths by %d chars" + % saved + ) + SHORTENING_REPORTED = True + return node + + +if os.environ.get("MESHCORE_ESP32_FULL_BUILD") == "1": + original = env.get("BUILD_FLAGS", []) # noqa: F821 + cleaned, removed = deduplicate_overlay( + original, os.environ.get("PLATFORMIO_BUILD_FLAGS", "") + ) + if removed: + env.Replace(BUILD_FLAGS=cleaned) # noqa: F821 + print( + "ESP32 FULL build: removed %d repeated external build flags" + % removed + ) + if ( + os.name == "nt" + and os.environ.get("MESHCORE_COMPANION_RADIO_FULL") == "1" + and not env.get( # noqa: F821 + "_MESHCORE_FULL_CPPPATH_SHORTENER_INSTALLED", False + ) + ): + env.Replace( # noqa: F821 + _MESHCORE_FULL_CPPPATH_SHORTENER_INSTALLED=True + ) + # Library builders inherit middleware from the root environment. Wait + # until PlatformIO starts the final Companion project sources, after + # dependency discovery has finished appending include directories. + env.AddBuildMiddleware( # noqa: F821 + shorten_full_build_cpppaths, + "*examples*companion_radio*.cpp", + ) diff --git a/test/test_full_build_flag_dedup.py b/test/test_full_build_flag_dedup.py new file mode 100644 index 00000000..59bed209 --- /dev/null +++ b/test/test_full_build_flag_dedup.py @@ -0,0 +1,293 @@ +#!/usr/bin/env python3 + +import contextlib +import fnmatch +import io +import os +from pathlib import Path +import runpy +import shlex +import unittest +from unittest import mock + + +ROOT = Path(__file__).resolve().parents[1] +SCRIPT = ROOT / "scripts" / "deduplicate_full_build_flags.py" +BUILD_SCRIPT = ROOT / "build.sh" + + +class FakeEnvironment: + def __init__(self, flags): + self.values = {"BUILD_FLAGS": flags} + self.replacements = [] + self.middlewares = [] + + def get(self, name, default=None): + return self.values.get(name, default) + + def Replace(self, **values): + self.values.update(values) + self.replacements.append(values) + + def AddBuildMiddleware(self, callback, pattern=None): + self.middlewares.append((callback, pattern)) + + +def flatten(values): + return [token for value in values for token in shlex.split(str(value))] + + +def apply_policy(flags, overlay, *, full=True, companion=True): + environment = FakeEnvironment(flags) + output = io.StringIO() + variables = { + "MESHCORE_ESP32_FULL_BUILD": "1" if full else "0", + "MESHCORE_COMPANION_RADIO_FULL": "1" if companion else "0", + "PLATFORMIO_BUILD_FLAGS": overlay, + } + with mock.patch.dict(os.environ, variables, clear=False): + with contextlib.redirect_stdout(output): + namespace = runpy.run_path( + str(SCRIPT), + init_globals={ + "Import": lambda _name: None, + "env": environment, + }, + ) + return environment, output.getvalue(), namespace + + +class FullBuildFlagDedupTest(unittest.TestCase): + def test_repeated_extends_overlay_keeps_last_exact_occurrence(self): + overlay = ( + "-DFEATURE=1 -UFEATURE -DFEATURE=1 " + "-D LABEL='\"Full build\"' -fno-exceptions" + ) + flags = [ + "-DBOARD=1 -Wextra " + overlay, + "-DWITH_MQTT=1 -Wextra " + overlay, + "-DCHILD_PROFILE=1 " + overlay, + ] + + environment, output, namespace = apply_policy(flags, overlay) + result = flatten(environment.values["BUILD_FLAGS"]) + logical = namespace["logical_flags"]( + environment.values["BUILD_FLAGS"] + ) + + self.assertEqual(result.count("-DFEATURE=1"), 2) + self.assertEqual(result.count("-UFEATURE"), 1) + macro_sequence = [ + token + for token in result + if token in {"-DFEATURE=1", "-UFEATURE"} + ] + self.assertEqual( + macro_sequence, + ["-DFEATURE=1", "-UFEATURE", "-DFEATURE=1"], + ) + self.assertEqual(logical.count(("-D", 'LABEL="Full build"')), 1) + self.assertEqual(result.count("-Wextra"), 2) + self.assertIn("-DBOARD=1", result) + self.assertIn("-DWITH_MQTT=1", result) + self.assertIn("-DCHILD_PROFILE=1", result) + # -fno-exceptions is not de-duplicated: unlike -D/-U, arbitrary + # compiler/linker options can be order- or repetition-sensitive. + self.assertEqual(result.count("-fno-exceptions"), 3) + self.assertIn("removed 8 repeated external build flags", output) + + def test_two_token_options_remain_paired_and_quoted(self): + overlay = "-D FEATURE=1 -I 'path with spaces'" + flags = [overlay, "-DLOCAL=1", overlay] + + environment, _output, namespace = apply_policy(flags, overlay) + logical = namespace["logical_flags"]( + environment.values["BUILD_FLAGS"] + ) + + self.assertEqual(logical.count(("-D", "FEATURE=1")), 1) + self.assertEqual(logical.count(("-I", "path with spaces")), 2) + self.assertIn(("-DLOCAL=1",), logical) + + def test_shell_tokens_round_trip_without_changing_quoting_semantics(self): + raw = ( + "-D NAME='\"quoted value\"' -U OLD -I 'include path' " + "-L lib -l mesh -F frameworks -arch xtensa " + "-include 'config file.h' -imacros macros.h " + "-isysroot sdk -isystem system -iquote quoted " + "-idirafter after --param max-inline-insns-single=20 " + "-Wl,--wrap=target $PROJECT_DIR/source.cpp" + ) + _environment, _output, namespace = apply_policy(raw, "") + logical = namespace["logical_flags"](raw) + serialized = namespace["serialize_flags"](logical) + + self.assertEqual(flatten(serialized), shlex.split(raw)) + + def test_only_column_zero_bang_is_preserved_as_a_command(self): + command = "!python make_flags.py --profile full" + quoted_literal = "'!not-a-command'" + indented_literal = " !also-not-a-command" + flags = [command, quoted_literal, indented_literal, "-DOVERLAY=1"] + + environment, _output, _namespace = apply_policy( + flags, "-DOVERLAY=1" + ) + result = environment.values["BUILD_FLAGS"] + + self.assertEqual(result[0], command) + self.assertFalse(result[1].startswith("!")) + self.assertFalse(result[2].startswith("!")) + self.assertEqual(shlex.split(result[1]), ["!not-a-command"]) + self.assertEqual(shlex.split(result[2]), ["!also-not-a-command"]) + + def test_non_overlay_duplicates_and_non_full_build_are_untouched(self): + flags = ["-Wextra -DOVERLAY=1", "-Wextra -DOVERLAY=1"] + environment, _output, _namespace = apply_policy( + flags, "-DOVERLAY=1" + ) + result = flatten(environment.values["BUILD_FLAGS"]) + self.assertEqual(result.count("-Wextra"), 2) + self.assertEqual(result.count("-DOVERLAY=1"), 1) + + environment, output, _namespace = apply_policy( + flags, "-DOVERLAY=1", full=False + ) + self.assertEqual(environment.values["BUILD_FLAGS"], flags) + self.assertEqual(environment.replacements, []) + self.assertEqual(output, "") + + def test_malformed_shell_quoting_fails_safe(self): + flags = ["-DBOARD=1", "-DNAME='unterminated"] + environment, output, _namespace = apply_policy( + flags, "-DOVERLAY=1" + ) + + self.assertEqual(environment.values["BUILD_FLAGS"], flags) + self.assertEqual( + [item for item in environment.replacements if "BUILD_FLAGS" in item], + [], + ) + self.assertNotIn("removed", output) + + def test_interposed_project_macros_keep_final_overlay_state(self): + overlay = "-DMODE=full -UOLD" + flags = [ + overlay, + "-DMODE=project -DOLD=1", + overlay, + ] + + environment, _output, _namespace = apply_policy(flags, overlay) + result = flatten(environment.values["BUILD_FLAGS"]) + + self.assertEqual( + [token for token in result if token.startswith("-DMODE=")], + ["-DMODE=project", "-DMODE=full"], + ) + self.assertLess(result.index("-DOLD=1"), result.index("-UOLD")) + + def test_commands_and_response_files_keep_duplicates(self): + command = "!python make_flags.py --profile full" + flags = [command, "@common.rsp -DOVERLAY=1"] * 2 + + environment, _output, _namespace = apply_policy( + flags, "@common.rsp -DOVERLAY=1" + ) + result = environment.values["BUILD_FLAGS"] + + self.assertEqual(result.count(command), 2) + self.assertEqual(flatten(result).count("@common.rsp"), 2) + self.assertEqual(flatten(result).count("-DOVERLAY=1"), 1) + + def test_include_order_and_repeated_inputs_are_not_deduplicated(self): + overlay = "-I external -include forced.h -l mesh -DFEATURE=1" + flags = [ + overlay, + "-I project -include forced.h -l mesh", + overlay, + ] + + environment, _output, _namespace = apply_policy(flags, overlay) + result = flatten(environment.values["BUILD_FLAGS"]) + + self.assertEqual( + [token for token in result if token in {"external", "project"}], + ["external", "project", "external"], + ) + self.assertEqual(result.count("forced.h"), 3) + self.assertEqual(result.count("mesh"), 3) + self.assertEqual(result.count("-DFEATURE=1"), 1) + + def test_cpppath_shortening_preserves_order_and_missing_entries(self): + _environment, _output, namespace = apply_policy("", "") + project = str(ROOT) + framework = str( + ROOT / "very-long-framework-directory" / "include" + ) + missing = str(ROOT / "does-not-exist" / "include") + aliases = { + os.path.abspath(framework): "C:/PIO/FRAMEW~1/include", + os.path.abspath(missing): None, + os.path.abspath(ROOT / "include"): "C:/MC/include", + } + + shortened, saved = namespace["shorten_cpppaths"]( + ["include", framework, missing], + project, + aliases.get, + ) + + # The relative project include is already shorter than its alias. + self.assertEqual(shortened[0], "include") + self.assertEqual(shortened[1], "C:/PIO/FRAMEW~1/include") + self.assertEqual(shortened[2], missing) + self.assertEqual( + saved, len(framework) - len("C:/PIO/FRAMEW~1/include") + ) + + def test_full_build_driver_installs_the_pre_script(self): + build_script = BUILD_SCRIPT.read_text(encoding="utf-8") + full_block_start = build_script.index( + 'if [ "$ESP32_FULL_BUILD" = "1" ] || ' + 'is_esp32_companion_radio_full_target "$env_name"; then' + ) + full_block_end = build_script.index("\n else", full_block_start) + full_block = build_script[full_block_start:full_block_end] + self.assertIn( + '"pre:scripts/deduplicate_full_build_flags.py"', full_block + ) + + def test_windows_shortener_waits_for_companion_project_sources(self): + environment, _output, _namespace = apply_policy("", "") + if os.name == "nt": + self.assertEqual(len(environment.middlewares), 1) + self.assertEqual( + environment.middlewares[0][1], + "*examples*companion_radio*.cpp", + ) + + environment, _output, _namespace = apply_policy( + "", "", companion=False + ) + self.assertEqual(environment.middlewares, []) + + def test_middleware_pattern_excludes_dependencies_with_full_env_name(self): + pattern = "*examples*companion_radio*.cpp" + self.assertTrue( + fnmatch.fnmatch( + ".pio/build/heltec_full/examples/companion_radio/MyMesh.cpp", + pattern, + ) + ) + self.assertFalse( + fnmatch.fnmatch( + ".pio/build/heltec_companion_radio_full/" + "lib123/ArduinoJson/src/Json.cpp", + pattern, + ) + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/test_indicator_render_profile.py b/test/test_indicator_render_profile.py index 8a22527c..79f63bb2 100644 --- a/test/test_indicator_render_profile.py +++ b/test/test_indicator_render_profile.py @@ -1,6 +1,8 @@ #!/usr/bin/env python3 from pathlib import Path +import os +import shutil import subprocess import tempfile import unittest @@ -24,6 +26,25 @@ WEBCONFIG_SOURCE = ROOT / "src" / "helpers" / "esp32" / "WebConfigServer.cpp" WIFI_QR_PAYLOAD = ROOT / "src" / "helpers" / "ui" / "WiFiSetupQrPayload.h" +def find_bash(): + # The legacy WindowsApps bash.exe bridge drops shell positional arguments + # and expands variables incorrectly. Prefer Git for Windows' real Bash; + # other platforms use the normal PATH lookup. + if os.name == "nt": + git_bash = ( + Path(os.environ.get("ProgramFiles", r"C:\Program Files")) + / "Git" + / "bin" + / "bash.exe" + ) + if git_bash.is_file(): + return str(git_bash) + return shutil.which("bash") or "bash" + + +BASH = find_bash() + + class IndicatorRenderProfileTest(unittest.TestCase): def test_transport_matrix(self): source = r''' @@ -308,7 +329,7 @@ int main() { def test_explicit_usa_cascadia_and_cascade_build_flags(self): command = r''' set -e -source "$1" +source ./build.sh PLATFORMIO_BUILD_FLAGS="" RADIO_FREQ_OVERRIDE=910.525 RADIO_BW_OVERRIDE=62.5 @@ -320,7 +341,8 @@ apply_firmware_profile_overrides printf '%s\n' "$PLATFORMIO_BUILD_FLAGS" ''' result = subprocess.run( - ["bash", "-c", command, "test", str(BUILD)], + [BASH, "-c", command], + cwd=ROOT, text=True, capture_output=True, check=False, @@ -353,7 +375,7 @@ printf '%s\n' "$PLATFORMIO_BUILD_FLAGS" def test_non_usa_radio_does_not_enable_wizard_skip(self): command = r''' set -e -source "$1" +source ./build.sh PLATFORMIO_BUILD_FLAGS="" RADIO_SETTING_TITLE="Australia" RADIO_FREQ_OVERRIDE=915.800 @@ -364,7 +386,8 @@ apply_radio_overrides printf '%s\n' "$PLATFORMIO_BUILD_FLAGS" ''' result = subprocess.run( - ["bash", "-c", command, "test", str(BUILD)], + [BASH, "-c", command], + cwd=ROOT, text=True, capture_output=True, check=False,