search

dotnet / Nerdbank.GitVersioning

Stamp your assemblies, packages and more with a unique version generated from a single, simple version.json file and include git commit IDs for non-official builds.
https://www.nuget.org/packages/Nerdbank.GitVersioning
MIT License
1.36k stars 167 forks source link

Updating to 3.1.89 breaks the build #474

Closed shana closed 4 years ago

shana commented 4 years ago

I just updated my repo https://github.com/spoiledcat/UnityTools to 3.1.89 and the build breaks while parsing previous commits to the version.json file: 

error MSB4018: Newtonsoft.Json.JsonReaderException: After parsing a value an unexpected character was encountered: ". Path 'pathFilters[13]', line 18, position

The version.json file in question is https://github.com/spoiledcat/UnityTools/blob/ba88c9c240bc52c2b425677f8a4a3fea51f01c74/src/version.json

I suspect that the changes in https://github.com/dotnet/Nerdbank.GitVersioning/pull/465 is the culprit, which is a shame, I could really use it :P

AArnott commented 4 years ago

@saul can you please investigate?

saul commented 4 years ago

You’re missing a comma after the 13th (0 indexed) element of that array in your JSON file from what I can see.

I’m not sure how to proceed here @AArnott - I remember mentioning how we would handle broken JSON files in my PR but can’t remember what your suggestion was. I’m not on desktop at the moment but will check when I am.

AArnott commented 4 years ago

Indeed, this is an unparsable file. But how would a nb.gv update have introduced the build break? It seems any version would have coughed on this.

shana commented 4 years ago

Yeah I don't know how to proceed, the version.json file that's currently there doesn't have a comma on the array entry and it doesn't complain, the build is running fine.

shana commented 4 years ago

I fixed the file, there weren't that many commits with it so it didn't screw up the history too much. I'm still confused how it parses fine with the previous version... 😕

AArnott commented 4 years ago

Did it actually parse? Or did nb.gv perhaps just discard the file?

AArnott commented 4 years ago

I can repro it in a trivially created repo. I'll take a look.

AArnott commented 4 years ago

Apparently this is a very bizarre Newtonsoft.Json behavior. We were on v9 both before and after this upgrade. This change is the crux of it. When Newtonsoft.Json deserializes a string[] it permits the missing comma. But when it deserializes PathFilter[] it requires a higher level of syntax correctness. I have no idea why. I had no idea that newtonsoft.json was allowing bad syntax before. We could conceivably 'fix' this, but I'd like to first try enforcing correct syntax.

For folks who hit this and can't rewrite history to fix it, here's the prescriptive solution:

  1. Delete the version.json file and commit that change.
  2. Restore the version.json file, fix the syntax errors, and commit that change.
  3. Push both of these commits directly onto the branch you want to fix. Do not merge and do not squash. Squashing would compress the two commits into one, but the workaround requires that there be a commit where no version.json file exists in order for NB.GV to stop walking history. Merging would provide nb.gv a direct path through one of the merge commit's parents to the bad version.json file.
shana commented 4 years ago

@AArnott Wow ok yeah, I felt like the change from string to PathFilter was triggering this in some way, but that's just crazy. Learning new things about newtonsoft every day... :P