JamesNK / Newtonsoft.Json

Json.NET is a popular high-performance JSON framework for .NET
https://www.newtonsoft.com/json
MIT License
10.84k stars 3.26k forks source link

Specify dependencies for .NET Core #618

Closed davidfowl closed 8 years ago

davidfowl commented 9 years ago

JSON.NET currently has PCL folder that targets portable-net45+wp80+win8+wpa81+dnxcore50 this works for older PCL projects and .NET Framework based projects because msbuild adds all of the facades to the compiler target. Moving forward, that's no longer the case. Instead packages are expected to specify what dependencies they use in the nuspec.

Here' an example of System.Collections.nuspec:

<?xml version="1.0"?>
<package xmlns="http://schemas.microsoft.com/packaging/2013/01/nuspec.xsd">
  <metadata minClientVersion="3.0">
    <id>System.Collections</id>
    <version>4.0.10-beta-23110</version>
    <title>System.Collections</title>
    <authors>Microsoft</authors>
    <owners>microsoft,dotnetframework</owners>
    <licenseUrl>http://go.microsoft.com/fwlink/?LinkId=329770</licenseUrl>
    <iconUrl>http://go.microsoft.com/fwlink/?LinkID=288859</iconUrl>
    <requireLicenseAcceptance>true</requireLicenseAcceptance>
    <description>Provides classes that define generic collections, which allow developers to create strongly typed collections that provide better type safety and performance than non-generic strongly typed collections.

Commonly Used Types:
System.Collections.Generic.List&lt;T&gt;
System.Collections.Generic.Dictionary&lt;TKey, TValue&gt;
System.Collections.Generic.Queue&lt;T&gt;
System.Collections.Generic.Stack&lt;T&gt;
System.Collections.Generic.HashSet&lt;T&gt;
System.Collections.Generic.LinkedList&lt;T&gt;
System.Collections.Generic.EqualityComparer&lt;T&gt;
System.Collections.Generic.Comparer&lt;T&gt;
System.Collections.Generic.SortedDictionary&lt;TKey, TValue&gt;</description>
    <copyright>© Microsoft Corporation.  All rights reserved.</copyright>
    <dependencies>
      <group targetFramework="MonoAndroid1.0" />
      <group targetFramework="MonoTouch1.0" />
      <group targetFramework=".NETFramework4.6" />
      <group targetFramework="Xamarin.iOS1.0" />
      <group targetFramework="Xamarin.Mac2.0" />
      <group targetFramework=".NETPlatform5.0">
        <dependency id="System.Runtime" version="4.0.0" />
      </group>
      <group targetFramework=".NETCore5.0">
        <dependency id="System.Runtime" version="4.0.20-beta-23110" />
        <dependency id="System.Resources.ResourceManager" version="4.0.0-beta-23110" />
        <dependency id="System.Diagnostics.Debug" version="4.0.0" />
        <dependency id="System.Threading" version="4.0.0" />
        <dependency id="System.Runtime.Extensions" version="4.0.0" />
      </group>
      <group targetFramework="DNXCore5.0">
        <dependency id="System.Runtime" version="4.0.20-beta-23110" />
      </group>
    </dependencies>
  </metadata>
</package>

Notice that the dependencies are specified in the TFM specific group. That's what JSON.NET should do for .NET Core based platforms. Here's the recommended solution, remove +dnxcore50 from the PCL folder, make a new folder called dotnet and put the dll in there. Specify the dependencies you use in your nuspec with a dependency group targeting dotnet/aka .NETPlatform5.0

JamesNK commented 9 years ago

.NETPlatform5.0? .NETCore5.0? DNXCore5.0? Is NuGet ever going to document this stuff because as someone who is not using the new .NET stuff on a regular basis I have no clue when is required of a NuGet package anymore.

:poop: :fire:

JamesNK commented 9 years ago

Also, you guys should fix https://github.com/aspnet/dnx/issues/1894

When you do that I could start using a project.json to build and Json.NET

davidfowl commented 9 years ago

.NETPlatform5.0? .NETCore5.0? DNXCore5.0? Is NuGet ever going to document this stuff because as someone who is not using the new .NET stuff on a regular basis I have no clue when is required of a NuGet package anymore.

I know we suck bad at this. There's some documentation coming out soon from the NuGet team that will clarify some of it.

Also, you guys should fix aspnet/dnx#1894

I totally want to fix this but I have to ask, who is using NuGet with .NET 2.0? 3.5 I can maybe understand (maybe...)

JamesNK commented 9 years ago

I know Glimpse targets .NET 3.5

clairernovotny commented 9 years ago

I have a tool that I'm publishing tomorrow that helps with this. It will generate the dependencies of a nuspec for dotnet properly. I'll try to remember to post a link back to this thread, but you don't have to do this by hand....

The tool is just a nuget package that you add to your PCL project -- the same one you're putting in the portable- dir today. No code changes. On build, it'll look for the nuspec and put the correct dependencies in. Then you just put your lib in \lib\dotnet as well and things should just work.

JamesNK commented 9 years ago

Two questions:

  1. When will NuGet documentation be done? I'll look at this when there is some documentation I can look at.
  2. How do I simply repro the error in a test? I don't want to install ArchLinux and Kestral to test whether the nuspec is correct.
clairernovotny commented 9 years ago

Hi James, this might help explain it: http://oren.codes/2015/07/29/targeting-net-core/

It should be really easy and I can submit a PR later today or tomorrow if you want.

clairernovotny commented 9 years ago

I'm going to submit a PR for to update the NuSpecs this morning.

JamesNK commented 9 years ago

Thanks.

JamesNK commented 9 years ago

@davidfowl How do I test this? Why did this error happen for the ppl at https://github.com/aspnet/dnx/issues/2364 and https://github.com/aspnet/Home/issues/761, and not for everyone?

clairernovotny commented 9 years ago

@JamesNK Just curious as to which part you want to test? The binary is the same. All that's been done is to list the dependencies. At that point, NuGet and DNU will see the dotnet section, list the dependencies and get the matching contracts/packages that are equal to or greater as per normal NuGet dependency rules.

One way to test this part out is to create a new "Console Application (Package)" project as that's based on DNX. Then add json.net to it's project.json and if you expand the node in VS under references, you should see it pull in all of your needed package references.

At that point it's up to those packages and the runtime to sort things out.

JamesNK commented 9 years ago

I never make changes before I can repro it myself.

clairernovotny commented 9 years ago

I can tell you what happened... and possibly how to construct the repro.

Create a new Console Application package as I describe above, then add the existing 7.0.1 library to it. In the project.json, only specify the minimum dependencies to run main -- System.Runtime. Then do something basic like serialize a type in a way that would use a type from System.Runtime.Serialization.Primitives (the contents of that contract lib escape me this moment).

If you do, then you'll get the exception since System.Runtime.Serialization.Primitives was not copied to the output directory.

The reason other people didn't necessarily see it is that they could reference another assembly that itself references System.Runtime.Serialization.Primitives, so it would get pulled in. They essentially got lucky.

clairernovotny commented 9 years ago

I have a repro, it's really easy and it's exactly what I said before: https://onedrive.live.com/redir?resid=B98D981155DF4D47!701387&authkey=!ANMjXa_ZUM3--BE&ithint=file%2czip

  1. New console app that's dnx based and set to run on coreclr.

Use the following project.json:

{
  "version": "1.0.0-*",
  "description": "ConsoleApp1 Console Application",
  "authors": [ "oren" ],
  "tags": [ "" ],
  "projectUrl": "",
  "licenseUrl": "",

  "dependencies": {
    "Newtonsoft.Json": "7.0.1"
  },

  "commands": {
    "ConsoleApp1": "ConsoleApp1"
  },

  "frameworks": {
    "dnxcore50": {
      "dependencies": {
        "System.Collections": "4.0.10-beta-23109",
        "System.Console": "4.0.0-beta-23109",
        "System.Linq": "4.0.0-beta-23109",
        "System.Threading": "4.0.10-beta-23109",
        "Microsoft.CSharp": "4.0.0-beta-23109"
      }
    }
  }
}

and Here's the program.cs

using System;
using Newtonsoft.Json;

namespace ConsoleApp1
{
    public class Program
    {
        public void Main(string[] args)
        {
            var foo = new Poco() { Foo = "Bar" };

            var str = JsonConvert.SerializeObject(foo);

            Console.WriteLine(str);
        }

    }

    public class Poco
    {
        public string Foo { get; set; }
    }
}

Try to run and you get an unhandled FileNotFoundException due to the missing dll.

JamesNK commented 9 years ago

Thanks! I'll look at this during the weekend.

ericstj commented 9 years ago

So the reason this isn't working is because DNX will not lift the packages to the version that contains support for DNXCore. The way we do this in the MSBuild based projects is through the Microsoft.NETCore.Targets package, though even including this in the DNX console project will not work due to https://github.com/aspnet/dnx/issues/2387.

A workaround would be to manually list the package versions that contain support for DNXCore in a DNXCore specific section of your project.json. You can see the minimum versions by looking at the runtime.json in the Microsoft.NETCore.Targets.DNXCore package. Long term I think this needs to happen automatically by the platform, similar to how it works for MSBuild based projects.

clairernovotny commented 9 years ago

Just to add one point to @ericstj's analysis, the issue at hand with dnxcore still repros with the changes in my PR for the reasons Eric mentions.

That said, the changes themselves are the correct changes so that the dependencies are transitively included.

I think the short version is that DNX users will need a workaround to consume a Profile 259 PCL.

JamesNK commented 9 years ago

So I'm playing around with getting Json.NET building via a project.json again and for some reason types from a dependency that I'm not directly referencing - System.Reflection.TypeExtensions - are generating conflicts. Is this new behavior? I don't remember having this problem in the past.

davidfowl commented 9 years ago

Dependencies are transitive by default. What conflict are you seeing?

JamesNK commented 9 years ago

I have my own internal BindingFlags enum. System.Reflection.TypeExtensions also has a BindingFlags enum.

Even though I'm not referencing System.Reflection.TypeExtensions, because I'm referencing Microsoft.CSharp and Microsoft.CSharp is referencing TypeExtensions, both are now in my project and conflicting with each other.

davidfowl commented 9 years ago

How's this change going @JamesNK ?

JamesNK commented 9 years ago

I'm at a conference this weekend. I'll look at it more late next week

JamesNK commented 9 years ago

https://github.com/JamesNK/Newtonsoft.Json/blob/master/Build/Newtonsoft.Json.nuspec

https://github.com/JamesNK/Newtonsoft.Json/blob/master/Src/Newtonsoft.Json/project.json

Yes/no?

clairernovotny commented 9 years ago

The nuspec looks good but in the project.json, I'd change up the floating versions: Right now you have "4.0.10-*", and that means if a beta version is available, it'll pick that up instead. I'd change that to either remove the wildcard or at least make it "4.0.*" so you only pickup stable packages.

clairernovotny commented 9 years ago

actually, you may need to add the following to the nuspec:

<group targetFramework="net45" />
<group targetFramework="wp8" />
<group targetFramework="win8" />
<group targetFramework="wpa81" />
<group targetFramework="xamarin.ios" />
<group targetFramework="monotouch" />
<group targetFramework="monoandroid" />

Otherwise any system.runtime platform will try to pull in the dependencies in dotnet and fail (because they're not applicable.

ericstj commented 9 years ago

If you do that you need to duplicate the dependencies in a UAP section, otherwise win8 will take precedence over the dotnet section.

Dependencies shouldn't fail, we've actually produced packages that will correctly install if they are supported, for example the System.Runtime 4.0.0 package installs into a net45 project. You do end up getting a bunch of extra packages in the packages.config for older project types and nuget will advertise upgrades that don't work since you'd be upgrading out of support for the old platforms like wp8. Once/if everything is moved to project.json both of these bad behaviors will go away and folks can author their packages with a single dotnet section.

clairernovotny commented 9 years ago

@ericstj It's true that System.Runtime 4.0.0 does install correctly, but 4.0.20 seems to be used as the system.runtime reference...and that does not work.

JamesNK commented 9 years ago

Why can't the NuGet team document any of this stuff?! :facepunch:

Here is a gist of the nuspec: https://gist.github.com/JamesNK/56b1c3b90b2a8d21c73d

Add comments to it of what needs to change to support all the platforms.

yishaigalatzer commented 9 years ago

@JamesNK what of this long list do you consider as not documented? Let us know and we will add it, or of course we always take PRs to our docs https://github.com/NuGet/NuGetDocs

JamesNK commented 9 years ago

There needs to be a sample or guide of how to create a nuspec that mixes old targets like net45/win8/portable-* with dotnet that works with new project.json apps like DNX and UAP while still playing nice with the old packages.json

Consider this - https://github.com/JamesNK/Newtonsoft.Json/issues/618#issuecomment-129879815 - will anyone reading the documentation understand that blank groups are required for some targets?

You need to be more prescriptive with telling people the best practices for creating packages. Look at what @davidfowl did in this bug description: he copied and pasted another package's nuspec and said do something like this. He should be able to link to a wiki page or guide somewhere with best practices for targets and dependencies. Muddling around, guessing by looking at what other people have done and discovery by word of mouth through the community sucks.

clairernovotny commented 9 years ago

@JamesNK :+1: This has been very frustrating to figure out what goes with what by trial and error.

yishaigalatzer commented 9 years ago

https://github.com/NuGet/Home/issues/1160

JamesNK commented 9 years ago

Test NuGet file: https://dl.dropboxusercontent.com/u/1917595/Newtonsoft.Json.7.0.2-beta1.nupkg

JamesNK commented 9 years ago

I've spent enough time on this. I'm going to wait until there is documentation or guides.

yishaigalatzer commented 9 years ago
  1. We did test 7.0.2, and it looks good.
  2. I agree with your decision to wait for a doc or guidance
JamesNK commented 9 years ago

So how are those docs coming along? I'm guessing the ASP.NET team would like to use a version of Json.NET that specifies dependencies before they go 1.0 RTM and they're going to be in RC's soon.

NickCraver commented 9 years ago

Any update on this? I was trying to use JSON.Net as an example for Dapper and...well, now I'm even more lost on what's "correct" all around. Some combinations work in CLI, some in VS, others neither. Every combination that works or doesn't or resolves dependencies correctly changes with every beta.

I'll be honest, I'm with @JamesNK here - I'm so confused and frustrated about what's proper that I'm giving up for now.

@davidfowl @yishaigalatzer can you please let us know when there's anything like a full example available?, and please include which versions of all the tooling it's assuming.

brthor commented 8 years ago

@krwq and I Ran into this today using JSON.NET version 8.0.1-beta3 using the dotnet cli.

Looking at the nuspec there doesn't seem to be any dependencies at all.

jefffhaynes commented 8 years ago

So refreshing to see I'm not the only one pulling my hair out over nuget + uap/dnx + dn of old.

Caldas commented 8 years ago

No i'm also facing some difficult to set newtonsoft to .Net Core 5.0 and a .Net 4.5.1 on some project

JamesNK commented 8 years ago

There won't be any progress until .NET Core RC2. When it is released I'll create a netstandard build and release a new version of Json.NET.

JamesNK commented 8 years ago

FYI there is a .NET CLI version of Json.NET - https://github.com/JamesNK/Newtonsoft.Json/compare/master...dotnet-cli - but I can't put in on NuGet because the RC2 packages aren't on NuGet.

bob-devereux-zocdoc commented 8 years ago

FYI RC2 is live now

JamesNK commented 8 years ago

Done https://www.nuget.org/packages/Newtonsoft.Json/8.0.4-beta1

brthor commented 8 years ago

👍

alexellis commented 8 years ago

What's the best way to add this to the project.json generated by dotnet new?

{
  "version": "1.0.0-*",
  "buildOptions": {
    "emitEntryPoint": true
  },
  "dependencies": {
    "Microsoft.NETCore.App": {
      "type": "platform",
      "version": "1.0.0-rc2-3002702"
    }
  },
  "frameworks": {
    "netcoreapp1.0": {
      "imports": "dnxcore50"
    }
  }
}

Is there any way to manage and add dependencies other than hacking .json?

heaths commented 8 years ago

Until recently announced tooling changes for Visual Studio "15" are made, project.json is how you manage dependencies. https://dogs.nuget.org has more details, but effectively you add general dependencies under "dependencies" and can most often do so as simple name/value strings:

"dependencies": {
  "Newtonsoft.Json": "8.0.4"
}
alexellis commented 8 years ago

@heaths thanks. Is DNCore going back to csproj files or just full fat .NET? I wondered how to find libraries which work on DNCore? Do you get the version number for this string by searching nuget's website? I'm imagining using the tools through VSCode without VS2015 and without Windows.

JamesNK commented 8 years ago

Please ask off topic questions like this somewhere else. You're spamming a lot of people with github notifications.