Creating separate issue based on things discussed with @natemcmaster in https://github.com/dotnet/source-build/issues/375.
Issue
Since rc1, ASP.NET Core templates no longer include a version for the _Microsoft.AspNetCore.App_/_Microsoft.AspNetCore.All_ packages.
<PackageReference Include="Microsoft.AspNetCore.App" />
For a self-contained application, the SDK will default to using the latest-patch (e.g. 2.1.1) version.
For a framework-dependent application, the SDK will default to using the .0-patch (e.g. 2.1.0) version. The actual version used is the shared-framework of the machine.
For source-build, there is no shared-framework. So the current behavior is to get the .0-patch version from nuget.org and publish it with the application. This is a security issue, since that version does not include the latest fixes.
Possible solution
The web sdk has some logic where it decides to use when to use the latest patch version:
<!-- Determine the default values of TargetLatestAspNetCoreRuntimePatch -->
<PropertyGroup Condition="'$(TargetLatestAspNetCoreRuntimePatch)' == ''">
<TargetLatestAspNetCoreRuntimePatch>false</TargetLatestAspNetCoreRuntimePatch>
<TargetLatestAspNetCoreRuntimePatch Condition="'$(SelfContained)' == 'true'">true</TargetLatestAspNetCoreRuntimePatch>
<TargetLatestAspNetCoreRuntimePatch Condition="'$(TargetLatestRuntimePatch)' != ''">$(TargetLatestRuntimePatch)</TargetLatestAspNetCoreRuntimePatch>
</PropertyGroup>
We could set this to true for source-build.
Secondary issue
When not using the latest sdk, the sdk is not aware of what the latest patch version is.
For example: sdk-2.1.301 will have some logic:
<!-- Latest patch version of .App Framework -->
<PropertyGroup Condition="'$(LatestAspNetCoreAppPatchVersion)' == ''">
<LatestAspNetCoreAppPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '2.1'">$(LatestPatchVersionForAspNetCoreApp2_1)</LatestAspNetCoreAppPatchVersion>
<LatestAspNetCoreAppPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '2.2'">$(LatestPatchVersionForAspNetCoreApp2_2)</LatestAspNetCoreAppPatchVersion>
<!-- If targeting the same release that is bundled with the .NET Core SDK, use the bundled package version
provided by Microsoft.NETCoreSdk.BundledVersions.props -->
<LatestAspNetCoreAppPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '$(BundledAspNetCoreAppTargetFrameworkVersion)'">$(BundledAspNetCoreAppPackageVersion)</LatestAspNetCoreAppPatchVersion>
<!-- If not covered by the previous cases use the target framework version for the latest patch version -->
<LatestAspNetCoreAppPatchVersion Condition="'$(LatestAspNetCoreAppPatchVersion)' == ''">$(_TargetFrameworkVersionWithoutV)</LatestAspNetCoreAppPatchVersion>
</PropertyGroup>
<!-- Latest patch version of .All Framework -->
...
When 2.1.2 becomes available, 2.1.1 will still be used by SDK 2.1.301. For source-build, this is an issue for framework-dependent and self-contained applications. For Microsoft build, this is only an issue for self-contained.
We may to some extent work around this by providing an up-to-date value of LatestPatchVersionForAspNetCoreApp2_1/LatestPatchVersionForAspNetCoreAll2_1.
These can be set using environment variables. (e.g. set in the dotnet s2i image, the rhel dotnet scl package or an /etc/profile.d file). This avoids having to rebuild all the sdks with updated patch versions. The package that provides the environment variables needs to be updated for each ASP.NET Core patch release.
@natemcmaster wdyt? if you have some alternatives, please share them.
CC @omajid
Setting TargetLatestAspNetCoreRuntimePatch=true seems like the best approach.
For a framework-dependent application, the SDK will default to using the .0-patch (e.g. 2.1.0) version.
Just FYI - this was our intention until we discovered https://github.com/aspnet/Universe/issues/1180. We are resetting the baseline for framework-dependent apps to 2.1.1 in the 2.1.301 SDK update.
To summarize (and check my understanding): it sounds like resetting the baseline to 2.1.1 means the flag has no immediate effect on the 2.1.1 (2.1.301 SDK) source-build release. However, adding TargetLatestAspNetCoreRuntimePatch=true will allow package maintainers to update the framework-dependent ASP.NET dependencies in the future using LatestPatchVersionForAspNetCoreApp2_1/LatestPatchVersionForAspNetCoreAll2_1 without rebuilding source-build, so it should be included. Let me know if that's not quite right.
@dagood that is correct!
it sounds like resetting the baseline to 2.1.1 means the flag has no immediate effect on the 2.1.1 (2.1.301 SDK) source-build release.
For subsequent SDK releases, it also has the effect that _using the latest SDK also means using the latest ASP.NET Core patch_ version.
So this means a distro can either:
dotnet-sdk-2.1 and users are expected to keep that package up-to-date to ensure they are targeting the latest ASP.NET Core release.LatestPatchVersionForAspNetCoreApp2_1/LatestPatchVersionForAspNetCoreAll2_1 to ensure older sdks are also using the latest ASP.NET Core patch releases.@natemcmaster it seems LatestPatchVersionForAspNetCoreApp2_1/LatestPatchVersionForAspNetCoreAll2_1 are not set in the 2.1.1 early access builds. Is this an oversight?
@tmds we didn't implement LatestPatchVersionForAspNetCoreApp2_1 in the 2.1.x SDK. This is something I introduced in the 2.2 SDK (https://github.com/aspnet/websdk/pull/348).
For 2.1, I still think setting TargetLatestAspNetCoreRuntimePatch=true is the best approach. I'm not sure about the second suggestion you've made...the suggestions about toggling the default package versions independently of SDK updates. We'd probably need to make more changes to the SDK to make this work well. cc @JunTaoLuo
@tmds we didn't implement LatestPatchVersionForAspNetCoreApp2_1 in the 2.1.x SDK. This is something I introduced in the 2.2 SDK (aspnet/websdk#348).
I see, I had grepped through the sdk, but I didn't notice this was actually in xml comments.
I'm not sure about the second suggestion you've made...the suggestions about toggling the default package versions independently of SDK updates.
The goal is to make an older sdk aware of what the latest ASP.NET Core patch version is, so that version is used when performing the publish operation.
I see for the 2.1 sdk, this version is read from LatestAspNetCoreAllPatchVersion/LatestAspNetCoreAppPatchVersion. The LatestPatchVersionForAspNetCoreApp2_1/LatestPatchVersionForAspNetCoreAll2_1 are nicer since they are version aware, but indeed they are not available.
I started looking at adding TargetLatestAspNetCoreRuntimePatch to the build and finally realized it's not a build property: it's used when the SDK runs. As far as I know, there's no precedent for the "plain" source-build to set environment variables like this, and it's up to package maintainers like it would be for LatestPatchVersionForAspNetCoreApp2_1/LatestPatchVersionForAspNetCoreAll2_1.
I could see asking CLI to add a build flag for source-build that adds/changes a default value, so no env variable would need to be set. Is that the kind of fix you're expecting?
For TargetLatestAspNetCoreRuntimePatch this would be a change in the source-build .NET Core (without requiring environment variables set).
The net effect is websdk Sdk.DefaultItems.targets behaving like:
<PropertyGroup Condition="'$(TargetLatestAspNetCoreRuntimePatch)' == ''">
<TargetLatestAspNetCoreRuntimePatch>true</TargetLatestAspNetCoreRuntimePatch>
</PropertyGroup>
LatestPatchVersionForAspNetCoreApp2_1/LatestPatchVersionForAspNetCoreAll2_1 (or 2.1 equivalent) are using environment variables because the environment (controlled by package maintainers) can update these values to reflect more recent patch versions of ASP.NET Core being available.
I'm on gitter if you want to discuss this further.
I see for the 2.1 sdk, this version is read from LatestAspNetCoreAllPatchVersion/LatestAspNetCoreAppPatchVersion. The LatestPatchVersionForAspNetCoreApp2_1/LatestPatchVersionForAspNetCoreAll2_1 are nicer since they are version aware, but indeed they are not available.
I don't like that these variables are not versioned: setting LatestAspNetCoreAllPatchVersion/LatestAspNetCoreAppPatchVersion may cause issues in the future...
So it would be nice to have the versioned variables available:
@@ -57,7 +57,8 @@ Copyright (c) .NET Foundation. All rights reserved.
<!-- Latest patch version of .All Framework -->
<PropertyGroup Condition="'$(LatestAspNetCoreAllPatchVersion)' == ''">
<!-- Placeholder for setting latest patch version using the bundled version from CLI -->
- <!-- <LatestAspNetCoreAllPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '2.1'">$(LatestPatchVersionForAspNetCoreAll2_1)</LatestAspNetCoreAllPatchVersion> -->
+ <LatestAspNetCoreAllPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '2.1'">$(LatestPatchVersionForAspNetCoreAll2_1)</LatestAspNetCoreAllPatchVersion>
+ <LatestAspNetCoreAllPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '2.2'">$(LatestPatchVersionForAspNetCoreAll2_2)</LatestAspNetCoreAllPatchVersion>
<!-- If targeting the same pre-release that is bundled with the .NET Core SDK, use the bundled package version
provided by Microsoft.NETCoreSdk.BundledVersions.props -->
@@ -69,7 +70,8 @@ Copyright (c) .NET Foundation. All rights reserved.
<!-- Latest patch version of .App Framework -->
<PropertyGroup Condition="'$(LatestAspNetCoreAppPatchVersion)' == ''">
<!-- Placeholder for setting latest patch version using the bundled version from CLI -->
- <!-- <LatestAspNetCoreAppPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '2.1'">$(LatestPatchVersionForAspNetCoreApp2_1)</LatestAspNetCoreAppPatchVersion> -->
+ <LatestAspNetCoreAppPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '2.1'">$(LatestPatchVersionForAspNetCoreApp2_1)</LatestAspNetCoreAppPatchVersion>
+ <LatestAspNetCoreAppPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '2.2'">$(LatestPatchVersionForAspNetCoreApp2_2)</LatestAspNetCoreAppPatchVersion>
<!-- If targeting the same pre-release that is bundled with the .NET Core SDK, use the bundled package version
provided by Microsoft.NETCoreSdk.BundledVersions.props -->
@@ -80,9 +82,7 @@ Copyright (c) .NET Foundation. All rights reserved.
<!-- Determine the default values of TargetLatestAspNetCoreRuntimePatch -->
<PropertyGroup Condition="'$(TargetLatestAspNetCoreRuntimePatch)' == ''">
- <TargetLatestAspNetCoreRuntimePatch>false</TargetLatestAspNetCoreRuntimePatch>
- <TargetLatestAspNetCoreRuntimePatch Condition="'$(SelfContained)' == 'true'">true</TargetLatestAspNetCoreRuntimePatch>
- <TargetLatestAspNetCoreRuntimePatch Condition="'$(TargetLatestRuntimePatch)' != ''">$(TargetLatestRuntimePatch)</TargetLatestAspNetCoreRuntimePatch>
+ <TargetLatestAspNetCoreRuntimePatch>true</TargetLatestAspNetCoreRuntimePatch>
</PropertyGroup>
<!-- Set the framework version of .All Framework -->
@JunTaoLuo @natemcmaster wdyt?
Looks okay to me. Looks like this is basically backporting some changes from https://github.com/aspnet/websdk/pull/348, in addition to changing the default for TargetLatestAspNetCoreRuntimePatch.
Here's what I'm thinking of adding to CLI to allow us to send in a source-build-specific default:
--- a/build/MSBuildExtensions.targets
+++ b/build/MSBuildExtensions.targets
@@ -174,6 +174,9 @@ Copyright (c) .NET Foundation. All rights reserved.
<LatestPatchVersionForNetCore1_0 Condition="'$(LatestPatchVersionForNetCore1_0)' == ''">1.0.11</LatestPatchVersionForNetCore1_0>
<LatestPatchVersionForNetCore1_1 Condition="'$(LatestPatchVersionForNetCore1_1)' == ''">1.1.8</LatestPatchVersionForNetCore1_1>
<LatestPatchVersionForNetCore2_0 Condition="'$(LatestPatchVersionForNetCore2_0)' == ''">2.0.7</LatestPatchVersionForNetCore2_0>
+
+ <!-- If true, always target the latest ASP.NET Core runtime by default -->
+ <TargetLatestAspNetCoreRuntimePatch Condition="'$(TargetLatestAspNetCoreRuntimePatch)' == ''">$(DefaultTargetLatestAspNetCoreRuntimePatch)</TargetLatestAspNetCoreRuntimePatch>
</PropertyGroup>
</Project>
]]>
@natemcmaster, can you help port the versioned properties to the release/2.1 branch in the websdk repo? If that can go in before any disruptive 2.1.vNext changes, we can move the submodule commit forward for source-build 2.1.1. Otherwise, I'll stick it in a patch for now.
the repo
to aspnet/websdk or dotnet/cli? Either way, those changes can't go in for 2.1.1. We can probably get them into a future patch, but we've locked down 2.1.1 to these hashes: https://github.com/dotnet/versions/tree/master/build-info/dotnet/product/cli/release/2.1.1#built-repositories.
Sorry, edited to say [aspnet/]websdk but not in time. A patch it is.
@natemcmaster I'm trying this by patching the 2.1.1 sdk.
It doesn't work as expected:
<LatestAspNetCoreAppPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '2.1'">$(LatestPatchVersionForAspNetCoreApp2_1)</LatestAspNetCoreAppPatchVersion>
is overridden by
<LatestAspNetCoreAppPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '$(BundledAspNetCoreAppTargetFrameworkVersion)'">$(BundledAspNetCoreAppPackageVersion)</LatestAspNetCoreAppPatchVersion>
I'm removing the latter to make it work. Is this good for you? Update diff:
@@ -57,11 +57,9 @@ Copyright (c) .NET Foundation. All rights reserved.
<!-- Latest patch version of .All Framework -->
<PropertyGroup Condition="'$(LatestAspNetCoreAllPatchVersion)' == ''">
<!-- Placeholder for setting latest patch version using the bundled version from CLI -->
- <!-- <LatestAspNetCoreAllPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '2.1'">$(LatestPatchVersionForAspNetCoreAll2_1)</LatestAspNetCoreAllPatchVersion> -->
+ <LatestAspNetCoreAllPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '2.1'">$(LatestPatchVersionForAspNetCoreAll2_1)</LatestAspNetCoreAllPatchVersion>
+ <LatestAspNetCoreAllPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '2.2'">$(LatestPatchVersionForAspNetCoreAll2_2)</LatestAspNetCoreAllPatchVersion>
- <!-- If targeting the same pre-release that is bundled with the .NET Core SDK, use the bundled package version
- provided by Microsoft.NETCoreSdk.BundledVersions.props -->
- <LatestAspNetCoreAllPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '$(BundledAspNetCoreAllTargetFrameworkVersion)'">$(BundledAspNetCoreAllPackageVersion)</LatestAspNetCoreAllPatchVersion>
<!-- If not covered by the previous cases use the target framework version for the latest patch version -->
<LatestAspNetCoreAllPatchVersion Condition="'$(LatestAspNetCoreAllPatchVersion)' == ''">$(_TargetFrameworkVersionWithoutV)</LatestAspNetCoreAllPatchVersion>
</PropertyGroup>
@@ -69,20 +67,16 @@ Copyright (c) .NET Foundation. All rights reserved.
<!-- Latest patch version of .App Framework -->
<PropertyGroup Condition="'$(LatestAspNetCoreAppPatchVersion)' == ''">
<!-- Placeholder for setting latest patch version using the bundled version from CLI -->
- <!-- <LatestAspNetCoreAppPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '2.1'">$(LatestPatchVersionForAspNetCoreApp2_1)</LatestAspNetCoreAppPatchVersion> -->
+ <LatestAspNetCoreAppPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '2.1'">$(LatestPatchVersionForAspNetCoreApp2_1)</LatestAspNetCoreAppPatchVersion>
+ <LatestAspNetCoreAppPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '2.2'">$(LatestPatchVersionForAspNetCoreApp2_2)</LatestAspNetCoreAppPatchVersion>
- <!-- If targeting the same pre-release that is bundled with the .NET Core SDK, use the bundled package version
- provided by Microsoft.NETCoreSdk.BundledVersions.props -->
- <LatestAspNetCoreAppPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '$(BundledAspNetCoreAppTargetFrameworkVersion)'">$(BundledAspNetCoreAppPackageVersion)</LatestAspNetCoreAppPatchVersion>
<!-- If not covered by the previous cases use the target framework version for the latest patch version -->
<LatestAspNetCoreAppPatchVersion Condition="'$(LatestAspNetCoreAppPatchVersion)' == ''">$(_TargetFrameworkVersionWithoutV)</LatestAspNetCoreAppPatchVersion>
</PropertyGroup>
<!-- Determine the default values of TargetLatestAspNetCoreRuntimePatch -->
<PropertyGroup Condition="'$(TargetLatestAspNetCoreRuntimePatch)' == ''">
- <TargetLatestAspNetCoreRuntimePatch>false</TargetLatestAspNetCoreRuntimePatch>
- <TargetLatestAspNetCoreRuntimePatch Condition="'$(SelfContained)' == 'true'">true</TargetLatestAspNetCoreRuntimePatch>
- <TargetLatestAspNetCoreRuntimePatch Condition="'$(TargetLatestRuntimePatch)' != ''">$(TargetLatestRuntimePatch)</TargetLatestAspNetCoreRuntimePatch>
+ <TargetLatestAspNetCoreRuntimePatch>true</TargetLatestAspNetCoreRuntimePatch>
</PropertyGroup>
<!-- Set the framework version of .All Framework -->
That should be fine. BundledAspNetCoreAppTargetFrameworkVersion doesn't mean anything in the context of source-build. Btw, I've opened https://github.com/aspnet/websdk/issues/355 to get this into aspnet/websdk for a future patch.
Here are the changes in PR for this: https://github.com/dotnet/source-build/pull/610/commits/a0b585f105ecb0264be88a6dfaa6aec81d5934bb. It should be the same as your diff, @tmds, except TargetLatestAspNetCoreRuntimePatch is set by setting a default in cli rather than hard-coding it in websdk.
Thanks @natemcmaster @dagood !
To test my locally patched SDK, I did some variations of setting LatestPatchVersionForAspNetCoreApp2_1, e.g.:
$ LatestPatchVersionForAspNetCoreApp2_1=2.1.10 dotnet publish
Microsoft (R) Build Engine version 15.7.179.62826 for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.
Restoring packages for /tmp/web/web.csproj...
/tmp/web/web.csproj : error NU1102: Unable to find package Microsoft.AspNetCore.App with version (>= 2.1.10)
/tmp/web/web.csproj : error NU1102: - Found 4 version(s) in nuget.org [ Nearest version: 2.1.0 ]
Generating MSBuild file /tmp/web/obj/web.csproj.nuget.g.props.
Generating MSBuild file /tmp/web/obj/web.csproj.nuget.g.targets.
Restore failed in 221.26 ms for /tmp/web/web.csproj.
Cool, same result in my build for LatestPatchVersionForAspNetCoreApp2_1=2.1.10 dotnet publish, and setting it to 2.1.1 succeeds since I have a nuget.config pointing at the ProdCon feed.
Running just dotnet publish gives me a .deps.json with the 2.1.0 ASP.NET Core packages, though... I expected 2.1.1 from what @natemcmaster said about the baseline being bumped up. I don't understand where the default version is coming from, other than Microsoft.NETCoreSdk.BundledVersions.props, which has:
<BundledNETCoreAppTargetFrameworkVersion>2.1</BundledNETCoreAppTargetFrameworkVersion>
<BundledNETCoreAppPackageVersion>2.1.1</BundledNETCoreAppPackageVersion>
<BundledNETStandardTargetFrameworkVersion>2.0</BundledNETStandardTargetFrameworkVersion>
<BundledNETStandardPackageVersion>2.0.2-servicing-26321-0</BundledNETStandardPackageVersion>
<BundledNETCorePlatformsPackageVersion>2.1.0</BundledNETCorePlatformsPackageVersion>
<BundledAspNetCoreAllTargetFrameworkVersion>2.1</BundledAspNetCoreAllTargetFrameworkVersion>
<BundledAspNetCoreAllPackageVersion>2.1.1</BundledAspNetCoreAllPackageVersion>
<BundledAspNetCoreAppTargetFrameworkVersion>2.1</BundledAspNetCoreAppTargetFrameworkVersion>
<BundledAspNetCoreAppPackageVersion>2.1.1</BundledAspNetCoreAppPackageVersion>
<_AspNetCoreAppSharedFxIsEnabled>false</_AspNetCoreAppSharedFxIsEnabled>
<_AspNetCoreAllSharedFxIsEnabled>false</_AspNetCoreAllSharedFxIsEnabled>
<NETCoreSdkVersion>2.1.301</NETCoreSdkVersion>
<_NETCoreSdkIsPreview></_NETCoreSdkIsPreview>
<!-- Default patch versions for each minor version of ASP.NET Core -->
<DefaultPatchVersionForAspNetCoreAll2_1>2.1.1</DefaultPatchVersionForAspNetCoreAll2_1>
<DefaultPatchVersionForAspNetCoreApp2_1>2.1.1</DefaultPatchVersionForAspNetCoreApp2_1>
<!-- Latest patch versions for each minor version of .NET Core -->
<LatestPatchVersionForNetCore1_0 Condition="'1.0.11' == ''">1.0.11</LatestPatchVersionForNetCore1_0>
<LatestPatchVersionForNetCore1_1 Condition="'1.1.8' == ''">1.1.8</LatestPatchVersionForNetCore1_1>
<LatestPatchVersionForNetCore2_0 Condition="'2.0.7' == ''">2.0.7</LatestPatchVersionForNetCore2_0>
<!-- If true, always target the latest ASP.NET Core runtime by default -->
<TargetLatestAspNetCoreRuntimePatch Condition="'' == ''">true</TargetLatestAspNetCoreRuntimePatch>
Maybe this is expected, and package maintainers will just always provide the latest 2_1, 2_2 etc. variables--I'm not sure if I understood the full issue and the fix.
Running just dotnet publish gives me a .deps.json with the 2.1.0 ASP.NET Core packages, though... I expected 2.1.1 from what @natemcmaster said about the baseline being bumped up.
That is also my expectation. From the user-side: using the latest sdk should imply using the latest ASP.NET Core version known to that SDK.
I think it may be caused by removing:
<LatestAspNetCoreAppPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '$(BundledAspNetCoreAppTargetFrameworkVersion)'">$(BundledAspNetCoreAppPackageVersion)</LatestAspNetCoreAppPatchVersion>
We want to use LatestPatchVersionForAspNetCoreApp2_1 when set, and BundledAspNetCoreAppTargetFrameworkVersion otherwise.
Instead of removing these lines for App and All:
<LatestAspNetCoreAppPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '$(BundledAspNetCoreAppTargetFrameworkVersion)'">$(BundledAspNetCoreAppPackageVersion)</LatestAspNetCoreAppPatchVersion>
we should AND the Condition with '$(LatestAspNetCoreAppPatchVersion)' == ''.
Confirmed, dotnet publish gives me 2.1.1 AspNetCore with that change in a fresh source-build. Thanks!
Fixed in #610.