Initial check-in for the System.Text.Json library and solution

[ahsonkhan]

cc @joshfree, @KrzysztofCwalina, @steveharter, @JeremyKuhne, @bartonjs, @GrabYourPitchforks, @steveharter, @terrajobst, @davidfowl, @JamesNK, @glennc, @stephentoub, cc @safern

[stephentoub]

I'm wondering if we should only mention reader/writer in this PR and then update it to include serializer/deserializer when they're actually there. I'm fine either way.

[davidfowl]

cc @DamianEdwards ?

[ahsonkhan]

These APIs will be targeting .NET Core specifically as part of v1.
Providing a netstandard implementation can be considered in the future.

I left it as netstandard1.1 for now but this is not a guarantee by any means.

[JamesNK]

Include netstandard2.0 to avoid large dependency graph from 1.x?

[ahsonkhan]

That diff is out-dated. I no longer have netstandard1.1 as a configuration. If I did, I would include netstandard2.0 as well, yes.

[stephentoub]

There's a netstandard 1.1 ref?

[stephentoub]

Same question... I was expecting to see netcoreapp here.

[stephentoub]

Why are we checking in GPLv3 stuff?!

[ahsonkhan]

From http://plantuml.com/faq

You can also use:
LGPL license
Apache license
Eclipse Public license
MIT license

[stephentoub]

The .svg says this:

[ahsonkhan]

I can upload the image as png instead of the svg then.

Edit: Oops, was looking at the wrong svg, I see you meant the license file (which I have now updated).

[joshfree]

Good catch @stephentoub. Lets remove the UML due to the licensing.

[ahsonkhan]

Fixed the licensing to MIT instead.

[stephentoub]

I'm not seeing the actual code here, other than a single enum. Am I missing it?

[ahsonkhan]

I'm not seeing the actual code here, other than a single enum. Am I missing it?

Nope, I wanted to get the initial skeleton checked-in first. The rest of the code will follow sooon.

[joshfree]

How about something with more symmetry with the XmlReader description such as "Provides the System.Text.Json.Utf8JsonReader type, that provides fast, noncached, forward-only access to JSON data."

[ahsonkhan]

For now, that works, but eventually the library will contain components outside of reader/writer. We can adjust it then.

[joshfree]

I thought we were going to go with the alternate name of Utf8JsonReader to be consistent with other types like Utf8String, no?

[ahsonkhan]

No, from the API review: https://github.com/dotnet/apireviews/pull/73/files#diff-244c336af7586c3fec32cb2d485d9dc7R63

Also, related discussion:
dotnet/corefxlab#2577 (comment)

We can re-visit the name if you feel strongly about it.

[bartonjs]

Despite being in the camp that suggested moving the "Utf8" to after "Json", I keep reading this now as some sort of JSON-ified "UTF-8 Reader". So maybe we should take another shot at the name. Utf8JsonReader or JsonReaderUtf8 being the obvious remaining candidates. "JsonReaderUtf8" sorts better, "Utf8JsonReader" seems more Englishy.

[stephentoub]

I'm not worried about sorting: it's all in the Json namespace, and IntelliSense handles such cases well IMO. So from my perspective if we're not going to go with JsonUtf8Reader, it should be Utf8JsonReader.

[danmoseley]

For what it's worth JsonUtf8Reader sounds like it's some sort of JSON-ified "UTF-8 Reader" to me as well. Either of those alternatives sound better to me.

[ahsonkhan]

Alright, Utf8JsonReader it is. No one ever said naming things was easy.

https://spin.atomicobject.com/2017/09/01/naming-things-is-hard/

[joshfree]

I'd add @steveharter here too.

[joshfree]

let's replace the UML with just a list of types in the markdown for now

[jkotas]

This is not consistent with what you have in src/System.Text.Json/src/Configurations.props

[ahsonkhan]

I left it as netstandard1.1 for now but this is not a guarantee by any means. I strongly suspect future APIs will require us to drop this naturally, but I didn't want to drop it artificially until necessary.

But, if it is causing confusion, I can drop to .NET Core only from the get go.

[ahsonkhan]

Is this the appropriate assembly version to start with? cc @ericstj

[safern]

We usually start with 4.0.0.0 from as far as I know.

[weshaggard]

It isn't necessary to start at 4.0.0.0 but we have generally done that for consistency with others which have a historical reason for doing so.

[safern]

Will you be having Unix specific implementation? Because this is only needed if you’re going to have specific details for Unix and for windows and then conditionally include them.

[ahsonkhan]

No, I will not. Should I just have netcoreapp-Windows_NT then? Having just netcoreapp fails to compile.

[jkotas]

It fails to compile because of you are referencing CoreLib and netcoreapp implementation directly in .csproj. This package should not be referencing CoreLib. It should be just referencing the public surface via reference assemblies. Once you fix that, you will be able to make this to just netcoreapp.

[safern]

Yeah you should be able to just reference System.Runtime and build for netcoreapp os agnostic.

[ahsonkhan]

Should I leave the uap configuration with the os specified?

Also, is it fine to keep the project reference to the System.Runtime ref within the System.Text.Json ref?

[safern]

Inside ref projects we strongly push toward having only project reference to other ref projects because the reference assemblies are the first thing we build and ordering is not guaranteed so we use project references in ref projects to make sure we build all its dependencies when building a project.

Having the uap being os specific doesn't hurt anything because uap is not supported in Unix, however we try to only use it os specific when we're going to have conditions in the csproj using $(TargetsWindows) property. Because in order for it to be set correctly the configuration needs to be os specific.

[safern]

Same question.

[ahsonkhan]

@dotnet-bot test Linux arm64 Release Build

https://mc.dot.net/#/user/ahsonkhan/pr~2Fjenkins~2Fdotnet~2Fcorefx~2Fmaster~2F/test~2Ffunctional~2Fcli~2F/b9fd17db15453f5dc0065998644fdcfba730c3bf/workItem/System.Net.NameResolution.Pal.Tests

System.Net.NameResolution.PalTests.NameResolutionPalTests/TryGetAddrInfo_HostName

Unhandled Exception of Type Xunit.Sdk.EqualException
Message :
Assert.Equal() Failure
Expected: Success
Actual:   TryAgain
Stack Trace :
   at System.Net.NameResolution.PalTests.NameResolutionPalTests.TryGetAddrInfo_HostName() in /mnt/j/workspace/dotnet_corefx/master/linux-TGroup_netcoreapp+CGroup_Release+AGroup_arm64+TestOuter_false_prtest/src/System.Net.NameResolution/tests/PalTests/NameResolutionPalTests.cs:line 54

[bartonjs]

Why do we need/want any of this PlantUML stuff? I don't see any other UML in corefx.

[stephentoub]

I agree. I don't think the UML/graphics add any real value to the readme.

[KrzysztofCwalina]

Are we going to have a portable package for this?

[ahsonkhan]

These APIs will be targeting .NET Core specifically as part of v1.
Providing a netstandard implementation can be considered in the future.

We will gather feedback from users and take that into account when deciding if we should have a portable package.

[weshaggard]

Does this really only have the one enum or are you just seeding the library?

[safern]

I think he is just seeding the library and then will add code progressively. Just so that the upstream plan with the package and what platforms it is going to support was clear.

[ahsonkhan]

@dotnet-bot test NETFX x86 Release Build
@dotnet-bot test OSX x64 Debug Build

Unrelated CI failures:
https://github.com/dotnet/corefx/issues/32025
https://mc.dot.net/#/user/ahsonkhan/pr~2Fjenkins~2Fdotnet~2Fcorefx~2Fmaster~2F/test~2Ffunctional~2Fcli~2F/a87b37dab77c7363646f9eef632571e7093cda73/workItem/System.IO.Compression.Tests/analysis/xunit/System.IO.Compression.DeflateStreamUnitTests~2FDispose_WithUnfinishedReadAsync

Message :
Assert.Throws() Failure
Expected: typeof(System.AggregateException)
Actual:   (No exception was thrown)
Stack Trace :
   at System.IO.Compression.CompressionStreamUnitTestBase.<Dispose_WithUnfinishedReadAsync>d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

https://mc.dot.net/#/user/ahsonkhan/pr~2Fjenkins~2Fdotnet~2Fcorefx~2Fmaster~2F/test~2Ffunctional~2Fcli~2F/a87b37dab77c7363646f9eef632571e7093cda73/workItem/System.Reflection.Tests/analysis/xunit/System.Reflection.Tests.AssemblyNameTests~2FConstructor_String_LoadVersionTest

Message :
Assert.Throws() Failure
Expected: typeof(System.IO.FileNotFoundException)
Actual:   (No exception was thrown)
Stack Trace :
   at System.Reflection.Tests.AssemblyNameTests.Constructor_String_LoadVersionTest() in /Users/dotnet-bot/j/workspace/dotnet_corefx/master/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/src/System.Reflection/tests/AssemblyNameTests.cs:line 559

[ahsonkhan]

@dotnet-bot test Windows x64 Debug Build
@dotnet-bot test Windows x86 Release Build

[ahsonkhan]

@dotnet/dnceng, any ideas what's up with the Windows legs? They keep timing out after several hours.

It looks like other PRs are seeing a similar issue: https://github.com/dotnet/corefx/pulls
For instance: #33224

https://ci3.dot.net/job/dotnet_corefx/job/master/job/windows-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/18082/console

15:14:40 Done Building Project "D:\j\workspace\windows-TGrou---74aa877a\src\upload-tests.proj" (default targets).
15:14:40 
15:14:40 Build succeeded.
15:14:40     0 Warning(s)
15:14:40     0 Error(s)
15:14:40 
15:14:40 Time Elapsed 00:00:30.08

...

D:\j\workspace\windows-TGrou---74aa877a>taskkill /F /IM VBCSCompiler.exe 
ERROR: The process "VBCSCompiler.exe" not found.
[Pipeline] cleanWs
[WS-CLEANUP] Deleting project workspace...Cannot delete workspace :remote file operation failed: D:\j\workspace\windows-TGrou---74aa877a at hudson.remoting.Channel@5ad8114f:JNLP4-connect connection from 23.100.42.22/23.100.42.22:18368: java.io.IOException: Unable to delete 'D:\j\workspace\windows-TGrou---74aa877a\.dotnet\dotnet.exe'. Tried 3 times (of a maximum of 3) waiting 0.1 sec between attempts.
[Pipeline] echo
Some files could not be cleaned up because of running processes.  These processes will be killed immediately and cleanup will happen before the node upon node-reuse
[Pipeline] }

...

01:04:13 [Windows.7.Amd64.Open - x64 - Debug (8d8eca21-a0e4-428b-9f5d-3d995e9b101d)] Sending request to url: https://helix.dot.net/api/2017-04-14/aggregate/jobs?groupBy=job.name&maxResultSets=1&filter.name=8d8eca21-a0e4-428b-9f5d-3d995e9b101d
01:04:14 [Windows.7.Amd64.Open - x64 - Debug (8d8eca21-a0e4-428b-9f5d-3d995e9b101d)] Response Code: HTTP/1.1 200 OK
01:04:14 [Windows.7.Amd64.Open - x64 - Debug (8d8eca21-a0e4-428b-9f5d-3d995e9b101d)] Response: 
01:04:14 [Windows.7.Amd64.Open - x64 - Debug (8d8eca21-a0e4-428b-9f5d-3d995e9b101d)] [{"Key":{"job.name":"8d8eca21-a0e4-428b-9f5d-3d995e9b101d"},"Data":{"Analysis":[{"Name":"xunit","Status":{"pass":0,"passonretry":0,"fail":0,"skip":0}}],"WorkItemStatus":{"run":227,"pass":1}}}]
01:04:14 [Windows.7.Amd64.Open - x64 - Debug (8d8eca21-a0e4-428b-9f5d-3d995e9b101d)] Success code from [100‥399]
01:04:17 [Windows.7.Amd64.Open - x64 - Debug (8d8eca21-a0e4-428b-9f5d-3d995e9b101d)] Info: Passed 0 (0 skipped)
[Pipeline] [Windows.7.Amd64.Open - x64 - Debug (8d8eca21-a0e4-428b-9f5d-3d995e9b101d)] echo
[Pipeline] [Windows.7.Amd64.Open - x64 - Debug (8d8eca21-a0e4-428b-9f5d-3d995e9b101d)] }
01:04:21 [Windows.7.Amd64.Open - x64 - Debug (8d8eca21-a0e4-428b-9f5d-3d995e9b101d)] Will try again after 5 min 0 se

[ahsonkhan]

@dotnet-bot test Windows x64 Debug Build
@dotnet-bot test Windows x86 Release Build

[MattGal]

@ahsonkhan it looks like the Windows 7 Helix machines took a dive over the weekend, I've taken some hopefully corrective action that should fix this in the next 30 minutes (except work will be quite backed up for a while) and filed https://github.com/dotnet/core-eng/issues/4551 to fix it more correctly. My apologies for the inconvenience.

[MattGal]

@ahsonkhan fixed and catching up now, but many thousands of work items behind. I'd estimate we should catch up to current by ~10:30 AM PST

[MattGal]

Windows 7 processing has caught up to real time now. We'll continue to track this issue until we find and fix the underlying cause.

[ahsonkhan]

@dotnet-bot test Windows x64 Debug Build
@dotnet-bot test Windows x86 Release Build