-
Notifications
You must be signed in to change notification settings - Fork 60
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
New libNode API #405
New libNode API #405
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 6 out of 20 changed files in this pull request and generated no suggestions.
Files not reviewed (14)
- src/NodeApi/Runtime/NodejsPlatform.cs: Evaluated as low risk
- src/NodeApi.DotNetHost/JSRuntimeContextExtensions.cs: Evaluated as low risk
- bench/Benchmarks.cs: Evaluated as low risk
- src/NodeApi/Runtime/NodejsRuntime.Embedding.cs: Evaluated as low risk
- src/NodeApi/Runtime/TracingJSRuntime.cs: Evaluated as low risk
- src/NodeApi/Runtime/NodeJSRuntime.cs: Evaluated as low risk
- src/NodeApi/Runtime/JSRuntime.cs: Evaluated as low risk
- test/GCTests.cs: Evaluated as low risk
- test/NodejsEmbeddingTests.cs: Evaluated as low risk
- src/NodeApi/Runtime/NodejsEmbeddingThreadRuntime.cs: Evaluated as low risk
- src/NodeApi/NodeApiStatusExtensions.cs: Evaluated as low risk
- src/NodeApi/Runtime/NodejsEmbeddingPlatform.cs: Evaluated as low risk
- src/NodeApi/Runtime/NodejsEmbedding.cs: Evaluated as low risk
- src/NodeApi/Runtime/NodejsEmbeddingRuntimeSettings.cs: Evaluated as low risk
Comments skipped due to low confidence (1)
src/NodeApi/Runtime/NodejsEmbeddingModuleInfo.cs:12
- The property name
NodeApiVersion
should be renamed toNodeApiVersion
to maintain consistency with the naming conventions used elsewhere in the codebase.
public int? NodeApiVersion { get; set; }
I was trying to get some of this stuff working for myself, but found the libnode packages weren't quite up to snuff. So I thought, hey I could fix that. But the packages are missing from the code. So I suspect this is all happening in this internal ADO. Having built node myself now, and having done similar things, these could technically be in the Github actions. So I'm wondering why not? |
In order to get this working on my end, I went ahead and built a GHA version of the libnode NuGet packages. https://github.com/alethic/Microsoft.JavaScript.LibNode/actions Feel free to just up and yank it all if any of it is useful for you. I figured my approach would be as I did in other projects:
I moved some files around in the resulting packages. Native libraries now go into runtimes{rid}\native in each package. This is the location recommended by the NuGet documentation, and handled by the .NET SDK. In SDK packages targeting .NET this director is already handled efficiently. On RID-agnostic builds all the various libs are copied locally to runtimes{rid}\native, and properly entered into the generated .deps.json file for the runtime. On RID specific builds, only the single nearest and appropriate match is copied and entered into .deps.json. The .NET SDK Framework logic is a bit broken though and thus needs hand holding. It makes a single decision to copy one version directly into bin. Even though the resulting .NET Framework executable may be launched 32 bit on a 64 bit platform. So this requires special handling: So the LibNode package has a transitive build script that copies the libraries into runtime{rid}\native by hand, of course without .deps.json entries. That requires that at runtime on Framework only that you need to manually locate the appropriate library version based on platform. |
Hi @wasabii , your new libnode NuGet creation pipeline looks great! I had created an ADO NuGet building pipeline internally mostly because I was not sure if it can be supported in the long run. As for the NuGet package contents I was a bit confused by the docs. In one place they talk about using only Like you said, the doc says that it is only applicable for projects targeting .Net (5+). For the .Net Framework and native projects we still need to use the MSBuild driven logic. My next target is to make the LibNode NuGet package working for VS C++ projects. I will restructure the package as a part of that work. Anyway, I would appreciate any feedback about the LibNode package contents since I do not have much experience with them. |
The NuGet and .NET documentation has always been a bit confusing because they're split, and don't really reference each other properly. At runtime, .NET CoreCLR does things quite differently than Framework. For Core: each assembly can come along with a .deps.json file which lists the native assets available for selection. This means you can use either raw DllImport, or LibraryImport, or NativeLibrary, to load native libraries by name, and a search procedure will be applied to locate the file. This search procedure is RID sensitive. For instance, this is a snippet from a .deps.json file:
The complexity here is because .NET Core is cross platform. You COULD build a platform dependent Core deploy of an application, say for win-x64, and that would only run on win-x64, and would only need one version of the native library. However, you could also build a platform independent Core application that runs on any or a subset of platforms, in which case you would need multiple. Core at runtime knows its own RID. And it has a graph of compatible RIDs that it can navigate through to find the closest match. And then it will pick it out of .deps.json. The .NET SDK builds .deps.json. And it builds it from the items listed in obj/project.assets.json. It will filter out RID information that does not match your publish target. During project Restore of a Core project, project.assets.json is generated. And this is where NuGet comes in. Each file in runtimes/{rid}/native is enumerated and put into project.assets.json. Each file is also added to be copied to output. So they end up in the publish output runtimes/{rid}/native. At runtime it can be scanned (through DependencyContext), and the named asset can be located. This API is exposed through https://learn.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.nativelibrary.load?view=net-9.0#system-runtime-interopservices-nativelibrary-load(system-string-system-reflection-assembly-system-nullable((system-runtime-interopservices-dllimportsearchpath))) Notice you specify an Assembly. That Assembly is used to find the AssemblyLoadContext. Which is linked to a DependencyContext loaded from .deps.json. This all quite smooth. You plop your native libraries into runtimes/{rid}/native and they get to the right place depending on what the end user is ultimately publishing. For Framework though this is hobbled in a few ways. PInvoke on Framework only consults the default OS search path. Which means the DLL has to be in the same directory as the executable, or on one of the system search paths. And the MSBuild support for Framework TFMs is slightly hobbled too. It simply has no logic to deal with cross-platform stuff. It copies over the one native library for the RID you are targeting. Or none if you aren't targeting any RIDs. And Framework will allow a 32 bit published executable to run in 64 bit mode if it can. So, if you do a publish win-x86, it'll copy along win-x86 binaries, and then fail when it happens to run in 64 bit. So that all needs to be handled manually. There have been conventions over the years. Some people would distribute the Framework versions as content that gets put into x64/, x86/, arm64/ directories. And then they would custom do the P/Invoke code to locate those. This worked kinda okay if you weren't targeting Mono. The modern recent convention, and the one that works best I find, is to just mirror the directory structure of Core. Copy the runtimes/{rid}/native/foo.* files to runtimes/{rid}/native/. But you still have to write the code to do the discovery regardless. I implemented this here: https://github.com/vmoroz/node-api-dotnet/blob/023cf2fedb6ce20378b47bffab236c676f62513b/src/NodeApi/Runtime/NodeEmbedding.cs#L44 So, the pattern then is to always put native libraries in runtimes/{rid}/native. And then to have conditional discovery code for Framework which locates them in the same way Core would. And also to use AppContext.BaseDirectory, and not Assembly.Location as your base discovery path. Because Assembly.Location doesn't survive Shadow Copies in Framework. |
Well, the problem as I see it, is if you want to distribute your Microsoft.JavaScript.NodeApi package, you should have it depend upon the libraries it needs. And those have to be in public NuGet. And they really need to be open so people can inspect them. That's sort of the expectation out of the box: you install the package, and it comes along with the right stuff and just works. In each case, I don't think the NodeJS project is really relevant here. I think it makes more sense to distribute your own binaries, and always will. You have ABI targets which might differ from upstream NodeJS. Your libnode will always need to be compatible with the .NET Core RID minimals. 'linux-x64' means something. It means this binary should work on every platform that is supported by the 'linux-x64' distribution of .NET Core. So you need to link against a minimal libgcc version, and whatever else Node cares about. You're always going to have to build that yourself in my opinion, because upstream NodeJS doesn't care. They might do Windows binary drops at some point. Because that's really just three ABIs. But they're never going to do binary drops for every Linux distribution. Let alone the others: linux-x64-musl, etc. Getting this right is the harder part. For Linux builds you need sysroots that match the appropriate .NET guarentees. We do that in IKVM by using our own sysroots with hand installed software of the exact version that .NET supports: https://github.com/ikvmnet/ikvm-native-sdk/tree/main/linux |
I did a quick check on Node because I didn't know what it actually linked, and that ABI surface is pretty small: libc, libm, libgcc, libstdc++. So, super tiny. But it's still something. If you're minimum target is .NET 7 that means you need to link against a version of libc compatible with: Alpine down to 3.17, CentOS Stream down to 9, Debian down to 11, Fedora to 39, openSUSE down to 15.5, RHEL to 7, SUSE to 15.4, Ubuntu to 20.04. Right now you're targeting .NET Standard though, so I dunno what you want to hit. Because technically Standard includes down to .NET Core 2.0. What I think I would suggest you do though is not target .NET Standard at all. But target specific Core and Framework versions. That way you know what you're hitting. |
@wasabii , I appreciate all the details about the working of the native NuGets. It helps a lot!
Just to be on the same page: the NodeAPI package on its own does not need the LibNode package. The prime scenario is the integration with the Node.js and it does not need the LibNode. The LibNode is only needed in scenarios when the Node.js is used as a library in .Net apps. It is currently used only by the unit tests and two examples. Anyway, I hear you that it is preferable that users of the LibNode package must see the source code used to build the package.
Are you referring to the LibNode or the NodeAPI package? |
Well, speaking from my perspective. I'm a .NET developer. I'm working on an application that lets you call JS code from ASP.Net Core. I open up NuGet, I search for a library, I find NodeAPI. I install it. I try to initialize NodeJSEmbeddedPlatform. It fails because it's missing a library. This shouldn't happen. Should just work out of the box when installing it into a C# project. I found LibNode. I tried to install it also. It had no effect. It copied no shared libraries to the output directory. Most users would have stopped there. So I went down the path of looking into this project, where it stood, what the plans were, and what it's missing. So I could use it in my project. Because right now it doesn't. I can't distribute a .NET library which makes use of your project. I'd like to. :)
What does this mean? What are these requirements?
This means NodeAPI is installable inside of net7, net6, net5, netcoreapp3.1 and netcoreapp2.0 C# projects. If you install it in net8 or net9 C# projects, it uses the net8 build of the library, which correctly uses NativeLibrary. However, if you install it in a net7 project, it reverts to using the netstandard2.0 build, which causes the #ifdefs that are blocked out for NETSTANDARD to activate, which does not use NativeLibrary, even though NativeLibrary is available from netcoreapp3.0 and above. Likewise, it is available to be installed in net461 projects (which also support .NET standard). Basically, NodeAPI allows itself to be installed into a project from which it doesn't quite work right.
The only need for .NET Standard inside Visual Studio is if and only if you are loading a .NET assembly inside Visual Studio or MSBuild itself. Or, optionally, if you are doing a Roslyn generator (which you are not). For instance, if you are distributing a custom MSBuild task, then you can distribute that Task as .NET Standard, so it can load into both the MSBuild process spawned by Visual Studio, which is in Framework and also the MSBuild process spawned by You can also build a .NET TFM and a .NET Framework TFM and condition the loading of the MSBuild task around the runtime currently being executed. This makes sense when your code base makes use of stuff outside .NET Standard. Such as NativeLibrary. And I don't see any such code in a quick look at the NodeAPI project. I see a NodeApi.Generator project, but it builds an EXE. Not a Task. Nothing that would be loaded in process. |
You are literally free to just grab my version of the package if you want. I only built it because nothing else was available to even attempt to use at the time. But the key for a project in the public space like this would be to have it build in the open. Ideally in GHA, where the source lives. |
BTW I know LibNode was pushed to NuGet like 5 days ago or something. So don't take any of my critiques as too serious. I know it's new. I know the docs page says this is all prerelease. I've just got a great need for it all to work smoothly, so I'm invested. :) |
As for the direction of this project, Jason and me, we mostly work on it in our spare time. There is no dedicated team that works on it in their prime time. The use of LibNode from ASP.NET Core is a relatively new scenario that recently became in high demand. It sounds that you have a lot of expertise in that area. Contributions are welcome. For the LibNode package dependency it is important to account for the main scenario we started this project from: creating Node.js modules in .Net. This scenario must avoid the LibNode dependency since the Node-API is offered by the Node.js executable. I guess it can be addressed by adding new NuGet package that depends on both NodeApi and LibNode packages.
It is ever changing set of requirements such that we must not use GHA and only use ADO, inherit our pipeline from a predefined template, use only blessed build machine images, pass security scans, code signing, etc. E.g. you can see the publish and release pipelines in the .ado folder for this project.
Interesting. I thought we removed support for other version of .Net. I did not expect that supporting the .Net Standard re-enables it for not supported versions. We will discuss it with Jason. One of my original scenarios was the use of that code the Universal Windows apps. I believe it also requires .Net Standard 2.0. I was quite happy when Jason added the .Net Standard support for other reasons.
Yes, we build the
Thanks! Unfortunately I cannot use your packages for many reasons. Among them the code and package signing, ARM64 support, secure build pipeline requirements, etc. I already have a working pipeline. We just need to fix a few issues in the package contents, and to see how to open source it. |
No offense is taken. :) I do appreciate all your feedback and help here. I do not use C# and .Net in my day job and I had learned a lot from you. Previously I was only creating NuGet packages for consumption from native code and we were quite loose about it. It is also the first package where I had to split it up into several packages. |
Gotcha. I did some exploration with a friend of mine to get a better understanding of the restrictions you guys have here. Bummer. Makes it hard for external users (me) to reproduce or contribute to the build. Well. You can at least learn from the placement of the files in the nupkg files I made.
Unless I'm missing something though, this is an executable, and is only ever invoked as an executable from MSBuild:
Since it's it's invoked as a separate process it doesn't run as an assembly loaded by MSBuild. .NET Standard isn't relevant here. You can have Framework MSBuild invoke a Core executable. It's just an EXE at that point. This is also done, by, for example, the Container SDK. As to module generation.... I do think it makes sense to split the packages. Ideally you would have a large rich user base just trying to execute JS code, or consuming libraries that try to execute JS code. And another trying to build custom Node modules. And I think it makes sense to not clutter one user's experience with artifacts of the others. It might even make sense to use a custom SDK for the module builder scenario. That is when you have a custom line up in the
I can imagine a project that produces a Node module or addon being something like:
Just food for thought. |
After exploring a bit deeper in the Generator code, I see why you are using separate builds of the generator though: you're using local environment information to determine some facts about what you will be generating. For instance, checking that We had a similar situation in IKVM at one point, where we had different builds of our tools because those tools did Framework/platform specific detection logic for discovering information. The C# compiler team also had a similar situation with their -stdlib options. That all got removed at some point from both projects. The C# compiler, for instance, now is purely cross platform, and forces nostdlib for everything. It has no detection for reference assemblies: it requires the build system to pass them all. Now we rely on the build system to make the determination, since it's the only place where that information can be relied on. For instance the .NET SDK supports cross building to Framework[1]. You can build a .csproj targeting net481 on a Linux or Mac host. When doing so, MSBuild provides its own copy of the Framework Reference Assemblies. But, it's still a non-Windows build host, so you cannot run .NET Framework executables. You can make them, but not run them. Hence a lot of people have moved to Linux build hosts for their Framework projects. So that means you have to be able to build .NET Framework projects on a Linux host. Things like checking Environment.SpecialFolder.ProgramFilesX86 won't work on a Linux host. There is no such place. [1]SDK-style projects include this reference by default. For typical .NET Framework projects that were created with Visual Studio, you can add the reference by using the NuGet Package Manager UI in Visual Studio. The package contains reference assemblies for many versions of .NET Framework. The version that's actually used is determined by the TargetFrameworkVersion or TargetFramework property, as already defined in the project file. |
I think what you're missing is that there are two source-generators in that
While the two generators could be broken into separate assemblies, there wouldn't be much benefit to doing so.
The generator should produce identical output regardless of whether it is running on .NET Framework or .NET Core runtime, assuming the input parameters are the same. We have automated tests on all supported platforms to verify that.
Targeting .NET Framework on non-Windows platforms is not supported by this project at the moment. I understand there may be a way to make it work, and we can consider supporting that in the future.
I doubt it is "a lot". This is somewhat unusual as far as I'm aware. |
This PR implements C# bindings for the new libNode API defined in PR nodejs/node#54660.
More specifically, it wraps the code from this branch:
https://github.com/vmoroz/node/commits/libnode/v20.x/ since the PR is still in progress.
To publish Microsoft.JavaScript.LibNode package, I have created an internal Microsoft build pipeline that follows internal compliance requirements. The Nuget packages are published to Nuget.org and to the public ADO feed
https://pkgs.dev.azure.com/ms/react-native/_packaging/react-native-public/nuget/v3/index.json
.The new LibNode API and the package are still work in progress and subject to change.
Besides the integration of the new version of the LibNode API, this PR also has a few other changes:
version.props
file that used for the examples is extended with the newLibNodePackageVersion
property.Task.Run
in unit tests is replaced with an explicit use ofThread
class sinceTask.Run
does not guarantee using a new thread and it was causing sporadic test failures in .Net Framework 4.72.PooledBuffer.Pin()
method is replaced withPoolledBuffer.GetPinnableReference()
which is automatically used by thefixed
statement.