dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.13k stars 4.7k forks source link

Documentation status of _AggressiveAttributeTrimming #88805

Closed MichalStrehovsky closed 2 months ago

MichalStrehovsky commented 1 year ago

These options are currently not documented on learn, but Blazor WASM enables them by default. So customers who get broken by this have no recourse but to dig into undocumented properties. Should these be documented?

https://github.com/dotnet/sdk/blob/16546adca23bbbea314c734a684ea696b866d5fd/src/BlazorWasmSdk/Targets/Microsoft.NET.Sdk.BlazorWebAssembly.6_0.targets#L49-L50

Cc @dotnet/illink @eerhardt

ghost commented 1 year ago

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas See info in area-owners.md if you want to be subscribed.

Issue Details
These options are currently not documented on learn, but Blazor WASM enables them by default. So customers who get broken by this have no recourse but to dig into undocumented properties. Should these be documented? https://github.com/dotnet/sdk/blob/16546adca23bbbea314c734a684ea696b866d5fd/src/BlazorWasmSdk/Targets/Microsoft.NET.Sdk.BlazorWebAssembly.6_0.targets#L49-L50 Cc @dotnet/illink @eerhardt
Author: MichalStrehovsky
Assignees: -
Labels: `linkable-framework`, `area-Tools-ILLink`
Milestone: -
MichalStrehovsky commented 1 year ago

If _AggressiveAttributeTrimming (or it's renamed equivalent) become public, we need to do something for #88994.

Sergio0694 commented 8 months ago

Would a documented version of _AggressiveAttributeTrimming unconditionally trim all attributes, or would it be able to preserve some? Eg. those it can see code specifically looking for (ie. via calls to GetCustomAttribute<T>, etc.), or perhaps via opt-in in some way? Asking because it sounds nice on paper, and I'm wondering how/if we could benefit from this too in CsWinRT, which does need to preserve a few attributes needed to support the interop infrastructure though 🤔

MichalStrehovsky commented 8 months ago

Would a documented version of _AggressiveAttributeTrimming unconditionally trim all attributes, or would it be able to preserve some? Eg. those it can see code specifically looking for (ie. via calls to GetCustomAttribute<T>, etc.), or perhaps via opt-in in some way? Asking because it sounds nice on paper, and I'm wondering how/if we could benefit from this too in CsWinRT, which does need to preserve a few attributes needed to support the interop infrastructure though 🤔

You're basically asking for https://github.com/dotnet/runtime/issues/103934. Yep, it would be great if attribute stripping was done in way that it doesn't break apps. It currently does break apps.

sbomer commented 3 months ago

Closing this because the feature switch was removed in https://github.com/dotnet/runtime/pull/103970, and we don't want to support/document the current attribute trimming model. See more context in https://github.com/dotnet/runtime/issues/103004#issuecomment-2146688120.

MichalStrehovsky commented 3 months ago

_AggressiveAttributeTrimming still exists though and looks like Android enables it: https://github.com/dotnet/android/blob/1f822a6fa58e78981b3c80de696a815fb1e65026/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.DefaultProperties.targets#L119-L120

sbomer commented 3 months ago

I'm not sure what we should do here (aside from implementing https://github.com/dotnet/runtime/issues/103934 as a better long-term solution).

_AggressiveAttributeTrimming is an underscored property, which I understand to mean "intentionally undocumented and technically unsupported". Do we really want to document it? Maybe we should push on Android to stop using it instead? I opened https://github.com/dotnet/android/issues/9060 for discussion.

MichalStrehovsky commented 3 months ago

The issue is that if Android opts into it and someone gets broken by it, they don't know what hit them and how to disable it. Documenting would involve dropping the underscore. Android SDK would ideally not do that. We can still keep the underscored feature for experimentation or just delete it.

sbomer commented 3 months ago

Android removed it in https://github.com/dotnet/android/pull/9062.

sbomer commented 2 months ago

Closing this since the only remaining work is for xamarin-macios to remove use of the property.