fix(windows): pin Conan's CMake generator to match the project's - #3605
Open
fzipi wants to merge 2 commits into
Open
fix(windows): pin Conan's CMake generator to match the project's#3605fzipi wants to merge 2 commits into
fzipi wants to merge 2 commits into
Conversation
conan install (line before cmake --fresh) let Conan auto-detect its own CMake generator for dependencies built from source (--build=missing, e.g. yajl), based on whatever Visual Studio version its auto-detected profile picked up. On a machine with more than one VS version installed, that can differ from the "Visual Studio 17 2022" generator this script explicitly requests for the main project two lines later. yajl/2.1.0's own CMakeLists.txt still does `cmake_policy(SET CMP0026 OLD)`, a policy newer CMake versions have dropped OLD-behavior support for entirely. Under a newer, auto-detected generator/CMake (observed: "Visual Studio 18 2026" on a GitHub-hosted windows-2025 runner that had more than one VS install), yajl's configure step hard-errors on that unsupported policy before it ever gets to build. Under the generator this project actually targets, it configures fine. Fix: explicitly pass the same generator to `conan install` via `-c tools.cmake.cmaketoolchain:generator`, so dependency builds use the identical toolchain as the main project rather than whatever a given machine's Conan profile auto-detection happens to prefer. Reported downstream via ModSecurity-nginx's Windows CI (owasp-modsecurity/ModSecurity-nginx#388 built this via vcbuild.bat and hit this exact failure). Fixes #3604. Note on validation: this repo's own PR-triggered Windows CI (ci.yml/ci_new.yml) runs on windows-2022, which -- as far as I can tell -- only has one Visual Studio version installed, so it's unlikely to reproduce the generator-mismatch this fixes even without the fix. The pin is a no-op when only one matching VS version is present either way, so it shouldn't regress that CI; real confirmation that this resolves the original failure will need a windows-2025-class runner (or a maintainer's machine with more than one VS version installed). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The previous commit fixed the generator mismatch but left the resulting "Visual Studio 17 2022" string duplicated across the Conan and CMake invocations -- exactly the kind of duplication that could drift out of sync on a future VS upgrade and silently reintroduce the same class of bug. Single source of truth via VS_GENERATOR instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes the Windows build script (vcbuild.bat) use a single, consistent CMake generator for both Conan-built dependencies (built from source under --build=missing) and the main CMake configure step, preventing generator/toolchain mismatches on machines with multiple Visual Studio installations.
Changes:
- Introduces a
VS_GENERATORvariable as the single source of truth for the targeted Visual Studio/CMake generator. - Pins Conan’s
CMakeToolchaingenerator via-c tools.cmake.cmaketoolchain:generator=...to match the project’s CMake-Ggenerator selection.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3604.
conan install(invcbuild.bat) let Conan auto-detect its own CMake generator for dependencies built from source (--build=missing, e.g.yajl), based on whatever Visual Studio version its auto-detected profile picks up. On a machine with more than one VS version installed, that can differ from the"Visual Studio 17 2022"generator this script explicitly requests for the main project two lines later viacmake --fresh .. -G "Visual Studio 17 2022".yajl/2.1.0's ownCMakeLists.txtstill doescmake_policy(SET CMP0026 OLD), a policy newer CMake versions have droppedOLD-behavior support for entirely. Under a newer, auto-detected generator/CMake (observed:"Visual Studio 18 2026"on a GitHub-hostedwindows-2025runner that had more than one VS install),yajl's configure step hard-errors on that unsupported policy before it ever gets to build. Under the generator this project actually targets, it configures fine.conan installvia-c tools.cmake.cmaketoolchain:generator, so dependency builds use the identical toolchain as the main project rather than whatever a given machine's Conan profile auto-detection happens to prefer.VS_GENERATORvariable referenced by both the Conan and CMake invocations, so a future VS-version bump only needs to change one line instead of two that must be kept in sync.Reported downstream via
ModSecurity-nginx's Windows CI (owasp-modsecurity/ModSecurity-nginx#388 built this viavcbuild.batand hit this exact failure).Note on validation
This repo's own PR-triggered Windows CI (
ci.yml/ci_new.yml) runs onwindows-2022, which — as far as I can tell — only has one Visual Studio version installed, so it's unlikely to reproduce the generator-mismatch this fixes even without the fix. The pin is a no-op when only one matching VS version is present either way, so it shouldn't regress that CI; real confirmation that this resolves the original failure will need awindows-2025-class runner (or a maintainer's machine with more than one VS version installed).Test plan
-c tools.cmake.cmaketoolchain:generatorconf key against Conan'sCMakeToolchain._get_generator()source to confirm it's the correct override point.ci.yml/ci_new.yml(windows-2022) still pass — expected to be a no-op there.windows-2025-class runner (or a machine with multiple VS installs) that this resolves the originalCMP0026failure.