Skip to content

ASP.NET Core patches for source-build .NET Core 2.1 #586

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tmds opened this issue Jun 4, 2018 · 22 comments
Closed

ASP.NET Core patches for source-build .NET Core 2.1 #586

tmds opened this issue Jun 4, 2018 · 22 comments
Assignees
Labels
area-product-experience Improvements in the end-user's product experience

Comments

@tmds
Copy link
Member

tmds commented Jun 4, 2018

Creating separate issue based on things discussed with @natemcmaster in #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

@natemcmaster
Copy link

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 aspnet/Universe#1180. We are resetting the baseline for framework-dependent apps to 2.1.1 in the 2.1.301 SDK update.

@dseefeld dseefeld added triaged area-product-experience Improvements in the end-user's product experience labels Jun 8, 2018
@dagood
Copy link
Member

dagood commented Jun 11, 2018

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.

@tmds
Copy link
Member Author

tmds commented Jun 12, 2018

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

  • provide only the 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.
  • also provide sdk feature packages (e.g. dotnet-sdk-2.1.3xx). And provide via environment variables LatestPatchVersionForAspNetCoreApp2_1/LatestPatchVersionForAspNetCoreAll2_1 to ensure older sdks are also using the latest ASP.NET Core patch releases.

@tmds
Copy link
Member Author

tmds commented Jun 12, 2018

@natemcmaster it seems LatestPatchVersionForAspNetCoreApp2_1/LatestPatchVersionForAspNetCoreAll2_1 are not set in the 2.1.1 early access builds. Is this an oversight?

@natemcmaster
Copy link

@tmds we didn't implement LatestPatchVersionForAspNetCoreApp2_1 in the 2.1.x SDK. This is something I introduced in the 2.2 SDK (dotnet/websdk#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
Copy link
Member Author

tmds commented Jun 12, 2018

@tmds we didn't implement LatestPatchVersionForAspNetCoreApp2_1 in the 2.1.x SDK. This is something I introduced in the 2.2 SDK (dotnet/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.

@dagood
Copy link
Member

dagood commented Jun 12, 2018

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?

@tmds
Copy link
Member Author

tmds commented Jun 12, 2018

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.

@tmds
Copy link
Member Author

tmds commented Jun 12, 2018

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?

@natemcmaster
Copy link

Looks okay to me. Looks like this is basically backporting some changes from dotnet/websdk#348, in addition to changing the default for TargetLatestAspNetCoreRuntimePatch.

@dagood
Copy link
Member

dagood commented Jun 12, 2018

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.

@natemcmaster
Copy link

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.

@dagood
Copy link
Member

dagood commented Jun 12, 2018

Sorry, edited to say [aspnet/]websdk but not in time. A patch it is.

@tmds
Copy link
Member Author

tmds commented Jun 13, 2018

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

@markwilkie markwilkie added this to the S137 June 11 - June 29 (6/11/2018) milestone Jun 13, 2018
@natemcmaster
Copy link

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.

@dagood
Copy link
Member

dagood commented Jun 13, 2018

Here are the changes in PR for this: a0b585f. 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.

@tmds
Copy link
Member Author

tmds commented Jun 13, 2018

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.

@dagood
Copy link
Member

dagood commented Jun 13, 2018

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.

@tmds
Copy link
Member Author

tmds commented Jun 13, 2018

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.

@tmds
Copy link
Member Author

tmds commented Jun 13, 2018

Instead of removing these lines for App and All:

<LatestAspNetCoreAppPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '$(BundledAspNetCoreAppTargetFrameworkVersion)'">$(BundledAspNetCoreAppPackageVersion)</LatestAspNetCoreAppPatchVersion>

we should AND the Condition with '$(LatestAspNetCoreAppPatchVersion)' == ''.

@dagood
Copy link
Member

dagood commented Jun 13, 2018

Confirmed, dotnet publish gives me 2.1.1 AspNetCore with that change in a fresh source-build. Thanks!

@dagood
Copy link
Member

dagood commented Jun 14, 2018

Fixed in #610.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-product-experience Improvements in the end-user's product experience
Projects
None yet
Development

No branches or pull requests

5 participants