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

macOS native SDK integration #2625

Closed
wants to merge 13 commits into from

Conversation

bruno-garcia
Copy link
Member

@bruno-garcia bruno-garcia commented Sep 16, 2023

An experiment to introduce specific mac target and bundle the native SDK (cocoa or sentry-native)

TODO:

  1. Build sentry-cocoa for desktop and remove the bundled dylib
  2. Compile the bridge code as part of CI
  3. Make sure native symbol upload include the native bits needed
  4. Portable PDB upload works: Sentry CLI crashes when running debug-files upload sentry-cli#1746
  5. test with NativeAOT (on a Mac, requires .NET 8 build with dotnet 8 sdk #2622)

First successful crash captured natively: https://sentry-sdks.sentry.io/issues/4483237642/?project=5428537&query=is%3Aunresolved&referrer=issue-stream&statsPeriod=90d&stream_index=0

@github-actions
Copy link
Contributor

github-actions bot commented Sep 16, 2023

Fails
🚫 Please consider adding a changelog entry for the next release.
Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- macOS native SDK integration ([#2625](https://github.com/getsentry/sentry-dotnet/pull/2625))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against 9316c46

@@ -18,12 +18,14 @@
</PropertyGroup>

<!-- NO_MOBILE can be passed in via an environment variable or a build property to disable all mobile targets -->
<PropertyGroup Condition="'$(NO_MOBILE)' == 'true'">
<PropertyGroup Condition="'$(NO_MOBILE)' == ''">
Copy link
Collaborator

Choose a reason for hiding this comment

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

We probably don't want this checked into source control

Copy link
Member Author

Choose a reason for hiding this comment

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

We might need a second sln after all. Unles we find another way to avoid this.

Can we have a symlink? Sentry.NoMobile.sln is a symlink to Sentry.sln? So you can add the checks back on the msbuild files based on sln file name?

@vaind vaind changed the base branch from main to feat/4.0.0 October 6, 2023 06:28
@vaind vaind changed the title macos native crash reporting Native SDK integration Oct 6, 2023
@vaind vaind changed the title Native SDK integration macOS native SDK integration Oct 6, 2023
@vaind
Copy link
Collaborator

vaind commented Oct 6, 2023

I've originally wanted to update this PR and make the fixes but it's not possible to build macos native AOT apps at this time (before .net 8 RC2), so I'm going to fork this further and only do Windows+sentry-native in the first step.

@vaind
Copy link
Collaborator

vaind commented Oct 30, 2023

Although #2767 brings native AOT debug images by using sentry-native for macOS (because it's much easier then getting cocoa working and bundled properly), I'll leave this open for the overall approach taken with all the scope sync, etc. We may revisit this later when there are features only sentry-cocoa can fulfill.

@bruno-garcia
Copy link
Member Author

Replaced by #2770

@bruno-garcia bruno-garcia deleted the feat/native-crash-reporting branch November 3, 2023 20:09
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.

3 participants