Repository navigation
Conversation
- Add the Translate sample and C# host build and run targets with runtime deployment. - Keep the CMake host build isolated from MSBuild references and vcpkg integration. - Document Phase 4 usage, parity, and CLion validation status. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
With samples enabled by default, finding dotnet but not pwsh aborts configuration of the entire native build even though PowerShell is needed only to launch the optional host.
1 open issue
What changed in this PR
Extends the CMake migration from native libraries and tests to the Translate sample and C# ActiveX host, keeping their runtime output separate from legacy MSBuild builds.
Changes:
- Adds sample and host build/run targets plus a CLion host run configuration.
- Deploys native runtime DLLs and prevents CMake-driven host builds from invoking legacy native builds or MSBuild vcpkg integration.
- Aligns linker options and documents Phase 4 usage and validation.
Builds and tests were not run during this review.
| File | Description |
|---|---|
| tests/MapWinGIS.Core.Tests/CMakeLists.txt | Enables runtime DLL deployment for tests. |
| samples/CSharp/CMakeLists.txt | Adds host build and launch targets. |
| samples/CSharp/ActiveXTestHost/Run-ActiveXTestHost.ps1 | Supports an explicit runtime directory. |
| samples/CSharp/ActiveXTestHost/ActiveXTestHost.csproj | Disables the native project reference for CMake builds. |
| samples/Cpp/CMakeLists.txt | Adds the Translate sample and DLL deployment. |
| README.md | Points to CMake sample and host instructions. |
| docs/CMAKE-MIGRATION.md | Records Phase 4 integration and validation. |
| docs/BUILDING.md | Documents sample and host target commands. |
| Directory.Build.targets | Skips vcpkg integration for CMake-driven builds. |
| Directory.Build.props | Guards legacy output paths and vcpkg imports. |
| CMakeLists.txt | Includes sample subdirectories when enabled. |
| cmake/MwgRuntimeDeploy.cmake | Adds a reusable runtime DLL deployment helper. |
| cmake/MwgCopyRuntimeDlls.cmake | Copies listed DLLs with error reporting. |
| cmake/MwgCompilerOptions.cmake | Sets explicit Debug and Release linker options. |
| .run/ActiveXTestHost.run.xml | Adds a CLion host run configuration. |
Review effort: Balanced
Julian-MW
left a comment
Julian-MW
left a comment
There was a problem hiding this comment.
Automated review. Final decision is with the human reviewer.
Overall: the Phase 4 CMake wiring is sound — the MwgCMakeBuild guards in Directory.Build.props/.targets and the csproj keep the legacy MSBuild path untouched, the linker options now match MapWinGIS.Cpp.props and the docs/CMAKE-MIGRATION.md linker table exactly, and the runtime-DLL deployment helper is correctly ordered ahead of gtest_discover_tests. Only minor/robustness findings below; nothing blocking. Note: GitHub reports this PR as "dirty" against its base while the base SHA still matches PR #64's head — likely a stale mergeability computation, worth a rebase/nudge if it persists.
- Skip optional host run targets when pwsh is unavailable. - Declare custom-target ordering with add_dependencies. - Warn when the .NET SDK is missing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Integrate the latest CMake phase 3 base. - Preserve the CMake sample and host guidance with updated build instructions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Implements Phase 4 of the CMake migration: the Translate sample and C# ActiveX host now build and run from the isolated CMake runtime folder.
.vcxprojor MSBuild vcpkg integration; the conditional project reference remains enabled for legacy builds.Stacked on Phase 3 PR #64 (
cmake-phase-3-gtest-ctest) because the prior CMake phases are not yet merged intodevelopment.