-
Notifications
You must be signed in to change notification settings - Fork 18
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
Add support for nuget package generation #17
base: master
Are you sure you want to change the base?
Conversation
Hi. I tried to test this and I failed. I may have done it wrong, though. Here were my steps:
It seems like I am missing something (like a call to SetDllPath or some .json nonsense). I also tried giving lsl.dll the copy always property. This copies it into a folder called runtimes in the bin/netstandard2.0 folder but this does not help. Then I tried adding lsl.dll to the liblsl project and giving it 'copy always'. Now the nupkg works, but I still get the yellow warning tags on my dependencies, which is bad. |
Finally, just looking ahead a bit, I would say that once this whole process is fixed up, we need to do some adjustment to the package names and other meta-data. 'lsl_csharp' is probably not the best name for the package. Obviously it is .NET if it is on NuGet. I think just 'LSL' is correct. I also think that due to the contributions of others (especially @tstenner and also yourself) that listing Christian Kothe as the sole author is not appropriate anymore. Then I think it would be good to CI this thing and upload it to nuget.org automatically every time we need to update it. |
@dmedine thanks for giving it a try! I would be curious to hear more about the warnings you got when installing the package, do you have any error message text you could share? Regarding native binary resolution, there is indeed not a good unified story yet for how they should be handled. For the "old" .NET framework, what me and several others have used in the past is to include However, there was no convention for identifying native platforms besides the In this new world the The story is complicated outside ASP.NET land however as there is no host that can automatically load up the dependencies. This has led to confusion from library authors which end up hacking different ways to do the runtime resolution. Some of them use I personally quite dislike these options as it makes it really easy to end up with different incompatible strategies for doing native dependency loading (some of them even incompatible with different underlying OSes) which compounds the DLL hell problem further. My current inclination then is to keep things as simple as possible, using all of the agreed upon standards while avoiding any premature decisions on things which are not standard. The p/invoke level already uses the underlying library resolution mechanism of the host OS, so no need to reinvent that wheel and try to reimplement complicated search strategies. Of course, we need a way to make sure the native dependencies included in the package end up in at least one of the places which gets scanned. Currently I know how to do this with build scripts for .NET framework but really need to study the story better for .NET Core. This PR was meant as a way to start this conversation for liblsl which I will need to find a solution for in the short/medium-term. .NET 6.0 is also around the corner with more insights on the p/invoke story. I will investigate a bit further and report back more details in this thread or the original issue. I totally agree with your thoughts regarding package name, but kept it to avoid changing the existing project conventions right away. |
Regarding copyright its a complex story. You are totally correct regarding contributions, but tracking these things can get messy. As a rule of thumb I assume that if the original author picked the MIT license then their intention is to keep the legal struggles to a minimum and simply provide an umbrella for contributions. The other extreme is GNU and hybrid model is Apache or a CLA (contributor license agreement). We can have a contributors file and change the copyright to "and contributors" but maybe I would raise a new specific issue for this? |
Hmm. I hadn't thought that there would be legal ramifications to extend the authors list. I just wanted to acknowledge some of the more recent contributions. I don't think anyone is going to cry if they don't see their name on the package, though ;-). In any case, the license should also be rolled into the package... but let's deal with the cart before the horse. |
|
Sounds like Microsoft has not dealt with this very well. This has been my experience with other projects that rely on native code in C-sharp as well---even ones published published by Microsoft! It is possible, of course, to insert xml inside the .csproj file that organizes all the dependencies properly (e.g. https://github.com/microsoft/onnxruntime/blob/master/csharp/sample/Microsoft.ML.OnnxRuntime.InferenceSample/Microsoft.ML.OnnxRuntime.InferenceSample.csproj), although Visual Studio doesn't seem to have any features that let you do this apart from by hand. This doesn't help much, though, since there is no way to inject these kinds of instructions into a client application .csproj file from a NuGet install. Then again, this stuff is for developers. No one would ever expect a C++ developer not to have to provide include and linker directives when using building against 3rd party library. I want it to be as 'easy' as pip, but maybe it just isn't. That onnxruntime library I pointed out above, for example, is on NuGet, but you still have to find and copy the underlying native dll for it to work in your code. |
Actually, you can include MSBuild snippets in a NuGet package which will be included as if they are part of the final build project using the So in theory you could do an automatic deployment script which is correctly executed at build time. However, this is the old way of deploying a build in .NET framework, which doesn't seem to be the way that the new .NET Core / ASP.NET Core operate, where its the runtime, rather than the build, which is responsible for locating these dependencies (unless you are building an AoT project). I'll experiment a bit more and update this PR accordingly. |
Thanks for tackling this problem :-) Due to the current situation I don't have a PC with all the .Net SDKs and Visual Studio at hand, but I'll give my two cents anyway: CI builds are trivial, I've already added a commit to build liblsl-Csharp and the examples. The hard problem is 1) putting the native library somewhere 2) loading the correct library at runtime, and 3) work across .Net versions. NuGet packages are zip-files with a control file, so we can create them alongside the regular liblsl-builds and users will then have to depend on liblsl_csharp (or whatever better name you find) and e.g. liblsl.win.x86_64.
From my understanding that's perfectly fine, but I'd be thankful if you could open a separate issue.
I guess the LGPL license would make it a lot more simple: everyone's free to use liblsl_csharp as long as they provide the source code for liblsl_csharp upon request. Contributions are derivative works, so they are automatically under the LGPL. With the MIT license, I'd assume that contributors add their name to the license file / authors tag in the project file if they regarded their contributions as substantial or just overlook it otherwise. To be sure, we should ask previous contributors.
You wouldn't have this problem if you didn't stray from the Right Path, i.e. pure C# or the occasional Windows-only DLL 😉
It's a good start, and if we were to merge your PR and publish NuGet packages without the native library included it'd be better than the status quo and with new all-batteries-included releases we would only improve the situation. |
Thanks for your feedback. Diving deeper I am happy to say I can now give concrete answers to all of the above and if you test drive the latest commit following the folder structure outlined in the README file it should work out of the box.
You have to lay out the native dependencies exactly like so:
Documentation is extremely hard to find, but it exists Basically the thing that kept tripping me is the extra
It turns out the compiler will copy the dependencies to the right place and resolve them automatically if the above layout is followed. On .NET Core the runtime knows how to resolve dependencies straight from the runtimes folder, so everything just works cross-platform (!) On .NET Framework each build can only support a specific architecture which can be specified using the <Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net472</TargetFramework>
<RuntimeIdentifier>win-x64</RuntimeIdentifier>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="lsl_csharp" Version="2.0.0" />
</ItemGroup>
</Project> This will make the compiler copy specifically the 64-bit native binary to the project. By default, if no For completion, specifying a
It works across both .NET Core and .NET Framework without changing any code 🥳 Regarding license, I would say in this case sticking with MIT is much easier, especially with a single-file project where any derived works can simply place the copy of the license in |
For the record I am very, very, very opposed to changing the license from MIT to LGPL. I can't think any scenario in which a commercial software producer would be willing to use an open source library if it was licensed under LGPL. I think it is also a potential legal vulnerability to switch the license since so many people are already using this library. Many of them are in research, so it is unlikely to bite us (or, more specifically, Christian), but better safe than sorry. I am in between computers at the moment and the new one (which is where I am set up to test this right now) is at home. I will play with the new commit tomorrow and get back.
That is very interesting! I was looking for some clues about this yesterday and couldn't find much. Can you point me to some examples/docs about how to do this? I keep finding hints on MSDN, but we all know how spotty those pages are. |
@dmedine of course, I use this technique all the time! The only requirement is to include the Relevant documentation is here: https://docs.microsoft.com/en-us/nuget/create-packages/creating-a-package#include-msbuild-props-and-targets-in-a-package |
Hiding in plain sight. |
Nope. Still doesn't work. I put the native .dll in the correct folder, I copied the .props and .targets files from obj to the correct target directory for lsl_csharp, and I added the RunTimeIdentifier tag into SendData.csproj. But when I run the build, it still doesn't have access to lsl.dll and throws the System.DllNotFoundException as soon as the code tries to use it. Here is a screenshot of my setup. Maybe you can see something I've missed |
Hmm, it looks like in this example you still have a Also in this case you don't need to add any .props or .targets files, the package should already have everything that is necessary. If it still doesn't work, I can package a working example in a zip file to see if you can reproduce a successful build. |
Ah yes, that seems to have done the trick! I would propose the following
|
Great work 👍
The compiler, nuget or MSBuild? As long as it works I mostly don't care, but when it doesn't it'd be good to know which tool to blame.
.NET Framework is Windows-only (from a practical perspective) and Unity doesn't use nuget packages, so I don't this this is a big deal.
I can think of at least one: they don't link against a static library and have someone who will sent out the source code for this specific library on request. The Qt guys would like everyone to think that commercial usage without source disclosure isn't allowed, but that's one of the major selling (pun intended) points of the LGPL compared to the regular GPL. Well, back to topic: Do I understand it correctly that one nuget package has to contain the native libraries for all target platforms? It it is so, I'd rather have the C# wrapper depend on a separate package with the binaries so it's easier to add platforms (e.g. android and all the arm family members) without changing the wrapper package. |
Indeed this is what LGPL is meant for; and, you are right there are circumstances where it makes sense. But, in my experience, vendors are loath to relying on open source producers doing this for them. The nice thing about MIT is that you can simply set it and forget it. Otherwise, vendors will prefer to pay a premium for garaunteed stability and support. There are exceptions. FFTW is notable and there are probably others.
I believe the answer to this is MSBuild. It is then our task to make the nuget package glom-on-able enough for MSBuild to be able to get it right. Please correct me if I am wrong. |
I am not clear on this point either. I am not sure, though, if it is right to have 2 packages or simply add support for new targets piecemeal. 2 packages seems hard for the client, but bumping a versions just for a new target support seems wrong to me. Maybe there is another strategy for this in dotnet land that I don't know about---but I kind of doubt it :/ |
Sony has a page where they put the source code for all the libraries they are using and from what I've understood it'd be enough to push the "fork" button once and link to the forked repository. But you're right that any licensing requirements are hard to sell in corporate environments.
Other package managers (apt, conda, rpm, …) offer a way to say "Package A depends on foobar" and one of several packages providing "foobar" can be used to resolve the dependency. Other package managers allow dependencies like "Package A depends on PackageB>=1.2 | PackageC>=1.4". As far as I can see, Nuget doesn't allow this. Still, "liblsl_wrapper x.y" could depend on "liblsl_native x.y" and additions to the native package could then be pushed as "liblsl_native x.y.2". Question for the nuget experts: is it sufficient to refresh the nuget packages so the updates to transient dependencies are pulled? One thing I'd like to see is a way to tell "I want to use liblsl_wrapper, but I have my own lsl.dll/.so/.dylib". Is there an attribute (e.g. |
That is actually an excellent point that I hadn't considered. However, I think the answer is that people who want to link against some custom/non-standard lsl.dll/so/dylib are going to have to roll their own. I think that it is reasonable to expect people that want to do such a thing don't need NuGet---especially since we have pretty clear build instructions.
That, as they say, is the rub. I fear we are still no closer to resolving this. Actually, I have been using Microsoft's onnxruntime library for some projects and you can get the C# wrapper on NuGet no problem, but your code will not run until you right-click on the project, change the extension to 'all .', etc. just as we suggest with adding lsl.dll to a Visual Studio project. On the other hand, that is a pretty Linux-y development project---even though it is from Microsoft---so what do they know? One idea occurred to me and that is to issue one just with the standard Windows/Linux/Mac shared libs and then for Android or other more exotic platforms have a LSL-Android/whatever package. I believe there is some precedent for that on NuGet. |
If we were to somehow solve the "add a reference to liblsl-wrapper but ignore the liblsl-native dependency"-problem, the wrapper could depend on a native package with the most common libraries and users on exotic platforms could just depend on liblsl-wrapper-without-native-libs and liblsl-native-android-arm8. |
@dmedine @tstenner I believe you can do this already at package import time when you specify which assets to include or exclude from each package (see here). For example, to exclude the built-in native dependencies, it is enough to do the following: <ItemGroup>
<PackageReference Include="lsl_csharp" Version="2.0.0">
<ExcludeAssets>native</ExcludeAssets>
</PackageReference>
</ItemGroup> |
That would solve the problem with exotic targets, but those users would still download several megabytes of irrelevant binaries. I guess we can just release a batteries-included version of the wrapper and then later remove the libraries and depend on a separate liblsl-native package. Regarding the versioning, I have already suggested something in a different PR:
|
@tstenner I like the suggestion regarding versioning, it is useful to be able to map the wrapper version directly to the base library version I agree it would not be a problem to have two packages, one with the common native targets, and another one without. I would probably start with the package with batteries-included, since advanced users can still just copy Regarding project name, is there a conclusion? To avoid name clashes I like to use the .NET suffix to indicate the wrapper (I prefer it to csharp, since technically you are able to use it with VB or managed C++), here are some suggestions:
The middle one keeps acronym capitalization following .NET convention. The first and last ones break this, but they have enough of their own justification to be picked too. |
This one 👍 |
Hello everyone! I recently wrote a C# wrapper for liblsl and plan to submit some PRs to liblsl-csharp. Currently, SharpLSL has implemented packaging for liblsl native libraries and supports .NET Framework 3.5+ and .NET 6.0+. You can check it out here: https://github.com/myd7349/SharpLSL/tree/main/Source/SharpLSL.Natives . I'm thinking that once this PR is merged, I might be able to further improve the NuGet packaging based on it. |
Fixes #2
This PR expands support for nuget package generation including common metadata such as projectURL, package license and package tags. A
README.md
file has been included in the runtime identifier folder to explain how to correctly build the package.At the moment, assemblies in the runtimes folder
need to be resolved manually by callingare resolved automatically by both .NET Core and .NET Framework according to the relevant runtime identifier, as long as the package architecture-specific folders are correctly declared. This package follows theSetLibraryPath
or using thedeps.json
file when publishing a .NET Core app. Nevertheless tforeseeablestandards for platform-specific native dependencies using runtime identifiers going forward from .NET 5.0 as documented in RID Catalog and in the section on architecture-specific folders.