code_size: run report and diff on Windows (#4086) `tools/code_size.py report` and `diff` run on native Windows with `python -X utf8`: '/' size keys and drive-less, case-insensitive filters, linker scripts read through ninja's cmd.exe wrapper, CMake-shortened objects keyed through build.ninja, and a timeout stops the whole command tree with taskkill. ESP-IDF, snapshot/compare and paths with spaces are not supported there; CI stays Linux-only.
diff --git a/.claude/skills/code-size/SKILL.md b/.claude/skills/code-size/SKILL.md index 6be613a..b7e19a7 100644 --- a/.claude/skills/code-size/SKILL.md +++ b/.claude/skills/code-size/SKILL.md
@@ -14,7 +14,7 @@ | all examples, CI-pinned, combined | `python3 tools/code_size.py diff --ci` | | one tree, no diff | `python3 tools/code_size.py report -b BOARD -e device/cdc_msc` | -By default the base comes from the master snapshots CI stored for `--base-branch`'s commit (`--baseline-repo`, default `hathach/tinyusb`, through your `gh` login; cached per master run in `${XDG_CACHE_HOME:-~/.cache}/tinyusb/code-size-baseline/`, shared by every checkout), else its nearest first-parent ancestor's; the `Base:` line names the commit used and how far back it is, and master changes after it count as yours. `--base-branch` means its tip; pass a merge-base SHA to diff against that. When the CI base cannot be had (no `gh` or login, no stored run within 30 commits, a base off master, a failed download) or `-f`, `--bloaty` or another `--engine` needs an elf, the base is built locally in a temporary worktree and the report's `Base:` line says why; `--base-source ci` fails instead, `--base-source local` always builds and makes no GitHub call. Against a CI base, a board whose compiler or membrowse version differs from yours, or is unknown, is still compared with a warning naming both. A local diff never builds CI's `-DMA` variants; DMA-path changes are measured only by the PR's CI comment. A local base worktree symlinks this checkout's fetched dependencies, so a `tools/get_deps.py` pin bump's own size change is not in its diff, while a CI base was built with CI's. A local `report` or `diff` cannot size a `-D` variant build. +By default the base comes from the master snapshots CI stored for `--base-branch`'s commit (`--baseline-repo`, default `hathach/tinyusb`, through your `gh` login; cached per master run in `${XDG_CACHE_HOME:-~/.cache}/tinyusb/code-size-baseline/` (`%LOCALAPPDATA%` on Windows), shared by every checkout), else its nearest first-parent ancestor's; the `Base:` line names the commit used and how far back it is, and master changes after it count as yours. `--base-branch` means its tip; pass a merge-base SHA to diff against that. When the CI base cannot be had (no `gh` or login, no stored run within 30 commits, a base off master, a failed download) or `-f`, `--bloaty` or another `--engine` needs an elf, the base is built locally in a temporary worktree and the report's `Base:` line says why; `--base-source ci` fails instead, `--base-source local` always builds and makes no GitHub call. Against a CI base, a board whose compiler or membrowse version differs from yours, or is unknown, is still compared with a warning naming both. A local diff never builds CI's `-DMA` variants; DMA-path changes are measured only by the PR's CI comment. A local base worktree symlinks this checkout's fetched dependencies, so a `tools/get_deps.py` pin bump's own size change is not in its diff, while a CI base was built with CI's. A local `report` or `diff` cannot size a `-D` variant build. ## Setup and choices @@ -25,6 +25,7 @@ - `--symbols` when Flash/RAM cancel: a pair counts as changed when its sections or symbols moved. - `--ci` for "all boards" / "CI"; it needs the arm, riscv, msp430 and ft9xx toolchains, and ESP-IDF or docker. - An espressif board builds each example `tools/build.py` picks as its own ESP-IDF project: with the exported ESP-IDF (`. "$IDF_PATH/export.sh"`), else in CI's image through docker, as your user (once: `docker pull espressif/idf:v5.5.3 && docker tag espressif/idf:v5.5.3 espressif/idf:tinyusb`, about 8.4 GB). +- Windows: run `python -X utf8 tools/code_size.py …` (or set `PYTHONUTF8=1`), else redirected output fails on `Δ`. `report` and `diff` work for paths without spaces, and a non-ASCII checkout, CMake or toolchain path needs ninja >= 1.11 whose `ninja -t wincodepage` reports UTF-8; espressif boards are refused. A local base, including the fallback, symlinks its dependencies: enable Developer Mode unless running elevated. ## Results
diff --git a/test/hil/test/test_code_size.py b/test/hil/test/test_code_size.py index 83b1ea0..25a282d 100644 --- a/test/hil/test/test_code_size.py +++ b/test/hil/test/test_code_size.py
@@ -8,8 +8,10 @@ import contextlib import errno import functools +import hashlib import io import json +import ntpath import os import re import shutil @@ -27,6 +29,9 @@ sys.path.insert(0, os.path.join(REPO, 'tools')) import code_size as sd # noqa: E402 +posix_only = unittest.skipIf(os.name == 'nt', 'needs a POSIX shell and signals') +no_esp_on_windows = unittest.skipIf(os.name == 'nt', 'code_size.py refuses ESP-IDF on Windows') + def fake_report(symbols): return {'symbols': symbols} @@ -66,6 +71,7 @@ self.assertEqual(cmd[cmd.index('--def') + 1], 'X=1') self.assertEqual(run.call_args.kwargs['cwd'], build) + @no_esp_on_windows def test_an_esp_idf_build_uses_its_generated_scripts(self): with tempfile.TemporaryDirectory() as build: open(os.path.join(build, 'build.ninja'), 'w').close() @@ -636,7 +642,7 @@ class DwarfSources(unittest.TestCase): - @unittest.skipUnless(shutil.which('gcc'), 'needs gcc to build an elf with DWARF') + @unittest.skipUnless(shutil.which('gcc') and os.name != 'nt', 'needs a gcc that builds an elf with DWARF') def test_compile_units_are_found_by_basename(self): with tempfile.TemporaryDirectory() as tmp: for rel, code in (('src/device/usbd.c', 'int usbd(void) { return 1; }\n'), @@ -1037,6 +1043,7 @@ error = sd.build_board(self._esp_src(tmp), os.path.join(tmp, 'b'), 'esp', example, 'build') return error, runs + @no_esp_on_windows def test_an_espressif_board_builds_each_examples_app_as_an_idf_project(self): with tempfile.TemporaryDirectory() as tmp: error, runs = self._build_esp(tmp) @@ -1045,6 +1052,7 @@ '-B', os.path.join(tmp, 'b', 'device', 'a_freertos'), '-GNinja', '-DBOARD=esp', 'app'], 600)]) + @no_esp_on_windows def test_without_an_exported_idf_it_builds_in_cis_image_as_this_user(self): with tempfile.TemporaryDirectory() as tmp: error, runs = self._build_esp(tmp, which=('docker',)) @@ -1068,6 +1076,7 @@ self.assertEqual(runs, []) self.assertIn('esp needs ESP-IDF: source $IDF_PATH/export.sh', error) + @mock.patch.object(sd, 'WINDOWS', new=False) def test_without_idf_report_and_diff_refuse_an_espressif_board_before_building(self): pinned = ['stm32f407disco', 'espressif_s3_devkitm'] for argv in (['report', '-b', pinned[0], '-b', pinned[1]], ['diff', '-b', pinned[0], '-b', pinned[1]], @@ -1090,6 +1099,7 @@ error, runs = self._build_esp(tmp, example=example) self.assertEqual((error, runs), (f'esp builds no {example}', [])) + @no_esp_on_windows def test_a_timed_out_or_interrupted_docker_build_removes_its_container(self): rm = ['docker', 'rm', '-f', f'tinyusb-code-size-{os.getpid()}'] for which, rc, removed in ((('docker',), 124, True), (('docker',), 2, False), (('idf.py',), 124, False)): @@ -1114,6 +1124,7 @@ self.assertEqual((ret.returncode, ret.stdout), (rc, 'w: \ufffd\n')) self.assertTrue(ret.stderr.startswith('\ufffd')) + @posix_only def test_sigterm_to_the_script_alone_stops_its_command_and_runs_cleanup(self): """Popen's exit waits for the command, so the handler passes the signal on, and kills a command ignoring it after the grace.""" @@ -1157,6 +1168,7 @@ ret = sd.run(['sh', '-c', "trap '' TERM; while :; do sleep 0.1; done"], timeout=1) self.assertEqual(ret.returncode, 124) + @posix_only def test_a_child_holding_the_pipes_after_the_kill_does_not_hang_it(self): start = time.monotonic() with mock.patch.object(sd, 'TERMINATE_GRACE', 0.5): @@ -1167,6 +1179,7 @@ self.assertEqual(ret.returncode, 124) self.assertIn('Command timed out after 1s', ret.stderr) + @posix_only def test_a_child_holding_the_pipes_after_sigterm_does_not_hang_it(self): start = time.monotonic() with mock.patch.object(sd, 'TERMINATE_GRACE', 0.5), mock.patch.object(sd, 'KILL_DRAIN', 0.2): @@ -1236,6 +1249,7 @@ self.assertRegex(sd.short_hash(sd.TINYUSB_ROOT), r'^[0-9a-f]{7,}(-dirty)?$') +@unittest.skipIf(os.name == 'nt', 'the Windows filter drops the drive') class SymlinkedCheckout(unittest.TestCase): def test_the_root_matches_its_own_filter_through_a_symlink(self): with tempfile.TemporaryDirectory() as tmp: @@ -1248,6 +1262,165 @@ self.assertEqual(out, 'True\n') +def cmake_shortened(obj_dir, obj_name): + """CMake v4.1.2 cmLocalGeneratorCheckObjectName and cmLocalGeneratorShortenObjectName + (Source/cmLocalGenerator.cxx) at Windows' CMAKE_OBJECT_PATH_MAX of 250.""" + max_len = 250 - len(obj_dir) + if len(obj_name) <= max_len: + return obj_name + pos = obj_name.find('/', len(obj_name) - max_len + 32) + return hashlib.md5(obj_name[:pos].encode()).hexdigest() + obj_name[pos:] + + +@mock.patch.object(sd, 'WINDOWS', new=True) +class CMakeShortenedObject(unittest.TestCase): + CHECKOUT = 'C:/Users/username/code/tinyusb/.worktrees/claude/code-size-windows/cmake-code-size/_worktree' + BUILD = 'C:/Users/username/code/tinyusb/.worktrees/claude/code-size-windows/cmake-code-size/base/' + TARGET = 'device/audio_4_channel_mic_freertos/CMakeFiles/audio_4_channel_mic_freertos.dir/' + FILTER = CHECKOUT[2:] + '/src/' + USBD = CHECKOUT + '/src/device/usbd.c' + CDC = CHECKOUT + '/src/class/cdc/cdc_device.c' + STARTUP = CHECKOUT + '/hw/mcu/st/cmsis_device_f4/Source/Templates/gcc/startup_stm32f407xx.s' + + def setUp(self): + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + self.build_dir = tmp.name + self.elf = os.path.join(tmp.name, 'device', 'audio_4_channel_mic_freertos', 'audio_4_channel_mic_freertos.elf') + + def obj(self, source): + """The map path of source's object, and its build.ninja compile edge.""" + obj = self.TARGET + cmake_shortened(self.BUILD + self.TARGET, source.replace(':', '_') + '.obj') + self.assertRegex(obj, r'\.dir/[0-9a-f]{32}/') + self.assertNotIn(self.FILTER, obj) + edge = f'build {obj}: C_COMPILER__audio_4_channel_mic_freertos_unscanned_MinSizeRel {source.replace(":", "$:")}' \ + ' || cmake_object_order_depends_target_audio_4_channel_mic_freertos\n' + return obj.replace('/', '\\'), edge + + def sizes(self, edges, filters=(FILTER,)): + with open(os.path.join(self.build_dir, 'build.ninja'), 'w') as f: + f.writelines(edges) + return sd._Sizes(list(filters), self.elf) + + def test_a_shortened_vendor_object_counts_in_all_only(self): + obj, edge = self.obj(self.STARTUP) + sizes = self.sizes([edge]) + sizes.add(obj, '.isr_vector', {'flash'}, 4, 'g_pfnVectors') + self.assertEqual(sizes.result()['files'], {}) + self.assertEqual(sizes.result()['all'], {'flash': 4, 'ram': 0}) + + def test_a_shortened_tinyusb_object_keeps_its_source_key(self): + obj, edge = self.obj(self.USBD) + sizes = self.sizes([edge]) + sizes.add(obj, '.text', {'flash'}, 4, 'tud_task_ext') + self.assertEqual(sizes.result()['files'], {'device/usbd.c': {'flash': 4, 'ram': 0}}) + + def test_a_later_filter_matching_the_kept_tail_keys_as_for_the_full_source(self): + obj, edge = self.obj(self.CDC) + self.assertIn('/class/cdc/', obj.replace('\\', '/')) + sizes = self.sizes([edge], (self.FILTER, 'class/')) + sizes.add(obj, '.text', {'flash'}, 4, 'tud_cdc_n_write') + self.assertEqual(sizes.result()['files'], {'class/cdc/cdc_device.c': {'flash': 4, 'ram': 0}}) + + def test_a_shortened_object_without_build_ninja_fails(self): + obj, _ = self.obj(self.USBD) + with mock.patch.object(sd, '_find_ninja_build_dir', return_value=None): + with self.assertRaisesRegex(RuntimeError, 'CMAKE_OBJECT_PATH_MAX .* no build\\.ninja was found above'): + sd._Sizes([self.FILTER], self.elf).add(obj, '.text', {'flash'}, 4, 'tud_task_ext') + + def test_a_shortened_object_build_ninja_does_not_build_fails(self): + obj, _ = self.obj(self.USBD) + _, edge = self.obj(self.STARTUP) + with self.assertRaisesRegex(RuntimeError, 'CMAKE_OBJECT_PATH_MAX .* no compile edge'): + self.sizes([edge]).add(obj, '.text', {'flash'}, 4, 'tud_task_ext') + + def test_a_build_ninja_not_in_utf_8_fails(self): + obj, edge = self.obj(self.USBD) + with open(os.path.join(self.build_dir, 'build.ninja'), 'wb') as f: + f.write(b'# Calf\xe9\n' + edge.encode()) + sizes = sd._Sizes([self.FILTER], self.elf) + with self.assertRaisesRegex(RuntimeError, r'build\.ninja is not UTF-8: .* use ninja >= 1\.11 whose ' + r'`ninja -t wincodepage` reports UTF-8') as cm: + sizes.add(obj, '.text', {'flash'}, 4, 'tud_task_ext') + self.assertNotIn('position', str(cm.exception)) # relative to the decoder's chunk, not the file + + def test_linux_does_not_resolve_a_shortened_object(self): + obj, _ = self.obj(self.USBD) + with mock.patch.object(sd, 'WINDOWS', new=False): + sizes = self.sizes([]) + sizes.add(obj.replace('\\', '/'), '.text', {'flash'}, 4, 'tud_task_ext') + self.assertEqual(sizes.result()['files'], {}) + + +@mock.patch.object(sd, 'WINDOWS', new=True) +class WindowsHost(unittest.TestCase): + def test_a_backslash_object_path_matches_the_forward_slash_filter(self): + for f in ('/a/tinyusb/src/', '\\a\\tinyusb\\src\\'): + self.assertEqual(sd._relative_key(r'cdc_msc.dir\D_\a\tinyusb\src\device\usbd.c.obj', [sd.filter_arg(f)]), + 'device/usbd.c') + + def test_a_drive_is_stripped_from_a_filter(self): + with mock.patch.object(sd.os, 'path', ntpath): + self.assertEqual(sd.filter_arg('D:\\a\\tinyusb\\src\\'), '/a/tinyusb/src/') + + def test_a_backslash_example_names_the_forward_slash_elf_id(self): + self.assertEqual(sd.example_arg('device\\cdc_msc\\'), 'device/cdc_msc') + + def test_a_filter_matches_regardless_of_case_and_the_key_keeps_the_recorded_case(self): + self.assertEqual(sd._relative_key(r'cdc_msc.dir\D_\A\TinyUSB\src\Device\usbd.c.obj', + ['/nomatch/', '/a/tinyusb/src/']), 'Device/usbd.c') + + def test_espressif_boards_are_refused(self): + self.assertRegex(sd.esp_without_idf(['espressif_s3_devkitc', 'stm32f407disco']), + r'^espressif_s3_devkitc need ESP-IDF, .* not support on Windows') + self.assertIsNone(sd.esp_without_idf(['stm32f407disco'])) + + def test_a_timeout_kills_the_command_tree(self): + with mock.patch('subprocess.run') as taskkill: + ret = sd.run([sys.executable, '-c', 'import time; time.sleep(30)'], timeout=0.5) + self.assertEqual(ret.returncode, 124) + argv = taskkill.call_args.args[0] + self.assertEqual(argv[:2] + argv[3:], ['taskkill', '/PID', '/T', '/F']) + + @unittest.skipUnless(os.name == 'nt', 'a real Windows process tree') + def test_a_timeout_stops_a_grandchild_holding_the_pipes(self): + # as ninja's compilers do: terminate() alone stops only the direct child + parent = ('import subprocess, sys, time; ' + 'p = subprocess.Popen([sys.executable, "-c", "import time; time.sleep(60)"], ' + 'stdout=sys.stdout, stderr=sys.stderr); ' + 'print(p.pid, flush=True); time.sleep(60)') + start = time.monotonic() + ret = sd.run([sys.executable, '-c', parent], timeout=5) + elapsed = time.monotonic() - start + pid = ret.stdout.strip() + try: + self.assertEqual(ret.returncode, 124) + self.assertTrue(pid.isdigit(), ret.stdout) + tasks = subprocess.run(['tasklist', '/FI', f'PID eq {pid}', '/FO', 'CSV', '/NH'], + capture_output=True, text=True, check=True, timeout=10).stdout + self.assertNotIn(f'"{pid}"', tasks) + self.assertLess(elapsed, 30) # well before the grandchild's own exit + finally: + if pid.isdigit(): + subprocess.run(['taskkill', '/PID', pid, '/F'], capture_output=True) + + def test_a_symlink_without_the_privilege_names_developer_mode(self): + denied = OSError(22, 'A required privilege is not held by the client') + denied.winerror = 1314 + with tempfile.TemporaryDirectory() as main, tempfile.TemporaryDirectory() as wt: + os.makedirs(os.path.join(main, 'lib/lwip')) + os.makedirs(os.path.join(wt, 'tools')) + with open(os.path.join(wt, 'tools', 'get_deps.py'), 'w') as f: + f.write("deps_all = {'lib/lwip': []}\n") + with mock.patch('os.symlink', side_effect=denied), \ + self.assertRaisesRegex(SystemExit, 'Developer Mode, or use --base-source ci'): + sd.symlink_deps(main, wt) + + def test_a_path_on_another_drive_is_shown_absolute(self): + with mock.patch('os.path.relpath', side_effect=ValueError('path is on mount C:, start on mount D:')): + self.assertEqual(sd._shown('C:/x/report.md'), 'C:/x/report.md') + + # main() tests stub the builds and sizing, so need no engine tool @mock.patch.object(sd, 'engine_missing', new=lambda _engine: False) class MainFailure(unittest.TestCase): @@ -1949,6 +2122,7 @@ '"uncovered": []}') self.assertEqual(built, ['extra', 'b1', 'b2']) + @mock.patch.object(sd, 'WINDOWS', new=False) def test_ci_builds_pinned_espressif_boards_too(self): with tempfile.TemporaryDirectory() as tmp: built = self._boards_built(tmp, '{"boards": [{"board": "b1"}, ' @@ -2015,6 +2189,13 @@ 'name': 'arm-none-eabi-gcc', 'build_type': 'MinSizeRel'}) self.assertEqual(sd._cmake_compiler('/nonexistent'), {'id': '', 'version': '', 'name': '', 'build_type': ''}) + def test_a_windows_compiler_is_named_without_exe(self): + with tempfile.TemporaryDirectory() as tmp: + os.makedirs(os.path.join(tmp, 'CMakeFiles', '4.1.2')) + with open(os.path.join(tmp, 'CMakeFiles', '4.1.2', 'CMakeCCompiler.cmake'), 'w') as f: + f.write('set(CMAKE_C_COMPILER "C:/arm/bin/arm-none-eabi-gcc.EXE")\n') + self.assertEqual(sd._cmake_compiler(tmp)['name'], 'arm-none-eabi-gcc') + def test_an_esp_idf_board_reads_its_first_example_project(self): with tempfile.TemporaryDirectory() as tmp: for ex, ver in (('device/b', '14.2.0'), ('device/a', '14.2.0'), ('device/a/bootloader', '1.0')): @@ -2139,7 +2320,7 @@ self.assertEqual(json.load(f)['boards'], ['b1-DMA']) with open(os.path.join(out, 'code-size-b1-DMA.json')) as f: self.assertEqual(json.load(f)['board'], 'b1-DMA') - self.assertEqual([os.path.relpath(p, build_root) for p in sized], ['cmake-build-b1-DMA/device/a/a.elf']) + self.assertEqual([os.path.relpath(p, build_root) for p in sized], [os.path.join('cmake-build-b1-DMA', 'device', 'a', 'a.elf')]) # the board's own skip rules, with the leg's defines as build.py applied them boards.assert_called_once_with([], ['b1'], ['device/a'], ('MAX3421_HOST=1',)) skip.assert_called_with('device/a', 'b1', ('MAX3421_HOST=1',))
diff --git a/test/hil/test/test_membrowse_cli.py b/test/hil/test/test_membrowse_cli.py index a752716..8021662 100644 --- a/test/hil/test/test_membrowse_cli.py +++ b/test/hil/test/test_membrowse_cli.py
@@ -17,6 +17,10 @@ import membrowse_cli as cli # noqa: E402 +# the tests run executable #! stubs, and onboard composes a bash build script +posix_only = unittest.skipIf(os.name == 'nt', 'needs POSIX executables and shell') + + def _write_stub(directory, name, body): path = os.path.join(directory, name) with open(path, 'w') as f: @@ -36,6 +40,15 @@ self.assertEqual(cli.extract_ld_scripts(text), ['C:/work/tinyusb/board.ld', '/work/a b/board.ld']) + def test_ld_script_extraction_unwraps_the_windows_ninja_cmd_wrapper(self): + # shortened from a windows-latest `ninja -t commands` link line + text = (r'C:\Windows\system32\cmd.exe /C "cd . && C:\gcc\bin\arm-none-eabi-gcc.exe -Os ' + r'-Wl,-Map=D:/a/out/cdc_msc.elf.map -Wl,--script=D:/a/bsp/STM32F407VGTx_FLASH.ld ' + r'main.c.obj -o device\cdc_msc\cdc_msc.elf lib/libboard.a && C:\Windows\system32\cmd.exe /C ' + r'"cd /D D:\a\out && C:\gcc\bin\arm-none-eabi-objcopy.exe -Obinary D:/a/out/cdc_msc.elf ' + r'D:/a/out/cdc_msc.bin""' '\n') + self.assertEqual(cli.extract_ld_scripts(text), ['D:/a/bsp/STM32F407VGTx_FLASH.ld']) + def test_defsym_extraction_both_separators(self): text = 'cc -Wl,--defsym=FOO=0x10 -Wl,--defsym,BAR=1 -o out.elf\n' self.assertEqual(cli.DEFSYM_RE.findall(text), ['FOO=0x10', 'BAR=1']) @@ -46,6 +59,7 @@ class LinkCommand(unittest.TestCase): + @posix_only def test_asks_ninja_for_only_the_elfs_final_command(self): # -s: without it, a target's commands include helper executables' links # (pico-sdk's boot_stage2) whose linker scripts are not the elf's @@ -136,6 +150,7 @@ ['membrowse', 'report', 'x']) +@posix_only class ReportCli(unittest.TestCase): """End-to-end CLI tests: real `python3 tools/membrowse_cli.py report` subprocess, with a stub `membrowse` injected into PATH so the real tool is never invoked.""" @@ -318,6 +333,7 @@ cmd = cli.compose('b', 'device/x', 5, False, 'k', ['--commits', 'a b']) self.assertNotIn('5', cmd[:4]) + @posix_only def test_each_historical_build_fetches_its_own_deps_and_linker_settings(self): cmd = cli.compose('b', 'device/x', 5, False, 'k', []) i = cmd.index('--ld-scripts') @@ -341,6 +357,7 @@ with open(out) as f: self.assertEqual(f.read(), 'FLASH_SIZE = 256K;\nINCLUDE "/tree/board.ld"\n') + @posix_only def test_explicit_scripts_are_resolved_from_the_build_dir(self): with tempfile.TemporaryDirectory() as tmp: build = os.path.join(tmp, 'build') @@ -359,6 +376,7 @@ f'INCLUDE "{script_dir}/memory.ld"\n' f'INCLUDE "{script_dir}/sections.ld"\n') + @posix_only def test_subcommand_runs_as_onboards_build_script_calls_it(self): # the historical build script runs `<python> <abs membrowse_cli.py> linker-shim ninja ...` with tempfile.TemporaryDirectory() as tmp: @@ -381,6 +399,7 @@ self.assertNotIn('linker-shim', r.stdout) +@posix_only class DisposableWorktree(unittest.TestCase): """`membrowse onboard` checks out and `git clean -fdx`s every historical commit unconditionally, in whatever directory it runs (membrowse/utils/
diff --git a/tools/code_size.py b/tools/code_size.py index a15013a..89e5692 100644 --- a/tools/code_size.py +++ b/tools/code_size.py
@@ -60,9 +60,10 @@ # resolved like tinyusb_src_filter(), so a symlinked checkout still matches its filter TINYUSB_ROOT = os.path.dirname(os.path.dirname(os.path.realpath(__file__))) CODE_SIZE_DIR = os.path.join(TINYUSB_ROOT, 'cmake-code-size') +WINDOWS = os.name == 'nt' # a master run's snapshots are the same for every checkout; XDG ignores a relative XDG_CACHE_HOME -_xdg_cache = os.environ.get('XDG_CACHE_HOME', '') -BASELINE_CACHE_DIR = os.path.join(_xdg_cache if os.path.isabs(_xdg_cache) else os.path.expanduser('~/.cache'), +_cache_home = os.environ.get('LOCALAPPDATA' if WINDOWS else 'XDG_CACHE_HOME', '') +BASELINE_CACHE_DIR = os.path.join(_cache_home if os.path.isabs(_cache_home) else os.path.expanduser('~/.cache'), 'tinyusb', 'code-size-baseline') CI_PINNED_BOARDS = os.path.join(TINYUSB_ROOT, '.github', 'ci-pinned-boards.json') # a diff's side names when git cannot give their commit hashes @@ -131,11 +132,14 @@ def _relative_key(src, filters): """Source path relative to the first matching filter, object suffix stripped, or None when no filter matches.""" + if WINDOWS: + src = src.replace('\\', '/') for f in filters: - idx = src.find(f) - if idx < 0: + # Windows paths are case-insensitive; the key keeps the recorded case + m = re.search(re.escape(f), src, re.IGNORECASE if WINDOWS else 0) + if not m: continue - key = src[idx + len(f):] + key = src[m.end():] for suffix in ('.obj', '.o'): if key.endswith(suffix): return key[:-len(suffix)] @@ -268,15 +272,61 @@ return buckets +# CMake replaces the start of an over-long object name with its md5 (cmLocalGenerator.cxx, +# cmLocalGeneratorShortenObjectName): <target>.dir/<md5>/<rest of the source path>.obj +_SHORTENED_OBJECT_RE = re.compile(r'\.dir/[0-9a-f]{32}/') +# first output, rule and first input of a build.ninja edge; '$' escapes a path's ':' and ' ' +_NINJA_EDGE_RE = re.compile(r'build ((?:[^$:\s]|\$.)+)(?: (?:[^$:]|\$.)*)?: \S+ ((?:[^$\s]|\$.)+)') + + +@functools.lru_cache(maxsize=None) +def _shortened_object_sources(build_dir): + """{shortened object: source} of the compile edges in build_dir's build.ninja.""" + path = os.path.join(build_dir, 'build.ninja') + sources = {} + try: + with open(path, encoding='utf-8') as f: + for line in f: + m = _NINJA_EDGE_RE.match(line) + if m and _SHORTENED_OBJECT_RE.search(m[1]): + obj, src = (re.sub(r'\$(.)', r'\1', p).replace('\\', '/') for p in m.groups()) + sources[obj] = src + except OSError as e: + raise RuntimeError(f'cannot read {path}: {e}') from e + except UnicodeDecodeError as e: + raise RuntimeError(f'{path} is not UTF-8: CMake writes it in the ANSI code page when ninja is older than ' + '1.11 or `ninja -t wincodepage` is not UTF-8; use ninja >= 1.11 whose ' + '`ninja -t wincodepage` reports UTF-8, or keep the checkout, CMake and toolchain paths ' + 'ASCII-only') from e + return sources + + class _Sizes: """Accumulates one elf's filtered per-file flash/RAM, section and symbol sizes, and its total flash/RAM.""" - def __init__(self, filters): - self.filters, self.files, self.all = filters, {}, {'flash': 0, 'ram': 0} + def __init__(self, filters, elf): + self.filters, self.elf, self.files, self.all = filters, elf, {}, {'flash': 0, 'ram': 0} self.sections, self.symbols = {}, {} + def _shortened_key(self, path): + """Key of the source build.ninja compiles to a CMake-shortened object; the object + path no longer holds the filtered dir.""" + shortened = f'{path}: CMake shortened this object path to fit CMAKE_OBJECT_PATH_MAX (250 on Windows) and' + build_dir = _find_ninja_build_dir(self.elf) + if build_dir is None: + raise RuntimeError(f'{shortened} no build.ninja was found above {self.elf} to name its source - ' + 'use a shorter checkout path') + src = _shortened_object_sources(build_dir).get(path.replace('\\', '/')) + if not src: + raise RuntimeError(f'{shortened} the build.ninja above {self.elf} has no compile edge for it ' + 'to name its source - use a shorter checkout path') + return _relative_key(src, self.filters) + def add(self, path, section, buckets, size, name): - key = _relative_key(path, self.filters) if path else None + if path and WINDOWS and _SHORTENED_OBJECT_RE.search(path.replace('\\', '/')): + key = self._shortened_key(path) # a filter could match the kept tail of the source path + else: + key = _relative_key(path, self.filters) if path else None for b in buckets: self.all[b] += size if key is not None: @@ -333,7 +383,7 @@ """Symbols of `membrowse report`; linker-defined symbols without a section (`__StackLimit`) are not counted.""" buckets = section_buckets(elf, _map_path(elf)) - sizes = _Sizes(filters) + sizes = _Sizes(filters, elf) for sym in report_for_elf(elf).get('symbols', []): section = sym.get('section') if not sym.get('size') or not section: @@ -372,7 +422,7 @@ sections = _linkermap().parseSections(f) if sections is None: raise RuntimeError(f'{map_path}: no Memory Configuration, not a GNU ld map') - sizes = _Sizes(filters) + sizes = _Sizes(filters, elf) for out in sections: if not out.children: # memory regions continue @@ -394,7 +444,7 @@ if not {'compileunits', 'sections', 'symbols', 'vmsize'} <= set(rows.fieldnames or ()): raise RuntimeError(f'unexpected bloaty csv columns for {elf}: {rows.fieldnames}') buckets = section_buckets(elf, _map_path(elf)) - sizes = _Sizes(filters) + sizes = _Sizes(filters, elf) for row in rows: section = row['sections'] try: @@ -836,7 +886,13 @@ in `checkout_dir`. The substring is the absolute path to the checkout's `src/` dir — collision-free with vendored deps (pico-sdk, lwip, FreeRTOS, etc.) which live at unrelated paths.""" - return os.path.realpath(os.path.join(checkout_dir, 'src')) + os.sep + return filter_arg(os.path.realpath(os.path.join(checkout_dir, 'src')) + os.sep) + + +def filter_arg(path): + """A path filter in the form sizes are matched in: on Windows, past the drive with '/', + as CMake names an out-of-tree object D_/a/.../src/x.c.obj and DWARF D:/a/...""" + return os.path.splitdrive(path)[1].replace('\\', '/') if WINDOWS else path verbose = False @@ -861,6 +917,10 @@ try: out, err = proc.communicate(timeout=timeout) except BaseException as stopped: + if WINDOWS: # terminate() and kill() stop only the command, not ninja's compilers + with contextlib.suppress(OSError, subprocess.SubprocessError): + subprocess.run(['taskkill', '/PID', str(proc.pid), '/T', '/F'], + capture_output=True, timeout=TERMINATE_GRACE) # SIGTERM lets ninja stop its jobs, which it runs in process groups of their own; # a descendant can outlive the SIGKILL holding the pipes open, so stop reading then for stop, wait in ((proc.terminate, TERMINATE_GRACE), (proc.kill, KILL_DRAIN)): @@ -880,8 +940,9 @@ def exit_on_termination(): """Exit on a SIGTERM or SIGHUP sent to this process alone as on Ctrl-C: through run()'s stop of its command and main()'s worktree removal, instead of orphaning the build.""" - for signum in (signal.SIGTERM, signal.SIGHUP): - signal.signal(signum, lambda sig, _frame: sys.exit(128 + sig)) + for name in ('SIGTERM', 'SIGHUP'): # Windows has no SIGHUP + if hasattr(signal, name): + signal.signal(getattr(signal, name), lambda sig, _frame: sys.exit(128 + sig)) def symlink_deps(main_root, worktree_dir): @@ -894,7 +955,13 @@ dst = os.path.join(worktree_dir, rel) if os.path.isdir(src) and not os.path.lexists(dst): os.makedirs(os.path.dirname(dst), exist_ok=True) - os.symlink(src, dst) + try: + os.symlink(src, dst, target_is_directory=True) + except OSError as e: + if getattr(e, 'winerror', None) == 1314: # ERROR_PRIVILEGE_NOT_HELD + sys.exit(f'Error: cannot symlink {rel} into the base worktree: enable Windows Developer ' + 'Mode, or use --base-source ci when CI stored a baseline') + raise def short_hash(checkout): @@ -953,7 +1020,10 @@ if line.strip() and not _NINJA_PROGRESS.match(line) and not line.startswith(('ninja: build stopped', 'ninja: Entering directory', 'FAILED: '))] error = build_utils.first_error(out) or (lines[-1] if lines else 'no output') - return error.replace(src_dir.rstrip(os.sep) + os.sep, '') + root = src_dir.rstrip(os.sep) + for prefix in {root + os.sep, root.replace(os.sep, '/') + '/'}: # Windows: CMake passes the compiler D:/a/... + error = error.replace(prefix, '') + return error ESP_IDF_IMAGE = 'espressif/idf:tinyusb' # .github/actions/setup_toolchain/espressif's tag @@ -991,6 +1061,8 @@ def esp_without_idf(boards): """The error for espressif `boards` no exported ESP-IDF or CI's image can build, else None.""" esp = [b for b in boards if is_espressif(b)] + if esp and WINDOWS: + return f'{", ".join(esp)} need ESP-IDF, which code_size.py does not support on Windows' if esp and not (shutil.which('idf.py') or _idf_image()): return f'{", ".join(esp)} need {ESP_IDF_MISSING}' return None @@ -1128,7 +1200,7 @@ f'or pick another --engine')] sizes, errors = {}, [] for elf, (elf_sizes, error) in zip(elfs, results): - rel = os.path.relpath(elf, build_dir) + rel = os.path.relpath(elf, build_dir).replace(os.sep, '/') # one elf id on every host sizes[rel] = elf_sizes if error: errors.append((rel, error)) @@ -1178,7 +1250,10 @@ def _shown(path): """`path` relative to the working directory when under it.""" - rel = os.path.relpath(path) + try: + rel = os.path.relpath(path) + except ValueError: # on another Windows drive + return path return path if rel.startswith('..') else rel @@ -1290,7 +1365,9 @@ ('name', 'CMAKE_C_COMPILER')): m = re.search(rf'^set\({var} "([^"]*)"\)', text, re.M) if m: - info[key] = os.path.basename(m.group(1)) if key == 'name' else m.group(1) + info[key] = m.group(1) + # without .exe, so a Windows build's name matches a Linux snapshot's + info['name'] = re.sub(r'\.exe$', '', os.path.basename(info['name']), flags=re.I) cache = os.path.join(build_dir, 'CMakeCache.txt') if os.path.isfile(cache): with open(cache) as f: @@ -1818,7 +1895,7 @@ def example_arg(value): - example = value.rstrip('/') + example = (value.replace('\\', '/') if WINDOWS else value).rstrip('/') if not example: raise argparse.ArgumentTypeError(f'{value!r} names no example') return example @@ -1830,7 +1907,7 @@ common = argparse.ArgumentParser(add_help=False) common.add_argument('-b', '--board', action='append', default=[], help='Board name (repeatable). Required unless diff --ci is given.') - common.add_argument('-f', '--filter', action='append', default=None, + common.add_argument('-f', '--filter', action='append', default=None, type=filter_arg, help='Path-substring filter (repeatable). When given, ' 'overrides the default and is applied to every build. ' 'Default: each build\'s own absolute <checkout>/src/ path, ' @@ -1893,7 +1970,7 @@ help='CI event; pull_request reads base/head from the merge commit ' '(default: $GITHUB_EVENT_NAME, else push)') snap.add_argument('--build-outcome', default='success', help='The Build step\'s outcome, recorded as is') - snap.add_argument('-f', '--filter', action='append', default=None, + snap.add_argument('-f', '--filter', action='append', default=None, type=filter_arg, help='Path-substring filter (repeatable); default: this checkout\'s <checkout>/src/') snap.add_argument('--symbols', action='store_true', help='Include each file\'s symbols') snap.add_argument('-v', '--verbose', action='store_true', help='Print commands')
diff --git a/tools/membrowse_cli.py b/tools/membrowse_cli.py index 12b06f7..053beb4 100644 --- a/tools/membrowse_cli.py +++ b/tools/membrowse_cli.py
@@ -42,6 +42,9 @@ for line in commands_text.splitlines(): try: args = shlex.split(line) + # Windows ninja links through `cmd.exe /C "cd . && <link command>"` + if len(args) >= 3 and args[0].lower().endswith('cmd.exe') and args[1].lower() == '/c': + args = shlex.split(args[2]) except ValueError: continue for i, arg in enumerate(args):