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

Split Android and Cocoa bindings into separate projects #1983

Merged
merged 8 commits into from
Oct 12, 2022

Conversation

mattjohnsonpint
Copy link
Contributor

@mattjohnsonpint mattjohnsonpint commented Oct 12, 2022

For the net6.0-android, net6.0-ios, and net6.0-maccatalyst targets, we currently treat the main Sentry project as both a regular class library and as the binding library for the Android and Cocoa SDKs. While this compiles, it seems to not be the normal expectation in documentation and tooling support.

To make things worse, Rider's solution analysis fails if a binding library is loaded at all. I reported that here: https://youtrack.jetbrains.com/issue/RIDER-83532. The workaround is to unload the binding libraries from the solution, or exclude them with a solution filter. We can't do that - since we don't have them broken out into their own projects.

Presently, just with the SentryCore.slnf solution filter, there are over 100K analysis errors by default. Most of them are in source generated automatically from the bindings. If I add src/Sentry/obj to the exclusions list, then it ignores those and the number is reduced to 338 errors in the actual code. This makes it very difficult to work with code that uses the bindings, as there is virtually no Intellisense. It also makes it hard to see anything important in the solution-wide analysis - whether from Andriod/iOS or not.

Example Screenshot (with src/Sentry/obj excluded):

image

This PR separates the binding libraries into their own projects, Sentry.Bindings.Android and Sentry.Bindings.Cocoa. It adds them to the solution, but keeps them out of most of the filters. The result for SentryCore.slnf is a clean analysis and working Intellisense on platform-specific code. No exclusion of src/Sentry/obj is necessary now.

image

This PR also moves the native libraries to /lib to keep them out of the .NET projects, simplifying a few things. There are some other minor associated fixes as well.

There is a tradeoff with this approach though - We'll have two new NuGet packages to publish. The Sentry package will take a dependency on a new Sentry.Bindings.Android package for the net6.0-android target, and on a new Sentry.Bindings.Cocoa package for the net6.0-ios and net6.0-maccatalyst targets.

I investigated the possibility of keeping everything in the Sentry package. However, due to the current state of the dotnet ecosystem, this isn't possible yet.

So - we go with separate packages for the binding projects. We can use InternalsVisibleTo to ensure they are only used as dependencies for Sentry, and we can make sure the package descriptions and readme files direct users to consume Sentry or Sentry.Maui.

#skip-changelog

@mattjohnsonpint mattjohnsonpint merged commit 77ccdb2 into main Oct 12, 2022
@mattjohnsonpint mattjohnsonpint deleted the split-ios-android branch October 12, 2022 20:50
@mattjohnsonpint
Copy link
Contributor Author

This is working. See iOS/Android dependencies under 3.23.0 release
https://www.nuget.org/packages/Sentry/3.23.0#dependencies-body-tab

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.

2 participants