GitVersionTask 5.1.3 fails on appveyor FileLoadException

[jetersen]

GitVersionTask is unable to load Newtonsoft.Json

Unsure what makes it fail on appveyor: https://ci.appveyor.com/project/nmklotas/gitlabapiclient/builds/29753347#L138

works just fine locally and on GitHub Action

[jetersen]

@arturcic seems this commit might have changed this related to packing
9d18780

[jetersen]

Tested both with MSBuild and dotnet cli on AppVeyor Visual Studio 2019

Microsoft (R) Build Engine version 16.4.0+e901037fe for .NET Framework
Microsoft (R) Build Engine version 16.4.0+e901037fe for .NET Core

[jetersen]

Applied workaround for FileLock mentioned in #1031 which also meant I worked around the file loading issue with Newtonsoft.Json.

[asbjornu]

@Casz, would @rose-a's suggested reorganization in #1031 (comment) help?

[jetersen]

I would prefer if I could use GitVersionTask without any issues and without calling the dotnet gitversion on appveyor.
I believe the suggested would perhaps solve the file locking.

However unrelated to loading issues with Newtonsoft.Json

[asbjornu]

Thanks for the feedback, @Casz. I hope we figure out the FileLoadException with Newtonsoft.Json, although I don't have any good ideas what the problem might be as of now. Do you have many other projects referencing (different versions of) Newtonsoft.Json in the same solution?

[jetersen]

GitLabApiClient reference Newtonsoft.Json v12.0.3 however this is not a issue with GitVersionTask v5.1.2 and only fails on appveyor with v5.1.3 (works locally)

[jetersen]

I'll see if I can provide a repro :)

[jetersen]

https://ci.appveyor.com/project/casz/gitversiontask-fileloadexception-repro
https://github.com/casz/gitversiontask-fileloadexception-repro

I'll try some variants on the some branches

[jetersen]

Okay looking into the differences on the GitVersionCore.dll it seems we have different referenced assemblies mentioned

v5.1.2

LibGit2Sharp, Version=0.26.0.0, Culture=neutral, PublicKeyToken=7cbde695407f0333
Microsoft.Extensions.DependencyInjection.Abstractions, Version=3.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60
Microsoft.Extensions.Options, Version=3.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60
netstandard, Version=2.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
YamlDotNet, Version=8.0.0.0, Culture=neutral, PublicKeyToken=ec19458f3c15af5e

v5.1.3

LibGit2Sharp, Version=0.26.0.0, Culture=neutral, PublicKeyToken=7cbde695407f0333
Microsoft.Extensions.DependencyInjection.Abstractions, Version=3.0.1.0, Culture=neutral, PublicKeyToken=adb9793829ddae60
Microsoft.Extensions.Options, Version=3.0.1.0, Culture=neutral, PublicKeyToken=adb9793829ddae60
netstandard, Version=2.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
Newtonsoft.Json, Version=12.0.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed
YamlDotNet, Version=8.0.0.0, Culture=neutral, PublicKeyToken=ec19458f3c15af5e

Seems that without referencing Newtonsoft.Json we still manage to work because GitVersionTask is running inside the MSBuild context which has Newtonsoft.Json loaded.

[jetersen]

I'd like to question the usage/reference of newtonsoft.json as it is only used here: https://github.com/GitTools/GitVersion/blob/0e9abcdc2ee4cae30ae41d715bb25169368f76ae/src/GitVersionCore/BuildServers/AppVeyor.cs

[asbjornu]

Seeing how /output json manages to do without Newtonsoft.Json, I have to agree, @Casz. The JsonOutputFormatter.ToJson() method could probably be duplicated and modified to serve the need of AppVeyor as well.

GitVersion/src/GitVersionCore/OutputFormatters/JsonOutputFormatter.cs

Lines 11 to 28 in 0e9abcd

	public static string ToJson(VersionVariables variables)
	{
	var builder = new StringBuilder();
	builder.AppendLine("{");
	var last = variables.Last().Key;
	foreach (var variable in variables)
	{
	var isLast = (variable.Key == last);
	// preserve leading zeros for padding
	if (int.TryParse(variable.Value, out var value) && NotAPaddedNumber(variable))
	builder.AppendLineFormat(" \"{0}\":{1}{2}", variable.Key, value, isLast ? string.Empty : ",");
	else
	builder.AppendLineFormat(" \"{0}\":\"{1}\"{2}", variable.Key, variable.Value, isLast ? string.Empty : ",");
	}
	builder.Append("}");
	return builder.ToString();
	}

When this code is touched, I'd like to see JsonOutputFormatter removed and the ToJson method moved to the VersionVariables class as its overridden ToString method. A PR that removes the Newtonsoft.Json dependency would be warmly welcomed. 🙏

[jetersen]

@asbjornu I doubt you need to refactor at all.

I would simply write the JSON inside appveyor class. We are talking about two objects, first one is with one key value, second is with two key value.

[asbjornu]

I agree that AppVeyor's needs don't require a refactor. I still would like to see a refactor, though. 😄 You decide how large you want the PR to be, it will be appreciated no matter how small or large. 😃