Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade Json.NET version? #1058

Closed
mattpwhite opened this issue Aug 25, 2018 · 4 comments · Fixed by #1165
Closed

Upgrade Json.NET version? #1058

mattpwhite opened this issue Aug 25, 2018 · 4 comments · Fixed by #1165
Assignees
Milestone

Comments

@mattpwhite
Copy link

PSScriptAnalyzer currently comes with a slightly outdated version of Json.NET. Any chance it could be updated to 11.x?

The problem this is actually causing for me is the usual collision between different assembly versions within an AppDomain. This is always frustrating in PowerShell, but most of the time an AssemblyResolve handler can get around it if the newer version is loaded first. I have some things that are using Json.NET 11 (and actually using new features like generic JsonConverters) that end up breaking if ScriptAnalyzer and its older Json.NET gets loaded first. PS Core 6.1 appears to ship with the current version of Json.NET already, so this probably only affects Windows PowerShell. Our module repo is a long way from being Core-friendly so I honestly didn't test.

@bergmeister
Copy link
Collaborator

bergmeister commented Aug 26, 2018

@mattpwhite Currently, an upgrade would break PowerShell Core 6.0 due to this issue because 6.0 ships with 10.0.3 (there was no change to that in the recently patched version 6.0.4).
Once 6.0 gets deprecated 6 months after the release of 6.1, we can definitely upgrade it though. In theory we could shipping 2 differently compiled versions for 6.0 and 6.1, similar to how we already have to do it for v 3, 4, 5, 6 but the question is if the temporarily added complexity is worth the gain. WDYT @JamesWTruher ?
If you only use Windows PowerShell 5.1, then you could just take this custom fork that I built you.

@mattpwhite
Copy link
Author

Thanks, I appreciate the private build. I actually already created a patched build and we are distributing that internally. There are other contexts where a patched version of script analyzer is more difficult to use (the VS Code extension).

I'm not totally sure I follow how this would break 6.0 though. I may just be missing something, but it looks like you don't actually ship a version of Json.NET for use with 6.x. This makes sense because PS 6.x itself ships with Json.NET. So how would updating the version of Json.NET that gets loaded by 5.x (only) create a problem?

@bergmeister
Copy link
Collaborator

bergmeister commented Aug 27, 2018

You are right that we do not ship the json dll by design for the netstandard build (i.e. PowerShell Core) because PowerShell Core already ships with this dll and PowerShell will know how to find it. This is something that the C# Compiler does not know about, therefore a specific version of the JSON NuGet package still needs to be referenced at compile time and due to this PowerShell issue, PowerShell will throw an DllNotFoundException at runtime (see the PSCore build here) if the referenced DLL of a cmdlet has a higher version than the one that ships with PowerShell Core.
We could try to conditionally compile the newer Json DLL for the net451 (Windows PowerShell) builds though, which would solve your problem without adding too much complexity to PSSA.

@bergmeister
Copy link
Collaborator

@mattpwhite With PowerShell 6.0 running out of support next month and us hoping to ship the next release soon, I think/hope that we will be able to update Json.Net to 11.0.2 (this is the version that PowerShell 6.1 ships with).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment