-
Notifications
You must be signed in to change notification settings - Fork 382
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
IL2104 when trimming application using System.CommandLine #1465
Comments
Without looking into this more closely, my guess is this is mostly around the name-based command handler APIs. We're looking at layering those into a separate library which might help here. The ones that don't use name-based matching are much more performant anyway. |
Sounds good @jonsequitur. I ended up disabling the linker warnings altogether in my project 🙈 for a few reasons:
All of these might be resolvable, but for now, putting the head in the sand seemed like the easiest way to move forward. 😁 Having that said, feel free to keep this issue since it might be relevant to others running into similar problems. |
I am seeing a functional impact here. From my deps.json: "targets": {
".NETCoreApp,Version=v6.0": {},
".NETCoreApp,Version=v6.0/linux-x64": {
"Turkey/9-3664abe": {
"dependencies": {
"Microsoft.CodeQuality.Analyzers": "2.6.0",
"Microsoft.NetCore.Analyzers": "2.6.0",
"Newtonsoft.Json": "12.0.1",
"System.CommandLine": "2.0.0-beta1.21308.1",
"Text.Analyzers": "2.6.0",
"runtimepack.Microsoft.NETCore.App.Runtime.linux-x64": "6.0.0"
... I am publishing it using: Running this application shows:
I copied some code from |
We plan to relayer to remove the dependency on |
Related: #1472 |
Thanks! Do you have any suggestions on how to make the current releases work? Is there a particular |
Hi. I just resolved the same issue in another library I'm using, then my build failed because System.CommandLine also suffers from it, and I had warnings as errors enabled. The solution required targeting |
If a library generates trimming warnings, adding IsTrimmable to it will not fix them. The library is still not trimmable, but you're allowing trimming anyway. There are two outcomes of that:
Adding The right workflow for libraries is documented at https://docs.microsoft.com/en-us/dotnet/core/deploying/trimming/prepare-libraries-for-trimming. You first deal with the warnings, and then mark the library trimmable. You can do an equivalent of adding IsTrimmable to library X by using the E.g. for System.CommandLine you would do: <ItemGroup>
<TrimmableAssembly Include="System.CommandLine" />
</ItemGroup> If you're in category 1 above, the warnings will disappear. If you're in category 2, you'll see more warnings. |
@MichalStrehovsky good to know, thanks, I'll look into it when I get the time. However I observed different behavior in the mentioned library that I "fixed", i.e. I started by just targeting Thanks. |
It will remove the warnings if the consuming app is in category 1 above - trimming the library got rid of code that couldn't be reasoned about and there's nothing left that would be problematic. But a different user of the library might have a different experience (unless the problematic code in question is always unreachable, in which case it would be better to just delete the unreachable code). The main thing with |
@MichalStrehovsky Ok I see and understand a bit better now. Thanks. So to sum up:
Is that correct? |
Yup. Without IsTrimmable, the trimming process considers everything in the assembly implicitly used, won't trim it, and will raise warnings if there's any problems in the code. |
I better go back and tell them to revert my "fix" then :D. Thanks for taking time to explain this to me. |
One remark is that setting
I still get the aggregate warning (and not dozens of specific warnings, or nothing). Maybe because the library is targeting |
Using latest daily build of System.CommandLine (2.0.0-beta1.21576.2) i'm getting following AOT warnings:
Publish with
|
I'm still AOT warning when compiling System.CommandLine 2.0.0-beta3.22101.1 with NativeAOT
It seems to point to this method https://github.com/dotnet/command-line-api/blob/main/src/System.CommandLine/Binding/ArgumentConverter.DefaultValues.cs#L22-L23 |
I closed this issue because we got rid of the trim warnings, and we've also now removed the warnings for single file publishing. Native AOT is more work and worth opening a separate issue for. |
@jonsequitur I'm hitting this when referencing System.CommandLine.Hosting.
Is there a way around this? |
Those libraries would have to be updated to support trimming. System.CommandLine.Hosting can possibly be updated to support this, but System.CommandLine.NamingConventionBinder would likely need an almost complete rewrite to move from reflection APIs to source generators. |
Makes sense. I turned TrimmerSingleWarn off and got these:
|
Do you want me to open a separate issue to track this, or is it better to reopen this one? |
A separate issue is preferable. This issue was specifically about System.CommandLine and the work is done. |
Hi,
When starting to prepare my application for .NET 6 (perlang-org/perlang#223), I am seeing errors like this:
I wonder if there would be any way to fix this, apart from disabling the trimming warnings altogether? 🤔 .NET 6 does introduce a bunch of trimming-related changes (https://devblogs.microsoft.com/dotnet/announcing-net-6/#il-trimming), so I guess this is a consequence of the more advanced analysis it does before performing the trimming.
The text was updated successfully, but these errors were encountered: