Repository navigation
v26.2 Packaging Touch-ups - #11826
Conversation
…gh in the macOS installer name [decent_ci_skip]
mitchute
added
the
DoNotPublish
|
This built packages on the first try here: https://github.com/NatLabRockies/EnergyPlus/releases/tag/v26.2.0-RC3.1 |
|
|
||
| - name: arm64 | ||
| os: windows-11-arm | ||
| os: windows-11-vs2026-arm |
There was a problem hiding this comment.
right.
Two points for future though:
- We should probably use Ninja here too, like we do for pull request. (Perhaps this should use the setup-system in-repo action. Pro: less repetition; Con: more obfuscated)
- It'd probably be a good idea to transition the x64 to windows-2025-vs2026 sooner than later. GHA supports at most two releases, so I suppose we have about a year until a Windows server 2028 is released
https://github.com/actions/runner-images/blob/main/README.md#software-and-image-support
We support (at maximum) 2 GA images and 1 beta image at a time. We begin the deprecation process of the oldest image label once the newest OS image label has been released to GA.
| Python_EXECUTABLE="${{ steps.setup-python.outputs.python-path }}" | ||
| Python_EXECUTABLE="${Python_EXECUTABLE//\\//}" | ||
| cmake -G "Visual Studio 17 2022" -A ${{ matrix.vs-generator }} \ | ||
| cmake -G "${{ matrix.cmake-generator }}" -A ${{ matrix.vs-generator }} -T "${{ matrix.cmake-toolset }}" \ |
There was a problem hiding this comment.
I doubt you have to explicitly pass the toolset?
| # Visual Studio 2026 defaults to the v145 toolset. Continue building Windows | ||
| # packages with v143 while using its native ARM64 generator. | ||
| MSVC_TOOLSET_MAPPING = { | ||
| MSVC.V18: "v143", | ||
| } |
There was a problem hiding this comment.
Hum ok. I don't think that's really necessary though.
| toolset = msvc_version.toolset_name() | ||
| if toolset: | ||
| command_line.extend(["-T", toolset]) |
| def toolset_name(self) -> Optional[str]: | ||
| """Get an explicitly selected CMake toolset, when needed.""" | ||
| return MSVC_TOOLSET_MAPPING.get(self) |
There was a problem hiding this comment.
Right, dict.get(val, default=None)
| choices=list(MSVC), | ||
| default=None, | ||
| help="For MSVC builds, this is the Visual Studio version ('16', '17')", | ||
| help="For MSVC builds, this is the Visual Studio version ('16', '17', '18')", |
There was a problem hiding this comment.
nitpick
| help="For MSVC builds, this is the Visual Studio version ('16', '17', '18')", | |
| help=f"For MSVC builds, this is the Visual Studio version ({', '.join(repr(str(m.value)) for m in MSVC)})", |
jmarrec
left a comment
jmarrec
left a comment
There was a problem hiding this comment.
This is all good and can merge as-in.
I added one nitpick suggestion to avoid hardcoding the help text.
My other point is that we probably don't need to specifically use an older toolset, but it's a conservative strategy, which makes kinda sense given how close we are to release date. (The other point could be that the newer toolset wouldn't build due to new warnings or whatnot, but I have recently tested that with success locally)
I would remove the explicit toolset v143 right after release.
I don't think there is any incompatibly risk.
The MSVC 14.x toolsets (v140 through v145) share the same ABI and DLL names (vcruntime140.dll, msvcp140.dll)
The risk would be shipping a binary that relies on newer CRT libs while the running OS would have an older VC++ Redistributable.
But we redistribute the CRT libs already: see InstallRequiredSystemLibraries + CMAKE_INSTALL_UCRT_LIBRARIES cmake/Install.cmake#L622-L645
| pretty: "Mac x64" | ||
| - os: macos-15 | ||
| macos_dev_target: 14.0 | ||
| macos_dev_target: '14.0' |
There was a problem hiding this comment.
Right, that's an annoying quirk.
unquoted 14.0 in YAML is a float. GitHub Actions turns it into the number 14 and then into the string "14" when it interpolates ${{ matrix.macos_dev_target }}.
|
Thanks @jmarrec. Given where we are at, I'm going to merge this one as-is. Let's add these issues to the to-do list for right after release. |
2 participants
Pull request overview
Description of the purpose of this PR
Pull Request Author
Reviewer