Default MSVC static CRT for static yaml-cpp builds - #1484
Conversation
Signed-off-by: harleywilsoneng <harleywilsoneng@users.noreply.github.com>
Signed-off-by: harleywilsoneng <harleywilsoneng@users.noreply.github.com>
Signed-off-by: harleywilsoneng <harleywilsoneng@users.noreply.github.com>
Signed-off-by: harleywilsoneng <harleywilsoneng@users.noreply.github.com>
Signed-off-by: harleywilsoneng <harleywilsoneng@users.noreply.github.com>
Signed-off-by: harleywilsoneng <harleywilsoneng@users.noreply.github.com>
|
Amazing! This is highly appreciated! I took the liberty to convert it to a "draft" PR. Please convert it back to a normal PR when it is ready to be reviewed. |
|
Thanks @SGSSGene — marking this ready for review. CI is green on the latest push (including Windows static consumer CRT alignment in |
|
Thanks @SGSSGene — marking this ready for review. CI is green on the latest push (including Windows static consumer CRT alignment in |
SGSSGene
left a comment
There was a problem hiding this comment.
Thank you for tackling this issue!
I only have a small remark about the README. Please feel free to reformulate my suggestion it in any other way!
| the dynamic CRT (`/MD`) instead. When linking yaml-cpp statically into your project, define | ||
| `YAML_CPP_STATIC_DEFINE` in your target (or consume the `yaml-cpp::yaml-cpp` imported target, | ||
| which sets it automatically). |
There was a problem hiding this comment.
I would rewrite this slightly, It sounds like by default everyone should define YAML_CPP_STATIC_DEFINE.
I'd rather recommend consume target yaml-cpp::yaml-cpp and have the information about YAML_CPP_STATIC_DEFINE as a side note.
| the dynamic CRT (`/MD`) instead. When linking yaml-cpp statically into your project, define | |
| `YAML_CPP_STATIC_DEFINE` in your target (or consume the `yaml-cpp::yaml-cpp` imported target, | |
| which sets it automatically). | |
| the dynamic CRT (`/MD`) instead. When using cmake consume `yaml-cpp::yaml-cpp`, e.g.: `target_link_libraries(your_project_target yaml::yaml`. | |
| (If cmake is not used you must define `YAML_CPP_STATIC_DEFINE` for your target.) | |
There was a problem hiding this comment.
Good catch — updated the README to lead with yaml-cpp::yaml-cpp, and mention YAML_CPP_STATIC_DEFINE only as the non-CMake fallback.
Signed-off-by: harleywilsoneng <harleywilsoneng@users.noreply.github.com>
Fixes #1354
Summary
YAML_MSVC_SHARED_RTto${YAML_BUILD_SHARED_LIBS}so static yaml-cpp builds use/MTby default on MSVC.YAML_CPP_STATIC_DEFINEfor consumers linking the static library.Test plan
/MTis selected by default.-DYAML_MSVC_SHARED_RT=ONand verify/MDis still available.