IL2104 when trimming application using System.CommandLine

perlun commented 2 years ago

Hi,

When starting to prepare my application for .NET 6 (https://github.com/perlang-org/perlang/pull/223), I am seeing errors like this:

/home/per/.nuget/packages/system.commandline/2.0.0-beta1.21216.1/lib/netstandard2.0/System.CommandLine.dll : error IL2104: Assembly 'System.CommandLine' produced trim warnings. For more information see https://aka.ms/dotnet-illink/libraries [/home/per/git/perlang/src/Perlang.ConsoleApp/Perlang.ConsoleApp.csproj]

I wonder if there would be any way to fix this, apart from disabling the trimming warnings altogether? :thinking: .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.

jonsequitur commented 2 years ago

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.

perlun commented 2 years ago

Sounds good @jonsequitur. I ended up disabling the linker warnings altogether in my project :see_no_evil: for a few reasons:

The attribute-based approach was not completely straightforward for some of the warnings (was getting them for some serialization assembly or something)
I'm not sure how to silence it for 3rd party dependencies like System.CommandLine
It feels like it does clutter my codebase in a somewhat unpleasant way.

All of these might be resolvable, but for now, putting the head in the sand seemed like the easiest way to move forward. :grin:

Having that said, feel free to keep this issue since it might be relevant to others running into similar problems.

omajid commented 2 years ago

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: dotnet publish -c Release -r linux-x64 --self-contained -p:PublishSingleFile=true -p:PublishTrimmed=true

Running this application shows:

Unhandled exception. System.IO.FileNotFoundException: Could not load file or assembly 'Microsoft.CSharp, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'. The system cannot find the file specified.

File name: 'Microsoft.CSharp, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'
   at System.CommandLine.IO.ConsoleExtensions.ResetTerminalForegroundColor(IConsole console)
   at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c__DisplayClass14_0.<UseExceptionHandler>g__Default|1(Exception exception, InvocationContext context)
   at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c__DisplayClass14_0.<<UseExceptionHandler>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c.<<CancelOnProcessTermination>b__5_0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.Invocation.InvocationPipeline.InvokeAsync(IConsole console)
   at System.CommandLine.CommandExtensions.InvokeAsync(Command command, String[] args, IConsole console)
   at Turkey.Program.Main(String[] args)
   at Turkey.Program.<Main>(String[] args)

I copied some code from ResetTerminalForegroundColor into sharplab.io here. It does look like there's an actual dependency from System.CommandLine.IO.ConsoleExtensions to Microsoft.CSharp.RuntimeBinder.

jonsequitur commented 2 years ago

We plan to relayer to remove the dependency on Microsoft.CSharp and move the reflection-dependent CommandHandler.Create methods out to a separate assembly.

jonsequitur commented 2 years ago

Related: #1472

omajid commented 2 years ago

Thanks! Do you have any suggestions on how to make the current releases work? Is there a particular DynamicDependencyAttribute or TrimmerRootDescriptor that's known to work? I have been playing around with them based on advice from this blog post but haven't had any luck. Or should I give up on PublishTrimmed with .NET 6 and current versions of System.CommandLine?

Caleb9 commented 2 years ago

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 net6.0 and adding <IsTrimmable>true</IsTrimmable> to .csproj file (see here - this tag is only valid in .NET 6+). I could make a similar PR to this repo but I'm not sure if you're OK with targeting net6.0 aside from netstandard2.0? Seeing that this manifests itself only in .NET 6 projects, which requires adding that <IsTrimmable> tag I'm not sure if there's a way around it though (assuming it would also solve the problem here).

MichalStrehovsky commented 2 years ago

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:

People who use the library with trimming and happen to only use parts that are fine for trimming (no unpredictable reflection) will see warnings disappear because the problematic code got trimmed away.
People who use the library with trimming and happen to use parts that are not okay with trimming will now see dozens of warnings for each individual part of the library instead of a single "Assembly 'System.CommandLine' produced trim warnings". IsTrimmable enables detailed warning reporting instead of the aggregate "There were some problems in this library".

Adding IsTrimmable without changing anything else in the library is never the right fix for "Assembly 'X' produced trim warnings".

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 TrimmableAssembly item in your app's csproj:

https://docs.microsoft.com/en-us/dotnet/core/deploying/trimming/trimming-options#trim-additional-assemblies

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.

Caleb9 commented 2 years ago

@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 net6.0 and NOT adding <IsTrimmable> and the warning was still generated from dotnet publish in a project that referenced the library. According to your description, adding the tag should generate even more warnings, if I understand correctly? But it actually removed the warning altogether :man_shrugging:. If you happen to know what's going on, then please let me know.

Thanks.

MichalStrehovsky commented 2 years ago

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 PublishTrimmed that people often don't realize is that by default, only assemblies that are marked IsTrimmable will participate in trimming. The other assemblies will be analyzed for trimming problems, and preserved in their entirety by the trimming process for compat reasons. Marking a library as IsTrimmable enables trimming the library. As of right now, very few libraries outside the framework are marked trimmable.

Caleb9 commented 2 years ago

@MichalStrehovsky Ok I see and understand a bit better now. Thanks.

So to sum up:

Without <IsTrimmable> you'll get one aggregate warning when there are potential trimming problems within the library. Even when the code using the library does not use the problematic parts. But the library will not actually get trimmed so risk of creating code that will break because of trimming is removed. If the trimming analyzer does not find problems within the library, you don't get any warnings.
With <IsTrimmable> you'll get detailed warnings when code is using problematic parts of the library, but you won't get any if either the library does not have any problems, or if the code using it is not using the problematic parts (the latter must've been my case and that's why adding the tag removed the warning).

Is that correct?

MichalStrehovsky commented 2 years ago

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.

Caleb9 commented 2 years ago

I better go back and tell them to revert my "fix" then :D. Thanks for taking time to explain this to me.

Caleb9 commented 2 years ago

One remark is that setting

<ItemGroup>
  <TrimmableAssembly Include="System.CommandLine" />
</ItemGroup>

I still get the aggregate warning (and not dozens of specific warnings, or nothing). Maybe because the library is targeting netstandard2.0 and not net6.0? It would make sense if it really is equivalent to setting <IsTrimmable> on that assembly but from the consuming app level, and <IsTrimmable> is not available in netstandard2.0.

tomrus88 commented 2 years ago

Using latest daily build of System.CommandLine (2.0.0-beta1.21576.2) i'm getting following AOT warnings:

using System;
using System.CommandLine;
using System.CommandLine.Invocation;
using System.IO;

namespace ConsoleApp7
{
    class Program
    {
        static void Main(string[] args)
        {
            // Create a root command with some options
            var rootCommand = new RootCommand
            {
                new Option<DirectoryInfo>(new[] { "-o", "--out", "--outFolder" }, () => new DirectoryInfo("extracted"), "Path to the folder"),
                new Argument<FileInfo>("inputFile", "Path to the file") { Arity = ArgumentArity.ExactlyOne }
            };

            rootCommand.Description = "My sample app";

            // Note that the parameters of the handler method are matched according to the names of the options
            rootCommand.Handler = CommandHandler.Create<DirectoryInfo, FileInfo>((outFolder, inputFile) =>
            {
                Console.WriteLine($"The value for --out is: {outFolder}, inputFile {inputFile}");
            });

            // Parse the incoming args and invoke the handler
            rootCommand.Invoke(args);
        }
    }
}

Publish with

dotnet publish -c Release -r win-x64 --self-contained -p:PublishTrimmed=true

/_/src/System.CommandLine/Binding/ArgumentConverter.cs(253): AOT analysis warning IL9700: System.CommandLine.Binding.ArgumentConverter.<ConvertStrings>g__CreateList|4_0(Type,Int32): Calling 'System.Type.MakeGenericType(Type[])' which has `RequiresDynamicCodeAttribute` can break functionality when compiled fully ahead of time. The native code for this instantiation might not be available at runtime. [C:\Users\tom_rus\Documents\Visual Studio 2019\Projects\ConsoleApp7\ConsoleApp7\ConsoleApp7.csproj]
/_/src/System.CommandLine/Binding/ArgumentConverter.cs(267): AOT analysis warning IL9700: System.CommandLine.Binding.ArgumentConverter.<ConvertStrings>g__CreateArray|4_1(Type,Int32): Calling 'System.Array.CreateInstance(Type,Int32)' which has `RequiresDynamicCodeAttribute` can break functionality when compiled fully ahead of time. The native code for the array might not be available at runtime. [C:\Users\tom_rus\Documents\Visual Studio 2019\Projects\ConsoleApp7\ConsoleApp7\ConsoleApp7.csproj]
/_/src/System.CommandLine/Binding/Binder.cs(60): AOT analysis warning IL9700: System.CommandLine.Binding.Binder.<GetDefaultValue>g__CreateEmptyArray|3_2(Type): Calling 'System.Array.CreateInstance(Type,Int32)' which has `RequiresDynamicCodeAttribute` can break functionality when compiled fully ahead of time. The native code for the array might not be available at runtime. [C:\Users\tom_rus\Documents\Visual Studio 2019\Projects\ConsoleApp7\ConsoleApp7\ConsoleApp7.csproj]
/_/src/System.CommandLine/Binding/Binder.cs(55): AOT analysis warning IL9700: System.CommandLine.Binding.Binder.<GetDefaultValue>g__GetEmptyEnumerable|3_1(Type): Calling 'System.Reflection.MethodInfo.MakeGenericMethod(Type[])' which has `RequiresDynamicCodeAttribute` can break functionality when compiled fully ahead of time. The native code for this instantiation might not be available at runtime. [C:\Users\tom_rus\Documents\Visual Studio 2019\Projects\ConsoleApp7\ConsoleApp7\ConsoleApp7.csproj]
/_/src/System.CommandLine/Binding/Binder.cs(50): AOT analysis warning IL9700: System.CommandLine.Binding.Binder.<GetDefaultValue>g__GetEmptyList|3_0(Type): Calling 'System.Type.MakeGenericType(Type[])' which has `RequiresDynamicCodeAttribute` can break functionality when compiled fully ahead of time. The native code for this instantiation might not be available at runtime. [C:\Users\tom_rus\Documents\Visual Studio 2019\Projects\ConsoleApp7\ConsoleApp7\ConsoleApp7.csproj]
/_/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/EnumConverter.cs(177): AOT analysis warning IL9700: System.ComponentModel.EnumConverter.ConvertTo(ITypeDescriptorContext,CultureInfo,Object,Type): Calling 'System.Enum.GetValues(Type)' which has `RequiresDynamicCodeAttribute` can break functionality when compiled fully ahead of time. It might not be possible to create an array of the enum type at runtime. Use the GetValues<TEnum> overload instead. [C:\Users\tom_rus\Documents\Visual Studio 2019\Projects\ConsoleApp7\ConsoleApp7\ConsoleApp7.csproj]

tomrus88 commented 2 years ago

I'm still AOT warning when compiling System.CommandLine 2.0.0-beta3.22101.1 with NativeAOT

System.CommandLine.Binding.ArgumentConverter.CreateEmptyArray(Type,Int32): Using member 'System.Array.CreateInstance(Type,Int32)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for the array might not be available at runtime.

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

jonsequitur commented 2 years ago

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.

azchohfi commented 2 years ago

@jonsequitur I'm hitting this when referencing System.CommandLine.Hosting.

...\.nuget\packages\system.commandline.hosting\0.4.0-alpha.22272.1\lib\netstandard2.1\System.CommandLine.Hosting.dll : error IL2104: Assembly 'System.CommandLine.Hosting' produced trim warnings. For more information see https://aka.ms/dotnet-illink/libraries
...\.nuget\packages\system.commandline.namingconventionbinder\2.0.0-beta4.22272.1\lib\netstandard2.0\System.CommandLine.NamingConventionBinder.dll : error IL2104: Assembly 'System.CommandLine.NamingConventionBinder' produced trim warnings. For more information see https://aka.ms/dotnet-illink/libraries
...\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.ILLink.targets(109,5): error NETSDK1144: Optimizing assemblies for size failed. Optimization can be disabled by setting the PublishTrimmed property to false.

Is there a way around this?

jonsequitur commented 2 years ago

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.

azchohfi commented 2 years ago

Makes sense. I turned TrimmerSingleWarn off and got these:

/_/src/System.CommandLine.Hosting/HostingExtensions.cs(113,21): Trim analysis error IL2077: System.CommandLine.Hosting.HostingExtensions.<>c__DisplayClass6_0.<UseCommandHandler>b__1(IServiceCollection): 'serviceType' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors' in call to 'Microsoft.Extensions.DependencyInjection.ServiceCollectionServiceExtensions.AddTransient(IServiceCollection,Type)'. The field 'System.CommandLine.Hosting.HostingExtensions.<>c__DisplayClass6_0.handlerType' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
/_/src/System.CommandLine.Hosting/HostingExtensions.cs(116,17): Trim analysis error IL2080: System.CommandLine.Hosting.HostingExtensions.UseCommandHandler(IHostBuilder,Type,Type): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods' in call to 'System.Type.GetMethod(String)'. The field 'System.CommandLine.Hosting.HostingExtensions.<>c__DisplayClass6_0.handlerType' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
/_/src/System.CommandLine.NamingConventionBinder/ModelDescriptor.cs(38,9): Trim analysis error IL2075: System.CommandLine.NamingConventionBinder.ModelDescriptor.ConstructorDescriptors.get: 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors', 'DynamicallyAccessedMemberTypes.NonPublicConstructors' in call to 'System.Type.GetConstructors(BindingFlags)'. The return value of method 'System.CommandLine.NamingConventionBinder.ModelDescriptor.ModelType.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
/_/src/System.CommandLine.NamingConventionBinder/ModelDescriptor.cs(47,9): Trim analysis error IL2075: System.CommandLine.NamingConventionBinder.ModelDescriptor.PropertyDescriptors.get: 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicProperties', 'DynamicallyAccessedMemberTypes.NonPublicProperties' in call to 'System.Type.GetProperties(BindingFlags)'. The return value of method 'System.CommandLine.NamingConventionBinder.ModelDescriptor.ModelType.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

azchohfi commented 2 years ago

Do you want me to open a separate issue to track this, or is it better to reopen this one?

jonsequitur commented 2 years ago

A separate issue is preferable. This issue was specifically about System.CommandLine and the work is done.

dotnet / command-line-api

IL2104 when trimming application using System.CommandLine #1465