Docfx: Migrating our projects from net46 to netstandard 2.0

[vicancy]

https://github.com/dotnet/standard/blob/master/docs/versions/netstandard2.0.md
https://gist.github.com/davidfowl/8939f305567e1755412d6dc0b8baf1b7
Steps:

• [ ] 1. Upgrade projects to net461
• [ ] 2. Update documentation that VS 2017 Preview Update 3 is required
• [ ] 3. Upgrade base projects to netstardard2.0 one by one

Blockers:

• [ ] 1. CallContext
  http://www.cazzulino.com/callcontext-netstandard-netcore.html
• [ ] 2. AppDomain
  https://github.com/dotnet/docfx/issues/138#issuecomment-332336247
• [ ] 3. Nustache to .NET standard

[vwxyzh]

v3.0?

[tintoy]

Hi @vicancy, do you have any particular place you'd prefer that I start? Otherwise, I'm happy to just dive in at the deep end and see where I get up to :)

[tintoy]

Work starting here.

[vicancy]

@tintoy Looks cool!

[tintoy]

A couple of tests are failing, but I'm just checking to see if they actually succeed without these changes :)

[vicancy]

@tintoy If you are using VS 15.3 and the tests are Metadata related, it is possibly an env issue with MSBuild, opening All.sln from VS2017 Developer Command can fix it.

[tintoy]

Ah yes, thanks - those are the ones that are failing :)
I can probably fix that in another PR, I think (ran into a similar problem on another project).

[tintoy]

All good, with your suggested workaround, all tests pass so far. I'll keep going.

[tintoy]

Ok, bit of a blocker - all dependencies except Nustache support .NET Standard (project seems to be somewhat dead - jdiamond/Nustache#121). I will see if I can find a compatible alternative (the project owner suggests stubble).

[tintoy]

Wow, OK, yak-shaving time. There is _no_ Mustache implementation currently compatible with .NET Standard (any version!). Looks like I'll have to create a .NET Standard port of Nustache (or similar).

Anyone else who'd like to help - now's the time to pitch in :)

[tintoy]

Actually, let me see if it'll still run on .NET Standard 2.0 first (I get the usual NU1701 but it may still work correctly).

[tintoy]

Ok, a more serious blocker: dotnet/roslyn#17968. There's currently no MSBuildWorkspace for .NET Standard. I might have another poke around the OmniSharp source code to see how hard it would be to create an interim workaround for this.

[tintoy]

Have ported everything except CLI, tests, and Microsoft.DocAsCode.Metadata.ManagedReference for now. Will have a think about what it would take to make Microsoft.DocAsCode.Metadata.ManagedReference work on .NET Standard but it sounds like this library in particular may be a bit trickier to port.

[tintoy]

Probably have a solution for Microsoft.DocAsCode.Metadata.ManagedReference:

https://gist.github.com/tintoy/76b48591e2a34429c1f5c02556b8b4d2#file-program-cs

Essentially, we can use an AdhocWorkspace and populate it using the MSBuild engine to examine the project's properties and items; a slightly-less-sophisticated version of what MSBuildWorkspace does.

This may not work for _every single_ project out there (e.g. if a project has <Compile> items that are added by custom tasks that can only run on the full .NET framework because they're AppDomain-isolated) but it should be fine for 95% of use-cases (chances are, the existing metadata extractor doesn't work for 100% of projects right now anyway).

@vicancy - before I go any further on this can I get some feedback from you on whether you find this approach acceptable?

[vicancy]

@tintoy Sorry for late response. I was on vacation and am back now. I will take a look at it today.

[tintoy]

@vicancy no problem, take your time - I've got plenty of other stuff to keep me busy :)

[vicancy]

@tintoy Thinking of changing from Mustache to Handlebars, https://www.nuget.org/packages/Handlebars.Net/ suports .NET standard

[tintoy]

Nice - I totally forgot about Handlebars; how compatible is the syntax between the 2? It's been ages since I used Handlebars, but I seem to remember it's basically a superset of Mustache...

[vicancy]

@tintoy It generally compatible with Mustache, the main difference is that it does not search up when context does not found, which is exactly what we expect. There may be other differences that leads to a breaking change (which is my main concern) so I need more time to test before switching. 😢

[tintoy]

Yeah, I see what you mean, and it looks like there are only a couple of tests for the template processor at the moment. Still, that's the great thing about Xunit theories - write 1 test and then just keep adding test data 😎

[vicancy]

@tintoy with the 95% support, is this <Comiple case the only case that the AdhocWorkspace does not support?

[tintoy]

It can support it, we just have to use Microsoft.Build.Evaluationto evaluate the Compile item group :) I can whip up an example if you like

[tintoy]

(just may not work for _every_ project e.g. the exceptions listed above)

[tintoy]

@vicancy - this demonstrates evaluating the <Compile/> items and adding them to the workspace.

[vicancy]

Hi @tintoy

• Do you think it would be better to add a comprehensive test suites to docfx project before we moving to AdhocWorkspace, so that we can have a thorough idea what it supports and what is not, as similar to the test cases in Omnisharp? https://github.com/OmniSharp/omnisharp-roslyn/tree/master/test-assets/test-projects

• Another thing I am doing is investigating the possibility to migrate to Handlebars however blocked, as Handlebars.Net does not support such syntax as Mustache do, and does not implement what handlebars.js claims to support as configurable to look context up. https://stackoverflow.com/questions/10555820/what-are-the-differences-between-mustache-js-and-handlebars-js

{{#summary}}
{{summary}}
{{/summary}}

So I don't have a decent solution for Nustache project migrating to netstardard .. 😿 any idea?

cc @vwxyzh @superyyrrzz

[tintoy]

Do you think it would be better to add a comprehensive test suites to docfx project before we moving to AdhocWorkspace

Yep, I agree this would be very worthwhile :)

As for Nustache, we might need to wind up working (or working with someone) to port an existing implementation?

[tintoy]

(I have a couple of projects I use for testing MSBuild Project Tools that we can use as well)

[tintoy]

Nustache looks like it would be pretty easy to port, BTW.

[tintoy]

@vicancy - which bits of Nustache are used by DocFX? I just ported Nustache.Core, Nustache.Compilation, and Nustache.Core.Tests to netstandard2.0 / netcoreapp2.0 in less than 15 minutes and they're fine apart from a small fraction of the unit tests that rely on a library (DynamicExpresso) that itself has not been ported to netstandard.

[tintoy]

No code changes required, BTW, except to the unit tests (upgraded to NUnit 3, so some attribute / property names in the NUnit framework have changed).

[tintoy]

BTW, around 4 (out of 285) of the Nustache unit tests don't pass, but they don't pass in the original project either. I had to disable one test because it alters a static global that breaks around 20 other tests.

If I can get you a set of packages or a copy of the ported code would you be able to see if it works when plugged into existing DocFX tests?

[vicancy]

@tintoy we are using Nustache.Core. I created a https://github.com/jdiamond/Nustache/issues/129 asking them about the plan, looks like that project is silent since Oct 10, 2016

[vicancy]

We can test it with a package of ported Nustache.Core

[tintoy]

Ok I'll create a myget feed with packages you can use to test.
On Mon, 23 Oct 2017 at 6:32 pm, Liangying.Wei [e-mail]
wrote:

We can test it with a package of ported Nustache.Core

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/dotnet/docfx/issues/1963#issuecomment-338571650, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABkezCRM44eiNYGsmLCB0aZTwoCJb6foks5svEEegaJpZM4O4ZWH
.

[tintoy]

@vicancy - you can pull from https://www.myget.org/F/nustache-netstandard/api/v3/index.json (built against netstandard2.0).

[Romanx]

@tintoy If you'd like to open a pull request on Nustache with your changes i'd be happy to look them over and get an official release out

[tintoy]

Thanks @Romanx :)

I may need to first submit a PR to an upstream project (DynamicExpresso) used by some of your tests since that doesn't support .NET Standard yet either. But as soon as I can get that done I'll submit the PR. In the meanwhile I can at least fork the original repo for Nustache and apply my changes there (at the moment it's all in a local repo only).

[tintoy]

Ok - initial port is sitting in https://github.com/tintoy/Nustache/tree/feature/netstandard. Would you mind having a poke-around the project to see what you think before I open a PR, @Romanx?

There are a couple of tests (around Sections) that fail, but as far as I can tell, they failed before the port, too (had to upgrade to a more recent version of NUnit, among other changes).

[Romanx]

@tintoy I'll have a look later today and see whats up with those tests.

[Romanx]

@tintoy I just checked out the repo. Seemed to work when I ran all the tests. Which are you finding don't seem to work?

[tintoy]

@Romanx - was that on the feature/netstandard branch? These ones fail or are inconclusive / skipped for me:

[Romanx]

@tintoy it's quite probably I derped and wasn't on the right branch. Let me confirm again 👍

[Romanx]

@tintoy I've worked out what the change was that caused the test failures. One is that the changing to use Json.Net returns the object in a different format to the JavascriptSerializer and the other is that originally it was pulling the data from the submodule specs. After refactoring the folder structure the pathing was off so you added the specs to the repo which are now different to the submodule which is where the "new" failure come from as the submodule hasn't been updated in awhile.

I'm pretty sure i've got a fix for the object format and i'm just working on regenerating the strongly typed objects from the spec, Would you like me to open a pull request to your repo when i'm done?

[tintoy]

@Romanx - thanks, that'd be awesome!

[vicancy]

Hi @Romanx Any progress 😄 ?

[vicancy]

Hi @tintoy I'd like to move forward with the AdhocWorkspace approach. Theoretically it can resolve #1752 & #138 if I am understanding correctly, is it? And after this approach, I plan to setup some CI to test it under different environment.
It would be fantastic if you could create an PR with this approach. 😄

[tintoy]

Ok - I'm about to go camping for the weekend but I'll get onto it early next week :)

[Romanx]

@vicancy We're having issues with the changed tests not working on @tintoy's environment and working on mine. Hoping to find some time this weekend to get to it

[tintoy]

@vicancy have a look at 6864ff239daac7bbc54c545a07811c112df182da - that should give you an idea of the scope of changes I have to make (it compiles but I haven't tried any of the tests yet).

[tintoy]

Actually, I'm thinking I might create extension methods for AdhocWorkspace to emulate the old OpenProjectAsync APIs. What do you reckon?

[tintoy]

Turns out AdhocWorkspace is mutable and therefore may not be thread-safe. Will investigate :)

[tintoy]

Ok turns out it's thread-safe.

[tintoy]

@vicancy would you mind pulling down the latest code from my fork and running the tests? There are a couple I don't understand well enough to work out why they are failing... It's the feature/netstandard branch.

[vicancy]

@tintoy Sure, I will take a look

[vicancy]

Hi @tintoy Can build pass with the feature/netstandard branch? Mine can't pass build, looks like System.Runtime.Remoting.Lifetime; is not available in netstandard.

[tintoy]

Which project was that? I didn't see any build errors but I was just running tests from within VS. I'll have a look at that first thing tomorrow, should be easy to fix either way.

[tintoy]

I haven't tried porting the main executable yet, only the libraries it depends on. The exe will take some more work because it currently relies on Appdomain but I do have a proposed design that will work cross-platform. In the meanwhile, would you mind trying to run the unit tests ignoring that executable build error?

[tintoy]

BTW, am I correct in assuming that use of Appdomains is mainly to control base path for loading assemblies? If so, AssemblyLoadContext is the replacement.

[vicancy]

@tintoy AppDomain is to isolate assemblies loaded from plugins. We are thinking of creating a new process instead.

[tintoy]

Yep that'd be the way :)

They can communicate over STDOUT / STDIN (have code that does this in an
LSP language service that might be helpful). Were you able to run the tests
if you ignore the build error in that project?

On Thu, 30 Nov. 2017, 4:14 pm Liangying.Wei, [e-mail]
wrote:

@tintoy https://github.com/tintoy AppDomain is to isolate assemblies
loaded from plugins. We are thinking of creating a new process instead.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/dotnet/docfx/issues/1963#issuecomment-348084783, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABkezLKhN6WsValNir-cVI-KBIRe7ac5ks5s7jmngaJpZM4O4ZWH
.

[vicancy]

@tintoy I changed docfx and docfx.test projects back to use base.props and can pass build now. I am running test and meet some runtime assembly loading errors. I am looking into it.

[tintoy]

BTW, here is one way to use an external process to handle assembly loading:

https://github.com/tintoy/msbuild-project-tools-server/tree/master/src/LanguageServer.TaskReflection

Called from here:
https://github.com/tintoy/msbuild-project-tools-server/blob/master/src/LanguageServer.SemanticModel.MSBuild/MSBuildTaskScanner.cs

[vicancy]

Hi @tintoy I also meet several kind of ut failures with your changes. However that branch is quite behind the latest branch and contains a lot of changes. I'd like to make small moves step by step for easy debugging and code review, so I am cherry-picking your changes and sending out PR for every commit.

[tintoy]

@vicancy - I recently stumbled across Buildalyzer, which seems to have fairly full-featured support for building a Roslyn AdhocWorkspace from project files in most formats:

Might be worth considering?

[vicancy]

Hi @tintoy thank you for the info, I will take a look. With a first glance, it looks like also dependent on msbuild.dll to load and parse the project, so I am now a little confused why we not use MSBuildWorkspace, it is from https://www.nuget.org/packages/Microsoft.CodeAnalysis.Workspaces.Common/ and has a netstardard1.3 version...

[tintoy]

I think MSBuildWorkspace is in the package Microsoft.CodeAnalysis.Workspaces.Desktop, which does not target .NET standard (yet)?

[tintoy]

AdhocWorkspace is in Microsoft.CodeAnalysis.Workspaces.Common though.

[vicancy]

@tintoy Oh yes, netstardard1.3 does not contain Microsoft.CodeAnalysis.Workspaces.Desktop assembly

[superyyrrzz]

Close as v2 is not likely to support it. v3 will.