Rebuild native modules whose objects predate a header change

[thejambi]

Start here: Remove-NativeObjDirsWithStaleInputs in Build/Agent/FwBuildHelpers.psm1, then the RemoveStaleNativeObjects target in Build/mkall.targets that calls it.

Before the first native module builds, MSBuild now deletes Obj/<Config>/<Module> for Views, FwKernel, Generic or DebugProcs when any header or makefile that module compiles against, including headers generated into Output/<Config>/Common, is newer than its oldest .obj. nmake then compiles the whole module.

Why. The rules in Bld/_rule.mak rebuild an object only when its own .cpp changes, never when an included header does. After #1166 added NfcOffsetMap to LayoutPassCache, an incremental Debug build recompiled UniscribeSegment.obj but kept a 2026-09-02 VwTextBoxes.obj, whose ParaBuilder embeds the old, smaller LayoutPassCache. Loading a project then tripped the Vector.h:309 AssertValid assert. Any existing Debug tree that pulled #1166 is exposed; CI builds clean, so it never is.

Will it rebuild everything every time? No. Once a module rebuilds, every object is newer than every header and the check goes quiet. It is a no-op on CI and after -Clean, and skipped with SkipNative. The cost is deliberate coarseness: one header edit rebuilds its whole module.

Where to look

• GenerateCellarConstants rewrote CellarConstants.h on every build (its Outputs item used a property set only at run time). It now generates into Obj and copies only changed content; otherwise this check would rebuild Views every build.
• Placement: the target depends on CopyDlls (restored LCM .idh and ICU headers) and GenerateCellarConstants, and DebugProcs depends on the target.
• The folder lists in Get-NativeMakefileModules. A test fails when a module's sources include a header from an unlisted folder by path; folders reached through a makefile's UI= line are checked by eye.
• bldinc.h is deliberately not watched: it holds only version stamps that change daily.

Not here: real header-dependency tracking (/sourceDependencies in _rule.mak), the file-precise fix.

Verification. On a real tree, updating LayoutCache.h's modified time made MSBuild log [INFO] Views: Src\views\lib\LayoutCache.h ... removing Obj\Debug\Views and rebuild only Views. After the generated-header change, one build rebuilt Views, FwKernel and Generic once; the next flagged nothing, and CellarConstants.h kept its date. NativeObjStaleness.Tests.ps1 (13 fixture cases plus folder and include checks) passes under Windows PowerShell 5.1, with mutation checks failing as expected. CI ran the tests under PowerShell 7 and 5.1 on the previous push, all green.

Next: approve, or point at a folder list that looks wrong.

---

Reading this a year from now -- start here

This branch had no design documents; the reasoning below is the only record of why the check is shaped the way it is. If the native modules have moved to real MSBuild C++ projects (which track headers through .tlog files) or Bld/_rule.mak emits compiler-generated dependency files, this check is redundant and the RemoveStaleNativeObjects target and its helper can be deleted.

Decisions, and why

• Timestamps, not a recorded commit hash. Comparing header times against object times also catches uncommitted header edits and branch switches, and needs no stamp file that can go stale. Git writes every file it changes with the current time, so a pull or checkout always looks newer than old objects.
• Oldest object, not newest. The failure is one unedited object left behind while its neighbours rebuild, so the oldest object is the one that matters.
• Delete the whole module folder. Deciding which objects include which header is the dependency tracking nmake lacks. Deleting the folder also clears the precompiled header and any orphaned objects.
• Top-level folders only. Scanning Src/views recursively would pull in Src/views/Test headers, which only the MSBuild-built test projects include.
• Run inside MSBuild, after CopyDlls and GenerateCellarConstants. CopyDlls (Build/PackageRestore.targets) is where restored .idh and ICU headers land in the tree, GenerateCellarConstants writes CellarConstants.h, and DebugProcs is the first native module every native target path goes through. Hooking between them sees package bumps and model changes, and covers MSBuild runs that skip build.ps1.
• Watch Output/<Config>/Common. Views, FwKernel and Generic include CellarConstants.h, CellarBaseConstants.h, FwKernelTlb.h and ViewsTlb.h from there. CellarConstants.h comes from MasterLCModel.xml in the LCM package, so no source-tree file changes when the model does.
• Content compare, not Inputs/Outputs, for GenerateCellarConstants. Fixing the Outputs path would make the target incremental by timestamp, but NuGet keeps package file dates (beta0178's MasterLCModel.xml is dated 06:43:48.000), so a newer model could look older than the header and be skipped. The target regenerates every build into Obj/<Config>/CellarConstants and copies only when content differs.
• Skip bldinc.h. It holds only version stamps (BUILD_NUMBER, BUILDLABEL) that local builds regenerate daily. Watching it would rebuild Generic and Views on the first build of each day for a version-string change that alters no layout.
• .pdb/.idb leftovers are tolerated. The compiler's program-database server can hold a debug database open between builds. A leftover debug database does not change what gets compiled; a leftover object or precompiled header would.

Reversals

• The check first ran from build.ps1, just before the MSBuild traversal. That missed headers copied by CopyDlls during the same build, so a package bump could still mix objects until the next build, and direct MSBuild runs skipped the check entirely. The third commit moves it into Build/mkall.targets; build.ps1 is unchanged against main.
• The folder lists first mirrored only each makefile's UI= line. Review found that Views reaches Src/AppCore/Res/AfAppRes.h and Src/Cellar/FwXml.h by relative path and never watched generated headers. The fourth commit adds those folders, the generated-header folder, and a test that fails on any path-qualified include into an unlisted folder.

Paths not taken

• Convert the Makefile-type vcxprojs to real MSBuild C++ projects. Most correct (headers, PCH and flags all tracked), but it means translating CL_OPTS/DEFS, the multiple PCH setups, MIDL and proxy/stub steps and custom link steps for every module. Worth doing only as part of a wider native-build modernization. XAmpleCOMWrapper.vcxproj is the in-repo precedent.
• Compiler-generated dependencies in nmake. Add /sourceDependencies to Bld/_rule.mak, convert the JSON to nmake rules with a small script, and !INCLUDE the result. One central change and file-precise, but it needs handling for deleted headers, PCH dependencies, generated MIDL headers and atomic writes. The natural next step if native churn continues.
• Hand-written header lists in each .mak. Rejected; they rot. testViews.mak and testGenericLib.mak already carry partial hand-written lists.
• Parse each makefile's UI= line instead of hard-coding folders. Rejected for now; parsing nmake macros is more fragile than a short list that changes rarely, and it would still miss path-qualified includes, which the include test covers.

Evidence

• Real tree, after updating the modified time of Src/views/lib/LayoutCache.h (content unchanged), ./build.ps1 logged from MSBuild: [INFO] Views: Src\views\lib\LayoutCache.h (2026-10-01 17:30) is newer than autopch\ViewsGlobals.obj (2026-10-01 16:55); removing Obj\Debug\Views so it rebuilds completely. Only Views was flagged, every Views object was recompiled, the build passed, and a second wrapper run printed nothing.
• Generated headers, real tree: the first build after the change logged [INFO] for Views, FwKernel and Generic, each flagged by Output\Debug\Common\CellarConstants.h (2026-10-01 19:30), a copy the old target had rewritten. The next build flagged nothing; CellarConstants.h stayed at 19:30:31 while the staged copy in Obj/Debug/CellarConstants was regenerated at 19:38:34. DebugProcs, which does not watch that folder, was untouched.
• The content-compare Copy condition was checked in a scratch MSBuild project with file contents holding quotes, $(), @() and %(): missing destination copies, identical content skips, changed content copies.
• Build/Agent/NativeObjStaleness.Tests.ps1 builds throwaway trees with chosen timestamps. Cases: header newer than oldest object; shared Include header newer; makefile newer; generated header newer; another configuration's generated header newer (no-op); only bldinc.h newer (no-op); all objects newer (no-op); only a .cpp newer (no-op); header in an unlisted subfolder (no-op); no object folder; folder without objects; locked .pdb skipped with every object removed; locked .obj fails the build. It also checks every real module's listed folders exist, and that every path-qualified include in a scanned file lands in a listed folder.
• Mutation checks: disabling the timestamp comparison fails the three removal cases; disabling the leftover-file filter fails the locked-object case; dropping Src\AppCore\Res and Src\Cellar from Views fails the include check for both; disabling the bldinc.h skip fails its case.
• Restored headers do not trip the check on every build: CopyDlls copies Src/Kernel/*.idh and Include/unicode/*.h with SkipUnchangedFiles.
• The original failure, on the affected tree: Obj/Debug/Views/autopch/VwTextBoxes.obj dated 2026-09-02; UniscribeSegment.obj, UniscribeEngine.obj and Src/views/lib/LayoutCache.h dated 2026-10-01.

Preflight review details

Code Review Summary

Branch: native-stale-header-objs
Base: main
Date: 2026-10-01
Review model: Claude Opus 5.5 (Claude Code)
Files changed: 5

Overview

Author's purpose: incremental native builds silently reuse objects compiled
against old headers (this caused the Vector.h:309 assert after #1166). Make
the build detect that and force a full rebuild of the affected native module.

The nmake rules in Bld/_rule.mak only map .cpp to .obj, so a header
change rebuilds nothing that merely includes it. After #1166 added an
NfcOffsetMap member to LayoutPassCache, Obj/Debug/Views/autopch/VwTextBoxes.obj
(2026-09-02) still embedded the old, smaller LayoutPassCache in
ParaBuilder, while the rebuilt UniscribeSegment.obj wrote past it. The
branch adds Remove-NativeObjDirsWithStaleInputs, which deletes
Obj/<Config>/<Module> for Views, FwKernel, Generic, or DebugProcs when any
header or makefile in that module's include folders is newer than its oldest
object. A new RemoveStaleNativeObjects target in Build/mkall.targets runs
it after CopyDlls (which copies restored LCM and ICU headers) and before
the first Make task. The check is deliberately coarse (whole-module
rebuild) and self-healing (once rebuilt, every object is newer than every
header).

Developer-only build tooling: no Jira ticket needed.

Contract/API Changes

• Build/Agent/FwBuildHelpers.psm1 exports two new functions:
  Get-NativeMakefileModules and Remove-NativeObjDirsWithStaleInputs.
• New script Build/Agent/Remove-StaleNativeObjects.ps1 (-RepoRoot,
  -Configuration; exits 1 when objects could not be removed).
• Build/mkall.targets adds target RemoveStaleNativeObjects; DebugProcs
  now depends on it. It is skipped when SkipNative is true. Any native
  build may now delete Obj/<Config>/<Module> folders before compiling.
• build.ps1 is unchanged.
• .github/workflows/CI.yml adds two steps that run the new fixture tests
  under PowerShell 7 and Windows PowerShell 5.1.

Findings

Critical - Must address before merge

None.

Important - Should address before merge

• Package restore ran after the check. CopyDlls in
  Build/PackageRestore.targets copies LCM .idh files into Src/Kernel and
  ICU headers into Include/unicode inside the MSBuild traversal, after the
  original build.ps1 call site. (fixed during review: the check moved into
  Build/mkall.targets as RemoveStaleNativeObjects, which depends on
  CopyDlls and runs before DebugProcs. This also covers MSBuild runs that
  do not go through build.ps1. The author first accepted this as a
  documented limitation; the hook point surfaced while verifying the PR text
  and the author chose to fold it in.)

• Views missed path-included headers (DevinReview, after the PR
  opened). Src/views/Main.h includes Src/AppCore/Res/AfAppRes.h and
  Src/Cellar/FwXml.h by relative path, bypassing the makefile UI= list.
  (fixed: both folders added to Views; a new test fails when any scanned
  file's path-qualified include lands in an unlisted folder.)

• Generated headers were not watched (DevinReview). Views includes
  CellarConstants.h from Output/<Config>/Common, generated from the LCM
  package's MasterLCModel.xml after the check ran. (fixed: the check runs
  after GenerateCellarConstants and watches Output/<Config>/Common for
  Views, FwKernel and Generic, skipping version-only bldinc.h. The
  reviewer's fix alone would have rebuilt Views every build, because
  GenerateCellarConstants rewrote the header every build: its Outputs item
  used $(dir-fwoutputCommon), set only at run time. The target now
  generates into Obj and copies only changed content.)

Minor - Consider

• A locked .pdb could abort the build. Remove-Item -ErrorAction Stop
  stopped at the first locked file, possibly leaving objects behind.
  (fixed during review: deletion continues past errors; only a leftover file
  other than .pdb/.idb fails the build, otherwise a warning is printed.
  Two fixture tests cover a locked .pdb and a locked .obj.)
• Folder lists are hand-maintained. Get-NativeMakefileModules mirrors
  each makefile's UI= include line; the fixture test only checks the folders
  exist, not that the list is complete. (Accepted: directory-level lists
  change rarely, and parsing nmake syntax would be more fragile.)

Required Validation / Evidence

• Build/Agent/NativeObjStaleness.Tests.ps1 under Windows PowerShell 5.1:
  13 fixture cases plus the real-module folder and include checks pass.
• Generated headers, real tree: the first build after the fix rebuilt
  Views, FwKernel and Generic once; the next flagged nothing, and
  CellarConstants.h kept its date while its staged copy was regenerated.
• Content-compare Copy condition checked in a scratch MSBuild project
  (missing, identical and changed destination; contents with quotes and
  $()/@()/%()).
• Mutation checks: disabling the timestamp comparison fails the 3 removal
  cases; disabling the lock filter fails the locked-object case; dropping
  Src\AppCore\Res and Src\Cellar fails the include check; disabling the
  bldinc.h skip fails its case.
• Build/Agent/powershell-compat.ps1: clean for 5.1 and 7.0.
• Build/Agent/comment-hygiene.ps1: clean.
• ./build.ps1 -CommentHygiene -TokenHygiene: passed after each change.
• Real-tree trigger: after updating the modified time of
  Src/views/lib/LayoutCache.h (content unchanged), ./build.ps1 logged
  [INFO] Views: Src\views\lib\LayoutCache.h (2026-10-01 17:30) is newer than autopch\ViewsGlobals.obj (2026-10-01 16:55); removing Obj\Debug\Views so it rebuilds completely. from inside MSBuild. Only Views was flagged, every
  Views object was recompiled (oldest now 17:40), the build passed, and a
  second run of the wrapper was a no-op.
• Restored-header folders checked: Src/Kernel/*.idh and
  Include/unicode/*.h are copied with SkipUnchangedFiles, so they do not
  cause a rebuild on every build.
• PowerShell 7 run of the fixture tests: pwsh is not installed locally;
  the new CI step is the first PowerShell 7 run.

Positive Observations

• Scoped to the four nmake modules; MSBuild-built native test projects already
  track headers and are untouched.
• No-op on CI and after -Clean (empty Obj), so CI behavior is unchanged.
• The diagnostic names the header and the object that triggered the rebuild.
• Fixture tests follow the existing dependency-free *.Tests.ps1 style and run
  under both engines in CI.

Interview Notes

• Author confirmed the branch purpose as stated in the Overview.
• Restore-order gap: author first chose to accept and document it. After the
  CopyDlls hook point was found, author chose to fold the fix in before
  opening the PR.
• Minor findings: author chose to fix the locked-.pdb abort and accept the
  hand-maintained folder lists.
• Nothing else to flag.

In-Review Quality Check

• INTERVIEW_CHANGES: lock-tolerant deletion in
  Remove-NativeObjDirsWithStaleInputs plus two fixture tests
  (locked-debug-database-is-skipped, locked-object-fails-the-build).
• INTERVIEW_CHANGES: check moved from build.ps1 to the
  RemoveStaleNativeObjects target in Build/mkall.targets, via the new
  Build/Agent/Remove-StaleNativeObjects.ps1 wrapper.
• Fixture tests, mutation checks, and ./build.ps1 -CommentHygiene -TokenHygiene
  re-run after each change.

Suggested Review Focus

• The per-module folder lists in Get-NativeMakefileModules against each
  makefile's UI= line.
• Whether deleting a whole module's Obj folder is acceptable rebuild cost
  for a single header edit.
• The RemoveStaleNativeObjects placement: after CopyDlls, before
  DebugProcs.

🤖 Generated with Claude Code

---

This change is 

[github-actions]

NUnit Tests

    1 files  ±0      1 suites  ±0   10m 26s ⏱️ +4s
6 365 tests ±0  6 280 ✅ ±0  85 💤 ±0  0 ❌ ±0 
6 374 runs  ±0  6 289 ✅ ±0  85 💤 ±0  0 ❌ ±0 

Results for commit 3ccb4bb. ± Comparison against base commit 6b8a752.

♻️ This comment has been updated with latest results.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 39.30%. Comparing base (37b7024) to head (3ccb4bb).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1174      +/-   ##
==========================================
- Coverage   39.30%   39.30%   -0.01%     
==========================================
  Files        1523     1523              
  Lines      353047   353117      +70     
  Branches    40750    40761      +11     
==========================================
+ Hits       138782   138805      +23     
- Misses     185001   185040      +39     
- Partials    29264    29272       +8     

see 5 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[thejambi]

Closing, duplicates #1168