Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions Directory.Build.targets
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,6 @@
</PropertyGroup>

<ItemGroup>
<!-- Workaround while there is no 5.0 SDK available, copy known apphost/framework reference info from 3.0. -->
<KnownAppHostPack Include="@(KnownAppHostPack->WithMetadataValue('TargetFramework', 'netcoreapp3.0'))"
TargetFramework="netcoreapp5.0" />
<KnownFrameworkReference Include="@(KnownFrameworkReference->WithMetadataValue('TargetFramework', 'netcoreapp3.0'))"
TargetFramework="netcoreapp5.0" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


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

@JunTaoLuo JunTaoLuo Sep 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@JunTaoLuo JunTaoLuo Sep 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Expand Down
4 changes: 2 additions & 2 deletions global.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"tools": {
"dotnet": "3.0.100-preview8-013656",
"dotnet": "5.0.100-alpha1-014696",
"runtimes": {
"dotnet": [
"2.0.9",
Expand All @@ -9,7 +9,7 @@
}
},
"sdk": {
"version": "3.0.100-preview8-013656"
"version": "5.0.100-alpha1-014696"
},
"msbuild-sdks": {
"Microsoft.DotNet.Arcade.Sdk": "1.0.0-beta.19461.7"
Expand Down
2 changes: 1 addition & 1 deletion src/dotnet-ef/dotnet-ef.nuspec
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<packageType name="$packageType$" />
</packageTypes>
<dependencies>
<group targetFramework=".NETCoreApp3.1" />
<group targetFramework=".NETCoreApp5.0" />
</dependencies>
</metadata>

Expand Down