Use Ninja in Windows CI and explicitly set the VS toolset version. #4759
Conversation
This file contains 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
|
This pull request has been linked to Shortcut Story #41556: Fix windows 2022 runners.. |
teo-tsirpanis
force-pushed
the
teo/vcpkg-pin-vs-toolset
branch
from
775b9e7
to
73a7c1b
Compare
teo-tsirpanis
force-pushed
the
teo/vcpkg-pin-vs-toolset
branch
from
9adc7ab
to
64ea96e
Compare
teo-tsirpanis
force-pushed
the
teo/vcpkg-pin-vs-toolset
branch
3 times, most recently
from
0f39834
to
2629a1a
Compare
teo-tsirpanis
force-pushed
the
teo/vcpkg-pin-vs-toolset
branch
from
2629a1a
to
4384dbd
Compare
teo-tsirpanis
changed the title
teo-tsirpanis
changed the title
teo-tsirpanis
force-pushed
the
teo/vcpkg-pin-vs-toolset
branch
3 times, most recently
from
df405db
to
3a0d1f8
Compare
teo-tsirpanis
force-pushed
the
teo/vcpkg-pin-vs-toolset
branch
from
3a0d1f8
to
0471ee7
Compare
teo-tsirpanis
force-pushed
the
teo/vcpkg-pin-vs-toolset
branch
from
0471ee7
to
a4adb0d
Compare
ihnorton
approved these changes
Feb 26, 2024
|
(I'm ok w/ using Ninja for some builds, but we should make sure we continue to test and support normal solution builds, because that is how most windows devs will build by default) |
Assuming CMake does its job correctly there should not be any difference. And vcpkg will build with Ninja on all platforms either way. |
ihnorton
approved these changes
Feb 26, 2024
There was a problem hiding this comment.
Assuming CMake does its job correctly there should not be any difference.
With cmake (and in general) "trust, but verify". I'm ok with this since we'll still have the VS solution path for 2019, but when we remove that we should have some job that doesn't use Ninja directly for the main TileDB build.
This was referenced Feb 26, 2024
KiterLuc
pushed a commit
that referenced
this pull request
Feb 27, 2024
KiterLuc
pushed a commit
that referenced
this pull request
Feb 28, 2024
8 tasks
Shelnutt2
pushed a commit
that referenced
this pull request
Apr 21, 2024
Shelnutt2
pushed a commit
that referenced
this pull request
Apr 21, 2024
Shelnutt2
pushed a commit
that referenced
this pull request
Apr 21, 2024
Shelnutt2
pushed a commit
that referenced
this pull request
Apr 21, 2024
Shelnutt2
added a commit
that referenced
this pull request
Apr 21, 2024
KiterLuc
pushed a commit
that referenced
this pull request
Apr 22, 2024
[SC-44745](https://app.shortcut.com/tiledb-inc/story/44745/remove-ninja-workaround-for-windows-builds) In #4759, I switched the VS2022 builds to use Ninja due to errors caused by MSVC toolset version mismatch but could not make the VS2019 builds use Ninja as well and resorted to some conditional logic to support both Ninja and MSBuild. I subsequently found out what the problem with VS2019 was (the action to install Ninja was running before the checkout action, and the latter deleted the downloaded Ninja), and this PR enables Ninja on all Visual Studio versions and deletes the conditional logic, simplifying the workflow. Ninja is preferred to MSBuild because of the superior parallelization capabilities. For coverage, MSBuild is still being used on nightly builds. --- TYPE: NO_HISTORY
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.
x-refs for future reference
Original PR Description
[SC-41556]
Since the GHA
windows-2022runners updated their version of Visual Studio yesterday, we have been seeing errors of the formerror LNK2019: unresolved external symbol _Thrd_sleep_for. After some investigation, the cause of the errors was found to be that the vcpkg dependencies were built with the new MSVC toolset (14.39), while the main CMake project was built with the old one (14.38)1.After efforts to make the main project build with the 14.39 toolset (by passing
-T v143,version=14.39or-DCMAKE_VS_PLATFORM_TOOLSET_VERSION=14.39) were unsuccessful, this PR switches to using the Ninja generator instead of Visual Studio. With Ninja the toolset version is specified by a GitHub action that sets the necessary environment variables.TYPE: BUILD
DESC: Fix linker errors when building with MSVC
Footnotes
If the opposite were to happen, it would be supported. Objects built with varying versions of MSVC toolsets are binary-compatible with each other, but the final linking must be done with the newest version. ↩