Improve Visual Studio toolchain detection for Windows builds#766
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves Windows build-toolchain reliability by preferring the correct MSVC host tool bin directory, making nmake.exe discovery more robust, and updating dependency verification to match current expectations.
Changes:
- Prefer Visual Studio MSVC
Hostx64\x64tool bin directories when resolving build tools. - Improve dependency verification UX (compact vs detailed) and add an opt-in structured results mode.
- Update WiX dependency detection logic to be more structured (XML-based) and remove legacy NuGet CLI checks.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| Build/Src/FwBuildTasks/Make.cs | Adds MSVC tool-bin discovery under VCINSTALLDIR\Tools\MSVC\*\bin\... and prefers Hostx64/x64 for tool resolution. |
| Build/Agent/Verify-FwDependencies.ps1 | Adds -Detailed / -PassThru, adjusts summary output, and updates WiX detection; removes NuGet CLI check. |
| Build/Agent/FwBuildEnvironment.psm1 | Ensures the preferred MSVC HostX64\x64 bin path is moved to the front of PATH during VS env initialization. |
98915ea to
42e67f1
Compare
|
I think that this change should be redone to use the microsoft provide vswhere package. |
42e67f1 to
616ab32
Compare
|
Update on the toolchain work: Managed paths are now covered by the shared Visual Studio helper in
For unmanaged/native paths, the remaining work is mostly cleanup and consistency rather than blocking correctness:
Net: the managed path is now aligned on one shared discovery model; the native side is functional but still has a few legacy resolution paths that could be simplified in a follow-up. |
80c2be8 to
036af1d
Compare
|
These two installer commands should be using /p:Configuration=Release |
036af1d to
874f6f4
Compare
|
@jasonleenaylor addressed in e5ba3a4. I simplified I also reran |
jasonleenaylor
left a comment
There was a problem hiding this comment.
If you are certain that our current build does not require the 'config' to build the c++ properly then
I had an informational comment about a fragile line which can be addressed at developer discretion.
@jasonleenaylor reviewed 6 files and all commit messages, and made 2 comments.
Reviewable status: 6 of 8 files reviewed, all discussions resolved.
Build/Agent/FwBuildEnvironment.psm1 line 111 at r5 (raw file):
} $vcTargetsPath = Join-Path $installationPath 'MSBuild\Microsoft\VC\v170'
Ok, on my second pass I noticed this. I will not hold up the review for it but this path might be a fragile part of this script. The next version may change the 'v170' part so we will either need fallbacks here or it may be possible to get this info from vswhere. This can be a follow up.
|
Follow-up for the I changed the branch so that The branch now has a repo-controlled toolchain policy in
That keeps control in the repo when we intentionally move toolchains, instead of floating to whatever is newest on a machine or keeping separate hardcodes in multiple places. Pushed in Validation on this branch:
|
a06d110 to
5cc0459
Compare
|
Validation update after chasing the local failure:
|
jasonleenaylor
left a comment
There was a problem hiding this comment.
@jasonleenaylor reviewed 18 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on johnml1135).
f6f16a4 to
631fa8f
Compare
This PR splits out the Windows build-toolchain reliability work from the local liblcm source-mode changes.
Included in this PR:
nmake.exediscovery in the customMakeMSBuild taskNot included:
liblcmsource-mode wiringDistFiles/Parts/StandardParts.xmlStandardParts.xmlwas reviewed during the split and left out intentionally. It is not part of the toolchain work, and the added part IDs are not referenced elsewhere on the branch, so it does not belong in this standalone PR.This PR does not change how FieldWorks resolves liblcm.
Validation:
Build/Agent/FwBuildEnvironment.psm1,Build/Agent/Verify-FwDependencies.ps1, andBuild/Src/FwBuildTasks/Make.csThis change is