Upgrade to a 6.0 SDK #22918

dougbu · 2020-10-07T23:23:27Z

use the latest SDK and contained runtimes when building this repo

global.json

bricelam · 2020-10-08T01:35:46Z

generated assemblies are unusable

Can you explain? ~~Is .NET 6 not backwards compatible with .NET Standard 2.1?~~

- without this, generated assemblies are unusable in aspnetcore after that brands to 6.0.0 - in other words, this is needed for dotnet/aspnetcore#24983 to test correctly

dougbu · 2020-10-10T22:02:08Z

Can you explain?

I don't think the EF assemblies need to be built against the latest runtime anymore. See my description of the problem impacting dotnet/aspnetcore#24983 in dotnet/aspnetcore#26776. The apparent problem is with Microsoft.AspNetCore.Diagnostics.EntityFrameworkCore itself, not the EF assemblies it's built against. For one thing, the template tests work fine in dotnet/aspnetcore#24983 when run on build agents and not Helix.

New commit in this PR reacts to a breaking change in dotnet/runtime that didn't impact dotnet/aspnetcore because that repo does not use NetworkException. See dotnet/runtime@aaecee064662 aka dotnet/runtime#41648 for more information.

But, that commit needs more work because there is no System.Net.Connections package…

dougbu · 2020-10-10T22:14:51Z

@ericstj @geoffkizer instead of trying to reference System.Net.Connections in reaction to dotnet/runtime#41648, should we use IOException and not NetworkException❔ Or does NetworkException still ship somewhere I haven't found❔

- `NetworkException` unavailable in shared framework or a shipping package

dougbu · 2020-10-10T22:24:52Z

test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosDbConfiguredConditionAttribute.cs

@@ -73,7 +73,7 @@ private static bool IsNotConfigured(Exception exception)
            {
                HttpRequestException re => re.InnerException is SocketException
 #if NET5_0
-                    || (re.InnerException is NetworkException networkException
+                    || (re.InnerException is IOException networkException
                        && networkException.InnerException is SocketException)


More specifically @geoffkizer @ericstj, is this extra HttpRequestException ➡️ IOException ➡️ SocketException check worth doing❔

@bricelam the good news here is this is a test-only change and doesn't imply a breaking change that will impact earlier EF packages when running against recent a shared framework from dotnet/runtime.

- eliminate need to change the .NET SDK version in two files - `$(DotNetCliVersion)` must be set explicitly - Helix SDK does not pick up .NET SDK version from our global.json - fortunately, `$(NETCoreSdkVersion)` is available in helix.proj

dougbu · 2020-10-11T00:13:55Z

New third commit both corrects the .NET SDK used on Helix agents and avoids need to update two files for .NET SDK changes in the future

dougbu · 2020-10-11T00:15:49Z

@bricelam please re-review since a bit has changed since you last looked.

@ajcvickers any objection to this set of changes in the main branch❔

dougbu · 2020-10-11T00:22:11Z

~~Is .NET 6 not backwards compatible with .NET Standard 2.1?~~

/fyi a future .NET SDK update will require reaction in this repo but likely only in the $(DefaultNetCoreTargetFramework) value and the few #if NET5_0 preprocessor directives.

dougbu · 2020-10-11T01:30:18Z

BTW this PR is green but I'm waiting for double-checks from @ajcvickers and / or @bricelam

bricelam · 2020-10-12T16:42:00Z

eng/helix.proj

@@ -8,7 +8,7 @@

    <IncludeDotNetCli>true</IncludeDotNetCli>
    <DotNetCliPackageType>sdk</DotNetCliPackageType>
-    <DotNetCliVersion>5.0.100-rc.1.20452.10</DotNetCliVersion>
+    <DotNetCliVersion>$(NETCoreSdkVersion)</DotNetCliVersion>


Oh funny, I thought we had to do a bunch of extra work to make this work. Awesome that it just works...

roji · 2020-10-16T09:49:26Z

@bricelam can we stay with 5.0 SDKs for now like we stayed with 3.1 during a lot of the 5.0 development?

bricelam · 2020-10-16T18:32:17Z

@dougbu seemed to assert that this was needed to get an end-to-end build of 6.0. I admit that I still don't understand why though since EF is just a .NET Standard 2.1 NuGet package...

dougbu · 2020-10-16T21:45:06Z

As I said in this comment above, I backed away from my initial assumption that this was needed to get dotnet/aspnetcore building 6.0 packages. The main reason to move forward on a regular basis is to keep up w/ shared framework changes affecting the tests.

I wouldn't suggest this repo needs to change anything every week and it may not need even to rev every month. But, keeping somewhat up-to-date w/ the SDK and runtimes will avoid a cliff at the end of the release.

smitpatel · 2020-10-16T22:28:12Z

We used to update monthly from public release in 5.0 timeframe. We should do similar for 6.0 unless blocked by something else. (That means current builds should be on 5.0-rc2 SDK which will upgrade in future when new SDK is released to public.

dougbu · 2020-10-16T22:34:49Z

Stepping back a bit: Why not settle on this SDK and let the world catch up in a few months❔ I'm not sure why we'd go back to any 5.0 SDK. It's not like this repo can build without a locally-installed SDK in any case (due to the need for the supplemental 3.1.8 runtime).

dougbu · 2020-10-16T22:35:29Z

That is, @roji what drove your question❔

roji · 2020-10-17T06:30:55Z

This is a general recurring discussion we also had during 5.0 dev. My main motivation here is is for EF to be as standard .NET project, without any special steps or build scripts - this lowers the bar for contributors (who don't understand why they can't just clone and build), and also for me personally (having to run build.sh very frequently because the SDK has changed).

When we last discussed this as a team during 5.0, we agreed that syncing monthly on previews is more than sufficient in the early phases, and switch to more frequent syncs as we get later in the release (I think it was preview8 or 9 for 5.0).

dougbu · 2020-10-17T06:47:05Z

I can understand the motivation. My main counters are

the 3.1.8 runtime reference means this is not a standard .NET repo and it cannot build without first having that runtime and the requested SDK in the local .dotnet/ folder
going back to a 5.0 SDK at this point would add additional churn
monthly (or whatever regular cadency the EF team decides on) updates will minimize reacting to SDK changes later in the release

So, overall, my recommendation is to stick w/ this SDK for a while (a month or two) then move to a later 6.0 SDK. But, I don't live in this repo and leave the choices up to you, @roji and the EF team.

roji · 2020-10-17T07:40:17Z

the 3.1.8 runtime reference means this is not a standard .NET repo and it cannot build without first having that runtime and the requested SDK in the local .dotnet/ folder

I don't know much about this (am missing context)... Would users installing 3.1.8 (or later) in the standard way not be enough? If not, why do we need to be special on this?

My vote at this point would be to wait for 5.0 GA and then switch to that until 6.0-preview1 is released. I'm hoping at that point the repo can just build after cloning for anyone with the 5.0 GA SDK?

roji · 2020-10-17T07:41:13Z

monthly (or whatever regular cadency the EF team decides on) updates will minimize reacting to SDK changes later in the release

As I wrote above, as we get to the later phases of 6.0 we would switch to a faster cadence - that's what we did in 5.0 and I think everyone was happy.

ajcvickers · 2020-10-17T16:11:56Z

Agree with the comments from others here. We want to always be using a publicly released and easily obtainable SDK. That means there needs to be at least a public preview. Doing otherwise puts a big burden on the community. If this doesn't naturally fall out of the build system then maybe it means we need to have a stronger community commitment across the org so that making it easy for the community to be included is something that is always a priority.

dougbu · 2020-10-17T20:36:16Z

/fyi the context here is the following from global.json:

"runtimes": {
  "dotnet": [
    "3.1.8"
  ],
  "aspnetcore": [
    "3.1.8"
  ]
}

The SDK itself doesn't contain any 3.1.x runtime bits and this repo apparently needs them, at least for tests. Having the above satisfies that requirement. But, it also prevents use of a globally-installed .NET installation, at least when building from the command line.

smitpatel · 2020-10-17T20:55:57Z

@dougbu - If a contributor has installed latest LTS (3.1 SDK) and latest public preview (5.0 RC2) SDK installed on machine, does it work out of box?

dougbu · 2020-10-17T22:18:56Z

does it work out of box?

Not for command-line builds. Arcade will still download both into .dotnet/. The SDK team is aware of the issue but there isn't a workaround allowing runtimes to be outside the SDK's root folder.

roji · 2020-10-18T07:52:09Z

and this repo apparently needs them

Does anyone know where this comes from? @bricelam or others?

roji · 2020-10-18T07:56:54Z

does it work out of box?

Not for command-line builds

FYI I just manually downloaded SDK 6.0.100-alpha.1.20472.11 (the one required by the latest global.json, via dotnet-install.sh), and everything seems to build (and test) fine from the commandline. I assume that's because I had also installed the correct 3.1.x SDK (see below), but that still makes us a compatible plain old dotnet project - or am I missing something?

_  EFCore.Tests git:(main) dotnet --list-sdks
3.1.107 [/home/roji/.dotnet/sdk]
3.1.402 [/home/roji/.dotnet/sdk]
3.1.403 [/home/roji/.dotnet/sdk]
5.0.100-rc.1.20452.10 [/home/roji/.dotnet/sdk]
6.0.100-alpha.1.20472.11 [/home/roji/.dotnet/sdk]
_  EFCore.Tests git:(main) dotnet --list-runtimes
Microsoft.AspNetCore.App 3.1.7 [/home/roji/.dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 3.1.8 [/home/roji/.dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 3.1.9 [/home/roji/.dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 5.0.0-rc.1.20451.17 [/home/roji/.dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 5.0.0-rc.2.20466.8 [/home/roji/.dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.NETCore.App 3.1.7 [/home/roji/.dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 3.1.8 [/home/roji/.dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 3.1.9 [/home/roji/.dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 5.0.0-rc.1.20451.14 [/home/roji/.dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.0-alpha.1.20468.7 [/home/roji/.dotnet/shared/Microsoft.NETCore.App]

dougbu · 2020-10-18T17:42:08Z

Your build used the .dotnet/ folder, not a globally-installed SDK and the 3.1.8 runtime was added to it. Or, you already had the listed runtimes in your local folder.

roji · 2020-10-18T19:25:22Z

I just double-checked, and it works fine without any .dotnet directory under my efcore repo. The dotnet being used comes from my home dir's .dotnet, which is where I install SDKs with dotnet-install.sh (I have both 5.0.0 and 3.1.9 there). I do understand that if people use the official packages (e.g. deb/rpm), this may not work as the 5.0.0 package doesn't come with the 3.1.x runtime.

Basically if feels like we should figure out why we have this special dependency on an older runtime though we're using a newer SDK, and fix it so that simply have the 5.0.0 GA (once it comes out) is enough to work out of the box.

dougbu · 2020-10-18T20:18:05Z

I just double-checked

Ah, I get it now. That worked because you were using dotnet and not build.sh or build.cmd. Those two scripts are what will unconditionally populate and use the .dotnet/ folder.

BTW the netcoreapp3.1 requirement comes from the dotnet-ef and benchmarks projects. It's also a standard test TFM.

Pilchie · 2020-10-19T16:46:50Z

building 3.1 doesn't require a 3.1 SDK or runtime, though running obviously does require a 3.1 runtime.

I'm supportive of trying to allow (mostly) leaf repos to remain on publicly released SDKs/runtimes whenever possible as it makes them so much more approachable.

ajcvickers · 2020-10-19T18:57:55Z

@dougbu We discussed this in triage and agreed to revert this PR. As far as we can tell it is not needed, and as discussed above we want to stay on a public preview SDK.

roji · 2020-10-19T21:10:08Z

PR: #23051

dougbu added the area-platform label Oct 7, 2020

dougbu requested review from bricelam and a team October 7, 2020 23:23

dougbu commented Oct 7, 2020

View reviewed changes

global.json Show resolved Hide resolved

bricelam approved these changes Oct 8, 2020

View reviewed changes

Upgrade to a 6.0 SDK

2d98217

- without this, generated assemblies are unusable in aspnetcore after that brands to 6.0.0 - in other words, this is needed for dotnet/aspnetcore#24983 to test correctly

dougbu force-pushed the dougbu/5.0.sdk branch from 089b75f to 28c5974 Compare October 10, 2020 21:47

Check for IOException instead of missing NetworkException

889e295

- `NetworkException` unavailable in shared framework or a shipping package

dougbu force-pushed the dougbu/5.0.sdk branch from 28c5974 to 889e295 Compare October 10, 2020 22:22

dougbu commented Oct 10, 2020

View reviewed changes

Update .NET SDK version used in Helix tests

8bbc8c4

- eliminate need to change the .NET SDK version in two files - `$(DotNetCliVersion)` must be set explicitly - Helix SDK does not pick up .NET SDK version from our global.json - fortunately, `$(NETCoreSdkVersion)` is available in helix.proj

bricelam approved these changes Oct 12, 2020

View reviewed changes

dougbu merged commit bdbeb48 into main Oct 12, 2020

dougbu deleted the dougbu/5.0.sdk branch October 12, 2020 16:56

dougbu mentioned this pull request Oct 12, 2020

[release/5.0] Update dependencies from dotnet/arcade #22977

Merged

ajcvickers mentioned this pull request Oct 22, 2020

Entity Framework Weekly Status Updates (2020) #19549

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to a 6.0 SDK #22918

Upgrade to a 6.0 SDK #22918

dougbu commented Oct 7, 2020 •

edited

Loading

bricelam commented Oct 8, 2020 •

edited

Loading

dougbu commented Oct 10, 2020

dougbu commented Oct 10, 2020

dougbu Oct 10, 2020

dougbu Oct 10, 2020

dougbu commented Oct 11, 2020

dougbu commented Oct 11, 2020

dougbu commented Oct 11, 2020

dougbu commented Oct 11, 2020

bricelam Oct 12, 2020

roji commented Oct 16, 2020

bricelam commented Oct 16, 2020

dougbu commented Oct 16, 2020

smitpatel commented Oct 16, 2020

dougbu commented Oct 16, 2020

dougbu commented Oct 16, 2020

roji commented Oct 17, 2020

dougbu commented Oct 17, 2020

roji commented Oct 17, 2020

roji commented Oct 17, 2020

ajcvickers commented Oct 17, 2020

dougbu commented Oct 17, 2020

smitpatel commented Oct 17, 2020

dougbu commented Oct 17, 2020

roji commented Oct 18, 2020

roji commented Oct 18, 2020

dougbu commented Oct 18, 2020

roji commented Oct 18, 2020

dougbu commented Oct 18, 2020

Pilchie commented Oct 19, 2020

ajcvickers commented Oct 19, 2020

roji commented Oct 19, 2020

Upgrade to a 6.0 SDK #22918

Upgrade to a 6.0 SDK #22918

Conversation

dougbu commented Oct 7, 2020 • edited Loading

bricelam commented Oct 8, 2020 • edited Loading

dougbu commented Oct 10, 2020

dougbu commented Oct 10, 2020

dougbu Oct 10, 2020

Choose a reason for hiding this comment

dougbu Oct 10, 2020

Choose a reason for hiding this comment

dougbu commented Oct 11, 2020

dougbu commented Oct 11, 2020

dougbu commented Oct 11, 2020

dougbu commented Oct 11, 2020

bricelam Oct 12, 2020

Choose a reason for hiding this comment

roji commented Oct 16, 2020

bricelam commented Oct 16, 2020

dougbu commented Oct 16, 2020

smitpatel commented Oct 16, 2020

dougbu commented Oct 16, 2020

dougbu commented Oct 16, 2020

roji commented Oct 17, 2020

dougbu commented Oct 17, 2020

roji commented Oct 17, 2020

roji commented Oct 17, 2020

ajcvickers commented Oct 17, 2020

dougbu commented Oct 17, 2020

smitpatel commented Oct 17, 2020

dougbu commented Oct 17, 2020

roji commented Oct 18, 2020

roji commented Oct 18, 2020

dougbu commented Oct 18, 2020

roji commented Oct 18, 2020

dougbu commented Oct 18, 2020

Pilchie commented Oct 19, 2020

ajcvickers commented Oct 19, 2020

roji commented Oct 19, 2020

dougbu commented Oct 7, 2020 •

edited

Loading

bricelam commented Oct 8, 2020 •

edited

Loading