-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[sentry-native] Add 0.7.8 #24720
[sentry-native] Add 0.7.8 #24720
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Hi @uilianries ! Do you have any idea why the build fails because of the range library ? Requiring C++ 20 for 0.7.7 and upper is not enough ? |
@ErniGH Do you have a clue to help me understand what is going on with these error ? 0.7.9 is out. Shall I push a new version ? |
return "17" | ||
return "17" if Version(self.version) < "0.7.7" else "20" |
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.
Hi @MartinDelille , I've been testing the PR on my machine without using this version requirement, and both on Mac and Linux, version 0.7.8 works for me. I've checked upstream, and it seems that the version is 17.
https://github.com/getsentry/sentry-native/blob/master/CMakeLists.txt#L33
Maybe we can just undo this change and see how it works.
As for the other change from clang 5.1 to 10.0, I can't find a reason for it. Could you provide any justification for this change?
Sorry for the delay :(
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.
Just revert the related change but I had the problem before. I also don't reproduce locally. I suspected that the issue was due to the fact that crashpad is requiring C++20 so I updated the requirements without success.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Uilian Ries <uilianries@gmail.com>
There is no newer Apple Clang version than 13.0 in CCI, but testing locally with Apple Clang 15.0 is working fine: sentry-native-0.7.0-apple-clang-15.log The Cpp Reference says Apple Clang 14.0 should be enough for C++20 bit_cast: |
Conan v1 pipeline ✔️All green in build 7 (
Conan v2 pipeline ✔️
All green in build 7 (
|
Hooks produced the following warnings for commit 247d4a4sentry-native/0.7.6@#2958881c5014ce4f81f05a4ecc68365c
sentry-native/0.7.8@#fe5f3770bd1e73592172ece78705a2c9
sentry-native/0.6.6@#4bb33700f1bf78c93e5f3bc41f458575
sentry-native/0.7.5@#2ce2aa8d31ac80438d7d7b10954625a3
sentry-native/0.5.4@#96d5d8039560078998398d328b620fcd
|
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.
Hey @MartinDelille , it looks like it's finally working. It seems that the new version indeed needed C++ 20 to support Concepts
and bit_cast
, @uilianries saw it.
Many thanks to both of you. ❤️
Summary
Add version : sentry-native/0.7.7
Motivation
Details