[BUILD] Configure ryml to use exceptions when using CMAKE#3852
Conversation
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3852 +/- ##
=======================================
Coverage 89.99% 89.99%
=======================================
Files 225 225
Lines 7170 7170
=======================================
Hits 6452 6452
Misses 718 718 🚀 New features to boost your workflow:
|
|
Waiting for my employer to hear back regarding the CLA; will get back to this as soon as I have the required info. |
|
@krook can you check here? I've added @BugelNiels to our org, how do we trigger a recheck for the CLA? |
a7d0e4f to
1f42168
Compare
Nevermind, force push did it and i see the check now passes. |
|
@rochaporto thanks for the effort! I did a rebase of the branch and all seems good now. |
There was a problem hiding this comment.
Thanks. LGTM.
Side note (not related to this PR): unlike the core API/SDK which were designed to compile with -fno-exceptions, the configuration module relies on exceptions by design. @marcalff - curious if there are any thoughts on this for the future, or if this is the intended direction for the configuration module?
The configuration code relies on a yaml parser, and currently requires to use exceptions, this is intended. To have the same feature without exceptions:
Not sure if this is feasible with the yaml parser, and in any case this will make the code in The feature is optional, and not available without exception support. |
Fixes #3851
Changes
When building
opentelemetry-cppand ryml is fetched from the repo using CMake, ryml will useabort()when it encounters errors. This results in unintended crashes and prevents the calling application from gracefully dealing with these errors. This PR ensures that when we fetch ryml, we configure it to use exceptions instead.For significant contributions please make sure you have completed the following items:
CHANGELOG.mdupdated for non-trivial changes