code cleanup: remove json output formatter #2019

jetersen · 2020-01-03T14:51:03Z

No description provided.

jetersen · 2020-01-03T14:51:45Z

I much have a twitchy finger 😭

jetersen · 2020-01-03T15:30:28Z

src/GitVersionCore/OutputVariables/VersionVariables.cs

+
+        public override string ToString()
+        {
+            var builder = new StringBuilder();


Would you prefer I moved this to a JsonBuilder?
So it could be reused by #2017 ?

Hm. How would a generic JsonBuilder look like? If you have a suggestion, please add it to the PR.

jetersen · 2020-01-03T15:35:39Z

Not sure why build is failing

jetersen · 2020-01-06T20:41:53Z

🤷‍♂ Not sure what makes this minor change explode the entire testsuite.

jetersen · 2020-01-06T20:42:57Z

Would love to add an generic JsonBuilder but it does not help when CI is utterly bogus 😭

asbjornu · 2020-01-07T08:33:17Z

Good question, @Casz. Seems like there's some cleanup job that fails.

gep13 · 2020-01-07T09:00:23Z

Not sure why this would be a problem, but on the AppVeyor build there is this:

InnerException:	System.Xml.XmlException: The token 'false' was expected but found 'fix-r'.

It is a little coincidental that the branch name for the PR starts with fix as well. Could this be part of the problem? @asbjornu @arturcic

asbjornu · 2020-01-07T09:05:49Z

That sounds both crazy and plausible at the same time, @gep13. Why would a branch name cause an XmlException, I wonder?

jetersen · 2020-01-21T06:26:48Z

Trying to run tests locally I get:

System.ComponentModel.Win32Exception : The specified executable is not a valid application for this OS platform.
   at System.Diagnostics.Process.StartWithCreateProcess(ProcessStartInfo startInfo)
   at System.Diagnostics.Process.Start()
   at System.Diagnostics.Process.Start(ProcessStartInfo startInfo)
   at System.Diagnostics.Process.Start(String fileName, String arguments)
   at Shouldly.Configuration.DiffTool.Open(String receivedPath, String approvedPath, Boolean approvedExists) in C:\projects\shouldly\src\Shouldly\Configuration\DiffTool.cs:line 59
   at Shouldly.ShouldMatchApprovedTestExtensions.ShouldMatchApproved(String actual, Func`1 customMessage, Action`1 configureOptions) in C:\projects\shouldly\src\Shouldly\ShouldlyExtensionMethods\ShouldMatchApprovedTestExtensions.cs:line 79
   at Shouldly.ShouldMatchApprovedTestExtensions.ShouldMatchApproved(String actual, Action`1 configureOptions) in C:\projects\shouldly\src\Shouldly\ShouldlyExtensionMethods\ShouldMatchApprovedTestExtensions.cs:line 25
   at GitVersionCore.Tests.JsonVersionBuilderTests.Json() in C:\git\code\GitVersion\src\GitVersionCore.Tests\JsonVersionBuilderTests.cs:line 41

jetersen · 2020-01-21T06:30:09Z

~~Think I'll recreate the PR to see if it fixes the odd xml exception~~

jetersen · 2020-01-21T07:42:52Z

Think I figured it out, it related to https://github.com/cake-build/cake/tree/develop/src/Cake.Common/Tools/GitVersion basically this is a breaking change.

gep13 · 2020-01-21T07:50:19Z

@Casz just so I am clear... are you saying that we would need to update the Cake aliases for GitVersion to account for the change that you are making here? I haven't been following this issue/PR very closely, but can you give me a recap of the change that is being made here, and what would need to change in Cake to make this happen?

jetersen · 2020-01-21T10:21:11Z

The idea was to Remove the JSON output formatter and override the to string method

gep13 · 2020-01-21T10:23:18Z

@Casz said...
The idea was to Remove the JSON output formatter and override the to string method

That part I understand, but why is this a breaking change?

What specifically needs to be addressed in the Cake aliases to account for this?

jetersen · 2020-01-21T10:31:27Z

Not sure I haven't had a chance to dig into the code. I just see it's going through the GitVersionAlias and complains about the JSON Output:

See:

StackTrace:        at System.Runtime.Serialization.XmlObjectSerializer.ReadObjectHandleExceptions(XmlReaderDelegator reader, Boolean verifyObjectName, DataContractResolver dataContractResolver)
   at System.Runtime.Serialization.Json.DataContractJsonSerializerImpl.ReadObject(XmlDictionaryReader reader)
   at System.Runtime.Serialization.Json.DataContractJsonSerializerImpl.ReadObject(Stream stream)
   at System.Runtime.Serialization.Json.DataContractJsonSerializer.ReadObject(Stream stream)
   at Cake.Common.Tools.GitVersion.GitVersionRunner.Run(GitVersionSettings settings) in C:\projects\cake\src\Cake.Common\Tools\GitVersion\GitVersionRunner.cs:line 65
   at Cake.Common.Tools.GitVersion.GitVersionAliases.GitVersion(ICakeContext context, GitVersionSettings settings) in C:\projects\cake\src\Cake.Common\Tools\GitVersion\GitVersionAliases.cs:line 121
   at Submission#0.GitVersion(GitVersionSettings settings)
   at Submission#0.GetVersion(BuildParameters parameters)
   at Submission#0.<<Initialize>>b__0_83(ISetupContext context)
        Message:        There was an error deserializing the object of type Cake.Common.Tools.GitVersion.GitVersionInternal. The token 'false' was expected but found 'fix-r'.
        Data:   [  ]
        InnerException: System.Xml.XmlException: The token 'false' was expected but found 'fix-r'.
   at System.Xml.XmlExceptionHelper.ThrowXmlException(XmlDictionaryReader reader, String res, String arg1, String arg2, String arg3)
   at System.Runtime.Serialization.Json.XmlJsonReader.Read()
   at System.Xml.XmlBaseReader.ReadElementContentAsString()
   at ReadGitVersionFromJson(XmlReaderDelegator , XmlObjectSerializerReadContextComplexJson , XmlDictionaryString , XmlDictionaryString[] )

Specifically: ReadGitVersionFromJson and Cake.Common.Tools.GitVersion.GitVersionRunner.Run

gep13 · 2020-01-21T10:32:56Z

Do we have a comparison of what is output before and after this change?

jetersen · 2020-01-21T10:35:37Z

The issue is located in the GitVersionInternal method as I have changed the ToString method.

https://github.com/cake-build/cake/blob/7e8a8013a43c4bd5b15e16eea3c6347ffc572f35/src/Cake.Common/Tools/GitVersion/GitVersionInternal.cs#L202-L206

jetersen · 2020-01-21T10:42:15Z

Let me try and revert the ToString method change.

jetersen · 2020-01-21T11:03:37Z

No... not related to my changes.

I reset my branch to upstream/master and still got the same error.

gep13 · 2020-01-21T11:06:11Z

@Casz ok, can we go back to my suggestion regarding the branch name? Can you submit your PR from a different branch name, so that we can rule that out. And if that is the problem, then we have a bug that needs to be addressed.

jetersen · 2020-01-21T11:06:24Z

I tried see #2058

jetersen · 2020-01-21T11:06:43Z

but it's also failing locally when I checkout master branch

jetersen · 2020-01-21T11:11:06Z

FYI!!!! This is also failing in CI on Master branch but we are apparently ignoring it:

https://ci.appveyor.com/project/GitTools/gitversion/builds/30222649#L138

jetersen · 2020-01-21T11:14:57Z

git checkout -b test
./build.ps1 -script build.cake -target Default

Results in

Resolving assembly System.Private.DataContractSerialization.resources, Version=4.1.5.0, Culture=en-US, PublicKeyToken=b03f5f7f11d50a3a
Exception while resolving assembly System.Private.DataContractSerialization.resources: Could not load file or assembly 'System.Private.DataContractSerialization.resources, Culture=neutral, PublicKeyToken=null'. The system cannot find the file specified.
Resolving assembly System.Private.Xml.resources, Version=4.0.2.0, Culture=en-US, PublicKeyToken=cc7b13ffcd2ddd51
Exception while resolving assembly System.Private.Xml.resources: Could not load file or assembly 'System.Private.Xml.resources, Culture=neutral, PublicKeyToken=null'. The system cannot find the file specified.
        TargetSite:     System.Object ReadObjectHandleExceptions(System.Runtime.Serialization.XmlReaderDelegator, Boolean, System.Runtime.Serialization.DataContractResolver)
        StackTrace:        at System.Runtime.Serialization.XmlObjectSerializer.ReadObjectHandleExceptions(XmlReaderDelegator reader, Boolean verifyObjectName, DataContractResolver dataContractResolver)
   at System.Runtime.Serialization.Json.DataContractJsonSerializerImpl.ReadObject(XmlDictionaryReader reader)
   at System.Runtime.Serialization.Json.DataContractJsonSerializerImpl.ReadObject(Stream stream)
   at System.Runtime.Serialization.Json.DataContractJsonSerializer.ReadObject(Stream stream)
   at Cake.Common.Tools.GitVersion.GitVersionRunner.Run(GitVersionSettings settings) in C:\projects\cake\src\Cake.Common\Tools\GitVersion\GitVersionRunner.cs:line 65
   at Cake.Common.Tools.GitVersion.GitVersionAliases.GitVersion(ICakeContext context, GitVersionSettings settings) in C:\projects\cake\src\Cake.Common\Tools\GitVersion\GitVersionAliases.cs:line 121
   at Submission#0.GitVersion(GitVersionSettings settings)
   at Submission#0.GetVersion(BuildParameters parameters)
   at Submission#0.<<Initialize>>b__0_83(ISetupContext context)
        Message:        There was an error deserializing the object of type Cake.Common.Tools.GitVersion.GitVersionInternal. The token 'true' was expected but found 'test'.
        Data:   [  ]
        InnerException: System.Xml.XmlException: The token 'true' was expected but found 'test'.
   at System.Xml.XmlExceptionHelper.ThrowXmlException(XmlDictionaryReader reader, String res, String arg1, String arg2, String arg3)
   at System.Runtime.Serialization.Json.XmlJsonReader.Read()
   at System.Xml.XmlBaseReader.ReadElementContentAsString()
   at ReadGitVersionFromJson(XmlReaderDelegator , XmlObjectSerializerReadContextComplexJson , XmlDictionaryString , XmlDictionaryString[] )
   at System.Runtime.Serialization.Json.JsonClassDataContract.ReadJsonValueCore(XmlReaderDelegator jsonReader, XmlObjectSerializerReadContextComplexJson context)
   at System.Runtime.Serialization.Json.JsonDataContract.ReadJsonValue(XmlReaderDelegator jsonReader, XmlObjectSerializerReadContextComplexJson context)
   at System.Runtime.Serialization.Json.XmlObjectSerializerReadContextComplexJson.ReadDataContractValue(DataContract dataContract, XmlReaderDelegator reader)
   at System.Runtime.Serialization.XmlObjectSerializerReadContext.InternalDeserialize(XmlReaderDelegator reader, String name, String ns, Type declaredType, DataContract& dataContract)
   at System.Runtime.Serialization.XmlObjectSerializerReadContextComplex.InternalDeserialize(XmlReaderDelegator xmlReader, Type declaredType, DataContract dataContract, String name, String ns)
   at System.Runtime.Serialization.Json.DataContractJsonSerializerImpl.InternalReadObject(XmlReaderDelegator xmlReader, Boolean verifyObjectName)
   at System.Runtime.Serialization.XmlObjectSerializer.InternalReadObject(XmlReaderDelegator reader, Boolean verifyObjectName, DataContractResolver dataContractResolver)
   at System.Runtime.Serialization.XmlObjectSerializer.ReadObjectHandleExceptions(XmlReaderDelegator reader, Boolean verifyObjectName, DataContractResolver dataContractResolver)
        HelpLink:
        Source: System.Private.DataContractSerialization
        HResult:        -2146233076

jetersen · 2020-01-21T12:16:37Z

Okay, I recloned the repo, and now I am not seeing any failures locally 🤔

jetersen · 2020-01-21T12:26:39Z

Hmm locally it works now, I need to prune or recreate my repo somehow...

jetersen closed this Jan 3, 2020

jetersen reopened this Jan 3, 2020

jetersen closed this Jan 3, 2020

jetersen reopened this Jan 3, 2020

jetersen commented Jan 3, 2020

View reviewed changes

jetersen mentioned this pull request Jan 20, 2020

remove usage of Newtonsoft.Json #2017

Merged

jetersen closed this Jan 21, 2020

jetersen reopened this Jan 21, 2020

add JSON Serializer

eb5145a

jetersen closed this Jan 21, 2020

jetersen deleted the fix/remove-json-output-formatter branch January 21, 2020 12:29

code cleanup: remove json output formatter #2019

code cleanup: remove json output formatter #2019

Uh oh!

Conversation

jetersen commented Jan 3, 2020

Uh oh!

jetersen commented Jan 3, 2020

Uh oh!

jetersen Jan 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asbjornu Jan 6, 2020

Choose a reason for hiding this comment

Uh oh!

jetersen commented Jan 3, 2020

Uh oh!

jetersen commented Jan 6, 2020

Uh oh!

jetersen commented Jan 6, 2020

Uh oh!

asbjornu commented Jan 7, 2020

Uh oh!

gep13 commented Jan 7, 2020

Uh oh!

asbjornu commented Jan 7, 2020

Uh oh!

jetersen commented Jan 21, 2020

Uh oh!

jetersen commented Jan 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jetersen commented Jan 21, 2020

Uh oh!

gep13 commented Jan 21, 2020

Uh oh!

jetersen commented Jan 21, 2020

Uh oh!

gep13 commented Jan 21, 2020

Uh oh!

jetersen commented Jan 21, 2020

Uh oh!

gep13 commented Jan 21, 2020

Uh oh!

jetersen commented Jan 21, 2020

Uh oh!

jetersen commented Jan 21, 2020

Uh oh!

jetersen commented Jan 21, 2020

Uh oh!

gep13 commented Jan 21, 2020

Uh oh!

jetersen commented Jan 21, 2020

Uh oh!

jetersen commented Jan 21, 2020

Uh oh!

jetersen commented Jan 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jetersen commented Jan 21, 2020

Uh oh!

jetersen commented Jan 21, 2020

Uh oh!

jetersen commented Jan 21, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jetersen Jan 3, 2020 •

edited

Loading

jetersen commented Jan 21, 2020 •

edited

Loading

jetersen commented Jan 21, 2020 •

edited

Loading