Improve loading of packages for .NET Core

[ermshiperete]

With .NET Core packages are no longer continuously downloaded and stored in the repository (under packages folder). It is stored in a NuGet cache (ie. %USERPROFILE%.nuget\packages). As a result, when running on .NET Core, the dependencies are resolved in their cache folder and not actually copied to the program's binary output folder. The .NET runtime knows how resolve these runtime dependencies when you use [DllImport] but since we are calling a Windows function LoadLibrary, it bypasses that logic.

See #20

[conniey]

@ermshiperete
Hi, I migrated the code to .NET Core as part of #8 , but I am having troubles loading the native libraries because of the current model for loading the native dependencies... and am unsure if can resolve the native dependencies without hacking in something.

Could you tell me what the motivation for moving away from using [DllImport] to using LoadLibrary/dlopen is to help me understand?

Thanks!
Connie

[ermshiperete]

@conniey: Linux installations have the ICU libraries installed in the system, so typically you don't install your own ICU libraries. The downside is that you'll have to deal with multiple ICU versions because e.g. Ubuntu 12.04 came with ICU 4.8, 14.04 comes with ICU 52, and 16.04 has ICU 55.

Using the LoadLibrary/dlopen approach allows to have one managed icu.net assembly that can work with any ICU version it finds on the system.

Another benefit is that it's no longer necessary to create separate icu.net nuget packages for each supported ICU version which greatly improves maintainability. We still need to provide separate icu4c nuget packages for Windows for each supported ICU version, but that's a one-time deal. For the more frequently changing icu.net nuget packages we'll have to create only one package at release time that works on both Windows and Linux and with any ICU version.

I hope we'll find a solution for the modified .NET Core behaviour.

One could think that on Linux we could use a similar approach as on Windows and simply install the native ICU libraries along with icu.net. However, that's not a solution. It would be a nightmare because these are native binaries so you'd need separate libraries for 32-/64-bit and for each OS version you support because of differences in libc. I don't think nuget supports that out-of-the-box, so it would need a fair amount of hacking there, and it would hugely increase the size of the nuget package.

Hope this clarifies the motivation for that change!

[NightOwl888]

@conniey

We are getting close to getting the build process working on Lucene.Net and I am now reviewing dependencies. One thing that stuck out is that the official icu-dotnet doesn't yet support .NET core.

I see you made a MyGet-based feed for a customized version that supports .NET Core, which is great. However, there are going to be a couple of issues:

1. The .NET Standard version is not available on NuGet. The workaround of putting the URLs in a NuGet.config file works on the build server, but isn't going to work on user machines.
2. The .NET Standard version has a higher version range than the official version.

So, although we have a solution that is working, we need to have a version that people can download from NuGet. I see that it also has a customized version of Icu4c.Win.Full.Lib that is also not on NuGet.

For the short term, we should probably change the package ids so they are prefixed with Lucene.Net:

1. Lucene.Net.Icu4c.Win.Full.Lib
2. Lucene.Net.icu.net

and then they can be uploaded to NuGet without any conflicts and we can use whatever version seems appropriate.

For the long term, we should aim to find a solution/merge the solution to this repository so we have an officially supported solution.

NOTE: I noticed that NuGet has planned to make it possible to make platform-specific dependencies, but it is not yet implemented. That may be useful for the Icu4c.Win.Full.Lib package.

Are you available to work on this? Also, can you shed some light on what the differences between the MyGet packages and the official packages are?

[conniey]

@NightOwl888

I've created the pull request #37 which allows this project to run on .NET Core.

The .NET Standard version has a higher version range than the official version.

Could you explain this statement a little more? We may have to upgrade Lucene.NET to netstandard1.6. Incrementing .NETStandard version increases the API surface and does not remove from it. I had to support netstandard1.6 for icu-dotnet because there were APIs missing in previous versions I needed.

[NightOwl888]

The .NET Standard version has a higher version range than the official version.

Could you explain this statement a little more?

What I mean is the official version is at 2.1.0 and the version that Lucene.Net references is 54.1.1-alpha. NuGet would make the wrong decision with a higher version like this if you just used the command Update-Package icu.net. But that is moot if we are merging everything back here.

We may have to upgrade Lucene.NET to netstandard1.6.

Actually, I was trying to work out how you came up with this - it is targeting netstandard1.5, but referencing netstandard1.6. It seems that Microsoft recommends using the lowest version you can.

I am taking a stab at upgrading to the VS2017 .csproj format. If successful, this will cut the amount of script down to almost nothing and has eliminated much of the complexity with the versioning (MSBuild allows you to pass version as a property and automatically adds the version attributes). The only downside is that I can't seem to get NUnit working in VS2017 on .NET Core - if I understand the threads on the NUnit VSTest Adapter correctly, it is still not supported.

The ICU-dependent classes have been added to a new package named Lucene.Net.Icu. This means that general users of Lucene.Net.Analysis.Common and Lucene.Net.Highlighter don't need to mess with the icu.net reference. Also, I noticed in the documentation comments that the Collator we have ported is worthless without the JDK's collator functionality. There is another Collator in the Analysis.ICU package that is meant for use with icu4j, which should be a closer match for this package. My thought is just to add that functionality to this new package as well, but put the functionality into the appropriate namespace where it belongs. I also have moved the BreakIterator and CharacterIterator abstract classes, IcuBreakIterator and StringCharacterIterator classes to this new package. I think it will be much easier to deal with the icu-dotnet dependency if we only have to reference it from one place.

[conniey]

Actually, I was trying to work out how you came up with this - it is targeting netstandard1.5, but referencing netstandard1.6. It seems that Microsoft recommends using the lowest version you can.

The reason I upgraded it to netstandard1.6 from the package we had before is that we now use DependencyContext.Default to find the referenced icu4c native package to load. This property only exists in netstandard1.6+.

I am taking a stab at upgrading to the VS2017 .csproj format.

I took a stab at this a couple of weeks ago... dotnet migrate tool does an alright job of moving the brunt of the template code... but I was running into annoying build issues where it does not exclude the right files if you have a <Compile Remove=""> ItemGroup with multiple items: dotnet/sdk#1074

My branch for that is here: https://github.com/conniey/lucenenet/tree/vs2017.

NUnit working in VS2017 on .NET Core - if I understand the threads on the NUnit VSTest Adapter correctly, it is still not supported.

In my PR #37 , I make reference to that issue nunit/nunit3-vs-adapter#297. Unfortunately there is no NUnit support in VS2017 for it yet. You have to create a wrapper project that uses NUnitLite to run it from command line, which I've done in this project as a workaround. :/

[ermshiperete]

Fixed/implemented in versions >= 2.3 (beta.77)