-
Notifications
You must be signed in to change notification settings - Fork 44
Update ci to update windows and ubuntu runners #249
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The build using arch: x86_64 using mingw64 on windows-2025 fails with a familiar error from CoinUtils PR 245 and CoinUtils PR 246. Notably, this commit -- thanks @tkralphs -- fixed this for windows-2022 runners. Any idea how to this could be failing only for windows-2025 runners for arch: x86_64 using mingw64? |
|
It can happen that a missing explicit include of a header becomes a problem only after updates to standard header files. Since CoinRational.hpp uses |
5eac273 to
5961168
Compare
.github/workflows/windows-ci.yml
Outdated
| run: | | ||
| msvc_version=${VisualStudioVersion%.*} | ||
| echo "package_suffix=w64-msvc${msvc_version}${{ matrix.suffix }}" >> $GITHUB_ENV | ||
| echo "package_suffix=${{ matrix.os}}-msvc${msvc_version}${{ matrix.suffix }}" >> $GITHUB_ENV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The OS name is specific to Github Actions and I guess it's not really indicating anything about the platform or toolchain. The w64 indicated 64-bit, now the string doesn't really have a meaning aside from just that this is windows. I guess assuming 64-bit across the board is reasonable, but then should we just maybe have win here? Or we could just leave it as w64?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, I had to add the OS to get unique artefact names, not because of toolchain differences and certainly not to give the impression that one package is for WS 2022 and the other for WS 2025. For regular ci builds I guess this is not such a problem, but this shouldn’t make it into different release artefacts (IMO).
Does this make it too difficult for the release packages?
If so, I can just reduce the matrix and keep only one toolchain/debug combo, not duplicated per OS. I like to keep duplicate combinations per OS exactly to catch unexpected issues like the cstdint/stdint identified for WS2025.
Sure, I will change w64 to win.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I would keep only one toolchain/debug combo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The artefact name changes have been reverted (no more OS name), and the build matrix was reduced to ensure the artefact names are still unique. In the revert, I left the old "w64" so didn't change it to "win".
For the "msvc" builds, the Visual Studio version is used in the yml to build the artefact name (package name). For windows-2019, this was VS 2019 (v16), but for windows-2022 and also windows-2025 this is VS 2022 (v17). Therefore, 2022 and 2025 builds with "arch: msvc" will both come out "msvc17". To ensure uniqueness of these artefact names while keeping both 2022 and 2025 builds, I made the 2022 msvc a "debug: true" build.
Same changes pushed to Cbc PR 728.
Once we're happy with the outcome I'll push the same to all other projects. Comments?
The windows-2022 and -2025 images both use Visual Studio 2022 (v17). This is used in the original package name for MSVCxx.
|
Everything looks good, did you say you wanted to open all the PRs and test before merging any of them? |
The windows-2022 and -2025 images both use Visual Studio 2022 (v17). This is used in the original package name for MSVCxx.
Yes, that was the idea.. |
See also COIN-OR-OptimizationSuite Issue 32 and Issue 33:
These changes are done for linux-ci, windows-ci (using coinbrew) and windows-msvs-ci (using Visual Studio solutions) workflows.