Updated LfMerge to .NET6 #278
Conversation
Includes updating .Core to .NET Standard. I picked this simply to instill more compatibility with other frameworks. Also includes updating various old Framework packages to the minimum versions that supported .NET Standard. Removed icu.net to depend on indirect reference. Upgraded MongoDB, per commandlineparser/commandline#225 and refactored some duplicated code into a base class in .Core. Referencing the "netcore" versions of the SIL packages -- these can go back once the "netcore" branches are in. Upgraded libpalaso references to 9, since most are already .NET Standard. There are lots of warnings for .NET Framework packages, some fixable, like SIL.Lexicon, and some not, like NHunspell. But these changes allow it to dotnet build
|
Looking forward to seeing this completed; building and running with .NET 6, if you can manage to get it all working, will allow me to use several features from recent C# versions that I've been looking forward to having available. It'll make for much nicer LfMerge code in the long run. Thanks for working on this! |
Couple notes: Removed RestoreBuildTasks. It doesn't appear to be necessary -- Nuget and the relevant projects should handle this if it turns out it is. Didn't replace --debug flag in mono with anything. It's unclear if the theoretical dotnet equivalent would perform as desired, but we could maybe use "verbosity" if needed. These changes are experimental, but a commit is required to test, since in docker creation pending changes are cleared.
|
@rmunn so the build is passing now, and i think i've gotten the usages of mono removed. however, i am definitely not the expert on the innerworkings of the surrounding scripts, so there may be some (major) things i'm missing. but feel free to take a preliminary look! note that the GHA workflows were flaky -- i had to do manual re-runs to get the FW8 ones to pass. Assuming this is not normal |
|
Another potential discussion point: i haven't upgraded the test projects. they reference the liblcm tests, which are still .NET Framework. so if we want the LFMerge tests to be .NET6 (and to remove Mono from them), we will likely need to upgrade the liblcm ones, as well |
Now that we're using signed MongoDB packages, MSBuild cares about exact version numbers. Since we don't care about patch releases, we have to tell MSBuild explicitly to use version 2.4.*.
With .NET 6, compiling defaults to producing Linux binaries that don't have a .exe extension, so our installation targets need to change.
|
Did some testing, and tweaked the build until it works. It fails to run, because the SyslogLogger class is looking for the Mono.Posix.NETStandard assemblty, which of course doesn't exist in a non-Mono runtime environment. But that will be easy to fix: just swich LfMerge over to logging everything to stdout, which is better in a Docker environment anyway, and get rid of the SyslogLogger. I'll work on that with you tomorrow. |
LfMerge and LfMergeQueueManager are now executable Linux binaries now that they're compiled with .NET 6, so we need mode 755 for them.
With the .NET 6 build, the Mono.Posix DLL is ending up in a different location than the other DLLs, along with a native library (in .so format for Linux) that it probably also needs, so we'll add those files to the installation script. Still doesn't work, but it was worth a try.
|
I got the missing DLL installing with the rest of the LfMerge files, but .NET still thinks it's missing, and I don't know why yet. But I did figure out where its dependency is coming from: it's depended on by the SIL.WritingSystems package, which is a core part of LfMerge that we can't do without. We call So we need to look at the SIL.Threading package and see if that can be converted to .NET Standard so as to not be looking for a Mono DLL. |
|
Mono.Posix.NETStandard is used by SIL.Core and SIL.LCModel.Utils, both of which are .NET Standard. if we need to remove these, I can look into that |
|
Looks like Mono.Posix is the older name, and the name has been changed to Mono.Unix: https://www.nuget.org/packages/Mono.Unix/7.1.0-final.1.21458.1 It might be necessary to update the .NET Standard versions of libpalaso to use Mono.Unix, or it might not. We'll find out, most likely. |
|
According to this issue comment, the Mono.Posix package may not work in .NET 4.8, let alone .NET 6.0. We'll definitely want to look into moving to the Mono.Unix packages for this. |
|
I've opened sillsdev/libpalaso#1186 to track the necessary work in libpalaso for switching to Mono.Unix. |
This is interesting. In that comment he mentions the theoretical .NET Standard package being compatible, so I'm not fully convinced there's something inherently wrong with all Mono.Posix.NetStandard packages. I think we'll just have to make a local package and see if it works |
|
So this morning I verified Mono.Posix.NETStandard works in a .NET6 environment, using the following simple program: Now I'm not saying we shouldn't move away from it. I'm just saying there's something else going on with that exception |
|
@rmunn have you tried |
|
@rmunn a bit more progress on this, and maybe you can provide further guidance. i noticed the
|
This fixes a warning, since v10 for this assembly targets .NET Standard
Installing it alongside the executable, like normal, resulted in a FileNotFoundException. For some reason it is expected to be in this exact place.
This allows "dotnet build" to succeed
|
Haven't managed to look into this in-depth, but a quick look showed the lfmerge process running successfully (yay), but displaying the error message "Can't acquire file lock - is another instance running?" on first run (in a clean container where there is no lock file yet). This error message is only displayed when the SIL.IO.FileLock.SimpleFileLock class returns false from the TryAcquireLock() method. If there was no other lfmerge instance running, then failing to acquire the lock is usually indicative of some sort of failure in the Mono.Posix or Mono.Unix calls that are used to acquire the file lock. In other words, the "Can't acquire file lock" error probably means that there's still more work to do before this is working, alas. |
|
The error is from SimpleFileLock in SIL.Core. File.Create() for /var/run/ is throwing the exception with this new framework. This is probably a simple fix, but I'm not good enough with linux operations in C# yet to have figured it out |
|
Commit abe050f fixes the permissions issue. |
Version 2.4 is old and has a bug where it doesn't work with .NET 6. Latest as of this commit is version 2.17.1.
This brings in two changes:
* Log to stdout by default
* Syslog logging no longer available, so warn if syslog chosen.
* Do not put projects on hold for entry failures, but record all
entry failures and save them to the state file.
MongoDB.Driver.signed version is still only at 2.14, though unsigned driver is up to 2.17. For now, stick with the signed driver.
We've replaced it with the ConsoleLogger.cs file.
There was a problem hiding this comment.
LGTM. Tested locally, and it successfully builds in GitHub Actions, so it's time to merge this into master and test on the staging server. I'm sure we'll find some other tweak that needs to be made once we test in a live environment, but this is ready to merge. Thanks for the good work, @josephmyers!
|
We'll deploy this when #232 is deployed. |
Includes updating .Core to .NET Standard. I picked this simply to instill more compatibility with other frameworks. Also includes updating various old Framework packages to the minimum versions that supported .NET Standard.
Removed icu.net to depend on indirect reference. Upgraded MongoDB, per commandlineparser/commandline#225 and refactored some duplicated code into a base class in .Core. Referencing the "netcore" versions of the SIL packages -- these can go back once the "netcore" branches are in. Upgraded libpalaso references to 9, since most are already .NET Standard.
There are lots of warnings for .NET Framework packages, some fixable, like SIL.Lexicon, and some not, like NHunspell. But these changes allow it to dotnet build
This change is