-
Notifications
You must be signed in to change notification settings - Fork 6.1k
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
[libmodplug] Fix WASM build by pinning C++ standard to 11 #37090
Conversation
ports/libmodplug/portfile.cmake
Outdated
-DCMAKE_CXX_STANDARD=11 | ||
) |
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.
-DCMAKE_CXX_STANDARD=11 | |
) | |
-DCMAKE_CXX_STANDARD=${CXX_STANDARD} | |
) | |
if(VCPKG_TARGET_ARCHITECTURE STREQUAL "wasm32") | |
set(CXX_STANDARD 11) | |
endif() |
Regarding specifying the c++ version, especially the lower standard approach, I think we should filter it。
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.
If we do this, I would probably go with VCPKG_TARGET_IS_EMSCRIPTEN
, since this is specific to the compiler. Off top of my head, we'd probably also have to move it above the configure.
I'm actually a bit surprised that it builds on all other major platforms as-is though, especially given that Emscripten just runs Clang under the hood. Maybe the major compilers are more lenient when it comes to accepting older C++ standards by default?
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.
Upstream has no special instructions for the C++ standard, so a lower standard may be accepted. :)
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.
It's better to use VCPKG_TARGET_IS_EMSCRIPTEN
.
Note: I will be converting your PR to draft status. When you respond, please revert to "ready for review". |
…37090) Fixes microsoft#37089 The port uses the register storage class specifier which was removed in C++17, pinning the standard to C++11 solves this. - [x] Changes comply with the [maintainer guide](https://github.com/microsoft/vcpkg-docs/blob/main/vcpkg/contributing/maintainer-guide.md). - [x] SHA512s are updated for each updated download. - [x] The "supports" clause reflects platforms that may be fixed by this new version. - [x] Any fixed [CI baseline](https://github.com/microsoft/vcpkg/blob/master/scripts/ci.baseline.txt) entries are removed from that file. - [x] Any patches that are no longer applied are deleted from the port's directory. - [x] The version database is fixed by rerunning `./vcpkg x-add-version --all` and committing the result. - [x] Only one version is added to each modified port's versions file.
[libmodplug] Fix WASM build by pinning C++ standard to 11 (microsoft#37090)
Fixes #37089
The port uses the register storage class specifier which was removed in C++17, pinning the standard to C++11 solves this.
./vcpkg x-add-version --all
and committing the result.