[spdlog] fix wchar support#24830
Conversation
|
|
There was a problem hiding this comment.
This is probably needed for integrations other than CMake. If you are annoyed by the warning do a #undef first.
There was a problem hiding this comment.
Is the mission and requirements of vcpkg to support its use outside cmake?
Please do point me to related requirements, as since vcpkg is based on cmake, that seems an unusual request.
When such is documented, I am happy to revisit the fixes as well as additional changes that may be needed. I doubt I would ever undef in my code as that would be special casing an issue that is actually in vcpkg. But like I wrote, I can revisit if you point me to related requirements
There was a problem hiding this comment.
Is the mission and requirements of vcpkg to support its use outside cmake?
Also internally it needs to work with meson/autotools and other build systems. So the headers need to define the required stuff without knowing about possible defines from cmake.
There was a problem hiding this comment.
ok, thanks for info. 👍
I will update this PR with changes so that someone can point to the vcpkg/install/........ directory manually without cmake and the needed defines will be present. I think I have to add one or two more.
There was a problem hiding this comment.
According to #18041, SPDLOG_WCHAR_SUPPORT and SPDLOG_WCHAR_FILENAMES are both valid and serve different purposes.
This change is invalid.
There was a problem hiding this comment.
There are two separate issues then...
- Today's port does not work correctly on non-cmake integration. Why? Because while there is a mapping of
SPDLOG_WCHAR_SUPPORTas a vcpkg feature, there is nothing in the port which changes the tweakme.h to make it work outside cmake. - Unicode filenames. Also known as wchar filenames. Unfortunately, the existing spdlog project doesn't overload both...it is one or the other. Therefore, I guess this falls into the "alternatives exclusion" rule for vcpkg. If that is true, then I will just do this on my local branch of vcpkg.
- correct vcpkg feature `wchar` - add support for integrations other than cmake - update port-version + vcpkg x-add-version spdlog
diablodale
force-pushed
the
fix-spdlog-wchar
branch
from
2d47143 to
349effc
Compare
|
force pushed with feedback changes |
LilyWangLL
added
the
category:port-feature
LilyWangLL
added
the
info:needs-maintainer-attention
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Awesome, thanks for the PR! Looks great after having taken in all the feedback :) |
5 participants
What does your PR fix?
Errant code/bugs in the
spdlogport itselfvcpkg_cmake_configure().Such duplication leads to compiler errors+warning of dups/conflicts
Which triplets are supported/not supported? Have you updated the [CI baseline]
No change in triplet support.
Does your PR follow the maintainer guide?
yes
If you have added/updated a port: Have you run
./vcpkg x-add-version --alland committed the result?yes