Skip to content
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

[Intention] Change platforms to .NET Core #159

Closed
11 tasks
atruskie opened this issue Mar 23, 2018 · 6 comments
Closed
11 tasks

[Intention] Change platforms to .NET Core #159

atruskie opened this issue Mar 23, 2018 · 6 comments

Comments

@atruskie
Copy link
Member

atruskie commented Mar 23, 2018

Expected behaviour

Our application should run on .NET Core - a newer, improved, .NET platform that has full cross-platform support, is faster, and has fewer quirks than Mono.

Actual behaviour

We can only run our app on Mono, which is slower and has less platform support.

Any other details

This change should be easy enough for our code. The problem is replacing dependencies that do not support .NET core.

UPDATE:
Another goal: I'd greatly prefer to still produce a self-contained executable for Windows machines. Currently, Windows machines all ship with a .NET framework installed which makes running our solution as simple as running the shipped exe. As I understand it, by default .NET Core does not produce executables anymore and requires a dotnet my.dll invocation to run any program.

Notes from @cofiem:

  • Remove the NonClosingStreamWrapper class in Acoustics.Shared as ObjRef is not in .NET Core, but the class wasn't used anyway.
  • Remove AForge.Net and replace with in Accord.Net, which does have .NET Core packages.
  • .NET Core does not support Binary, use bytes[].
  • .NET Core comes with a much smaller set of default BCLs. Add references to the NuGet packages:
  • alglibnet - the NuGet package is very old. However, the download from the source website has a .NET Core .dll. AT will need to check but I don't think we need this
  • ColorMine - the NuGet package is very old. However, either the original library (which is no longer available on github) could be manually converted to a .NET Core package, or there is a NuGet portable package that might work. AT: should be removable, if not all of what we need will be in in ImageSharp
  • DotSpinners - this package does not seem to have a .NET Core version
  • EnvDTE - I can't find a VS2017 or .NET Core-compatible version of this. The documentation is here. AT: we only use this has a hack for automatic debugger attaching, we should just #IF DEF out this reference for non-Windows, non DEBUG builds
  • fasterflect - It looks like work on a .NET Core/Standard port started, but is still in progress.
  • FSPowerPack - the FSPowerPack packages are very old (from 2012!). The project seems to be abandoned. I'm not sure how this can be upgraded or replaced.
  • QuickGraph - the NuGet package is very old. AT should be removable
@atruskie atruskie added this to the Move to dotnet core milestone Mar 23, 2018
@cofiem
Copy link
Contributor

cofiem commented Jul 24, 2018

I decided to run the .NET API Portability Analyzer over this repo.

I used the Debug for release v18.7.0.17.

The command line I used was:

E:\Dev\NetFrameworkNetCorePorting\ApiPort\net461\win7-x64\ApiPort.exe analyze -f E:\Dev\NetFrameworkNetCorePorting\QutEcoacousticsAudioAnalysis\Debug.18.7.0.17 -o "E:\Dev\NetFrameworkNetCorePorting\QutEcoacousticsAudioAnalysis\Debug.18.7.0.17.html" -r HTML -t ".NET Framework, Version=4.7.1" -t ".NET Core, Version=2.1" -t ".NET Standard, Version=2.0" -p -b -u

The output html file is attached.
Debug.18.7.0.17.html.zip

I haven't had much of a look at the output, but as is mentioned in #158, System.Drawing seems to be a big contributor to the issues.

@atruskie
Copy link
Member Author

Thanks for running this. It was quite useful to read.

@cofiem
Copy link
Contributor

cofiem commented Aug 4, 2018

I decided to give ~3 hours to seeing how far I could get with migrating AudioAnalysis to .NET Core.

Summary: the migration is definitely possible, however there are some dependencies that need updates before it is feasible to attempt. I did not manage to get it to build, but using the CoreCompat approach outlined below, I think it should be possible to make it build without many changes and without too much more work.

.Net Core seems to have much better support for System.Drawing, in System.Drawing.Common (src), which has replaced CoreCompat.System.Drawing and friends.

Accord.Net is the key dependency that is not ready - it is in the process of migrating, there is a commit in the development branch, but has not yet been released. The migration of AudioAnalysis could proceed without this change by using the current Accord.Net with CoreCompat, which in turn requires overcoming the type collisions for the types in System.Drawing.

The approach I took to do the actual migration was to create a new Solution in VS2017, and create each project as a .NET Core 2.0 Library (Console for AnalysisPrograms). I then copied the *.cs and *.fs files into the new projects. I added the same NuGet packages and added the same references to other projects.

A sample of the changes I made:

  • Removed the NonClosingStreamWrapper class in Acoustics.Shared as ObjRef is not in .NET Core, but the class wasn't used anyway.
  • AForge.Net does not have .NET Core packages, however all the functionality is present in Accord.Net, which does have .NET Core packages.
  • .NET Core does not support Binary, use bytes[].
  • .NET Core comes with a much smaller set of default BCLs. I had to add references to the NuGet packages System.Configuration.ConfigurationManager, System.Threading.Tasks, System.Drawing.{Common, Primitives}, EnvDTE.

I don't know if I'll give it any more time, but if I do I'll update my progress here. There is no branch or PR related to this, yet.

@cofiem
Copy link
Contributor

cofiem commented Aug 4, 2018

Other dependencies that need to be changed for .NET Core. Unfortunately none of these have an obvious migration path.

  • alglibnet - the NuGet package is very old. However, the download from the source website has a .NET Core .dll
  • ColorMine - the NuGet package is very old. However, either the original library (which is no longer available on github) could be manually converted to a .NET Core package, or there is a NuGet portable package that might work.
  • DotSpinners - this package does not seem to have a .NET Core version
  • EnvDTE - I can't find a VS2017 or .NET Core-compatible version of this. The documentation is here.
  • fasterflect - It looks like work on a .NET Core/Standard port started, but is still in progress.
  • FSPowerPack - the FSPowerPack packages are very old (from 2012!). The project seems to be abandoned. I'm not sure how this can be upgraded or replaced.
  • QuickGraph - the NuGet package is very old. This other NuGet package might work?

@atruskie
Copy link
Member Author

atruskie commented Aug 6, 2018

Awesome work.

Even though System.Drawing appears to exist in .Net Core compatible builds I'm still keen to replace it. AFAIK the limitations in GDI+ still exist.

I think your approach was useful. We should probably ramp up support for this migration slowly and maintain concurrent solutions. When both solutions compile, pass tests successfully, and are otherwise black box compatible, we can switch over.

I see that Accord has incorporated Aforge in its entirety so that problem should be a non-issue.

I've updated the original issue with a list of TODOs. More than likely, as you suggested, most of these dependencies can be removed, replaced, or we can submit a PR.

atruskie added a commit that referenced this issue Feb 17, 2020
F# projects not done - they'll need to be done manually later

Work done for #159
atruskie added a commit that referenced this issue Feb 17, 2020
And also migrating to using ImageSharp as drawing library.

Work done for #159 and #158
atruskie added a commit that referenced this issue Feb 17, 2020
Finished fixing build errors for dotnet core conversion

Also addressed some code style issues

Work done for #158 and #159
atruskie added a commit that referenced this issue Feb 17, 2020
Removed some code not needed now we're on dotnet core.

Also added more tests to ensure expected resources and build tasks work as expected.

Still need to fix assembly versioning.

Work done for #158 and #159
atruskie added a commit that referenced this issue Feb 17, 2020
Known bugs:
- short file name bug (se main entry)
- #289
- #288
- ImageSharp has bugs that crash with versions greater than  1.0.0-unstable0598
- ImageSharp has bug in parallel implementation so you'll find Drawing.NoParallelConfiguration sprinkled everywhere
- And another ImageSharp bug prevents us from writing text on concatenated images

All up though, the majority of things work. Yay!

Work done for #158 and #159
atruskie added a commit that referenced this issue Feb 17, 2020
F# projects not done - they'll need to be done manually later

Work done for #159
atruskie added a commit that referenced this issue Feb 17, 2020
And also migrating to using ImageSharp as drawing library.

Work done for #159 and #158
atruskie added a commit that referenced this issue Feb 17, 2020
Finished fixing build errors for dotnet core conversion

Also addressed some code style issues

Work done for #158 and #159
atruskie added a commit that referenced this issue Feb 17, 2020
Removed some code not needed now we're on dotnet core.

Also added more tests to ensure expected resources and build tasks work as expected.

Still need to fix assembly versioning.

Work done for #158 and #159
atruskie added a commit that referenced this issue Feb 17, 2020
Known bugs:
- short file name bug (se main entry)
- #289
- #288
- ImageSharp has bugs that crash with versions greater than  1.0.0-unstable0598
- ImageSharp has bug in parallel implementation so you'll find Drawing.NoParallelConfiguration sprinkled everywhere
- And another ImageSharp bug prevents us from writing text on concatenated images

All up though, the majority of things work. Yay!

Work done for #158 and #159
@atruskie
Copy link
Member Author

atruskie commented Apr 7, 2020

Closed (completed)

@atruskie atruskie closed this as completed Apr 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants