Skip to content
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

fix: upload crashes on linux #75

Merged

Conversation

BogdanLivadariu
Copy link

as per description and comments on: getsentry/sentry-native#773

as per description and comments on:  getsentry/sentry-native#773
@BogdanLivadariu
Copy link
Author

tested changes with a custom build of sentry-native on the latest release(0.5.3) and crashes do get uploaded now on linux hosts

Copy link
Member

@Swatinem Swatinem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you have to find / declare libcurl as a dependency to crashpad now.
For sentry-native that just works as it depends on curl itself already.

@Swatinem
Copy link
Member

You also have to add the found curl package to the dependencies :-)

@supervacuus can help you with that

@supervacuus
Copy link
Collaborator

Hi @BogdanLivadariu, thanks for starting this off.

@supervacuus
Copy link
Collaborator

@BogdanLivadariu, are you planning to finish this PR?

@BogdanLivadariu
Copy link
Author

Hi @supervacuus
I would ... but my experience with cmake is not at its best, if you could guide me on where to
add the found curl package to the dependencies that would be great

@supervacuus
Copy link
Collaborator

supervacuus commented Jan 5, 2023

@BogdanLivadariu, you can start by applying my change suggestions, which do most of the work. I just saw that I forgot to request them.

util/CMakeLists.txt Outdated Show resolved Hide resolved
util/CMakeLists.txt Outdated Show resolved Hide resolved
BogdanLivadariu and others added 2 commits January 12, 2023 16:15
Co-authored-by: Mischan Toosarani-Hausberger <mischan@abovevacant.com>
Co-authored-by: Mischan Toosarani-Hausberger <mischan@abovevacant.com>
@BogdanLivadariu
Copy link
Author

hi @supervacuus does this looks good to go ?
should I squash the commits ?

thanks for the suggestions

@supervacuus
Copy link
Collaborator

LGTM @BogdanLivadariu; no need to squash, we will do this. Thx!

@supervacuus
Copy link
Collaborator

@Swatinem, can I ask you to merge this?

@Swatinem Swatinem merged commit 4edb2bb into getsentry:getsentry Jan 17, 2023
@bsergean
Copy link

Could we do the if(NOT CURL_FOUND) around the find_package trick ?

https://github.com/bsergean/crashpad/pull/1/files

@supervacuus
Copy link
Collaborator

Could we do the if(NOT CURL_FOUND) around the find_package trick ?

There is no "trick" involved :-) This is how FindCURL is supposed to work in layered CMake projects. A few other changes are also needed before we update the submodule in the Native SDK, which I will finish today.

@bsergean
Copy link

bsergean commented Jan 25, 2023 via email

supervacuus added a commit to getsentry/sentry-native that referenced this pull request Feb 7, 2023
- Switch Crashpad transport on Linux to use libcurl ([#803](#803), [crashpad#75](getsentry/crashpad#75), [crashpad#79](getsentry/crashpad#79))
- Avoid accidentally mutating CONTEXT when client-side stack walking in Crashpad ([#803](#803), [crashpad#77](getsentry/crashpad#77))
- Fix various mingw compilation issues ([#794](#794), [crashpad#78](getsentry/crashpad#78))
- Updated Crashpad backend to 2023-02-07. ([#803](#803), [crashpad#80](getsentry/crashpad#80))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants