Downgrade Best.Conventional to target netstandard20 (rather than netstandard21 which excludes .NET Framework consumption) #98

todthomson · 2024-11-01T06:55:13Z

This commit a72a988 bumped Conventional from netstandard2.0 to netstandard2.1.

Unfortunately, this makes it impossible for .NET Framework projects to use Best.Conventional versions 10.x and 11.x.

Older versions of Best.Conventional depend on older versions of System.Data.SqlClient which have CVEs.

My understanding is that there is no way currently to release hotfixes to older versions of Best.Conventional.

By targeting back to netstandard2.0 older .NET Framework projects can still use the latest version of Best.Conventional (with the latest and greatest features) with any current or future CVEs also able to be addressed easily and correctly (here).

I have tested this using:

The existing Best.Conventional tests (all tests still pass).
A preexisting .NET Framework project (all tests still pass).
A preexisting .NET Core project (all tests still pass).

Please advise if there is anything else I can do to assist in getting this PR ready to land. :)

I will provide a separate PR to fix the same issue in Best.Conventional.Roslyn once this has landed, as Best.Conventional.Roslyn depends on the Best.Conventional NuGet package.

- Conventional can still be used on .NET Framework

- Feature 'nullable reference types' is not available in C# 7.3. - Please use language version 8.0 or greater.

- System.Data.SqlClient version 4.8.6 removes that. - See: https://www.nuget.org/packages/System.Data.SqlClient

- but, there is no reason to reference this directly AFAIK? - app still compiles fine - all tests still pass - I'm not sure what this is actually doing here? :D

NETSDK1138 The target framework 'netcoreapp3.1' is out of support and will not receive security updates in the future. Please refer to https://aka.ms/dotnet-core-support for more information about the support policy.

todthomson · 2024-11-01T07:20:46Z

Build is failing because the AppVeyor images being used are out of date. We need to change the following:

Visual Studio 2019 to Visual Studio 2022, and
Ubuntu1804 to Ubuntu2204.

I could do this, but I don't have access to your AppVeyor.

If you can provide access, I will fix this up. :)

andrewabest · 2024-11-03T09:07:36Z

This has reminded me of this issue #84.

I'm wondering since the 5.2.x version of Microsoft.Data.SqlClient is now released, should we be using the latest version of that instead of upgrading the reference to System.Data.SqlClient? Keen to hear your thoughts.

Update

I've tested the above issue and it seems to no longer be a big problem. I think for the moment we will go for the minor bump and leave it at that. One thing we do need to do in this PR is to also bump the global.json file to net8.0.

A nice improvement for the future will be to add some conditionals for build-time to run some net462 tests as well to prove that is all happy at build time, but I don't think we need to do that now.

andrewabest · 2024-11-03T09:07:57Z

I've updated the AppVeyor images, thanks for the prompt! Build is green.

andrewabest · 2024-11-03T09:09:13Z

My understanding is that there is no way currently to release hotfixes to older versions of Best.Conventional.

Yes, this is deliberate. I only wanted to support mainline of the library, so don't have the build infrastructure set up to build and publish hotfixes.

andrewabest · 2024-11-03T22:54:03Z

Fixes #84

todthomson · 2024-11-04T00:52:18Z

This also fixes: #91 (dependabot issue with System.Data.SqlClient).

andrewabest · 2024-11-04T01:18:35Z

@todthomson I've updated a previous comment, but will re-state this ask here in case it was missed:

One thing we do need to do in this PR is to also bump the global.json file to net8.0.

andrewabest · 2024-11-04T01:19:16Z

Something like this should do it:

{
  "sdk": {
    "version": "8.0.100",
    "rollForward": "latestFeature"
  }
}

todthomson · 2024-11-04T01:20:27Z

Something like this should do it:

{
  "sdk": {
    "version": "8.0.100",
    "rollForward": "latestFeature"
  }
}

All good, I didn't miss it, I'm updating my PR as we speak. :)

todthomson · 2024-11-04T01:27:40Z

The Visual Studio 2022 agent (Windows Build VM) in AppVeyor has .NET 8 SDK 8.0.401, so I propose we that for global.json:
https://www.appveyor.com/docs/windows-images-software/#net-framework

The latest version of the .NET SDK v8 is 8.0.403 so that should be fine:
https://dotnet.microsoft.com/en-us/download/dotnet/8.0

I agree that using latestFeature rather than minor seems like the correct approach:
https://learn.microsoft.com/en-us/dotnet/core/tools/global-json#rollforward
So that dotnet (CLI) will version 8.0.4** will be used to build Best.Conventional (is my understanding).

todthomson · 2024-11-04T01:28:01Z

I'm wondering since the 5.2.x version of Microsoft.Data.SqlClient is now released, should we be using the latest version of that instead of upgrading the reference to System.Data.SqlClient? Keen to hear your thoughts.

Update
I've tested the above issue and it seems to no longer be a big problem. I think for the moment we will go for the minor bump and leave it at that.

Agreed.

This can be done as a separate PR (and a separately "major" versioned release) anyhow if/when it's needed.

Being that this will move Conventional from System.Data.SqlClient to Microsoft.Data.SqlClient, we don't want to break anyone taking a transient (is that even the right term?) dependency on System.Data.SqlClient via Conventional.

It also separates that from the move back to netstandard2.0, which is a safer/good thing IMO.

todthomson · 2024-11-04T01:37:37Z

Something like this should do it:
{
  "sdk": {
    "version": "8.0.100",
    "rollForward": "latestFeature"
  }
}
All good, I didn't miss it, I'm updating my PR as we speak. :)

PR updated to update global.json (as above).

todthomson · 2024-11-04T01:42:46Z

dotnet pack "C:\projects\conventional\src\Core\Conventional.Tests\Conventional.Tests.csproj" --configuration Release --output "C:\Users\appveyor\AppData\Local\Temp\1\4jhi4ddw5c" --no-build
if ($env:APPVEYOR_BUILD_WORKER_IMAGE -eq "Visual Studio 2019") {
cd src\Core
dotnet test
}
else {
dotnet test --filter TestCategory!=Database
}
MSBUILD : error MSB1003: Specify a project or solution file. The current working directory does not contain a project or solution file.

The tests don't look to be running on "Visual Studio 2022" worker.

From looking at the above, I think it will be because the cd src\Core is missing from how it's running the tests.

I'll see if I can fix. :)

feature: Uses the latest patch level for the specified: major, minor, and feature band. If not found, rolls forward to the next higher feature band within the same major/minor and uses the latest patch level for that feature band. If not found, fails. See: https://learn.microsoft.com/en-us/dotnet/core/tools/global-json#rollforward (cherry picked from commit a575629)

todthomson · 2024-11-04T01:53:51Z

dotnet restore
...
Installed SDKs:
8.0.303 [/usr/share/dotnet/sdk]
Install the [8.0.401] .NET SDK or update [/home/appveyor/projects/conventional/src/Core/global.json] to match an installed SDK.

Will push an update to (hopefully) fix this.

andrewabest · 2024-11-04T01:57:22Z

That test script isn't right at all. I'll update it shortly once the build is 🟢 with your current changes.

todthomson · 2024-11-04T02:10:19Z

That test script isn't right at all. I'll update it shortly once the build is 🟢 with your current changes.

I was hoping to do that, but I couldn't find the source for the "test script" in the repo, so I'm assuming it's elsewhere.

It looks like it would be as simple as fixing this line:

if ($env:APPVEYOR_BUILD_WORKER_IMAGE -eq "Visual Studio 2019")

to:

if ($env:APPVEYOR_BUILD_WORKER_IMAGE -eq "Visual Studio 2022")

Though that may not be the best / most correct fix.

andrewabest · 2024-11-04T02:42:42Z

That test script isn't right at all. I'll update it shortly once the build is 🟢 with your current changes.

I was hoping to do that, but I couldn't find the source for the "test script" in the repo, so I'm assuming it's elsewhere.

It looks like it would be as simple as fixing this line:

if ($env:APPVEYOR_BUILD_WORKER_IMAGE -eq "Visual Studio 2019")

to:

if ($env:APPVEYOR_BUILD_WORKER_IMAGE -eq "Visual Studio 2022")

Though that may not be the best / most correct fix.

Yup, no scripts in the repo because I don't want rascals mining bitcoin via my CI builds 😆 that will change once I move to GHA.

andrewabest

Thanks for helping me work through this one @todthomson! Much appreciated.

todthomson · 2024-11-04T02:48:44Z

Easy, you're welcome. Ultimately, you're also helping me to squash some CVEs in 100+ test fixtures, so thanks to you too mate. Glad to see you got the tests running again too. 😁

tthomson-eml added 7 commits October 31, 2024 14:51

target netstandard2.0 so...

4ef3387

- Conventional can still be used on .NET Framework

hiding inherited members should be explicit

ad4ae4b

fix CS8370...

8af3948

- Feature 'nullable reference types' is not available in C# 7.3. - Please use language version 8.0 or greater.

System.Data.SqlClient 4.8.5 has CVE...

a50c5c1

- System.Data.SqlClient version 4.8.6 removes that. - See: https://www.nuget.org/packages/System.Data.SqlClient

System.Data.Common contains CVEs...

ebfad61

- but, there is no reason to reference this directly AFAIK? - app still compiles fine - all tests still pass - I'm not sure what this is actually doing here? :D

target tests to net8.0 instead of netcoreapp3.1...

dc9c3c9

NETSDK1138 The target framework 'netcoreapp3.1' is out of support and will not receive security updates in the future. Please refer to https://aka.ms/dotnet-core-support for more information about the support policy.

target Conventional.Tests at both net6.0 + net8.0

3d2f593

todthomson changed the title ~~Downgrade conventional to target netstandard20~~ Downgrade Best.Conventional to target netstandard20 (rather than netstandard21 which excludes .NET Framework consumption) Nov 1, 2024

todthomson mentioned this pull request Nov 1, 2024

Bump System.Data.SqlClient from 4.8.5 to 4.8.6 in /src/Core/Conventional #91

Closed

update dotnet CLI to v8.0.4** of the .NET SDK

9eeaf22

andrewabest approved these changes Nov 4, 2024

View reviewed changes

andrewabest merged commit 75a6b3b into andrewabest:master Nov 4, 2024
1 check passed

todthomson deleted the downgrade-conventional-to-target-netstandard20 branch November 4, 2024 21:59

Downgrade Best.Conventional to target netstandard20 (rather than netstandard21 which excludes .NET Framework consumption) #98

Downgrade Best.Conventional to target netstandard20 (rather than netstandard21 which excludes .NET Framework consumption) #98

Uh oh!

Conversation

todthomson commented Nov 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

todthomson commented Nov 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrewabest commented Nov 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrewabest commented Nov 3, 2024

Uh oh!

andrewabest commented Nov 3, 2024

Uh oh!

andrewabest commented Nov 3, 2024

Uh oh!

todthomson commented Nov 4, 2024

Uh oh!

andrewabest commented Nov 4, 2024

Uh oh!

andrewabest commented Nov 4, 2024

Uh oh!

todthomson commented Nov 4, 2024

Uh oh!

todthomson commented Nov 4, 2024

Uh oh!

todthomson commented Nov 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

todthomson commented Nov 4, 2024

Uh oh!

todthomson commented Nov 4, 2024

Uh oh!

todthomson commented Nov 4, 2024

Uh oh!

andrewabest commented Nov 4, 2024

Uh oh!

todthomson commented Nov 4, 2024

Uh oh!

andrewabest commented Nov 4, 2024

Uh oh!

andrewabest left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

todthomson commented Nov 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

todthomson commented Nov 1, 2024 •

edited

Loading

todthomson commented Nov 1, 2024 •

edited

Loading

andrewabest commented Nov 3, 2024 •

edited

Loading

todthomson commented Nov 4, 2024 •

edited

Loading

todthomson commented Nov 4, 2024 •

edited

Loading