build: Add installation support and fix cmake files by madebr · Pull Request #168 · getsentry/sentry-native

madebr · 2020-03-11T06:03:05Z

This pr:

refactors the cmake build script a bit by:
- reordering,
- adding a cmake install configuration script,
- installing all libraries for shared and dynamic builds of the sentry library
fix the enumeration in the readme
fix a warning about redefinitions of some compile definitions
fix error about wrong calling convention

I tested this on Linux and MSVC

…NINGS macros

Amphaal · 2020-03-11T06:42:40Z

Changing WIN32 flags to MSVC are indeed required since Linux>Windows cross-compilation will fail to build Crashpad right now.

Swatinem

This is already a lot better!
I do have headaches and nightmares dealing with cmake actually. I’m happy to learn how to effectively use it!

Would be nice if you could check out the failures on CI. Most are just failing due to missing curl, linux fails in the unwinder. not exactly sure what causes that though.

CMakeLists.txt

gocarlos · 2020-03-11T12:58:30Z

@madebr are you working on conan-io/conan-center-index#1089?

madebr · 2020-03-11T14:32:25Z

@madebr are you working on conan-io/conan-center-index#1089?

Looks like it 😄
Let's first have upstream a nice cmake build script before adding it to CCI (and having to patch it).

madebr · 2020-03-11T16:05:55Z

@Amphaal

Changing WIN32 flags to MSVC are indeed required since Linux>Windows cross-compilation will fail to build Crashpad right now.

Indeed.

After a small fix to sentry's crashpad cmake scripts (replacing WIN32 with MSVC), the build fails at:
https://github.com/getsentry/crashpad/blob/8a2ddfc0a475bff9e25bfc7af693bac373adebe3/compat/win/sys/types.h#L18-L19

…rsion

madebr · 2020-03-12T02:29:40Z

I have some questions

~~Until now, I haven't touched libunwindstack and libstacktrace. Are these backends, equivalent to inproc and crashpad?~~
~~Is libunwindstack a backend or can it be used together with another backend?~~
~~It looks like libbacktrace is not used. There is only mention of a WITH_LIBBACKTRACE variable, but nothing else.~~
Is CURL support usable on all platforms? Windows, Macos, Linux, Android, IPhone, ...?

Amphaal · 2020-03-12T07:59:42Z

@Amphaal

Changing WIN32 flags to MSVC are indeed required since Linux>Windows cross-compilation will fail to build Crashpad right now.

Indeed.

After a small fix to sentry's crashpad cmake scripts (replacing WIN32 with MSVC), the build fails at:
https://github.com/getsentry/crashpad/blob/8a2ddfc0a475bff9e25bfc7af693bac373adebe3/compat/win/sys/types.h#L18-L19

...And that's only the tip of the iceberg 😆 I'll try to push a patch for the Sentry's Crashpad fork once I get the whole process working with MinGW.

Swatinem

lgtm, would like some input from @jan-auer as well.
I do agree that throwing a hard error when curl is not found is a great idea! Rather than silently compiling without, and having people complain why no reports are actually sent.

src/CMakeLists.txt

tests/__init__.py

tests/conftest.py

CMakeLists.txt

Swatinem · 2020-03-12T11:26:37Z

I just merged breakpad support in #169 , which will cause conflicts here.

Sorry for the churn, and I hope I can learn some cmake skills by looking at how you adapt this PR.

madebr · 2020-03-12T17:33:44Z

A word of caution, all unit tess have now CURL disabled.
But with some pytest magic, I think it can be easily conditionally enabled.

Swatinem · 2020-03-12T20:00:22Z

A word of caution, all unit tess have now CURL disabled.

I think thats fine for now. The tests don’t test http yet anyway. I want to get there eventually.

Swatinem · 2020-03-12T20:11:39Z

Thank you very much @madebr , awesome work!

madebr added 4 commits March 11, 2020 06:57

feat: refactor + add install support for cmake

913cd40

docs: fix enumeration of SENTRY_BACKEND options

4cebaaf

fix: redefinitions of NOMINMAX/WIN32_LEAN_AND_MEAN/_CRT_SECURE_NO_WAR…

31ebf40

…NINGS macros

fix: calling convention of worker thread should be __stdcall on MSVC

77f721a

Swatinem suggested changes Mar 11, 2020

View reviewed changes

CMakeLists.txt Show resolved Hide resolved

CMakeLists.txt Show resolved Hide resolved

CMakeLists.txt Outdated Show resolved Hide resolved

CMakeLists.txt Outdated Show resolved Hide resolved

gocarlos reviewed Mar 11, 2020

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

madebr added 4 commits March 11, 2020 16:10

fix: address comments from review

6f79e7d

fix: re-enable hidden visibility

a462434

ttt

f3a9e7d

fix: lowercase windows includes to allow mingw on linux

c405210

madebr added 3 commits March 11, 2020 17:09

fix: pass WINVER and NTDDI_VERSION to set minimum required windows ve…

ed48d41

…rsion

fix: add CURL_COMPILE_DEFINITIONS + crashpad_handler dependency

fb50a85

fix: disable CURL on azure pipeline CI + fix pytest

b3b7a26

madebr added 4 commits March 12, 2020 09:14

fix: move WITH_LIBUNWINDSTACK and WITH_LIBBACKTRACE to top of file

85ac23c

fix: fix include_directory of libunwindstack

2939fa5

fix: get_target_property values can be NONFOUND which causes errors

64639e6

fix: set output directory for android on azure pipelines

2c0a779

Swatinem mentioned this pull request Mar 12, 2020

Error in sentry_sync.c file #174

Closed

Swatinem approved these changes Mar 12, 2020

View reviewed changes

src/CMakeLists.txt Outdated Show resolved Hide resolved

tests/__init__.py Show resolved Hide resolved

tests/conftest.py Show resolved Hide resolved

CMakeLists.txt Outdated Show resolved Hide resolved

Swatinem requested a review from jan-auer March 12, 2020 10:00

jan-auer changed the title ~~Add installation support for cmake + fix various things~~ build: Add installation support and fix cmake files Mar 12, 2020

madebr added 2 commits March 12, 2020 17:42

Merge remote-tracking branch 'origin/master' into cmake

0181afd

fix: breakpad backend

ae29729

madebr added 3 commits March 12, 2020 18:07

fix: breakpad and crashpad tests were wrong

1b303ac

fix: -E flag is only needed for tests + remove PREPEND

52321a4

fix: disable CURL_SUPPORT in unit tests

6fa3d6d

fix: require c++11 for crashpad

baa6a22

Swatinem merged commit 7acd79f into getsentry:master Mar 12, 2020

madebr deleted the cmake branch March 25, 2020 11:25

Uh oh!

Conversation

madebr commented Mar 11, 2020

Uh oh!

Amphaal commented Mar 11, 2020

Uh oh!

Swatinem left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gocarlos commented Mar 11, 2020

Uh oh!

madebr commented Mar 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

madebr commented Mar 11, 2020

Uh oh!

madebr commented Mar 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Amphaal commented Mar 12, 2020

Uh oh!

Swatinem left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Swatinem commented Mar 12, 2020

Uh oh!

madebr commented Mar 12, 2020

Uh oh!

Swatinem commented Mar 12, 2020

Uh oh!

Swatinem commented Mar 12, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

madebr commented Mar 11, 2020 •

edited

Loading

madebr commented Mar 12, 2020 •

edited

Loading