Msbuild Task factory - error when Multitargeting

[dazinator]

I have been testing a little more and have just found an issue that is puzzling me a little..

The task factory breaks if I use multi-targeting with more than one target framework specified.

For example - building with either of these and the task factory works:

• <TargetFrameworks>netcoreapp2.0</TargetFrameworks>
• <TargetFrameworks>netstandard1.3</TargetFrameworks>

However when I do this:

<TargetFrameworks>netstandard1.3;netcoreapp2.0</TargetFrameworks>

I get the following build errors:

Severity Code Description Project File Line Suppression State
Error NMSBT001 Exception in initialization: System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.IO.FileNotFoundException: Could not load file or assembly 'GitVersionCore, Version=4.0.0.0, Culture=neutral, PublicKeyToken=null' or one of its dependencies. The system cannot find the file specified.
at GitVersionTask.WriteVersionInfoToBuildLog..ctor()
--- End of inner exception stack trace ---
at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor)
at System.Reflection.RuntimeConstructorInfo.Invoke(BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
at UtilPack.NuGet.MSBuild.TaskReferenceCreator.CreateTaskReferenceHolder(String taskTypeName, NuGetAssemblyResolver resolver, String packageID, String packageVersion, String assemblyPath, String msbuildFrameworkAssemblyName, ResolverLogger logger)
at UtilPack.NuGet.MSBuild.TaskReferenceCreator.CreateTaskReferenceHolder(String taskTypeName, NuGetAssemblyResolver resolver, String packageID, String packageVersion, String assemblyPath, String msbuildFrameworkAssemblyName, ResolverLogger logger)
at UtilPack.NuGet.MSBuild.NuGetTaskRunnerFactory.CreateExecutionHelper(String taskName, XElement taskBodyElement, String taskPackageID, String taskPackageVersion, String taskAssemblyFullPath, String taskAssemblyPathHint, BoundRestoreCommandUser restorer, ResolverLogger resolverLogger, GetFileItemsDelegate getFiles, String assemblyCopyTargetFolder, AppDomainSetup& appDomainSetup)
at UtilPack.NuGet.MSBuild.NuGetTaskRunnerFactory.<>c__DisplayClass30_3.b__3()
at System.Lazy1.CreateValue() at System.Lazy1.LazyInitValue()
at System.Lazy1.get_Value() at UtilPack.ReadOnlyResettableLazy1.get_Value()
at UtilPack.NuGet.MSBuild.NuGetTaskRunnerFactory.Initialize(String taskName, IDictionary`2 parameterGroup, String taskBody, IBuildEngine taskFactoryLoggingHost) repro 1
Error MSB4175 The task factory "UtilPack.NuGet.MSBuild.NuGetTaskRunnerFactory" could not be loaded from the assembly "C:\Users\Daz.nuget\packages\utilpack.nuget.msbuild\2.4.0\build\net45\UtilPack.NuGet.MSBuild.dll". Exception has been thrown by the target of an invocation. repro C:\Users\Daz.nuget\packages\gitversiontask\4.0.0-pullrequest1269-1530\build\functionality\GitVersionBuild.targets 6

There is a branch here for repro: https://github.com/dazinator/gitversion_pr1269/tree/issue

[stazz]

Hmm, I just tried the repro - it is compiling without errors for me. The output is (I built the .csproj file, and not .sln):

dotnet msbuild /t:Build src\repro\repro.csproj
Microsoft (R) Build Engine version 15.6.82.30579 for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

[NuGet Minimal]: Restoring packages for C:\Users\Staz\AppData\Local\Temp\ananh2m3.oml\dummy...
[NuGet Minimal]: Restoring packages for C:\Users\Staz\AppData\Local\Temp\ananh2m3.oml\dummy...
[NuGet Minimal]: Restoring packages for C:\Users\Staz\AppData\Local\Temp\c2lshs5m.3ku\dummy...
[NuGet Minimal]: Restoring packages for C:\Users\Staz\AppData\Local\Temp\udyhnull.phu\dummy...
[NuGet Minimal]: Restoring packages for C:\Users\Staz\AppData\Local\Temp\c2lshs5m.3ku\dummy...
[NuGet Minimal]: Restoring packages for C:\Users\Staz\AppData\Local\Temp\udyhnull.phu\dummy...
[NuGet Minimal]: Restoring packages for C:\Users\Staz\AppData\Local\Temp\kvjna5zr.44b\dummy...
[NuGet Minimal]: Restoring packages for C:\Users\Staz\AppData\Local\Temp\yejcqb4e.hj2\dummy...
[NuGet Minimal]: Restoring packages for C:\Users\Staz\AppData\Local\Temp\kvjna5zr.44b\dummy...
[NuGet Minimal]: Restoring packages for C:\Users\Staz\AppData\Local\Temp\yejcqb4e.hj2\dummy...
[NuGet Minimal]: Restoring packages for C:\Users\Staz\AppData\Local\Temp\cm35px3q.wln\dummy...
  repro -> gitversion_pr1269\src\repro\bin\Debug\netstandard1.3\repro.dll
[NuGet Minimal]: Restoring packages for C:\Users\Staz\AppData\Local\Temp\cm35px3q.wln\dummy...
  repro -> gitversion_pr1269\src\repro\bin\Debug\netcoreapp2.0\repro.dll

I was running it from Windows - maybe the error happened on Linux? I remember that sometimes there were some subtle differences between those two within MSBuild that occasionally caused problems.

[dazinator]

Are you building with these target frameworks in the csproj?

 <TargetFrameworks>netstandard1.3;netcoreapp2.0</TargetFrameworks>

I am running on windows, doing the build within VS2017 15.6.1

[stazz]

Also, at least for me, if you just changed from using <TargetFramework> to <TargetFrameworks> by editing .csproj file, you needed to delete bin and obj folders from the project, otherwise MSBuild didn't want to Clear/Restore/Build the project.

[stazz]

Are you building with these target frameworks in the csproj?

Yeah, those two - you can see the corresponding DLL file information lines in the output I pasted. :) I did the build from commandline though. Maybe that has some differences compared to VS...?

[dazinator]

Ok so dotnet build works. This problem only happens when building within VS.
Deleting the bin / obj directories and restoring from fresh doesn't clear the problem

[stazz]

I think I found the problem. The VS build uses desktop version of the GitVersionTask, and the build/net461 folder inside the NuGet package does not have the GitVersionCore.dll, while the build/netstandard1.5 (used by dotnet) has it. Thus the desktop version does not "see" the required DLL. I copied it to the net461 folder, and then got a different error (which is related to the fact that this DLL is compiled against .NET Standard 1.3 and since it is missing from .nuspec (as I manually copied it), the NuGet cannot tell enough information to UtilPack).

I think once you add the GitVersionCore.dll to the net461 folder properly via .nuspec file, things should start working. :)

[dazinator]

Thanks for having a look :-)

The GitVersionCore assembly for the desktop version is actually embedded inside the GitVersionTask.dll using a Fody weaver called Costura. If you open up the build/net461/GitVersionTask.dll using something like Telerik JustDecompile tool, you can see the dependencies are all bundled into the dll as embedded resources.

Note this embedding of dependencies is only done for the Desktop version of GitVersionTask.

The Fody weaver, aside from just embedding the dependencies as resources, also generates and includes an AssemblyLoader class which can handle resolving the assembly when its requested at runtime. It does this via the normal attaching a handler to the AppDomain.CurrentDomain.AssemblyResolve event.

But if this is the problem, why would it only occur when multi targeting? I can build through VS absolutely fine if I just target the one targetframework or the other. Which means the desktop task is working fine in that scenario? The error only occurs once I target more than one framework.. If there was a problem with a missing assembly or the way fody is resolving the dependencies, then I would expect it to fail on every build from within VS, not just the multitargeting.. :-)

[stazz]

Thanks for the info, I totally missed the fact that certain dependencies are embedded in GitVersionTask.dll as resources! I see it now (btw, I use ILSpy for decompiling stuff, that is a good tool too).

After some testing, the current situation is this:

• Builds using dotnet via commandline are working fine in both single-framework and multi-framework scenarios.
• Builds using c:\Program Files (x86)\Microsoft Visual Studio\2017\<edition>\MSBuild\15.0\Bin\MSBuild.exe via commandline are also working fine in both single-framework and multi-framework scenarios.
• Builds using MSVS are working fine in sinle-framework scenario, but failing in multi-framework scenario.

I looked into output of the build in MSVS. Apparently in both scenarios, the UtilPack executes the following tasks successfully, in this order:

1. WriteVersionInfoToBuildLog
2. GenerateGitVersionInformation
3. GetVersion

Even in successful invocations, there is a text "Failed to resolve GitVersionCore, Version=4.0.0.0, Culture=neutral, PublicKeyToken=null." indicating that UtilPack failed to resolve the assembly. However, in those cases, I guess the Costura's assembly resolve trick kicked in and saved it. However, when the WriteVersionInfoToBuildLog is invoked second time (in multi-framework scenario, this is where the error happens), the Costura no longer seems to resolve the references.

My guess is that maybe when the assembly is loaded again from the same location, it's Module initializer method (where Costura registers itself to AssemblyResolve method) is not run? Or maybe it thinks it is already attached, via some quirk in static variables? Really hard to tell. But it's definetly has to do with Costura, VS environment, and invoking the task twice - I verified that the UtilPack, in this case, does not dispose AppDomain or anything like that between task usages.

What is the reason you guys ended up using Costura in first place? Can it be avoided somehow? If it is a reasonable option, it would be worth investigating it.

[dazinator]

Thank you so much for looking into this further.

To be honest I am not sure of the historic reasons but I know that we used to ILMerge the assemblies. I changed that in my PR to use Costura embedding because I thought this would simplify the build process a little. However it sounds like this has introduced the problem so I will take your advice and look at switching it back to ILMerge or potentially removing it all together and move to seperate assemblies. Im not sure why they were eager for a single assembly but it sounds unnecessary really.

Ok i'll try that :-) cheers

[stazz]

I played around a bit, and I added a Console.WriteLine call to the module initializer method of GitVersionTask.dll. The VS build output, unfortunately, does not show any of the output done via Console class, but when calling MSBuild.exe manually, I noticed that my test output was shown on every time when UtilPack task factory was called. I seriously suspect that in VS environment, some things get optimized in such way that the module initializer method simply does not get always called (e.g. when loading same assembly from same location again).

Using ILMerge would be a good thing, I would recommend my own project CILMerge, but unfortunately it is a bit out of date. If the reason for the assembly merge is (like me) that you want to ensure that architectural layers only reference each other as needed (and disable circular references), then NsDepCop might be good option. I have not yet used it myself, but it seems to aim at exactly what I was doing when most of my projects were CILMerged: making sure that intra-project dependencies are handled well and according to rules. This way, instead of splitting your project into several assemblies, you just need to split it into several namespaces within one assembly (which is the end-result for project consumers anyway).

[dazinator]

Interesting. It sounds like a time sink to me to trace the exact root cause, so I have taken the easy road and removed all forms of dependency embedding. I have updated the repro, and it all seems to be working fine. To be honest being a build time package, I can't understand why embedding would be necessary - it can't be aesthetics, and the dependency graph is not complicated. I can only think that at some point in the past, older versions of msbuild were not able to locate assemblies correctly necessitating in the need to embed dependencies.. In any case, this doesn't seem to be a problem anymore, I'm not sure if that is due to msbuild, or Utilpack handling the resolution now - but either way, it works :-)

[dazinator]

Quick question - why so many restores? It seems three restores per target framework youy are building against:-

1>[NuGet Minimal]: Restoring packages for C:\Users\Administrator\AppData\Local\Temp\gx3n052t.hmk\dummy...
1>[NuGet Minimal]: Restoring packages for C:\Users\Administrator\AppData\Local\Temp\qboeldvg.jt0\dummy...
1>[NuGet Minimal]: Restoring packages for C:\Users\Administrator\AppData\Local\Temp\mf3sw4ks.kji\dummy...
1>repro -> E:\Repos\gitversion_pr12692\src\repro\bin\Debug\netstandard1.3\repro.dll
1>[NuGet Minimal]: Restoring packages for C:\Users\Administrator\AppData\Local\Temp\crxbznty.p3r\dummy...
1>[NuGet Minimal]: Restoring packages for C:\Users\Administrator\AppData\Local\Temp\kpfjhx4q.xf2\dummy...
1>[NuGet Minimal]: Restoring packages for C:\Users\Administrator\AppData\Local\Temp\snfmia33.s04\dummy...

Why three?

[dazinator]

Also is there a way to leave the temp directories around if you want to have a peek to see what is going on :-)

[dazinator]

Also, I notice its using C:\Users\Administrator\

So I log in to windows as "Daz" which is not an Admin.
When I open VS it asks for elevation, I have to type the password of the Administrator account.

In this scenario, do you think it should it use Daz for the temp directory rather than Administrator? I'm not sure, because I guess the process has been elevated to run under the Admin user, but it still feels weird I have files being placed under a different profile.

[stazz]

I'm not sure if that is due to msbuild, or Utilpack handling the resolution now - but either way, it works :-)

Glad to hear! It is most likely the combination of both MSBuild and UtilPack, but the main thing is that it works now!

why so many restores?

It runs three tasks under the UtilPack task factory per framework, so when building for two frameworks you get six restores (since each restore is done for each task), three per each framework.

Also is there a way to leave the temp directories around if you want to have a peek to see what is going on :-)

Ah, those paths only to satisfy the NuGet - the restore will fail if I don't put something in certain properties of the RestoreRequest, but it will never actually write anything to the directory. So it must be string, which looks like directory, and I just use Path.Combine( Path.GetTempPath(), Path.GetRandomFileName() ), and don't even bother creating the directory, as the restore will never write there.

Also, I notice its using C:\Users\Administrator\

Yeah - that's because your VS is running as administrator, so the Path.GetTempPath() will return administrator's temporary path, rather than the path of whoever started VS. That is something that can not be circumvented, since doing it would be finding security hole in OS.

[dazinator]

It runs three tasks under the UtilPack task factory per framework, so when building for two frameworks you get three restores (since each restore is done for each task).

Ah I see. Is there any potential of an optimisation around that in the case that the tasks are all from the same assembly? i.e so the number of restores in this case would drop to one?

p.s I am not complaining, I am just pre-empting questions that people will have when inspecting the build output :-)

Ah, those paths only to satisfy the NuGet - the restore will fail if I don't put something in certain properties of the RestoreRequest, but it will never actually write anything to the directory.

Ah ok. Would you perhaps consider changing this to use the repo or package directory? If it doesn't actually write anything there it's just aesthetics but it's much less alarming seeing a repo or package path in an msbuild output log than seeing a user profile path, and it would help eliminate questions like the one I had about the Admin directory. Just a consideration.

Thanks for clarifying!

[stazz]

Is there any potential of an optimisation

Long ago I investigated the methods in IBuildEngine4 interface, which would allow to share state between MSBuild invocations, exactly in hopes of this kind of optimization. IIRC I ran into some problems there though (maybe the state sharing methods were unusable until the actual task execution started?), so I decided to drop it at that time. I'll open separate issue for that.

I am not complaining

No worries - I think you're making valid points, and this is good discussion. 👍

Would you perhaps consider changing this to use the repo or package directory?

It's a good idea - I'll open issue for that. :)