Version 3.x should support .NET Core LTS version(s)

[jzabroski]

We should support .netcoreapp3.1 and .netcoreapp2.1
The reason is that .netcoreapp3.0 and .netcoreapp2.2 is EOL (End Of Life), whereas .netcoreapp3.1 and .netcoreapp2.1 are LTS (Long Term Servicing).

This is supported in Version 4.x (develop branch), but we will likely be servicing Version 3.x for awhile, so it makes sense to add support for .NET Core 3.0

Process

1. Gather all FluentMigrator netstandard2.0 PackageReference dependencies
2. Gather all /lib/ folder dependencies and see if they now exist as Nuget Packages.
   • If they do, move to the Nuget Package for netstandard2.1
   • Delay moving for net461 and netstandard2.0
     • Reason: We don't have integration tests that perform end-to-end verification yet - we don't want a bad release, so let's be conservative here
3. For Microsoft.Extensions.* packages, in order to allow end users to load FluentMigrator from ASP.NET Core 2.x:
   • netstandard2.0 should use 2.2.0 dependencies (latest .NET Core 2.x dependencies)
   • netstandard2.1 should use 3.1.3 dependencies (latest .NET Core 3.y dependencies)

Out of scope: In the future, we should consider targeting netstandard TFM family that is parallel support to the underlying target frameworks. In practice, this means adding "netstandard2.1" to the list of TargetFrameworks

Lessons Learned

1. Next time, upgrade dependencies first so that the latest versions of each dependency work against currently supported TargetFrameworks.

2. Be careful of TargetFramework vs. TargetFrameworks.

3. .NET Core 3.0 introduced [NotNull] attribute. If you reference JetBrains.Annotations and System.Diagnostics.CodeAnalysis in the same file, you'll get a compiler error. The fix is to use the NETSTANDARD2_1 pre-defined symbol (see also Target Frameworks in SDK-style projects).

4. FluentMigrator.DotNet.Cli depended on an Obsolete extension method ILoggerFactory.AddDebug, which is now removed in .NET Core 3.x. There is a guide on Microsoft Docs for Migrate from Microsoft.Extensions.Logging 2.1 to 2.2 or 3.0:

       private static void Configure(ILoggerFactory loggerFactory)
       {
   #if NETSTANDARD2_0
           loggerFactory
               .AddDebug(LogLevel.Trace);
   #endif
   #if NETSTANDARD2_1
           LoggerFactory.Create(builder => builder.AddDebug(LogLevel.Trace));
   #endif
       }

5. Autofac-specific issues

   1. These are limited to the FluentMigrator.Tests project
   2. Autofac 4.9.4 is the last release that supports legacy .NET Framework. net45 TFM was dropped in 5.0.0.
   3. Autofac.Extensions.DependencyInjection 4.4.0 is the last release that supports netstandard1.0 TFM, and therefore is the last release that supports net461 TFM.

6. McMaster.Extensions.CommandLineUtils now 2.6.0

   1. Package has breaking changes from 2.6.0 to 3.0.0. As a result, we're using 2.6.0 as part of this issue. Upgrading to 3.0.0 is out-of-scope. Fortunately, 2.6.0 marks obsolete interfaces, so we can fix them soon after FluentMigrator 3.3.0 release.

7. FSharp.Core now 4.7.1

8. Microsoft.Build.Utilities.Core

   1. Stopped supporting net461 in major version 15. Last release to nuget 15.9.20.
   2. However, net461 targets major version 14. No idea why?

9. Sap.Data.SQLAnywhere doesn't exist for .NET Core.

   1. As a result, FluentMigrator.Tests project tests for SqlAnywhere will only run on net461.
   2. I've never seen a support request for SqlAnywhere in the last 2+ years, so it's FluentMigrator user base is likely small, but @eloekset might be one of them!

10. Microsoft Jet (via ADOX.dll) doesn't exist for .NET Core.

    1. We already had a check in FluentMigrator.Tests.csproj if the build was being built using MSBuild Core, but never had a check to guard against running tests on .NET Core.

       fluentmigrator/test/FluentMigrator.Tests/FluentMigrator.Tests.csproj

       Line 79 in 8427429

       <ItemGroup Condition=" '$(MSBuildRuntimeType)' == 'Core' or '$(OS)' != 'Windows_NT' ">

    2. As a result, JetTestTable.cs and other integration tests for Jet are disabled on .netstandard2.0 and .netstandard2.1 TFMs for xUnit test runner. I was wrong about this.
    3. Type.GetTypeFromProgID("ADOX.Catalog"); might be footgun code.

11. Wrapped ConnectionStringManagerTests.cs in a #if NET461 check

12. FluentMigratorConsoleTests.cs should be moved to its own Test project to avoid #if NET461

13. .NET Core 3.0 does not support AddJsonFile

    1. Add nuget package Microsoft.Extensions.Configuration.Json

14. Minor refactoring work

    1. Lifted out package tags from individual csproj files and into Global.props
    2. Lifted out assembly signing logic from individual csproj files and into new Package.AssemblySigning.props file. This will make it easier to remove assembly signing in the future (cf Remove assembly signing #1050 )
    3. Installed Paul Harrington's EditorGuidelines Visual Studio Extension and formatted <Description> field so it doesn't run on forever without a line break.
    4. Moved <Import ...> MSBuild directives to the top of each csproj file, so that it's clear what global variables exist from contexts outside the file itself.
       1. This could in theory cause an issue, so I checked that there are no variable name collisions that could cause replacing values.

TODO Items

• Investigate why Microsoft.Build.Utilities.Core for net461 has Major Version 14.
• Investigate why project references like FluentMigrator.Runner.SqlAnywhere needed to add netstandard2.1 to the TFM list in order for FluentMigrator.Tests to be happy.
• In files where JetBrains.Annotation.NotNullAttribute is still used in .netstandard2.1 target, should we update to explicitly use System.Diagnostics.CodeAnalysis.NotNullAttribute?
• Finish cleaning up <Description> field in .csproj
• Revisit Microsoft Jet assumptions as OleDebConnection should work on .NET Core?
• ConnectionStringManager.cs and NetConfigManager.cs is using/was using #if NETFRAMEWORK - should it use #if NET461 instead?
• Ask @fubar-coder why some tests use the SqlAnywhere .Password("pass") extension
• Update all the PackageReference(s) to use Nuget version ranges so that dependabot correctly auto-bumps dependencies.

[jzabroski]

Changed to say 3.0, as RollMajorVersion is not the default here.

[jzabroski]

We should support .netcoreapp3.1 and .netcoreapp2.1
The reason is that .netcoreapp3.0 and .netcoreapp2.2 is EOL (End Of Life), whereas .netcoreapp3.1 and .netcoreapp2.1 are LTS (Long Term Servicing).

[qhero]

We should support .netcoreapp3.1 and .netcoreapp2.1
The reason is that .netcoreapp3.0 and .netcoreapp2.2 is EOL (End Of Life), whereas .netcoreapp3.1 and .netcoreapp2.1 are LTS (Long Term Servicing).

Hi,
We definitively need that! We plan to upgrade our project to .NET Core 3.1 and we can't because of FM CLI not supporting that.
Also the title issue shouldn't be "Version 3.x should support .NET Core LTS version" ?

EDIT:
Do you have a planned date/timeline to resolve this ?

[jzabroski]

I plan to resolve it this weekend, barring something chaotic. I messed up one package in the 3.2.4 release (see: #1209) so 3.2.5 will fix that botched FluentMigrator.Console package.

[qhero]

Awesome! Thank you for the update.

[jzabroski]

@qhero I got pretty far with this over the weekend but there is still some finishing touches. I'd say realistically release 3.3.0 will go out next weekend. However, as you can see from the updates on the initial post, I did make huge progress and covered a ton of ground. I simply didn't anticipate the complexity/typing/fixing compile errors around SapAnywhere support.

[jzabroski]

Down to ~28 tests failing (21 in netstandard2.0, 21 in netstandard2.1, 28 in net461). All tests are BatchParserTests in ProcessorBatchParserTestsBase, with the following failure:

   Source: ProcessorBatchParserTestsBase.cs line 185
   Duration: 3 ms

  Message: 
    TearDown : Moq.MockException : Mock<DbConnection:22>:
    This mock failed verification due to the following unverified invocations:
    
       DbConnection.ConnectionString = "server=this"
       DbConnection.Open()
       DbConnection.State
       DbConnection.State
       DbConnection.Close()
  Stack Trace: 
    --TearDown
    Mock.VerifyNoOtherCalls(Mock mock, HashSet`1 verifiedMocks)
    Mock.VerifyNoOtherCalls(Mock mock, HashSet`1 verifiedMocks)
    Mock`1.VerifyNoOtherCalls()
    ProcessorBatchParserTestsBase.TearDown() line 83

I think this has something to do with SqlAnywhere batch parsing but I'm not sure - I would expect the net461 tests to still pass. @eloekset : Any ideas? Do you think if I pushed my progress to github.com/jzabroski/fluentmigrator you could take a look?

[jzabroski]

I pushed my progress so far.

[eloekset]

Ok, I'll take a look at it.

[jzabroski]

Orthogonal to review, I should update all the PackageReference(s) to use Nuget version ranges so that dependabot doesn't go crazy + it should make dependabot more useful to us in auto-bumping dependencies.

[eloekset]

I'll have to call it a day. This was a tricky one to understand. What I've tried to do, is running your master branch and origin/master and compare the results of ProcessorBatchParserTestBase.Issue442 for the SqlServer2016 processor.

They both get a mockedCommand that contains 6 Invocations, and it fails because they are marked as not verified, while they should be marked as verified. But when I step through the code, I can't see anything that goes differently in the two branches. (Be aware that the list of invocation increases as you hover over members while debugging.)

I'm starting to suspect that the bug is somewhere else. I haven't checked, but maybe there are two different versions of the mocking library? (BTW there's a build error in your master branch, but that's on the Console project, so it shouldn't have any influence on the code that gets executed in these processor unit tests.)

Maybe if I have another look at it some other day, I'll shift focus and find something interesting. It looks like I've just spent time looking into details of things that actually works today.

And I only ran the net461 tests. I haven't debugged the same test for the other target frameworks, but all fail so it shouldn't matter which one we focus on.

[eloekset]

If possible, we should compare the two branches using the same version of Moq.

[jzabroski]

Good catch, I forgot I upgraded that. I'm also suprised the depandabot build didn't fail for that PR.

We could be hitting this: devlooped/moq#858 (comment) (I found this by searching across all open/closed issues for VerifyNoOtherCalls)

It's not mentioned in the release notes but it seems to match our problem. Will dig deeper when I get a chance; feel free to beat me to the punch.

Here are the release notes that mention VerifyNoOtherCalls by name.

4.12.0

https://github.com/moq/moq4/blob/master/CHANGELOG.md#4120-2019-06-20

Fixed

Regression: VerifyNoOtherCalls causes stack overflow when mock setup returns the mocked object (@bash, #846)

4.10.0 https://github.com/moq/moq4/blob/master/CHANGELOG.md#4120-2019-06-20

Added

VerifyNoOtherCalls for MockRepository (@BlythMeister, #682)

Changed

Make VerifyNoOtherCalls take into account previous calls to parameterless Verify() and VerifyAll() (@stakx, #659)

[fubar-coder]

• Investigate why Microsoft.Build.Utilities.Core for net461 has Major Version 14
  I wanted to support the msbuild runner for net461 projects. AFAIK some users are (were?) expecting this. Just remove it if nobody uses it anymore.
• Investigate why project references like FluentMigrator.Runner.SqlAnywhere needed to add netstandard2.1 to the TFM list in order for FluentMigrator.Tests to be happy.
  Just some guesses: Different dependencies, type redirects, etc... IOW: DLL hell
• In files where JetBrains.Annotation.NotNullAttribute is still used in .netstandard2.1 target, should we update to explicitly use System.Diagnostics.CodeAnalysis.NotNullAttribute?
  My 5¢: Just remove all references to JetBrains.Annotations and use nullable annotations instead of the attributes directly. For TFW older than netstandard2.1, just use C# 8 and the project configuration <nullable>annotations</nullable> (nullable contexts).
• Revisit Microsoft Jet assumptions as OleDebConnection should work on .NET Core?
  It might work on .NET Core 3.1 as it improved compatibility with legacy stuff.
• ConnectionStringManager.cs and NetConfigManager.cs is using/was using #if NETFRAMEWORK - should it use #if NET461 instead?
  Only if NET461 is the only .NET Framework we're supporting. Was there some NET47 specific code in there somewhere? Maybe in the 4.0 branch?
• Ask @fubar-coder why some tests use the SqlAnywhere .Password("pass") extension
  I don't know why it might've been needed.

[jzabroski]

• Investigate why Microsoft.Build.Utilities.Core for net461 has Major Version 14

I wanted to support the msbuild runner for net461 projects. AFAIK some users are (were?) expecting this. Just remove it if nobody uses it anymore.

I think if we use any legacy .NET Framework MSBuild, COMReference will work correctly. In other words, it's the MSBuildRuntimeType that matters. Per Rainer Sigwald's directions, the right approach might be:

1. Check MSBuildVersion via new support in MSBuild Version 16.5 for instrinsic property functions to compare versions which addresses the gotcha with MSBuild casting versions to numbers in ways that create strange/unexpected/non-intuitive results: Need a performace and unified way to compare versions dotnet/msbuild#3212 (comment)
2. FluentMigrator.Console InitialTargets should be:

   <Project Sdk="Microsoft.Net.SDK" InitialTargets="RequireFullFramework">
     <Target Name="RequireFullFramework">
       <Error Condition=" '$(MSBuildRuntimeType)' != 'Full' "
              Text="Requires Legacy .NET Framework to use COMReference." />
     </Target>
   </Project>

• Investigate why project references like FluentMigrator.Runner.SqlAnywhere needed to add netstandard2.1 to the TFM list in order for FluentMigrator.Tests to be happy.

Just some guesses: Different dependencies, type redirects, etc... IOW: DLL hell

Yeah, makes sense. Was just hoping you knew a precise reason why. But I'm not surprised you just know its DLL Hell, as until recently, I lacked the vigor myself to pursue to root cause of each DLL Hell scenario individually. It's just that DLL Hell takes up so much of my time I wanted to be careful in enumerating every scenario.

I decided to push ahead with #1218 as part of this PR. It is a bit more work but will make the "test framework" vs the "Tests" more explicit, and allow us to more gracefully handle more migration runners in the future.

• In files where JetBrains.Annotation.NotNullAttribute is still used in .netstandard2.1 target, should we update to explicitly use System.Diagnostics.CodeAnalysis.NotNullAttribute?

My 5¢: Just remove all references to JetBrains.Annotations and use nullable annotations instead of the attributes directly. For TFW older than netstandard2.1, just use C# 8 and the project configuration <nullable>annotations</nullable> (nullable contexts).

Very good idea. Will do it and report back if I run into any problems.

• Revisit Microsoft Jet assumptions as OleDebConnection should work on .NET Core?

It might work on .NET Core 3.1 as it improved compatibility with legacy stuff.

Using Microsoft Access in .NET Core

[jzabroski]

I started work on splitting out SqlAnywhere tests into their own project as part of wrapping up this change. I'll probably push them as separate commits, effectively wrapping up the previous PR with rolling back updates to Moq for now.

[qhero]

Hi @jzabroski
Firstable I would like to thank you for your work on this issue, also I would like a quick catch up about the support of .NET Core 3.x. As far I can see this is still under development,but I would really appreciate if you can provide a rough date of patch. It will help us to know which strategy we will apply to our application (downgrade or upgrade).
Thank you!

[jzabroski]

I want to finish it by the end of the summer. I maintain two projects, this and RazorLight. This PR is rather enormous. To complicate things, I was falling behind accepting PRs to FluentMigrator, so I chose to accept those PRs and do small releases. So now I have to merge those PRs to my jzabroski/FluentMigrator branch, which is extra work. It's a fine balance between moving features forward and keeping project interest up and optimizing for what's easiest / least amount of work for me to do. In this case, I chose to do more work and accept more PRs and have to do more merging.

I've also taken some time to write up some design docs on where I want the project to go long-term, as I didn't write the current runner architecture. See #1288

Note, this is not my full time job and I don't get paid for it, so I do all this free work "on the side" as time permits with family and my other business objectives (I run my own software company).

[qhero]

I know this is not you fulltime job, that's why I can't thank you enough for what you do! Thank you for the catch up.

[jzabroski]

We will get there. I think once proper 3.1 support is implemented, adding features is gonna be a relative breeze. I have so many cool, innovative ideas in this space that nobody has even thought of yet. I haven't even written design docs for the really cool ideas because I don't want anyone stealing my original thoughts and then being like, "FluentMigrator doesn't do this" (as is common in open source software). It's going to really advance state of the art in "DbOps".

[jzabroski]

@fubar-coder

• In files where JetBrains.Annotation.NotNullAttribute is still used in .netstandard2.1 target, should we update to explicitly use System.Diagnostics.CodeAnalysis.NotNullAttribute?
  My 5¢: Just remove all references to JetBrains.Annotations and use nullable annotations instead of the attributes directly. For TFW older than netstandard2.1, just use C# 8 and the project configuration <nullable>annotations</nullable> (nullable contexts).

I tried this but I found it was tricky to "just use c# 8". I followed Stuart Lang's guide, in particular, and what I found is that when non-.NET Core 3.0 target frameworks use C# 8.0, if you include features like switch expressions in the code, you get lots of "Red ink" in the Visual Studio IDE that throw off the whole experience of using Visual Studio. I found Stuart Lang's guide to getting C# 8 to work in net461, but its a lot of hacks in my opinion.

Maybe I'll give it a second shot, but I looked back today at my notes on C# 8 in non-.NET Core 3 environments, and saw some problems.

[rballonline]

Is there anything we could do to help get this to completion? I wouldn't mind donating some time although admittedly have no idea where to start or how to help. I started looking into this and it started to become a rabbit hole for me.

[jzabroski]

Done in 5.0.0