-
Notifications
You must be signed in to change notification settings - Fork 556
Remove obsolete System.AndroidPlatform methods and native implementations #10385
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: jonathanpeppers <[email protected]>
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot check my review comments
Co-authored-by: jonathanpeppers <[email protected]>
Co-authored-by: jonathanpeppers <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot there is a new error:
(_BuildAndroidRuntimes target) ->
ld.lld : error : version script assignment of 'LIBMONO_ANDROID' to symbol '_monodroid_freeifaddrs' failed: symbol not defined [/Users/builder/azdo/_work/4/s/android/src/native/native-mono.csproj]
ld.lld : error : version script assignment of 'LIBMONO_ANDROID' to symbol '_monodroid_getifaddrs' failed: symbol not defined [/Users/builder/azdo/_work/4/s/android/src/native/native-mono.csproj]
clang++ : error : linker command failed with exit code 1 (use -v to see invocation)
Is something using these methods? or is there another file to update?
Co-authored-by: jonathanpeppers <[email protected]>
... The error was caused by the Mono version script file still referencing the removed symbols. I've fixed it by removing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (if the build passes)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot there are a few minor formatting issues
Co-authored-by: jonathanpeppers <[email protected]>
I would hold off with this PR until we branch for .NET10 release. The Mono changes can potentially be breaking ones, especially removal of the |
I find it very unlikely that any app would depend on it. This was internal implementation detail that has not been used since .NET 5 (unified .NET always shipped the shims in their native libraries). The reason I found this in the first place is that I looked into reusing the native code for CoreCLR GC bridge for NativeAOT (which now has the same API). I was very surprised to find that this is still lurking around and that it was even ported to CoreCLR host code base. (in fact, I would argue that from security perspective it's safer to ship less dead code in a LTS release; well, even if LTS is irrelevant for the mobile workloads) |
It must not affect app size too much, as we have tests that fail if file sizes change within a certain percentage. |
Isn't it internal APIs ? |
This PR removes obsolete managed code that references
System.AndroidPlatform
, which was part of the Mono Android runtime but was never used in modern .NET. The issue was identified by @filipnavara and confirmed safe to remove by @jonathanpeppers after verifying that these APIs don't exist in dotnet/runtime.Changes Made
Removed Methods from AndroidEnvironment.cs:
GetDisplayDPI()
- Previously called bySystem.Drawing.GraphicsAndroid.FromAndroidSurface()
(already marked as obsolete for android31.0)GetDefaultTimeZone()
- Previously called bySystem.Core!System.AndroidPlatform.GetDefaultTimeZone()
GetDefaultSyncContext()
- Previously called bymscorlib.dll!System.AndroidPlatform.GetDefaultSyncContext()
GetInterfaceAddresses()
- Previously called bySystem.dll!System.AndroidPlatform.getifaddrs
FreeInterfaceAddresses()
- Previously called bySystem.dll!System.AndroidPlatform.freeifaddrs
GetDefaultProxy()
- Previously called bySystem.dll!System.AndroidPlatform.GetDefaultProxy()
Additional Cleanup:
_Proxy
class that was only used byGetDefaultProxy()
RuntimeNativeMethods.cs
:_monodroid_timezone_get_default_id()
_monodroid_getifaddrs()
_monodroid_freeifaddrs()
Native Code Cleanup:
generate-pinvoke-tables.sh
_monodroid_timezone_get_default_id()
functions (CLR & Mono)xamarin_getifaddrs.cc
files and header (CLR & Mono - 2,200+ lines)Impact
This is a breaking change that removes internal APIs that were never intended for public use in modern .NET for Android. No external code should be affected as these were internal implementation details for obsolete System.AndroidPlatform methods.
Total lines removed: 2,456 lines across 16 files
Build status: ✅ Successful with no new warnings or errors
Fixes #10381.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.