Using the `dotnet msbuild` command produces invalid executables for apps targeting .NET Framework
0xced opened this issue · 17 comments
After upgrading our TeamCity installation, I followed JetBrains recommendataion to migrate from their Visual Studio (sln) runner to the new .NET runner. This basically means running dotnet msbuild
instead of MSBuild.exe
. Also, simply installing the .NET 5 SDK without having to deal with the complexity of Visual Studio Build Tools installation on the build agent seemed very appealing.
So I started to try building apps with dotnet msbuild
instead of the usual MSBuild.exe
command but had a very unpleasant surprise. The first app I tried was crashing at startup before even hitting the module initializer. The event viewer was showing a crash but without much usable details. When removing Costura, the app would stop crashing. So I enabled fusion logs to try to understand what was happening and saw something very unexpected for a .NET Framework app: a reference to System.Private.CoreLib
! (This is basically the .NET Core equivalent of mscorlib.)
Here is how to reproduce the problem with a minimal hello world app.
Program.cs:
using System;
namespace hellocostura
{
class Program
{
static void Main(string[] args)
{
Console.WriteLine("Hello World!");
}
}
}
hellocostura.csproj:
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net48</TargetFramework>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Costura.Fody" Version="5.2.0" PrivateAssets="all" />
</ItemGroup>
</Project>
assemblyrefs.csx (to check what are the referecend assemblies, runnable with the dotnet script global tool):
#r "nuget: dnlib, 3.3.2"
using (var stream = System.IO.File.OpenRead(Args[0]))
using (var module = dnlib.DotNet.ModuleDefMD.Load(stream))
{
Console.WriteLine(string.Join("\n", module.GetAssemblyRefs()));
}
✅ When building with the standard msbuild exectuable, the hellocostura.exe
has references to mscorlib
and System
, everything is fine.
rm -rf bin obj && \
dotnet restore --nologo --verbosity=quiet && \
msbuild -nologo -verbosity:quiet && \
dotnet script assemblyrefs.csx bin/Debug/net48/hellocostura.exe
mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089
System, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089
❌ When building with the dotnet msbuild
command, the hellocostura.exe
has references to mscorlib
and System.Private.CoreLib
, leading to the crash at startup.
rm -rf bin obj && \
dotnet restore --nologo --verbosity=quiet && \
dotnet msbuild -nologo -verbosity:quiet && \
dotnet script assemblyrefs.csx bin/Debug/net48/hellocostura.exe
mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089
System.Private.CoreLib, Version=5.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e
System.IO.Compression, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089
System.Runtime, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
Note: the only difference bewtween the two commands is msbuild
vs dotnet msbuild
.
I will investigate this issue and hopefully be able to provide a workaround, maybe even another pull request. 😉
Maybe because the dotnet version of msbuild makes Costura think that it's running against .NET Core? Although that would be a bit weird, it should check against the ModuleDefinition.
does this happen with other fody addins?
Good point! I just tried with PropertyChanged.Fody
instead of Costura.Fody
and there is no problem, the only assembly referenced are mscorlib
and System
and the weaved executable runs fine. So it looks like a Costura specific issue.
I narrowed where the System.Private.CoreLib
assembly reference is added, at AssemblyLoaderImporter.cs:57:
_targetType = new TypeDefinition("Costura", "AssemblyLoader", _sourceType.Attributes, Resolve(_sourceType.BaseType));
More precisely, at AssemblyLoaderImporter.cs:126:
// typeDefinition is "System.Object"
var typeReference = ModuleDefinition.ImportReference(typeDefinition);
Now I guess I'll have to learn a bit about Cecil…
Hmmm, could it be caused by the templates using 'is not null' and 'is null'? They should not be .NET core specific, but that's one of the changes recently made. Or that Costura.Fody reference assembly is targeting something special?
I think the root cause is that Costura.Template
is referenced by the Costura.Fody
project, not by the Costura
project.
This way Costura.Template
will follow the framework of the build environment, not the framework of the target assembly.
Maybe this sample helps: here the code to be imported is in the assembly containing the attributes, which is referenced by the target assembly - so it will match the target framework:
https://github.com/tom-englert/WeakEventHandler.Fody/blob/c0b39014e90351c59016f20b17c06fa6e44fcc45/WeakEventHandler.Fody/WeakEventHandlerWeaver.cs#L57-L62
Thanks @tom-englert , that is probably it. Will investigate later this week.
I don't think it has anything to do with the code in the templates. What I have noticed is that when using dotnet msbuild
Cecil takes the NET_CORE path in BaseAssemblyResolver.cs:137:
#if NET_CORE
assembly = SearchTrustedPlatformAssemblies (name, parameters);
if (assembly != null)
return assembly;
#else
When using MSBuild.exe
it takes the other path (#else branch) and eventually resolves the assembly in the GAC.
So I tried to force Fody to use the netclassictask
assembly like this:
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net48</TargetFramework>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Costura.Fody" Version="5.2.0" PrivateAssets="all" />
<PackageReference Include="Fody" Version="6.5.1" PrivateAssets="all" GeneratePathProperty="true" />
</ItemGroup>
<PropertyGroup>
<FodyAssembly>$(PkgFody)\netclassictask\Fody.dll</FodyAssembly>
</PropertyGroup>
</Project>
But this did not work as it ended in an MSBuild error:
%USERPROFILE%\.nuget\packages\fody\6.5.1\build\Fody.targets(40,5): error MSB4062: The "Fody.WeavingTask" task could not be loaded from the assembly %USERPROFILE%\.nuget\packages\fody\6.5.1\netclassictask\Fody.dll. Could not load file or assembly 'Microsoft.Build.Utilities.v4.0, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'. The system cannot find the file specified. Confirm that the declaration is correct, that the assembly and all its dependencies are available, and that the task contains a public class that implements Microsoft.Build.Framework.ITask.
I think the solution may be to write a custom IAssemblyResolver.
@0xced So if that's the root cause, WeakEventHandler.Fody should fail the same way - can you give it a try?
I just tried with WeakEventHandler.Fody instead of Costura but it doesn't fail, i.e. only mscorlib
and System
are referenced.
I'm currently trying to solve this issue by implementing a custom IAssemblyResolver
so that Cecil doesn't add reference to System.Private.CoreLib
when weaving a .NET Framework assembly. I have pushed the branch I'm working on https://github.com/0xced/Costura/tree/NetFrameworkAssemblyResolver.
Currently (as of 745a248) I have made some progress as the System.Private.CoreLib
assembly reference is now added when executing line 62 (by CopyMethod
) instead of previously line 58 (by Resolve(_sourceType.BaseType)
).
OK, so I finally figured it out and the fix was almost a one-line fix eventually! The pull request fixing this issue is up in #696.
Fun story: after basically rewriting the .NET Framework GAC assembly resolver by copy/pasting internal methods from Cecil's BaseAssemblyResolver
I searched for a way to properly identify if the weaved assembly was a .NET Framework or a .NET Core assembly. So I looked at the ModuleDefinition in the debugger and saw that that the module definition has an AssemblyResolver
property. And that was the eureka moment: just use this assembly resolver, it should work. And well, it worked. 🎉
New alpha available:
https://www.nuget.org/packages/Costura.Fody/5.3.0-alpha0028
If it works, I'll release stable
I just tried the published 5.3.0-alpha0028 on a couple of my projects and everything looks good.
Will release as stable, thanks for the quick turnaround!
thanks for the quick turnaround
Same. 😀
Stable version should be available now!