Update to 5.0 SDK #17945

wtgodbe · 2019-09-19T20:38:44Z

Part of https://github.com/aspnet/AspNetCore-Internal/issues/3016

src/dotnet-ef/dotnet-ef.nuspec

wtgodbe · 2019-09-19T20:48:42Z

F:\workspace_work\1\s\src\dotnet-ef\dotnet-ef.csproj : error NU1102: Unable to find package Microsoft.AspNetCore.App.Runtime.win-x64 with version (= 5.0.0-alpha1.19466.7) [F:\workspace_work\1\s\All.sln]

That version doesn't show up in Versions.props, thinking it comes from the SDK. Looking.

JunTaoLuo · 2019-09-19T20:57:19Z

Directory.Build.targets

-        TargetFramework="netcoreapp5.0" />

    <!-- Reference base shared framework at incoming dependency flow version, not bundled sdk version. -->
    <FrameworkReference Update="Microsoft.NETCore.App"


Hmm I wonder how this ever worked. @wtgodbe the version you're linking to is the runtime version for Microsoft.NETCore.App. The error message is complaining that it cannot find the appropriate version of Microsoft.AspNetCore.App. I wonder if we need to apply this workaround to M.AspNetCore.App as well. Also, I'm curious how this ever worked without this workaround.

@JunTaoLuo why wouldn't this work given the $(NETCoreAppMaximumVersion) setting? EF doesn't reference Microsoft.AspNetCore.App anywhere.

Suggest instead updating @(KnownFrameworkReference) items instead of @(FrameworkReference) but that's reasonably minor.

I think when we had a 3.0 SDK, the KnownFrameworkReference for Microsoft.AspNetCore.App was a 3.0 version, which we were able to restore from nuget (3.0.0-rc1-19456-20). Now with the 5.0 SDK, that default reference is versioned 5.0.0-alpha1.19466.7 per the binlog, which we can't restore since it's not in nuget or the dotnet-core feed. What feed to we publish AspNetCore stuff to? We could just add that to nuget.config.

Hmm, I think it's implicit since dotnet-ef targets 5.0 after my change (my bad) https://github.com/aspnet/EntityFrameworkCore/blob/master/src/dotnet-ef/dotnet-ef.csproj#L15. I think the recommendation is for dotnet CLI tools to target a lower fixed TFM and allow roll forward: dotnet/diagnostics#227.

So theoretically, we should make a change in 3.1 to make this tool target netcoreapp3.0 and then add the roll forward option in the json file. That should also fix the error @wtgodbe saw.

Again, exactly what in this repo references Microsoft.AspNetCore.App? That sounds like a bug.

I think it's the KnownFrameworkReference from the 5.0 SDK, which does try to pull in the package which the build complains it can't find (5.0.0-alpha1.19466.7). The old 3.0 SDK referenced a version that's present on nuget.org.

I think we're getting our wires crossed, we can discuss this in person at the next standup.

dotnet-ef project might have issues but that's a red herring for this problem. Instead, let's remove all Microsoft.AspNetCore.App items from the @(KnownFrameworkReference) group.

dougbu

Probably need similar changes in other SDK update PRs.

dougbu · 2019-09-19T21:27:26Z

Directory.Build.targets

-    <KnownAppHostPack Include="@(KnownAppHostPack->WithMetadataValue('TargetFramework', 'netcoreapp3.0'))"
-        TargetFramework="netcoreapp5.0" />
-    <KnownFrameworkReference Include="@(KnownFrameworkReference->WithMetadataValue('TargetFramework', 'netcoreapp3.0'))"
-        TargetFramework="netcoreapp5.0" />


Don't remove this logic; it'll help to have around the next time we deal w/ a new TFM before an SDK is ready. Instead, make this conditional on lack of '%(TargetFramework) == $(DefaultNetCoreTargetFramework)` items in each group.

dougbu · 2019-09-19T21:31:47Z

Directory.Build.targets

-        TargetFramework="netcoreapp5.0" />

    <!-- Reference base shared framework at incoming dependency flow version, not bundled sdk version. -->
    <FrameworkReference Update="Microsoft.NETCore.App"


@JunTaoLuo why wouldn't this work given the $(NETCoreAppMaximumVersion) setting? EF doesn't reference Microsoft.AspNetCore.App anywhere.

Suggest instead updating @(KnownFrameworkReference) items instead of @(FrameworkReference) but that's reasonably minor.

wtgodbe · 2019-09-20T20:11:30Z

Just pushed cac7707 to address feedback, @dougbu PTAL

wtgodbe · 2019-09-20T20:32:44Z

Failing with the package icon issue, I'll fix first in a separate PR:

##[error].dotnet\sdk\5.0.100-alpha1-014696\Sdks\NuGet.Build.Tasks.Pack\build\NuGet.Build.Tasks.Pack.targets(198,5): error NU5048: The 'PackageIconUrl'/'iconUrl' element is deprecated. Consider using the 'PackageIcon'/'icon' element instead. Learn more at https://aka.ms/deprecateIconUrl

dougbu · 2019-09-20T22:00:06Z

Suggestion: Do the package icon change in parallel with getting this in. For this PR, just disable the NuGet warning / error.

wtgodbe · 2019-09-20T22:11:54Z

Checking to see if the package icon fix works with these changes

- For additional context: dotnet/efcore#17945 (comment).

wtgodbe · 2019-09-23T19:49:24Z

This looks good locally now, assuming we don't need info about the packageIcon in the .nuspec. Waiting to hear back from @tmat on that one.

wtgodbe · 2019-09-23T22:24:25Z

As per dotnet/ef6#1286 (comment), looks like we don't need to worry about the .nuspec. @dougbu any other concerns, or is this good to go?

wtgodbe · 2019-09-23T22:24:40Z

(Will also contribute to https://github.com/aspnet/AspNetCore-Internal/issues/3103)

wtgodbe · 2019-09-24T16:44:50Z

With the last change, this now includes the icon metadata in all .nuspec's, including the manually generated ones - @dougbu PTAL

dougbu

Please split this in two because the package icon portion needs to be done in 'release/3.0' and that's somewhat urgent. Include the necessary <NoWarn/> w/ the 5.0 SDK bit until that change flows into 'master'.

Separately, the package icon changes should also remove <NoWarn>NU5048;$(NoWarn)</NoWarn> elements where they exist. That might need to be done separately since I doubt we have that in 'release/3.0' branches.

dougbu · 2019-09-24T17:07:10Z

Directory.Build.props

+    <None Include="$(PackageIconFullPath)" Pack="true" PackagePath="\"/>
  </ItemGroup>
-</Project>
+</Project>


nit: Avoid changing the final newlines. This results in files thrashing back and forth.

wtgodbe · 2019-09-24T20:11:34Z

@dougbu this now just handles the SDK update - #18027 takes care of the package icon change in 3.0.

Update to 5.0 SDK

19ab68b

wtgodbe requested review from a team and ajcvickers September 19, 2019 20:38

wtgodbe commented Sep 19, 2019

View reviewed changes

src/dotnet-ef/dotnet-ef.nuspec Show resolved Hide resolved

AndriySvyryd requested a review from bricelam September 19, 2019 20:41

bricelam approved these changes Sep 19, 2019

View reviewed changes

JunTaoLuo approved these changes Sep 19, 2019

View reviewed changes

JunTaoLuo reviewed Sep 19, 2019

View reviewed changes

dougbu suggested changes Sep 19, 2019

View reviewed changes

dougbu added the area-infrastructure label Sep 19, 2019

Add vNext SDK Workaround, remove ref to AspNetCore.App

cac7707

wtgodbe mentioned this pull request Sep 20, 2019

Update to newer SDK dotnet/aspnetcore#14168

Closed

NTaylorMullen pushed a commit to dotnet-maestro-bot/AspNetCore-Tooling that referenced this pull request Sep 20, 2019

Fix restore errors for .NET Core 5.0

42c7974

- For additional context: dotnet/efcore#17945 (comment).

JunTaoLuo approved these changes Sep 23, 2019

View reviewed changes

dougbu suggested changes Sep 24, 2019

View reviewed changes

NoWarn NU5048

8a600a8

wtgodbe force-pushed the 50SDK branch from af4dacd to 8a600a8 Compare September 24, 2019 20:08

dougbu approved these changes Sep 25, 2019

View reviewed changes

wtgodbe merged commit 612f9b1 into master Sep 25, 2019

ghost deleted the 50SDK branch September 25, 2019 15:58

Update to 5.0 SDK #17945

Update to 5.0 SDK #17945

Uh oh!

Conversation

wtgodbe commented Sep 19, 2019

Uh oh!

Uh oh!

wtgodbe commented Sep 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JunTaoLuo Sep 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JunTaoLuo Sep 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dougbu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wtgodbe commented Sep 20, 2019

Uh oh!

wtgodbe commented Sep 20, 2019

Uh oh!

dougbu commented Sep 20, 2019

Uh oh!

wtgodbe commented Sep 20, 2019

Uh oh!

wtgodbe commented Sep 23, 2019

Uh oh!

wtgodbe commented Sep 23, 2019

Uh oh!

wtgodbe commented Sep 23, 2019

Uh oh!

wtgodbe commented Sep 24, 2019

Uh oh!

dougbu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wtgodbe commented Sep 24, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

wtgodbe commented Sep 19, 2019 •

edited

Loading

JunTaoLuo Sep 19, 2019 •

edited

Loading

JunTaoLuo Sep 19, 2019 •

edited

Loading