-
Notifications
You must be signed in to change notification settings - Fork 127
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
.NET 6.0 removing ctor for Activator.CreateInstance()-created types #1633
Comments
The recommendation is to use Type.GetType that linker intrinsically recognizes. We could add intrinsic recognition to Assembly.GetType, but based on API usage telemetry, it's a rarely used API (around 1% on NuGet.org) and it's unclear whether the intrinsic recognition would be able to actually help those 1% (we would need to see the assembly being operated on statically, and that requires quite a bit of luck). It's about limiting the API surface the linker needs to hardcode (linker doesn't currently understand Is there a reason to believe this specific pattern would be common in Xamarin apps? |
Current stable Xamarin.Forms has a DI pattern, such as: [assembly: Dependency(typeof(DeviceOrientationService))]
public class DeviceOrientationService : IDeviceOrientationService
{
public DeviceOrientation GetOrientation() { ... }
} https://docs.microsoft.com/xamarin/xamarin-forms/app-fundamentals/dependency-service/introduction The implementation for this looks like it might break: Let me actually test if this works or not in a Xamarin.Forms app. |
The `HelloService.cs` example in this app works, but the case in Xamarin.Forms startup here does not: https://github.com/xamarin/Xamarin.Forms/blob/4e704d871f3379b58c6efa66b9942fb41df4de51/Xamarin.Forms.Core/Application.cs#L37 I suspect `HelloService.cs` works because `HelloForms.dll` is the root assembly. Using `@(TrimmerRootDescriptor)` solved the issue here.
It does seem like the implementation in Xamarin.Forms breaks, I was able to reproduce it here: It fails in a spot in Xamarin.Forms internally unless I use The We need to update these |
Is Xamarin planning to do more aggressive trimming modes in .NET 6? DI is one of the things that are fundamentally incompatible with more aggressive trim modes. Unless we add extra steps that specialcase the Xamarin DI, I wouldn't expect it to work with more aggressive trimming. I see some investigation is happening to use Microsoft.Extensions for DI in Xamarin - source generators are being investigated there to make that DI trimming friendly: dotnet/runtime#44432 |
Note that as of now, the source generator for |
In current Xamarin (pre .NET 6), the linker modes we have are a little different than .NET 5+: https://docs.microsoft.com/xamarin/android/deploy-test/linker#linker-behavior Users can opt into The |
The answer is that not by default but users won't be blocked to do that. I'm not sure we need to support "Full" mode this way unless you want to be in the pain to fight with the official dotnet CLI trimming settings. |
Do we need to consider a completely different UI here? The defaults we have right now seem to work, if you just have a default/empty For a .NET 5 console app, there is only a single |
A bit off-topic on this issue but nonetheless. I don't think you should consider any Android-specific UI for trimming. Firstly the same thing should work from CLI and secondly there is no need to specialize anything at UI level to be Android-specific. I'd suggest migrating from the old SDK/All concept (which is inaccurate in today's XA anyway) into common .NET Core PublishTrimmed setup. You could do that by simply setting For more details about how the trimming will be controlled internally see for example #1269 (comment) |
On the original topic - how did this work before? AFAIK linker never really recognized |
@vitek-karas I made a sample, you can build this in https://github.com/jonathanpeppers/AndroidLinkModeFull I used our (somewhat documented) feature of dumping the When running:
I see this, but it doesn't explain why the
|
Oh, this gave more info:
|
I think it'd be valuable to actually find out what pattern all this code is trying to persist. There is a lot of Android customization (e.g. Android.Runtime.Preserve) which might not work well going forward and has a possibly better alternative. I was talking to @vitek-karas and he will follow up to understand the scenario fully. |
In getting our tests passing for .NET 6, we have a test where the linker is removing the
.ctor
for this type:https://github.com/xamarin/xamarin-android/blob/9bc00032eebc30e91a66114eff9026c6a3b0e4d7/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs#L282-L284
https://github.com/xamarin/xamarin-android/blob/9bc00032eebc30e91a66114eff9026c6a3b0e4d7/tests/MSBuildDeviceIntegration/Resources/LinkDescTest/MainActivityReplacement.cs#L35-L45
Given the type:
It is only used with code such as:
typeof()
works fine, but the.ctor
has been linked away.This seems like it might be a common case that works today with current Xamarin.
Here are the assemblies from the linker output: linked.zip
UnnamedProject.dll
is the root assembly, andLibrary1.dll
contains this type.The text was updated successfully, but these errors were encountered: