Skip to content

Comments

Support 16 KB page sizes on Android - libsentrysupplemental#3723

Merged
jamescrosswell merged 45 commits intoversion-5.0.0from
align-libsentry
Nov 5, 2024
Merged

Support 16 KB page sizes on Android - libsentrysupplemental#3723
jamescrosswell merged 45 commits intoversion-5.0.0from
align-libsentry

Conversation

@jamescrosswell
Copy link
Collaborator

@jamescrosswell jamescrosswell commented Oct 31, 2024 •
edited
Loading

Resolves #3633

Summary

When targeting net9.0-android we get build warnings from Xamarin.Android.Common.Debugging.targets when it tries to build the apk:

warning XA0141: NuGet package '<unknown>' version '<unknown>' contains a shared library 'libsentry.so' which is not correctly aligned. See https://developer.android.com/guide/practices/page-sizes for more details
warning XA0141: NuGet package '<unknown>' version '<unknown>' contains a shared library 'libsentry-android.so' which is not correctly aligned. See https://developer.android.com/guide/practices/page-sizes for more details
warning XA0141: NuGet package '<unknown>' version '<unknown>' contains a shared library 'libsentrysupplemental.so' which is not correctly aligned. See https://developer.android.com/guide/practices/page-sizes for more details

This link contained in the warning explains.

Solution

sentry-dotnet repo

This PR addresses the warnings for libsentrysupplemental.so, the source for which is in this repository. The most relevant parts of the PR are:

sentry-dotnet/lib/sentrysupplemental/CMakeLists.txt

Lines 6 to 7 in 8669e4c

# Set alignment to 16 KB. See https://developer.android.com/guide/practices/page-sizes#cmake
target_link_options(sentrysupplemental PRIVATE "-Wl,-z,max-page-size=16384")

Rebuilding with those changes results in changes to the so files, which we have checked in to this repo (presumably because they change very infrequently).

And to test this (since it only presents for us when targeting net9.0-android) we bumped the target frameworks for samples/Sentry.Samples.Maui/Sentry.Samples.Maui.csproj from net8.0 to net9.0.

sentry-jave repo

The other shared libraries are part of the sentry-native repository that we get transitively through sentry-java when targeting net9.0-android.

Tracking issue:

jamescrosswell and others added 30 commits October 22, 2024 13:23
@jamescrosswell
Add net9.0 support
c64ca21
@jamescrosswell
Workaround CastleProxy errors in device tests
582045d
@jamescrosswell
Skip SystemDiagnosticsMetricsListenerTests in DeviceTests (will depre…
d3c40ca 
…cate)
@jamescrosswell
Disabled Handle_UnobservedTaskException_CaptureEvent (flaky test)
df2cd5e
@jamescrosswell
Update CHANGELOG.md
1154070
@jamescrosswell
Merge branch 'version-5.0.0' into net9
10550f7
@jamescrosswell
Made logic for detecting net8.0 or later more robust in Sentry.target…
f013b99 
…s and Sentry.Native.targets
@jamescrosswell
Bump device tests to Xcode 16
bdca1bb
@jamescrosswell
Moved error from runtime to compile time in SingleFileApp
8150ca8
@jamescrosswell
Bump to XHarness 10 (prerelease)
968e87b
@getsentry-bot
Format code
c3231da
@jamescrosswell
Merge branch 'version-5.0.0' into net9
93a950b
@jamescrosswell
Removed reference to obsolete member (Java SDK)
ba88d7e
@jamescrosswell
Disabled iOS device tests
4a30dfd
@jamescrosswell
Removed hard coded alpha VersionSuffix
813a119
@jamescrosswell
Update ApiApprovalTests.Run.DotNet9_0.verified.txt
5efc0d5
@jamescrosswell
Debug native library references in integration tests on CI
5beafd0
@jamescrosswell
Reenable iOS device tests
196d00d
@jamescrosswell
Fixed typo in MS Build condition on FrameworkSupportsNative
c9f3bc3
@jamescrosswell
Added temporary diagnostic logging to device-tests-ios.yml
faad873
@jamescrosswell
Update Sentry.Samples.Maui.csproj
306eefc
@jamescrosswell
Update device-tests-ios.yml
301a854
@jamescrosswell
Update device-test.ps1
ec2335e
@jamescrosswell
Update device-tests-ios.yml
58206a9
@jamescrosswell
Update build.yml
7261983
@jamescrosswell
Disable Flaky tests
bcc3f00
@jamescrosswell
Update Sentry.Native.targets
d45ea42
@jamescrosswell
Update Sentry.Native.targets
830ba36
@jamescrosswell
Merge branch 'version-5.0.0' into net9
867073e
@jamescrosswell
Update verify tests
4f59199
@jamescrosswell
Copy link
Collaborator Author

jamescrosswell commented Nov 4, 2024

Thanks @supervacuus,

I don't think what you're seeing is related to the version of sentry-native that the dotnet SDK references directly, but rather the one packaged via sentry-android.

Ah, OK that makes sense.

The changes here, together with a release of sentry-android that includes the 16kb compatible native SDK binaries, should fix #3633.

tracking issue: getsentry/sentry-java#3657 alpha release for testing: https://github.com/getsentry/sentry-java/releases/tag/7.17.0-alpha.1

Just tested with 7.17.0-alpha.1 and I can confirm that this resolves the alignment warnings 👍🏻

@jamescrosswell jamescrosswell linked an issue Nov 4, 2024 that may be closed by this pull request
Alignment warnings when targeting net9.0-android - MAUI #3633
Closed
@vaind
Copy link
Contributor

vaind commented Nov 4, 2024

The sentry-dotnet library appears to link sentry-native statically however:

sentry-dotnet/src/Sentry/Platforms/Native/buildTransitive/Sentry.Native.targets

Lines 30 to 34 in 830ba36

<ItemGroup Condition="'$(FrameworkSupportsNative)' == 'true' and ('$(RuntimeIdentifier)' == 'osx-x64' or '$(RuntimeIdentifier)' == 'osx-arm64')">
<DirectPInvoke Include="sentry-native" />
<NativeLibrary Include="$(MSBuildThisFileDirectory)..\sentry-native\osx\libsentry-native.a" />
<NativeSystemLibrary Include="curl" />
</ItemGroup>

Could that explain why we're getting alignment warnings from the sentry-native libraries we're linking in the sentry-dotnet repo?

Full disclosure: I know virtually nothing about native development... I vaguely remember studying C and C++ about 30 years ago 😛

We only do this for Native AOT on desktop afaik (do we also do this on mobile @vaind ?)

Sentry.Native subproject here is just desktop. I'm not aware of what the mobile side is doing in sentry-java but as @jamescrosswell have just noticed, there's a PR in sentry-java to fix this issue which would come in due time and I don't think we need to do anything in the dotnet repo.

@jamescrosswell jamescrosswell marked this pull request as ready for review November 4, 2024 10:19
bricefriha
bricefriha approved these changes Nov 4, 2024
View reviewed changes
Copy link
Contributor

@bricefriha bricefriha left a comment

Choose a reason for hiding this comment

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

lgtm

@bruno-garcia
Copy link
Member

bruno-garcia commented Nov 4, 2024

Sentry.Native subproject here is just desktop. I'm not aware of what the mobile side is doing in sentry-java but as @jamescrosswell have just noticed, there's a getsentry/sentry-java#3620 to fix this issue which would come in due time and I don't think we need to do anything in the dotnet repo.

We need both. This PR fixes the "suplemental" lib which is just an example crash we bundle.
And sentry-java monorepo has Android, which has NDK which has sentry-native, so native libs on Android also built there. Which that PR fixes.

So we can merge this, and independently bump Android once that PR is merged, and the all the warnings should be gone

bruno-garcia
bruno-garcia reviewed Nov 4, 2024
View reviewed changes
@jamescrosswell jamescrosswell merged commit d1e7392 into version-5.0.0 Nov 5, 2024
@jamescrosswell jamescrosswell deleted the align-libsentry branch November 5, 2024 05:08
@jamescrosswell jamescrosswell linked an issue Nov 17, 2024 that may be closed by this pull request
Support 16kb page size on Android 15 #3672
Closed
@jamescrosswell jamescrosswell mentioned this pull request Nov 17, 2024
Closed
@dependabot dependabot bot mentioned this pull request Aug 18, 2025
Closed
This was referenced Sep 8, 2025
Closed
Closed
This was referenced Sep 15, 2025
Closed
Closed
Closed
This was referenced Oct 6, 2025
Closed
Closed
@dependabot dependabot bot mentioned this pull request Oct 20, 2025
Closed
@dependabot dependabot bot mentioned this pull request Nov 3, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bruno-garcia bruno-garcia bruno-garcia left review comments

@bitsandfoxes bitsandfoxes Awaiting requested review from bitsandfoxes

+1 more reviewer

@bricefriha bricefriha bricefriha approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support 16kb page size on Android 15 Alignment warnings when targeting net9.0-android - MAUI

6 participants

@jamescrosswell @supervacuus @bruno-garcia @vaind @bricefriha @getsentry-bot