Unable to reference Newtonsoft.Json > 11.0.2

What You Are Seeing?

When referencing Newtonsoft.Json > 11.0.2, Cake fails when loading assembly. This is because Cake.Core (netstandard 2.0 only) and Cake.Nuget have a dependency on Newtonsoft.Json 11.0.2, which it loads during execution.

This is the same issue as captured under #2116, however the changes made to the resolver do not appear to work, as Cake is trying to load the assemblies based on absolute path.

I had previously submitted #2597 to pull in the latest version of Newtonsoft.Json at point of build, but this was rejected as it did not pin to a specific version (a point I concede).

I propose to update Cake.Core.csproj and Cake.Nuget.csproj to point directly to latest version of Newtonsoft.Json, which is, at the time of writing, 12.0.2.

What is Expected?

Assembly to be loaded without error

What version of Cake are you using?

0.34.1

Are you running on a 32 or 64 bit system?

64

What environment are you running on? Windows? Linux? Mac?

Windows & Linux, using dotnet core tool
Does not appear to affect framework version of Cake, as this does not appear to load Newtonsoft.Json during normal execution.

Are you running on a CI Server? If so, which one?

N/A - issue occurs locally too.

How Did You Get This To Happen? (Steps to Reproduce)

Add the following to build.cake:
#addin nuget:?package=Newtonsoft.Json&version=12.0.2

Output Log

Output:

Error: System.IO.FileLoadException: Could not load file or assembly 'Newtonsoft.Json, Version=12.0.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed'.
   at System.Runtime.Loader.AssemblyLoadContext.LoadFromPath(IntPtr ptrNativeAssemblyLoadContext, String ilPath, String niPath, ObjectHandleOnStack retAssembly)
   at System.Runtime.Loader.AssemblyLoadContext.LoadFromAssemblyPath(String assemblyPath)
   at System.Reflection.Assembly.LoadFrom(String assemblyFile)
   at Cake.Core.Reflection.AssemblyLoader.Load(FilePath path, Boolean verify) in C:\projects\cake\src\Cake.Core\Reflection\AssemblyLoader.cs:line 31
   at Cake.Core.Scripting.ScriptRunner.Run(IScriptHost host, FilePath scriptPath, IDictionary`2 arguments) in C:\projects\cake\src\Cake.Core\Scripting\ScriptRunner.cs:line 171
   at Cake.Commands.BuildCommand.Execute(CakeOptions options) in C:\projects\cake\src\Cake\Commands\BuildCommand.cs:line 41
   at Cake.CakeApplication.Run(CakeOptions options) in C:\projects\cake\src\Cake\CakeApplication.cs:line 45
   at Cake.Program.Main() in C:\projects\cake\src\Cake\Program.cs:line 73

@gep13 - issue as requested

@pritchums thank you for adding this, I appreciate it!

@mholo65 @daveaglick can either of you offer anything here?

While updating the Newtonsoft.Json PackageReference to 12.0.2 will likely fix the issue, I suspect that it will just happen again when a newer version comes out. Right?

however the changes made to the resolver do not appear to work, as Cake is trying to load the assemblies based on absolute path

It took me a minute to grok what's going on here:

The ScriptAssemblyResolver mentioned above hooks AppDomain.CurrentDomain.AssemblyResolve which only gets called if an assembly load is attempted by name and fails:

cake/src/Cake/Scripting/Roslyn/ScriptAssemblyResolver.cs

Line 29 in 325efb8

private Assembly AssemblyResolve(object sender, ResolveEventArgs args)
...however, the AssemblyHelper called by AssemblyLoader only attempts a load by name if the assembly path isn't valid:

cake/src/Cake.Core/Polyfill/AssemblyHelper.cs

Line 41 in 325efb8

if (path.Segments.Length == 1 && !fileSystem.Exist(path))

In this case ScriptRunner finds and loads references by path, which is why the binding fix in ScriptAssemblyResolver never comes into play:

cake/src/Cake.Core/Scripting/ScriptRunner.cs

Lines 161 to 179 in 325efb8

    
           // Load all references. 
        
           var applicationRoot = _environment.ApplicationRoot; 
        
           var assemblies = new HashSet<Assembly>(); 
        
           assemblies.AddRange(_conventions.GetDefaultAssemblies(applicationRoot)); 
        
           foreach (var reference in result.References) 
        
           { 
        
               var referencePath = new FilePath(reference); 
        
               if (host.Context.FileSystem.Exist(referencePath)) 
        
               { 
        
                   var assembly = _assemblyLoader.Load(referencePath, true); 
        
                   assemblies.Add(assembly); 
        
               } 
        
               else 
        
               { 
        
                   // Add a reference to the session. 
        
                   session.AddReference(referencePath); 
        
               } 
        
           }

...and because Cake.Core references Newtonsoft.Json 11.0.2, and that reference is already in the AppDomain when ScriptRunner tries to load an alternate version by path, we never redirect the binding to the already loaded version and BOOM!

Updating to the latest version of Newtonsoft.Json will fix the issue short-term - until a newer version of Newtonsoft.Json comes out and someone tries to reference that as an addin. To fix this long-term, we'll need a similar code-based binding redirection like we did for the ScriptAssemblyResolver inside either ScriptRunner (or probably better in AssemblyHelper).

My first instinct that we should replace this line with an Assembly.ReflectionOnlyLoadFrom() first, check if the current app domain contains an assembly with the same name but different version, and then skip the actual Assembly.LoadFrom() if so. That'll still cause runtime problems if the script is expecting and uses features from a newer version than what's already loaded, but at least it won't crash and will hopefully work in most cases. We should also still update Newtonsoft.Json to avoid as many runtime missing method problems as possible though.

@mholo65 can you check my work and assumptions here?

@daveaglick There is no AppDomain on .NET Core so it won't solve the problem there.
Also, does Cake.Core really reference Newtonsoft.Json?

@patriksvensson yes...

https://github.com/cake-build/cake/blob/develop/src/Cake.Core/Cake.Core.csproj#L19

@patriksvensson Yeah, poor choice of words. I think we'll need to check the AssemblyLoadContext equivalent for currently loaded assemblies when deciding to load a same-assembly-but-different-version in the AssemblyHelper.

@daveaglick said:

My first instinct that we should replace this line with an Assembly.ReflectionOnlyLoadFrom() first, check if the current app domain contains an assembly with the same name but different version, and then skip the actual Assembly.LoadFrom() if so.

I actually tried that in #2564 as I had problems loading System.Diagnostics.DiagnosticSource as it was already referenced by the app (via NetCoreApp IIRC). However I couldn’t find the assembly when searching loaded assemblies. I added a Try/Catch instead and ”it works...”.

”it works...”

😬

@gep13 @mholo65 @daveaglick Why is that assembly being referenced there? Does anyone know?

You know those times when you debug an App in VS and the debug window is flooded with ”Could not load Assembly XYZ...” but the app still works... 😄

@gep13 @mholo65 @daveaglick Why is that assembly being referenced there? Does anyone know?

Because NuGet client libs

@gep13 @mholo65 @daveaglick Why is that assembly being referenced there? Does anyone know?

Because NuGet client libs

I see no reason why it should be in Cake.Core because of Cake.NuGet?

@gep13 @mholo65 @daveaglick Why is that assembly being referenced there? Does anyone know?

Because NuGet client libs

I see no reason why it should be in Cake.Core because of Cake.NuGet?

git blame? Sorry not at computer.

@gep13 @mholo65 @daveaglick Why is that assembly being referenced there? Does anyone know?

Because NuGet client libs

I see no reason why it should be in Cake.Core because of Cake.NuGet?

git blame? Sorry not at computer.

It's in Cake.Core, because the Microsoft.Extensions.DependencyModel 2.0.4 is referenced for netstandard, and this references Newtonsoft.Json >= 9.0.1

(and Cake.NuGet via NuGet.Packaging)

Hi all. Just wondering what your current thoughts are on this?

The Cake.Core dependency on Microsoft.Extensions.DependencyModel looks superfluous to me, given Cake uses Autofac. The version history suggests this was added when Cake was originally modified to support dotnet core, and I suspect it was a MS default addition. Rider/ReSharper suggests the reference is unused, and (with a very simple test case) Cake appears to still function under dotnet core without this reference.

Removing the Cake.Core dependency on Microsoft.Extensions.DependencyModel would allow the Cake.Core dependency on Newtonsoft.Json to also be removed, but the Cake.NuGet dependency on Newtonsoft.Json will need remain due to the dependency on NuGet.Packaging.

It sounds like you have some good ideas to improve assembly loading, but would it be worth breaking out that improvement into a separate issue? While work on that issue in progress, we could update the existing references to Newtonsoft.Json to latest (12.0.2), so anyone using cake on dotnet core can reference packages using Newtonsoft.Json > 11.0.2.

Microsost.Extensions.DependencyModel contains abstractions for reading .deps files and AFAIK nothing that we need. Could be removed IMO.

Thanks @mholo65

Any objections if I submit a single PR removing the Cake.Core dependency on Microsoft.Extensions.DependencyModel, and updating Newtonsoft.Json in Cake.NuGet to 12.0.2?

Hi @devlead @gep13 @mholo65 @daveaglick
Wondering what your thoughts are on adding PR #2615 to the 0.35.0 milestone? This is currently a blocker for us, as one of our dependencies annoyingly requires Newtonsoft.Json 12.0.2.

Fixed by #2597

	// Load all references.
	var applicationRoot = _environment.ApplicationRoot;
	var assemblies = new HashSet<Assembly>();
	assemblies.AddRange(_conventions.GetDefaultAssemblies(applicationRoot));

	foreach (var reference in result.References)
	{
	var referencePath = new FilePath(reference);
	if (host.Context.FileSystem.Exist(referencePath))
	{
	var assembly = _assemblyLoader.Load(referencePath, true);
	assemblies.Add(assembly);
	}
	else
	{
	// Add a reference to the session.
	session.AddReference(referencePath);
	}
	}