Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
This PR replaces the previous “overlay/copy local liblcm outputs” approach with an explicit, repo-wide local-source mode rooted at Localizations/LCM, including dedicated build entrypoints and improved VS Code debugging ergonomics.
Changes:
- Introduces
-LcmMode Auto|Package|Localand-ManagedDebugTypeintobuild.ps1, plus local-source detection/state reporting and runtime-output syncing for VS Code symbols. - Adds
FieldWorks.LocalLcm.slnand MSBuild wiring (UseLocalLcmSource, package→project reference switching, build-task bootstrapping). - Updates developer setup + VS Code tasks/launchers, and improves toolchain discovery (MSVC path selection).
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/Agent/Copy-LocalLcm.ps1 | Removes the old “copy locally-built LCM DLLs over NuGet outputs” flow. |
| build.ps1 | Adds LcmMode + ManagedDebugType, local-source selection, and symbol/runtime-output refresh. |
| Src/Common/SimpleRootSite/EditingHelper.cs | Disposes VwPropertyStoreManaged via using. |
| Src/Common/FwUtils/FwUtilsTests/TestFwStylesheetTests.cs | Disposes VwPropertyStoreManaged in tests via using. |
| Src/Common/FieldWorks/FieldWorks.csproj | Enables unmanaged debugging for the FieldWorks project. |
| Src/Common/Controls/Widgets/FontHeightAdjuster.cs | Disposes VwPropertyStoreManaged via using. |
| Setup-Developer-Machine.ps1 | Installs .NET SDK 8.x when missing; adjusts helper-repo cloning/junction behavior for worktrees; clones liblcm nested. |
| FieldWorks.LocalLcm.sln | Adds a dedicated solution that includes local liblcm projects for local-source mode. |
| Docs/architecture/liblcm-local-source-checklist.md | Documents the migration checklist and expectations for local-source mode. |
| Docs/architecture/liblcm-debugging.md | Adds detailed debugging guidance for package vs local-source liblcm workflows. |
| Docs/architecture/dependencies.md | Points dependency debugging guidance to the new liblcm debugging doc and summarizes the workflow. |
| DistFiles/Parts/StandardParts.xml | Adds new part definitions (ImportResidue/Pictures). |
| Directory.Build.targets | Adds package→project reference switching for LCM + build-task bootstrapping/validation targets. |
| Directory.Build.props | Adds UseLocalLcmSource defaults for FieldWorks.LocalLcm and redirects LCM paths/artifacts to nested checkout. |
| Build/Src/FwBuildTasks/Make.cs | Improves MSVC tool path detection under modern VS layouts. |
| Build/Agent/Invoke-VsCodeDebugBuild.ps1 | Adds VS Code prelaunch build wrapper with change detection + mode/debug-type stamping. |
| Build/Agent/FwBuildEnvironment.psm1 | Prefers HostX64\x64 MSVC bin path early in PATH. |
| .vscode/tasks.json | Adds explicit package/local build tasks and “prepare debug” tasks; defaults Build to -LcmMode Auto. |
| .vscode/launch.json | Adds package vs local-LCM launch configs with prelaunch tasks + symbol search paths. |
| .vscode/extensions.json | Recommends the official C# extension while keeping Dev Kit unwanted (ReSharper-first). |
| .serena/project.yml | Adds Serena config keys for line ending and read-only memory patterns. |
NUnit Tests 1 files ± 0 1 suites ±0 6m 7s ⏱️ +23s Results for commit 899d70c. ± Comparison against base commit d58727c. This pull request removes 26 and adds 382 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
0112423 to
b25ed38
Compare
|
Cleaned up this PR to match the review feedback and the split described in the local-source plan. What changed:
What remains in this PR is the local liblcm source-mode implementation and the supporting docs/debugger/build-flow changes that are directly part of that workflow. The split toolchain PR is here for separate review: #766 |
b25ed38 to
3b0aa26
Compare
bf0bd50 to
899d70c
Compare
jasonleenaylor
left a comment
There was a problem hiding this comment.
This PR looks like it has conflated two different uses of LCM locally, there is a localization related path used mostly by installers or by local testing of localization and the current focus of being able to test LCM fixes by integrating locally built packages and symbols.
The developer is presumed to have LCM cloned locally. The previous helper script would run pack on lcm and capture the version information out of the build log and modify the package references to point at the new version. It would also copy the symbol artifacts from the lcm build output into the fieldworks build output.
I think a cleaner and more developer configurable change would be to have a -LocalLcm parameter which takes a path to the lcm root folder. Validate that location, run dotnet pack in that location, change the lcm package version in the centralized packaging location for fieldworks, copy symbol files to output if necessary.
@jasonleenaylor reviewed all commit messages and made 1 comment.
Reviewable status: 0 of 131 files reviewed, all discussions resolved.
Summary
Localizations/LCMFieldWorks.LocalLcm.sln, repo-wideUseLocalLcmSourcewiring, andbuild.ps1 -LcmMode Auto|Package|LocalOutput/Debugfor VS Code symbol loading and keep portable PDBs for theclrdebugger pathReview findings addressed
SIL.LCModel.Build.Tasks.dllfrom source when local artifacts are missingValidation
./build.ps1 -LcmMode Local -ManagedDebugType portable -SkipDependencyCheck./Build/Agent/Invoke-VsCodeDebugBuild.ps1 -LcmMode Local -ManagedDebugType portableThis change is