Document new linker options and trimming libraries #23766
Conversation
And fix typos, links
Fixes dotnet#16140 Relevant for dotnet/docs#23766
- Move sample up - Call out TrimMode link default - Clarify app vs library - Publish Release
| ``` | ||
|
|
||
| This means the library calls a method which has explicitly been annotated as incompatible with trimming, using [`RequiresUnreferencedCodeAttribute`]( | ||
| https://docs.microsoft.com/dotnet/api/system.diagnostics.codeanalysis.requiresunreferencedcodeattribute?view=net-5.0). To get rid of the warning, consider whether `Foo` needs to call `Bar` to do its job. If so, annotate the caller `Foo` with `RequiresUnreferencedCode` as well; this will "bubble up" the warning so that callers of `Foo` get a warning instead. Once you have "bubbled up" the attribute all the way to public APIs (so that these warnings are produced only for public methods, if at all), you are done. Apps which call your library will now get warnings if they call those public APIs, but these will no longer produce warnings like `IL2104: Assembly 'MyLibrary' produced trim warnings`. |
There was a problem hiding this comment.
Is this the right place to include the DynamicDependency, Descriptor options to solve the RequiresUnreferencedCode warning and be able to use UnconditionalSuppressMessage?
There was a problem hiding this comment.
Yes, I think either in this doc or one next to it would be a good place for these. I might do it in a separate PR if I don't get to it soon.
There was a problem hiding this comment.
When we add this, we should have a "warning" or "advanced" banner on it. It is definitely a "this is something you can do to make things work, but it is really only for people who know precisely what they are doing".
There was a problem hiding this comment.
Added a new section with UnconditionalSuppressMessage. I'm having some trouble coming up with examples for DynamicDependency that aren't super contrived. Do you have any suggestions?
There was a problem hiding this comment.
Do you have any suggestions?
One example is when serialization doesn't work.
- https://github.com/dotnet/runtime/blob/01b7e73cd378145264a7cb7a09365b41ed42b240/src/libraries/System.Runtime.Serialization.Xml/tests/TrimmingTests/EndToEndTest.cs#L13-L15
- https://github.com/dotnet/runtime/blob/01b7e73cd378145264a7cb7a09365b41ed42b240/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs#L177-L182
There was a problem hiding this comment.
Thanks! I will add an example for DynamicDependency in a follow-up.
- Fix typo - Avoid referencing sections with "above/below"
Add code snippet for examples after annotation
|
|
||
| Note that the analysis results depend on the implementation details of your dependencies. If you update to a new version of a dependency, this may introduce analysis warnings if the new version added non-understood reflection patterns, even if there were no API changes. In other words, introducing trim analysis warnings to a library is a breaking change when the library is used with `PublishTrimmed`. | ||
|
|
||
| ## Resolving trim warnings |
There was a problem hiding this comment.
How do you feel about combining @agocke's guidance blog here? dotnet/linker#1946
There are good things about both descriptions. I'd rather have 1 really good one that combines the best of both, than have 2 different ones with different information.
There was a problem hiding this comment.
I think it makes sense to have both a fixed announcement blog post and docs that we can expand over time. At the moment there's a lot of overlap, but I'd like to keep adding to these instructions so that they read more like a reference for authors who really have to dive into the annotations. @agocke thoughts?
I already see one discrepancy, which is that these docs don't yet mention UnconditionalSuppressMessage (I will add it). I'm also happy to take any other suggestions so that the docs contain a superset of the info in the blog post.
There was a problem hiding this comment.
I'm fine with 2 posts, as long as the official docs (i.e. this document) is better than the blog. The case I don't want to have happen is the blog to be really good, but the official docs being worse.
There was a problem hiding this comment.
I think this is good for long-term documentation. This looks more detailed, which I think is better for docs, and mine is more narrative, which may be useful for people just starting. I don't see any reason not to keep both up-to-date since the scenarios will be relevant long-term.
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
- Fix links - Clarify which assemblies are trimmed by default - Clarify which assemblies are affected by per-assembly metadata
- Change example to call reflection directly - Avoid mentioning .NET 5 - Point out benefits of Roslyn analyzer - Avoid Foo/Bar - Recommend not annotating virtuals
Which mentions UnconditionalSuppressMessage
Co-authored-by: David Pine <david.pine@microsoft.com>
Co-authored-by: David Pine <david.pine@microsoft.com>
Co-authored-by: David Pine <david.pine@microsoft.com>
Co-authored-by: David Pine <david.pine@microsoft.com>
Co-authored-by: David Pine <david.pine@microsoft.com>
Co-authored-by: David Pine <david.pine@microsoft.com>
Co-authored-by: David Pine <david.pine@microsoft.com>
Co-authored-by: David Pine <david.pine@microsoft.com>
Co-authored-by: David Pine <david.pine@microsoft.com>
sbomer
force-pushed
the
trimmingLibraries
branch
from
efdc041 to
35bdcfd
Compare
|
|
||
| The Roslyn analyzer is useful for a fast feedback cycle with IDE integration, but is currently incomplete. It doesn't cover all trim analysis warnings, but the set of patterns it understands will improve over time to give more complete coverage. The Roslyn analyzer also isn't able to analyze the implementations of reference assemblies that you depend on. It is important to follow the next steps to ensure that your library is fully compatible with trimming. | ||
|
|
||
| ### Show all warnings |
There was a problem hiding this comment.
This section is not really only about showing all warnings but about trimming everything, right? Should this section cover how to do it for a library and not only for an app?
There was a problem hiding this comment.
It is about showing library warnings - I added a sentence to clarify why the step of creating an app project is necessary.
if everyone else is good with this - thank you 🙏- Don't require un-suppressing warnings, as this is now implied by TrimmerDefaultAction=link: dotnet/sdk#16865 - Clarify why publishing an app is necessary for library warnings
|
Close/reopen to test status check. |
* Make TrimmerDefaultAction public Fixes #16140 Relevant for dotnet/docs#23766 * Don't suppress trim warnings when default action is link * Move SuppressTrimAnalysisWarnings to targets So that the condition takes into account properties set in the project file.
Fixes dotnet#16140 Relevant for dotnet/docs#23766
* Document new linker options and trimming libraries * Fix TOC * Document EnableTrimAnalyzer And fix typos, links * PR feedback - Move sample up - Call out TrimMode link default - Clarify app vs library - Publish Release * Change title to "Preparing libraries for trimming" * PR feedback - Fix typo - Avoid referencing sections with "above/below" * PR feedback Add code snippet for examples after annotation * Update docs/core/deploying/preparing-libraries-for-trimming.md Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com> * Update docs/core/deploying/preparing-libraries-for-trimming.md Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com> * Update docs/core/deploying/trimming-options.md Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com> * PR feedback - Fix links - Clarify which assemblies are trimmed by default - Clarify which assemblies are affected by per-assembly metadata * PR feedback - Change example to call reflection directly - Avoid mentioning .NET 5 - Point out benefits of Roslyn analyzer - Avoid Foo/Bar - Recommend not annotating virtuals * Add advanced section Which mentions UnconditionalSuppressMessage * Remove whitespace * PR feedback - Clarify why we need an exe - Improve comments in sample csproj * More comments * PR feedback Simplify wording * Update docs/core/deploying/preparing-libraries-for-trimming.md Co-authored-by: David Pine <david.pine@microsoft.com> * Update docs/core/deploying/preparing-libraries-for-trimming.md Co-authored-by: David Pine <david.pine@microsoft.com> * Update docs/core/deploying/preparing-libraries-for-trimming.md Co-authored-by: David Pine <david.pine@microsoft.com> * Update docs/core/deploying/preparing-libraries-for-trimming.md Co-authored-by: David Pine <david.pine@microsoft.com> * Update docs/core/deploying/trimming-options.md Co-authored-by: David Pine <david.pine@microsoft.com> * Update docs/core/deploying/preparing-libraries-for-trimming.md Co-authored-by: David Pine <david.pine@microsoft.com> * Update docs/core/deploying/preparing-libraries-for-trimming.md Co-authored-by: David Pine <david.pine@microsoft.com> * Update docs/core/deploying/preparing-libraries-for-trimming.md Co-authored-by: David Pine <david.pine@microsoft.com> * Update docs/core/deploying/preparing-libraries-for-trimming.md Co-authored-by: David Pine <david.pine@microsoft.com> * Update docs/core/deploying/preparing-libraries-for-trimming.md Co-authored-by: David Pine <david.pine@microsoft.com> * Update docs/core/deploying/preparing-libraries-for-trimming.md Co-authored-by: David Pine <david.pine@microsoft.com> * Update docs/core/deploying/preparing-libraries-for-trimming.md Co-authored-by: David Pine <david.pine@microsoft.com> * Update docs/core/deploying/preparing-libraries-for-trimming.md Co-authored-by: David Pine <david.pine@microsoft.com> * Update docs/core/deploying/preparing-libraries-for-trimming.md Co-authored-by: David Pine <david.pine@microsoft.com> * Update docs/core/deploying/preparing-libraries-for-trimming.md Co-authored-by: David Pine <david.pine@microsoft.com> * Update docs/core/deploying/preparing-libraries-for-trimming.md Co-authored-by: David Pine <david.pine@microsoft.com> * Update docs/core/deploying/preparing-libraries-for-trimming.md Co-authored-by: David Pine <david.pine@microsoft.com> * Update docs/core/deploying/preparing-libraries-for-trimming.md Co-authored-by: David Pine <david.pine@microsoft.com> * Update docs/core/deploying/trimming-options.md Co-authored-by: David Pine <david.pine@microsoft.com> * Add intro and fix links * Rename file to match title * Update trimming-options.md * Apply suggestions from code review * PR feedback - Don't require un-suppressing warnings, as this is now implied by TrimmerDefaultAction=link: dotnet/sdk#16865 - Clarify why publishing an app is necessary for library warnings Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com> Co-authored-by: David Pine <david.pine@microsoft.com>
* Make TrimmerDefaultAction public Fixes #16140 Relevant for dotnet/docs#23766 * Don't suppress trim warnings when default action is link * Move SuppressTrimAnalysisWarnings to targets So that the condition takes into account properties set in the project file.
This adds documents for the new linker options and instructions for trimming libraries. The "Trimming libraries" document will be the target of the aka.ms link in IL2104 and should (hopefully) nicely complement the blog post @agocke. For now this mostly uses similar examples to the blog post, but over time I think this would be a good place to add more involved examples that could be useful to developers.
For trimming libraries I think we should fix dotnet/sdk#16140 to get the behavior described in the doc.
This also fixes #23740.
Internal preview
✔️ Prepare libraries for trimming
✔️ Trimming options