-
Notifications
You must be signed in to change notification settings - Fork 640
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
.NET Core RC2 support #433
Comments
Perfect, thank you! |
Good progress so far, no real blockers. Despite Microsoft making huge changes between RC1 and RC2 to the way that the framework directory and packages are resolved, the managed and unmanaged code has been updated, compiles on Windows, and passes all of the pre-compiled (i.e. non-edge-cs-based) tests. As part of this support, I also copied over the PAL code that Microsoft uses in https://github.com/dotnet/cli, which has almost completely eliminated the platform-specific Dynamic compilation is up next and then a bunch of refactoring and fit-and-finish work and testing on the other OSes. All of this work is in https://github.com/medicomp/edge/tree/RC2. I'm traveling this weekend and then have my company's user group conference all next week, so I should be able to pick it back up the week after next. |
Wonderful, thank you for all the hard work. |
Dynamic compilation is in now and all tests are green on both Windows and Linux. I'm going to take care of OS X (which shouldn't involve too much work) later this week. I'm also going to make sure that standalone apps (ones with their own copy of the CLR via The fit-and-finish work is also done, but I may propose a change to the way that CoreCLREmbedding.dll is referenced and loaded. It would be a breaking change, but may result in a cleaner codebase in CoreCLREmbedding. It would also remove the requirement that the .NET Core SDK be installed: it would be possible to use Edge.js with only standalone (i.e. published) .NET Core apps, which could be very useful for folks who don't want to install the CLI on their production systems. Anyway, I'm going to do a bit more research and then I may write something up. |
This is fantastic, looking forward to it. I will probably want to pile up a few long outstanding fixes onto this release, and also add support for Node 6. Just FYI. |
OK, OS X support is done and all tests are green there as well. There is one important caveat: I had to up the Next, I'll be enabling support for |
Sounds good. I don't think the C++ 11 on OS X requirement is an issue, not many people use edge on OS X beyond doing development like you and me. |
Just wanted to tell you guys to keep up the good work. I'm eagerly waiting to be able to use |
I also wait for .NET Core RC2 support to be able to make a high quality bridges between https://github.com/linksplatform/Crawler and https://github.com/Konard/LinksPlatform |
@gingters and @Konard, thank you for sharing your plans, it's very useful in determining what I should be including in .NET Core support. Given the interest (from at least one person) in being able to use At issue is the fact that, in my current implementation, the This sort of violates the principal of Pros:
Cons:
Please chime in with your thoughts or comments. |
@lstratman I want to make sure you are aware there is already a NuGet package for Edge.js: https://www.nuget.org/packages/Edge.js/, which is the primary way in which Edge.js can be used to enable scripting Node.js from within desktop CLR apps on Windows? I believe there is a way to make the construction of this NugetGet package platform and CLR-flavor agnostic, but when I looked at this some months ago it was lacking any form of documentation beyond talking to @davidfowl on Twitter. The NuGet package is built from https://github.com/tjanczuk/edge/blob/master/tools/build_double.bat |
@tjanczuk Yep, and we can either merge the code for CoreCLREmbedding.dll in with that or create a new NuGet package with just the stuff from CoreCLREmbedding.dll. The former option would be less confusing when people are searching NuGet for the package to install so I imagine that's the route that we'll take. |
I agree, having a single Nuget package that covers all platforms is preferable. My head hurts every time I come back to CLR after a break. The entire which-runtime-needs-what-dependencies-on-what-platform-using-what-tools is confusing enough to confuse people with a further choice of NuGet package flavor. Are you saying the Nuget package for CoreCLR would have to contain the source code of CoreCLREmbedding? The Nuget for full CLR contains a precompiled assembly. |
No need for any of the source, just the precompiled assembly. |
This actually ended up being much more straightforward than I anticipated. I've gotten the initial implementation fully working on Windows and will test it on all of the other OSes next week. Once everything checks out, I'll do a writeup on what I did in this thread, update the docs, and then this should finally be ready for a PR. |
Just a few more information about our use-case: The idea is that we take the output of the publish process of our library, and just copy that 1:1 into our Electron application. In the best case we would simply include the folder in an asar package. In our electron application we use the It would be great, if (electron-)edge would be able to automatically detect which runtime to load (regarding the platform we are currently at) to load and execute the library, so that I as a developer would just have to point it to the root folder of the published application and it would fetch the correct platform itself. |
This is finally DONE. Or at least I am declaring it done 👍 What was formerly CoreCLREmbedding.dll that was in each of the OS/architecture/Node.js version directories under All of the compilers that we want to support on .NET Core should also have NuGet packages for them; I have updated my fork of edge-cs to do this and will send you a PR for it. The requirement that the .NET Core SDK and CLI be installed on servers running Edge.js has been removed. The NPM install process on non-Windows boxes will now always compile edge_coreclr.node since it no longer looks at the presence of the With the requirement that a NuGet package be referenced in order for a .NET Core application to work, @tjanczuk can either build and push the NuGet packages with a pre-release version or you can install .NET Core first and build the package using I've updated the documentation with all of the stuff for .NET Core that I could think of, but if I've missed anything or you want me to elaborate on something, please let me know. In addition, the Docker images still need to be updated: I didn't do anything with them. Feel free to cut loose with any comments or questions. Otherwise, @tjanczuk let me know which branch you would like me to submit my PR under. |
Woo hoo! This is great work @lstratman! Can you submit a PR against master? Let me make sure I understand a few things about how this hangs together:
|
You are correct that CoreCLREmbedding was the .NET runtime code that acted as the interop layer between Node.js and whatever .NET code that you wanted to run from Node.js. There were several reasons that I moved this over to a NuGet package. First, CoreCLREmbedding.dll depends on several packages (chief among which is Newtonsoft.Json) that a .NET application may specify a different version of. The initial code that I had for RC2 support handled this in a very non-transparent way: it had its own list of dependencies for CoreCLREmbedding.dll that it would load up and then it would merge in the list of dependencies for the application that were specified in the application's project.json to form the complete list of dependencies that we need to load when executing the .NET code. The problem is that the user would be unaware that any of this was happening and might wonder why a different version of Newtonsoft.Json or some other package was loaded since Edge.js might have required a higher version or something similar. The solution was to require the application's project.json to include a dependency on the Edge.js .NET Core runtime (the assembly formerly known as CoreCLREmbedding.dll) so that when the user ran The above is all well and good if you're developing a fully-formed .NET Core application (with its own project.json) and then using that application with Edge.js. But, if you just want to:
then we support that as well, it just requires you to have the .NET Core SDK and CLI installed on the machine that you're doing it on. During NPM install, we detect if the
without any issues. I believe that covers #1, #2, and #4. As for #3, yes, in order to script C# literals without a formal Sorry that was so long-winded, I hope that answers your questions. |
Perfect, I think I get it now. Can you PR? Can you remind me what EDGE_APP_ROOT defaults to? |
|
I wonder if it would make sense to default this to __dirname to enable predictable file layout? |
Actually it would have to be __dirname of the code calling edge.func... Hmmm... |
@lstratman could you send a PR for the edge-cs nuget package? |
Done. |
@lstratman Thanks. I am trying to wrap my head around the implications on the Edge.js dev process that the inclusion of coreclrmebedding in a nuget package has. In order to ship a new version of Edge.js, do we now need to:
Is that about right? If so, does running tests in step 3 require the bootstrap project to temporarily point to the locally built nuget packages? |
Yep, that's basically it. I would actually flip steps 1 and 2 around. For your question about step 3, it's already taken care of: in build.bat, I updated the |
I see, clever. How do you organize the dev flow on Linux and Mac given that #2 needs to happen on Windows? |
I build the NuGet package on Windows, create a directory on my Linux and On Thu, Jun 23, 2016 at 6:26 PM, Tomasz Janczuk [email protected]
|
Just FYI, the .NET Core RTM is supposed to drop at some point today, so I'd hold off on officially merging my PR until I have a chance to update the version numbers of the dependencies and do a final round of testing against the new version. Should be able to take care of that today or tomorrow, depending on when the RTM becomes available. |
Yes, good idea. |
Do you have any estimate @lstratman when this could be ready? ;) |
Looks like issue should be renamed to .NET Core 1.0.0 RTM support... |
@ChristianWeyer I was hoping to have everything ready the day after the RTM, but was busy with my day job. Now that I've had some time to get everything ready, something in .NET Core's custom assembly resolution process changed, meaning that Edge.js dies silently trying to resolve assemblies from the .deps.json. I'm not sure what's going on with it, but hope to have everything working in the next day or two. |
@ChristianWeyer what is your dependency on this by the way? |
We have a very large customer project which is by now blocked by this... |
I see you like playing it close to the chest ;) |
Cutting EDGE ;P |
Got the dependency resolution issue sorted out, got everything built, and successfully ran the unit tests on both Windows and Linux. The only thing outstanding is that the bootstrap application (which allows running Edge.js without a project.json) seems to need some additional dependencies, which I will look at tomorrow. |
FINALLY, everything is done. .NET Core RTM support is official, tests and builds are green across Windows, Linux, and OS X. @tjanczuk I updated the version numbers in the various |
Perfect, this is going to be great. I will go through the devops cycle today and try to ship. |
So... When the next release? Did the installation process changed? |
Does this include support for node 6.xxx?
|
Really want this on the the released version of CoreCLR so my c# code can call node functions. Is that what this topic is or the other way around? |
Looks like everything is ok except NuGet packages usage.. #451 |
[email protected] has now shipped. Supports CoreCLR 1.0.0 preview 2 and node 6.x. Big thanks to @lstratman! |
Putting this in here as a placeholder to track support for RC2 against. Microsoft changed a ton of stuff with regard to building and assembly loading between RC1 and RC2, so it's going to take me a bit to untangle everything. I'll post additional information/blockers as I progress.
The text was updated successfully, but these errors were encountered: