Fixes symbolication for net9.0-android applications in Release config#4221
Fixes symbolication for net9.0-android applications in Release config#4221bruno-garcia merged 10 commits intomainfrom
Conversation
bruno-garcia
left a comment
bruno-garcia
left a comment
There was a problem hiding this comment.
Should we test this on Symbol Collector Android?
| internal static string GetArchivePathForAbi(this string archivePath, string abi, DebugLogger? logger) | ||
| { | ||
| var basePath = Path.GetDirectoryName(archivePath) ?? string.Empty; | ||
| var abiPart = abi.Replace("-", "_"); |
There was a problem hiding this comment.
I wonder if we can avoid this by having supportedAbis be provided in the right format
There was a problem hiding this comment.
The abi.Replace("-", "_") you mean? Not really... most of the time it is used with a hyphen. When building the file names for the split_config APKs though it gets replaced with an underscore (I'm sure there's a reason... not sure what it is though).
| { | ||
| var basePath = Path.GetDirectoryName(archivePath) ?? string.Empty; | ||
| var abiPart = abi.Replace("-", "_"); | ||
| var splitFilePath = Path.Combine(basePath, $"split_config.{abiPart}.apk"); |
There was a problem hiding this comment.
if GetDirectoryName returned null and we ended up with "" does this code make sense?
That might give a bit of reassurance yes. I've only tested it manually on the Sentry.Samples.Maui project so far. I've tested both with I also tested what happens when using the bundletool to split the packages and install these on a simulator (rather than using the MAUI build targets to install the app), since the bundletool apparently does exactly what GooglePlay would do... like so:
Curiosly after step two, the However after step 3, what gets installed on the simulator is: So obviously some renaming happens during installation. None the less, it seems like the device specific file that contains the assemblies we want will always be named |
3 participants
Resolves #4209:
Testing
I ran into some challenges creating some automated tests for this. I think we should release this fix ASAP then and I've created #4227 to address the tests:
References