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

Remove support for the implicit package version of Microsoft.AspNetCore.App #3292

Open
natemcmaster opened this Issue Jul 3, 2018 · 43 comments

Comments

Projects
None yet
@natemcmaster
Member

natemcmaster commented Jul 3, 2018

Microsoft.AspNetCore.App, unlike normal NuGet packages, represents a shared framework. This means the package provides:

  • A) The minimum expected version of of the ASP.NET Core shared runtime (found in C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App$(PackageVersion))
  • B) A baseline of assemblies versions which the SDK expects to find in the shared runtime
  • C) References assemblies for the compiler
  • D) Implementations assemblies for self-contained deployments

There is tension between these as we want the lowest possible version of (A) to simplify deployment concerns, the highest version of (D) for security updates and patch fixes, and a version of (B) and (C) compatible with both (A) and (D).

We attempted to solve this tension in 2.1.0 by optimizing for (A) and hiding the package version from users and keeping it low, pinned to 2.1.0. That made it difficult to use packages which shared dependencies with the shared framework. In 2.1.1, we tried to fix this by easing off the restrictions and allowing package upgrades, but means users unintentionally upgrade out of the baseline (B), and end up with redundant copies of binaries in their publish output.

After lots of design discussion reviewing half a dozen alternatives, we think it will be better in the long run to back to reverse course and encourage users to use an explicit version on the PackageReference to Microsoft.AspNetCore.App, and it regularly update it to latest.

Proposed changes

  • Update dotnet new templates to include the latest Microsoft.AspNetCore.App package version in the .csproj file.

  • Update Microsoft.NET.Sdk.Web to warn when Microsoft.AspNetCore.App is missing a package version.

    • Text: The package reference to Microsoft.AspNetCore.App is missing the Version attribute. Defaulting to Version="2.1.1"..
  • Remove "IsImplicitlyDefined" = true from Microsoft.NET.Sdk.Web. This unblocks the NuGet UI from displaying updates.
    Before:

    After:

Downsides

One of the primary reasons for making the version implicit was to simplify deployment. As history since 2.0.0 has taught us, our users that updating a PackageReference should be sufficient to use the newest version of ASP.NET Core. However, there is one more step required: users need to install a compatible shared runtime. This step is not obvious from the NuGet Package UI in VS, and error messages indicating the missing runtime are sometimes hidden (e.g. IIS Express, Test Explorer).

Proposed enhancement

[Nate: July 9]: this was not implemented, and we are not planning to implement right now.

  • Update Microsoft.NET.Sdk to produce a warning when compiling for a .NET Core shared framework which is not present on the machine (cc @dsplaisted @nguerrera).
    • Text: This project is compiling for Microsoft.AspNetCore.App 2.1.3, but a compatible runtime was not found. .NET Core runtimes can be installed from https://aka.ms/dotnet-download
      image

cc @DamianEdwards @davidfowl @JunTaoLuo @vijayrkn

@vijayrkn

This comment has been minimized.

Show comment
Hide comment
@vijayrkn

vijayrkn commented Jul 3, 2018

@nguerrera

This comment has been minimized.

Show comment
Hide comment
@nguerrera

nguerrera Jul 3, 2018

Member

Cc @steveharter. I'm not sure how we would implement that warning. We should tread carefully before putting host internals into build logic.

Fundamentally, I'm always wary of build trying to decide things based on the current machine. We have a long history of tooling bugs from doing just that.

But I do think there needs to be a diagnostic of some kind somewhere.

Member

nguerrera commented Jul 3, 2018

Cc @steveharter. I'm not sure how we would implement that warning. We should tread carefully before putting host internals into build logic.

Fundamentally, I'm always wary of build trying to decide things based on the current machine. We have a long history of tooling bugs from doing just that.

But I do think there needs to be a diagnostic of some kind somewhere.

@nguerrera

This comment has been minimized.

Show comment
Hide comment
@nguerrera

nguerrera Jul 3, 2018

Member

Isn't a bug that IIS express and test explorer are hiding the clear runtime message?

Member

nguerrera commented Jul 3, 2018

Isn't a bug that IIS express and test explorer are hiding the clear runtime message?

@natemcmaster

This comment has been minimized.

Show comment
Hide comment
@natemcmaster

natemcmaster Jul 3, 2018

Member

@steveharter @nguerrera how much work do you think it would be to implement something like hostfxr_resolve_sdk, but instead hostfxr_resolve_shared_runtime?

Isn't a bug that IIS express and test explorer are hiding the clear runtime message?

Yes, yes it is. We should work with VSTest and IIS Express to see if we can improve this, too.

Member

natemcmaster commented Jul 3, 2018

@steveharter @nguerrera how much work do you think it would be to implement something like hostfxr_resolve_sdk, but instead hostfxr_resolve_shared_runtime?

Isn't a bug that IIS express and test explorer are hiding the clear runtime message?

Yes, yes it is. We should work with VSTest and IIS Express to see if we can improve this, too.

@nguerrera

This comment has been minimized.

Show comment
Hide comment
@nguerrera

nguerrera Jul 3, 2018

Member

Writing the function is easy enough but there are some deployment issues. Chief among them is that we often run in 32 bit msbuild but dotnet installed is 64 bit. Sdk resolver gets around this by carrying its own hostfxr, now we'd need access to same during build. It's messy already and I don't have a great feeling about more interaction between build and hostfxr after the experience so far.

Member

nguerrera commented Jul 3, 2018

Writing the function is easy enough but there are some deployment issues. Chief among them is that we often run in 32 bit msbuild but dotnet installed is 64 bit. Sdk resolver gets around this by carrying its own hostfxr, now we'd need access to same during build. It's messy already and I don't have a great feeling about more interaction between build and hostfxr after the experience so far.

@nguerrera

This comment has been minimized.

Show comment
Hide comment
@nguerrera
Member

nguerrera commented Jul 3, 2018

Adding @jeffschwMSFT

@natemcmaster

This comment has been minimized.

Show comment
Hide comment
@natemcmaster

natemcmaster Jul 3, 2018

Member

It's messy already and I don't have a great feeling about more interaction between build and hostfxr after the experience so far.

Presumably we can piggy back off the messiness you've already wrangled, right? From what I can tell, to implement this diagnostic we only need to read a runtimeconfig.json and check if a runtime exists to satisfy the runtime framework name and version. Are there complications in doing this that would make the messiness even worse?

Member

natemcmaster commented Jul 3, 2018

It's messy already and I don't have a great feeling about more interaction between build and hostfxr after the experience so far.

Presumably we can piggy back off the messiness you've already wrangled, right? From what I can tell, to implement this diagnostic we only need to read a runtimeconfig.json and check if a runtime exists to satisfy the runtime framework name and version. Are there complications in doing this that would make the messiness even worse?

@nguerrera

This comment has been minimized.

Show comment
Hide comment
@nguerrera

nguerrera Jul 3, 2018

Member

Not really. The resolver lives in its own world with its private hostfxr and no knowledge of the build tasks. And the build tasks have no knowledge of the resolver. So we'd either have to duplicate this messiness in the build tasks (yuck) or come up with some other activation scheme. I'd push for the latter but I don't have a solution in mind for it yet. It's definitely something that would need careful thought. I hope we don't decide to rush on it for 2.1.3.

Member

nguerrera commented Jul 3, 2018

Not really. The resolver lives in its own world with its private hostfxr and no knowledge of the build tasks. And the build tasks have no knowledge of the resolver. So we'd either have to duplicate this messiness in the build tasks (yuck) or come up with some other activation scheme. I'd push for the latter but I don't have a solution in mind for it yet. It's definitely something that would need careful thought. I hope we don't decide to rush on it for 2.1.3.

@nguerrera

This comment has been minimized.

Show comment
Hide comment
@nguerrera

nguerrera Jul 3, 2018

Member

And I'm still concerned about the general idea of warning about what won't run with the dotnet on PATH. It is valid to publish from an environment that can't run a program to an environment that can.

A suppressible warning isn't the worst sin of this kind. It pales in comparison to say RAR thinking the presence of an assembly in the build machine's GAC is relevant to whether it should be copied. But bad decisions like that give me pause whenever we discuss anything that ties the runtime environment of the build to the runtime environment of the program being built.

Member

nguerrera commented Jul 3, 2018

And I'm still concerned about the general idea of warning about what won't run with the dotnet on PATH. It is valid to publish from an environment that can't run a program to an environment that can.

A suppressible warning isn't the worst sin of this kind. It pales in comparison to say RAR thinking the presence of an assembly in the build machine's GAC is relevant to whether it should be copied. But bad decisions like that give me pause whenever we discuss anything that ties the runtime environment of the build to the runtime environment of the program being built.

@rockerinthelocker

This comment has been minimized.

Show comment
Hide comment
@rockerinthelocker

rockerinthelocker Jul 3, 2018

@natemcmaster , @coolcsh , While you're at it, #3257 might need the same treatment.

rockerinthelocker commented Jul 3, 2018

@natemcmaster , @coolcsh , While you're at it, #3257 might need the same treatment.

@poke

This comment has been minimized.

Show comment
Hide comment
@poke

poke Jul 3, 2018

As much as I dislike this going back and forth, I believe making the version explicit again is for the best to get rid of all the problems that came with the shared framework.

@natemcmaster The “that made it difficult” link in the opening post has the wrong link.

poke commented Jul 3, 2018

As much as I dislike this going back and forth, I believe making the version explicit again is for the best to get rid of all the problems that came with the shared framework.

@natemcmaster The “that made it difficult” link in the opening post has the wrong link.

@steveharter

This comment has been minimized.

Show comment
Hide comment
@steveharter

steveharter Jul 3, 2018

@natemcmaster can you elaborate on this:

In 2.1.1, we tried to fix this by easing off the restrictions and allowing package upgrades, but means users unintentionally upgrade out of the baseline (B), and end up with redundant copies of binaries in their publish output.

Are the redundant "binaries" assemblies, or some other binaries? Did this result in an error of some form?

For the proposed new API, to get a 100% accurate result it requires us to read the app's runtimeconfig.json in order to pull in the same roll-forward settings and potentially in the future resolve other references to the same framework. An alternative (< 100%) would be to just do a dotnet --info and parse the output (possibly json) and apply roll-forward semantics probably just using >=

steveharter commented Jul 3, 2018

@natemcmaster can you elaborate on this:

In 2.1.1, we tried to fix this by easing off the restrictions and allowing package upgrades, but means users unintentionally upgrade out of the baseline (B), and end up with redundant copies of binaries in their publish output.

Are the redundant "binaries" assemblies, or some other binaries? Did this result in an error of some form?

For the proposed new API, to get a 100% accurate result it requires us to read the app's runtimeconfig.json in order to pull in the same roll-forward settings and potentially in the future resolve other references to the same framework. An alternative (< 100%) would be to just do a dotnet --info and parse the output (possibly json) and apply roll-forward semantics probably just using >=

@jeffschwMSFT

This comment has been minimized.

Show comment
Hide comment
@jeffschwMSFT

jeffschwMSFT Jul 3, 2018

Text: This project is compiling for Microsoft.AspNetCore.App 2.1.3, but a compatible runtime was not found. .NET Core runtimes can be installed from https://aka.ms/dotnet-download

What does it mean to be compatible? It seems like there are practical considerations to get this right, which makes me think that it will be hard for our customers to get right as well. Is it as simple as 'matches .'?

jeffschwMSFT commented Jul 3, 2018

Text: This project is compiling for Microsoft.AspNetCore.App 2.1.3, but a compatible runtime was not found. .NET Core runtimes can be installed from https://aka.ms/dotnet-download

What does it mean to be compatible? It seems like there are practical considerations to get this right, which makes me think that it will be hard for our customers to get right as well. Is it as simple as 'matches .'?

@natemcmaster

This comment has been minimized.

Show comment
Hide comment
@natemcmaster

natemcmaster Jul 3, 2018

Member

The “that made it difficult” link in the opening post has the wrong link.

@poke Updated -> aspnet/Universe#1180

What does it mean to be compatible?

@jeffschwMSFT would an error message like this be better?

"This project is compiling for the Microsoft.AspNetCore.App 2.1.3 shared runtime, but a stable version of this runtime that is higher than or equal to 2.1.3 but no greater than 3.0.0, could not be found. Your app may fail to start unless you install the latest 2.1.* patch version of the Microsoft.AspNetCore.App shared runtime from https://aka.ms/dotnet-download"

@natemcmaster can you elaborate on this:

In 2.1.1, we tried to fix this by easing off the restrictions and allowing package upgrades, but means users unintentionally upgrade out of the baseline (B), and end up with redundant copies of binaries in their publish output.

Are the redundant "binaries" assemblies, or some other binaries? Did this result in an error of some form?

At the moment, we're not seeing runtime errors, but we're seeing the published output of apps starting to grow as we patch packages that overlap dependencies with the shared framework. In theory, it could happen with native binaries, too, such as sni.dll. We want to avoid putting users in a state in which they are overriding the shared runtime with app-local copies.

You can see this clearly in our daily builds. This project has 16 additional assemblies in publish output which are normally part of the Microsoft.AspNetCore.App shared runtime.

<Project Sdk="Microsoft.NET.Sdk.Web">
  <PropertyGroup>
    <TargetFramework>netcoreapp2.1</TargetFramework>
    <RestoreAdditionalProjectSources>https://dotnet.myget.org/F/aspnetcore-dev/api/v3/index.json</RestoreAdditionalProjectSources>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore.App" />
    <PackageReference Include="Microsoft.EntityFrameworkCore.SQLite" Version="2.1.2-rtm-30877" />
    <PackageReference Include="Microsoft.Extensions.Caching.Redis" Version="2.1.2-rtm-30877" />
  </ItemGroup>
</Project>

The way a user would fix this is to upgrade Microsoft.AspNetCore.App.

    <PackageReference Include="Microsoft.AspNetCore.App" Version="2.1.3-rtm-30877" />

to get a 100% accurate result

@steveharter IMO I'm okay with less than 100% accuracy in this warning. In the event there is an error in using the new hostfxr API or dotnet --info, I would just skip this check since there will be a runtime error anyways. The main point of the idea is to provide an early diagnostic, soon after a user upgrades their package references in NuGet, so they will know that they might have introduced an issue by upgrading Microsoft.AspNetCore.App.

Member

natemcmaster commented Jul 3, 2018

The “that made it difficult” link in the opening post has the wrong link.

@poke Updated -> aspnet/Universe#1180

What does it mean to be compatible?

@jeffschwMSFT would an error message like this be better?

"This project is compiling for the Microsoft.AspNetCore.App 2.1.3 shared runtime, but a stable version of this runtime that is higher than or equal to 2.1.3 but no greater than 3.0.0, could not be found. Your app may fail to start unless you install the latest 2.1.* patch version of the Microsoft.AspNetCore.App shared runtime from https://aka.ms/dotnet-download"

@natemcmaster can you elaborate on this:

In 2.1.1, we tried to fix this by easing off the restrictions and allowing package upgrades, but means users unintentionally upgrade out of the baseline (B), and end up with redundant copies of binaries in their publish output.

Are the redundant "binaries" assemblies, or some other binaries? Did this result in an error of some form?

At the moment, we're not seeing runtime errors, but we're seeing the published output of apps starting to grow as we patch packages that overlap dependencies with the shared framework. In theory, it could happen with native binaries, too, such as sni.dll. We want to avoid putting users in a state in which they are overriding the shared runtime with app-local copies.

You can see this clearly in our daily builds. This project has 16 additional assemblies in publish output which are normally part of the Microsoft.AspNetCore.App shared runtime.

<Project Sdk="Microsoft.NET.Sdk.Web">
  <PropertyGroup>
    <TargetFramework>netcoreapp2.1</TargetFramework>
    <RestoreAdditionalProjectSources>https://dotnet.myget.org/F/aspnetcore-dev/api/v3/index.json</RestoreAdditionalProjectSources>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore.App" />
    <PackageReference Include="Microsoft.EntityFrameworkCore.SQLite" Version="2.1.2-rtm-30877" />
    <PackageReference Include="Microsoft.Extensions.Caching.Redis" Version="2.1.2-rtm-30877" />
  </ItemGroup>
</Project>

The way a user would fix this is to upgrade Microsoft.AspNetCore.App.

    <PackageReference Include="Microsoft.AspNetCore.App" Version="2.1.3-rtm-30877" />

to get a 100% accurate result

@steveharter IMO I'm okay with less than 100% accuracy in this warning. In the event there is an error in using the new hostfxr API or dotnet --info, I would just skip this check since there will be a runtime error anyways. The main point of the idea is to provide an early diagnostic, soon after a user upgrades their package references in NuGet, so they will know that they might have introduced an issue by upgrading Microsoft.AspNetCore.App.

@jeffschwMSFT

This comment has been minimized.

Show comment
Hide comment
@jeffschwMSFT

jeffschwMSFT Jul 3, 2018

"This project is compiling for the Microsoft.AspNetCore.App 2.1.3 shared runtime, but a stable version of this runtime that is higher than or equal to 2.1.3 but no greater than 3.0.0, could not be found. Your app may fail to start unless you install the latest 2.1.* patch version of the Microsoft.AspNetCore.App shared runtime from https://aka.ms/dotnet-download"

It does, but I fear it too does not cover all the cases. If for instance, a pre-release was used then it would be slightly different rollforward logic. I don't have a great suggestion. But I dislike that our rollforward logic is bleeding into this experience. Between the two, I like this one more.

jeffschwMSFT commented Jul 3, 2018

"This project is compiling for the Microsoft.AspNetCore.App 2.1.3 shared runtime, but a stable version of this runtime that is higher than or equal to 2.1.3 but no greater than 3.0.0, could not be found. Your app may fail to start unless you install the latest 2.1.* patch version of the Microsoft.AspNetCore.App shared runtime from https://aka.ms/dotnet-download"

It does, but I fear it too does not cover all the cases. If for instance, a pre-release was used then it would be slightly different rollforward logic. I don't have a great suggestion. But I dislike that our rollforward logic is bleeding into this experience. Between the two, I like this one more.

@dsplaisted

This comment has been minimized.

Show comment
Hide comment
@dsplaisted

dsplaisted Jul 3, 2018

How about leaving the message as is but adding a link with the detailed explanation for how version selection works? Documentation for that is in PR here: dotnet/docs#6060

dsplaisted commented Jul 3, 2018

How about leaving the message as is but adding a link with the detailed explanation for how version selection works? Documentation for that is in PR here: dotnet/docs#6060

@natemcmaster

This comment has been minimized.

Show comment
Hide comment
@natemcmaster

natemcmaster Jul 3, 2018

Member

Yeah, we should add a link to this warning.

By the way, I agree with all of your concerns about a build-time diagnostic of a runtime behavior which may or may not be accurate.

But, I'm more concerned about how often users will find their applications failing to launch after updating their NuGet packages. I think we can address this with better tooling, and an MSBuild warning is the only tooling option we have at our disposal right now.

Member

natemcmaster commented Jul 3, 2018

Yeah, we should add a link to this warning.

By the way, I agree with all of your concerns about a build-time diagnostic of a runtime behavior which may or may not be accurate.

But, I'm more concerned about how often users will find their applications failing to launch after updating their NuGet packages. I think we can address this with better tooling, and an MSBuild warning is the only tooling option we have at our disposal right now.

natemcmaster added a commit to aspnet/Templating that referenced this issue Jul 3, 2018

Add package version to Microsoft.AspNetCore.App in project templates
As discussed in aspnet/Home#3292, we're reversing course on the implicit package version feature. This adds a package version for Microsoft.AspNetCore.App to the `dotnet new` and VS templates.
@natemcmaster

This comment has been minimized.

Show comment
Hide comment
@natemcmaster

natemcmaster Jul 6, 2018

Member

We talked about this in shiproom and a handful of design meetings. The plan is to move forward with the changes to the project templates and web SDK tweaks.

We'll take more time to investigate enhancements to tooling to help users identify when their .NET Core console or web projects are building against a runtime framework which is not installed on the machine.

Member

natemcmaster commented Jul 6, 2018

We talked about this in shiproom and a handful of design meetings. The plan is to move forward with the changes to the project templates and web SDK tweaks.

We'll take more time to investigate enhancements to tooling to help users identify when their .NET Core console or web projects are building against a runtime framework which is not installed on the machine.

@normj

This comment has been minimized.

Show comment
Hide comment
@normj

normj Jul 7, 2018

Adding another voice, in this case from AWS, that this setting needs to be explicit. This is breaking our customers using ASP.NET Core because they update their dev/build machines but AWS services usually get updated about a month after a new .NET Core release. Right now this seems like a major reversal back to the .NET Core 1.x days when every patch release would cause pain.

normj commented Jul 7, 2018

Adding another voice, in this case from AWS, that this setting needs to be explicit. This is breaking our customers using ASP.NET Core because they update their dev/build machines but AWS services usually get updated about a month after a new .NET Core release. Right now this seems like a major reversal back to the .NET Core 1.x days when every patch release would cause pain.

@dasMulli

This comment has been minimized.

Show comment
Hide comment
@dasMulli

dasMulli Jul 7, 2018

Another idea: what about setting the framework version that's written to the runtimeconfig not to the NuGet version, but to a version that the package provides via build targets. So the 2.1.2+ NuGet packages could for example set 2.0.0 as the sharedfx version to be used in runtimeconfig.json.
Of course comes with other requirements such as the public API not changing etc.
That way a self-contained deployment could get the latest assets (or whichever version they reference) but a framework-dependent deployment would be able to run off any 2.0.0+ fx

dasMulli commented Jul 7, 2018

Another idea: what about setting the framework version that's written to the runtimeconfig not to the NuGet version, but to a version that the package provides via build targets. So the 2.1.2+ NuGet packages could for example set 2.0.0 as the sharedfx version to be used in runtimeconfig.json.
Of course comes with other requirements such as the public API not changing etc.
That way a self-contained deployment could get the latest assets (or whichever version they reference) but a framework-dependent deployment would be able to run off any 2.0.0+ fx

@natemcmaster

This comment has been minimized.

Show comment
Hide comment
@natemcmaster

natemcmaster Jul 7, 2018

Member

Thanks for the feedback. I'd love to address your concerns. Let me share my thinking on both points just made. Let me know if you have follow up questions, or if my comments don't address your concerns.

@normj: this seems like a major reversal back to the .NET Core 1.x days when every patch release would cause pain.

Yes, this still haunts us and has been part of our conversations. We're only reversing a minor part of the experience, not the whole thing.

The problem I believe you're referring to was fixed when we stopped lifting the implicit version of the Microsoft.NETCore.App reference in SDK updates, and that isn't changing. Microsoft.NETCore.App will stay at 2.1.0 for all 2.1.x patches. Likewise, SDK updates will not change the version of Microsoft.AspNetCore.App/All. This will continue to use the version on the PackageReference in the .csproj and fallback to 2.1.1 if the PackageReference item did not specify a version.

VS and SDK updates won't change your AspNetCore.App version, but developers might. If they unaware of the consequences, they can make seemingly innocent changes to their project which will cause runtime failures. The Updates tab in the VS NuGet Package Manager leads users to making this change without warning about consequences. We tried to guard against this in 2.1.0 by hiding the AspNetCore.App package update from the UI, but it didn't really help. The majority of our users upgraded from netcoreapp2.0 and don't use the implicit package version feature. So, last month when 2.1.1 came out, they saw the advertised update in NuGet UI, updated, and hit runtime errors because the 2.1.1 runtime was not installed.

We want to help users understand that a NuGet package update may require installing a new runtime, so we're looking for a good way to implement design-time diagnostics. I'm hoping that warnings about compiling for an unavailable runtime will help, but we're open to suggestions.

@dasMulli: what about setting the framework version that's written to the runtimeconfig not to the NuGet version, but to a version that the package provides via build targets.

This is something we considered, but decided against. Setting the framework version in runtimeconfig.json to 2.1.0 when an app compiled against 2.1.3 will lead to runtime errors. This happens because your app's binary will have a reference to Microsoft.Extensions.Logging, Version=2.1.3.0, but the 2.1.0 shared runtime only has Microsoft.Extensions.Logging, Version=2.1.0.0. There are ways to mitigate this, such as introduce ref assemblies with assembly versions set to 2.1.0.0, but there be dragons, my friend. Handling all scenarios we support using this approach is massively complicated -- .NET Framework and binding redirects, .NET Standard, resolving across P2P references, console .NET Core apps, Xamarin, etc. We consulted with the corefx team who does this well, and concluded this is not something we can change in a patch. We are investigating ref assemblies for 3.0, but that's going to be a lot of work to get right.

Member

natemcmaster commented Jul 7, 2018

Thanks for the feedback. I'd love to address your concerns. Let me share my thinking on both points just made. Let me know if you have follow up questions, or if my comments don't address your concerns.

@normj: this seems like a major reversal back to the .NET Core 1.x days when every patch release would cause pain.

Yes, this still haunts us and has been part of our conversations. We're only reversing a minor part of the experience, not the whole thing.

The problem I believe you're referring to was fixed when we stopped lifting the implicit version of the Microsoft.NETCore.App reference in SDK updates, and that isn't changing. Microsoft.NETCore.App will stay at 2.1.0 for all 2.1.x patches. Likewise, SDK updates will not change the version of Microsoft.AspNetCore.App/All. This will continue to use the version on the PackageReference in the .csproj and fallback to 2.1.1 if the PackageReference item did not specify a version.

VS and SDK updates won't change your AspNetCore.App version, but developers might. If they unaware of the consequences, they can make seemingly innocent changes to their project which will cause runtime failures. The Updates tab in the VS NuGet Package Manager leads users to making this change without warning about consequences. We tried to guard against this in 2.1.0 by hiding the AspNetCore.App package update from the UI, but it didn't really help. The majority of our users upgraded from netcoreapp2.0 and don't use the implicit package version feature. So, last month when 2.1.1 came out, they saw the advertised update in NuGet UI, updated, and hit runtime errors because the 2.1.1 runtime was not installed.

We want to help users understand that a NuGet package update may require installing a new runtime, so we're looking for a good way to implement design-time diagnostics. I'm hoping that warnings about compiling for an unavailable runtime will help, but we're open to suggestions.

@dasMulli: what about setting the framework version that's written to the runtimeconfig not to the NuGet version, but to a version that the package provides via build targets.

This is something we considered, but decided against. Setting the framework version in runtimeconfig.json to 2.1.0 when an app compiled against 2.1.3 will lead to runtime errors. This happens because your app's binary will have a reference to Microsoft.Extensions.Logging, Version=2.1.3.0, but the 2.1.0 shared runtime only has Microsoft.Extensions.Logging, Version=2.1.0.0. There are ways to mitigate this, such as introduce ref assemblies with assembly versions set to 2.1.0.0, but there be dragons, my friend. Handling all scenarios we support using this approach is massively complicated -- .NET Framework and binding redirects, .NET Standard, resolving across P2P references, console .NET Core apps, Xamarin, etc. We consulted with the corefx team who does this well, and concluded this is not something we can change in a patch. We are investigating ref assemblies for 3.0, but that's going to be a lot of work to get right.

@normj

This comment has been minimized.

Show comment
Hide comment
@normj

normj Jul 7, 2018

Yes the implicit versioning of Microsoft.NETCore.App was what caused us lots of problems in .NET Core 1.x days and with 2.0 that was fixed which was great. We still had the problem that if our package store wasn't up to date applications referencing Microsoft.AspNetCore.All with a higher version then we had would fail. That was fairly easy to explain to users that their referenced version of the NuGet package was greater then the version in our package cache. Now that ASP.NET Core looks like a runtime and not a collection of packages it is confusing that the experience isn't the same Microsoft.NETCore.App and will be harder to explain. On the plus side the error message is better because it says you asked for 2.1.1 and only 2.1.0 is available. The confusion part will be understanding how did I ask for 2.1.1. By making the package reference explicit helps but it does seem weird that a package reference affects my choice in target runtime.

In services that we have more control of the running environment, (i.e. Lambda) our client tooling verifies the version of Microsoft.AspNetCore.All referenced in the csproj file is available in Lambda and if not fail the deployment. By pushing developers to set an explicit version for Microsoft.AspNetCore.App and Microsoft.AspNetCore.All we can continue to have that check. I believe what I will do at least for our first version of 2.1 support is require that the Version attribute be set so we can avoid getting the mass confusion.

I had hoped that the reason for moving the ASP.NET Core packages as a shared runtime was that it would have the same flexibility as Microsoft.NETCore.All. Looks like that is not true and I still need guide users to supported patch versions in Lambda,

Is the push to latest version just and SDK/build decision? Meaning if the deployment package says use Microsoft.AspNetCore.App version 2.1.1 will the application fail to start if only 2.1.2 is available in the execution environment or will it roll forward to 2.1.2?

normj commented Jul 7, 2018

Yes the implicit versioning of Microsoft.NETCore.App was what caused us lots of problems in .NET Core 1.x days and with 2.0 that was fixed which was great. We still had the problem that if our package store wasn't up to date applications referencing Microsoft.AspNetCore.All with a higher version then we had would fail. That was fairly easy to explain to users that their referenced version of the NuGet package was greater then the version in our package cache. Now that ASP.NET Core looks like a runtime and not a collection of packages it is confusing that the experience isn't the same Microsoft.NETCore.App and will be harder to explain. On the plus side the error message is better because it says you asked for 2.1.1 and only 2.1.0 is available. The confusion part will be understanding how did I ask for 2.1.1. By making the package reference explicit helps but it does seem weird that a package reference affects my choice in target runtime.

In services that we have more control of the running environment, (i.e. Lambda) our client tooling verifies the version of Microsoft.AspNetCore.All referenced in the csproj file is available in Lambda and if not fail the deployment. By pushing developers to set an explicit version for Microsoft.AspNetCore.App and Microsoft.AspNetCore.All we can continue to have that check. I believe what I will do at least for our first version of 2.1 support is require that the Version attribute be set so we can avoid getting the mass confusion.

I had hoped that the reason for moving the ASP.NET Core packages as a shared runtime was that it would have the same flexibility as Microsoft.NETCore.All. Looks like that is not true and I still need guide users to supported patch versions in Lambda,

Is the push to latest version just and SDK/build decision? Meaning if the deployment package says use Microsoft.AspNetCore.App version 2.1.1 will the application fail to start if only 2.1.2 is available in the execution environment or will it roll forward to 2.1.2?

@davidfowl

This comment has been minimized.

Show comment
Hide comment
@davidfowl

davidfowl Jul 7, 2018

Member

The problem is that ASP.NET Core has to deliver patches in multiple ways:

  • via .NET Standard packages that run anywhere (including .NET Framework, UWP etc)
  • via the shared framework which only works on .NET Core

This tension is the reason things are so brittle today. As @natemcmaster vividly described in his previous post, this is hard to get right and getting it wrong is extremely impactful (as we’ve seen in both 2.1.0 and 2.1.1).

Is the push to latest version just and SDK/build decision? Meaning if the deployment package says use Microsoft.AspNetCore.App version 2.1.1 will the application fail to start if only 2.1.2 is available in the execution environment or will it roll forward to 2.1.2?

Hmm I’m not sure if this works today but somebody else can confirm. The runtime does roll forward by default to the latest patch but I’m not sure if the target version needs to be installed. I know we do support minor version runtime roll forward if the target isn’t installed though...

@dsplaisted

Member

davidfowl commented Jul 7, 2018

The problem is that ASP.NET Core has to deliver patches in multiple ways:

  • via .NET Standard packages that run anywhere (including .NET Framework, UWP etc)
  • via the shared framework which only works on .NET Core

This tension is the reason things are so brittle today. As @natemcmaster vividly described in his previous post, this is hard to get right and getting it wrong is extremely impactful (as we’ve seen in both 2.1.0 and 2.1.1).

Is the push to latest version just and SDK/build decision? Meaning if the deployment package says use Microsoft.AspNetCore.App version 2.1.1 will the application fail to start if only 2.1.2 is available in the execution environment or will it roll forward to 2.1.2?

Hmm I’m not sure if this works today but somebody else can confirm. The runtime does roll forward by default to the latest patch but I’m not sure if the target version needs to be installed. I know we do support minor version runtime roll forward if the target isn’t installed though...

@dsplaisted

@normj

This comment has been minimized.

Show comment
Hide comment
@normj

normj Jul 7, 2018

Yes, I agree it is a difficult problem and I'm a big fan of having the both distributions. I'm just advising away from deciding any specific runtime version at build time be implicit which it sounds like we are in agreement.

In the future I would encourage the lever that chooses the runtime to be something more obvious then a version attribute on a PackageReference. A property in a PropertyGroup would have been more intuitive as well as it would allow the value to be overriden as a switch during build. So in my case I could see oh this is the latest version we support in Lambda when our tooling builds the package bundle we could set the correct value for our execution environment.

normj commented Jul 7, 2018

Yes, I agree it is a difficult problem and I'm a big fan of having the both distributions. I'm just advising away from deciding any specific runtime version at build time be implicit which it sounds like we are in agreement.

In the future I would encourage the lever that chooses the runtime to be something more obvious then a version attribute on a PackageReference. A property in a PropertyGroup would have been more intuitive as well as it would allow the value to be overriden as a switch during build. So in my case I could see oh this is the latest version we support in Lambda when our tooling builds the package bundle we could set the correct value for our execution environment.

@tmds

This comment has been minimized.

Show comment
Hide comment
@tmds

tmds Jul 7, 2018

The runtime does roll forward by default to the latest patch but I’m not sure if the target version needs to be installed.

The latest is enough.

A property in a PropertyGroup would have been more intuitive as well as it would allow the value to be overriden as a switch during build

+1 to have an easy way to build for a specific asp.net core version.

tmds commented Jul 7, 2018

The runtime does roll forward by default to the latest patch but I’m not sure if the target version needs to be installed.

The latest is enough.

A property in a PropertyGroup would have been more intuitive as well as it would allow the value to be overriden as a switch during build

+1 to have an easy way to build for a specific asp.net core version.

@natemcmaster

This comment has been minimized.

Show comment
Hide comment
@natemcmaster

natemcmaster Jul 7, 2018

Member

@normj: We still had the problem that if our package store wasn't up to date applications referencing Microsoft.AspNetCore.All with a higher version then we had would fail.

This was the primary reason we converted from a package runtime store to a shared framework. The package runtime store was not designed to handle servicing updates, and its error messages were not clear to users.

it does seem weird that a package reference affects my choice in target runtime.
In the future I would encourage the lever that chooses the runtime to be something more obvious then a version attribute on a PackageReference.

+💯 agree. It's something we plan to address in 3.0.

if the deployment package says use Microsoft.AspNetCore.App version 2.1.1 will the application fail to start if only 2.1.2 is available in the execution environment or will it roll forward to 2.1.2?

@tmds is right. It will rollforward. 2.1.1 does not need to be installed.

Member

natemcmaster commented Jul 7, 2018

@normj: We still had the problem that if our package store wasn't up to date applications referencing Microsoft.AspNetCore.All with a higher version then we had would fail.

This was the primary reason we converted from a package runtime store to a shared framework. The package runtime store was not designed to handle servicing updates, and its error messages were not clear to users.

it does seem weird that a package reference affects my choice in target runtime.
In the future I would encourage the lever that chooses the runtime to be something more obvious then a version attribute on a PackageReference.

+💯 agree. It's something we plan to address in 3.0.

if the deployment package says use Microsoft.AspNetCore.App version 2.1.1 will the application fail to start if only 2.1.2 is available in the execution environment or will it roll forward to 2.1.2?

@tmds is right. It will rollforward. 2.1.1 does not need to be installed.

@davidfowl

This comment has been minimized.

Show comment
Hide comment
@davidfowl

davidfowl Jul 7, 2018

Member

A property in a PropertyGroup would have been more intuitive as well as it would allow the value to be overriden as a switch during build.

I don’t see how that’s more obvious. It’s more like you’re saying to make it hard to target patch versions by making it less obvious.

Can you clarify? Here are 2 questions I have:

  • Should people target patch versions of ASP.NET Core at dev/build?
  • What sort of action in each of our existing distributions are require. (Shared framework, packages)
Member

davidfowl commented Jul 7, 2018

A property in a PropertyGroup would have been more intuitive as well as it would allow the value to be overriden as a switch during build.

I don’t see how that’s more obvious. It’s more like you’re saying to make it hard to target patch versions by making it less obvious.

Can you clarify? Here are 2 questions I have:

  • Should people target patch versions of ASP.NET Core at dev/build?
  • What sort of action in each of our existing distributions are require. (Shared framework, packages)
@normj

This comment has been minimized.

Show comment
Hide comment
@normj

normj Jul 8, 2018

@davidfowl In general people should not be targeting a patch version at dev/build time because they don't necessary have control or knowledge of the patch version in the execution environment. The exception to that case if there application was affected by an issue addressed by a patch and in which case they should just be saying what is the minimum version. That is what the property in the property group I imagine would set, is the minimum shared runtime version.

They way I thought the new ASP.NET Core shared runtimes would work, and just found out the hard way that I was wrong, was they would act just like the Microsoft.NETCore.App shared runtime. Meaning when I do a build the runtimeconfig.json file would be set to 2.1.0 so my deployment package would be compatible with any version of .NET Core 2.1 installed. Then as part of my devops I would keep the execution environment patched and the deployed applications would automatically start using them after their next restart.

Now that process is broken compared to Microsoft.NETCore.App because rutimeconfig.json is being set to the latest patch version known at build time without the knowledge of what the execution environment supports. The toggle for that is the version attribute on a NuGet package and I believe that re-purposing of a NuGet version field is not intuitive because developers don't know to associate a NuGet version with their choice in patch runtimes.

So in my ideal world using a shared framework the runtimeconfig.json sets the minimum 2.1.0 value unless some property explicitly says it needs a later version. For the packages distribution option that would already follow this behavior except using the Microsoft.NETCore.App framework instead.

normj commented Jul 8, 2018

@davidfowl In general people should not be targeting a patch version at dev/build time because they don't necessary have control or knowledge of the patch version in the execution environment. The exception to that case if there application was affected by an issue addressed by a patch and in which case they should just be saying what is the minimum version. That is what the property in the property group I imagine would set, is the minimum shared runtime version.

They way I thought the new ASP.NET Core shared runtimes would work, and just found out the hard way that I was wrong, was they would act just like the Microsoft.NETCore.App shared runtime. Meaning when I do a build the runtimeconfig.json file would be set to 2.1.0 so my deployment package would be compatible with any version of .NET Core 2.1 installed. Then as part of my devops I would keep the execution environment patched and the deployed applications would automatically start using them after their next restart.

Now that process is broken compared to Microsoft.NETCore.App because rutimeconfig.json is being set to the latest patch version known at build time without the knowledge of what the execution environment supports. The toggle for that is the version attribute on a NuGet package and I believe that re-purposing of a NuGet version field is not intuitive because developers don't know to associate a NuGet version with their choice in patch runtimes.

So in my ideal world using a shared framework the runtimeconfig.json sets the minimum 2.1.0 value unless some property explicitly says it needs a later version. For the packages distribution option that would already follow this behavior except using the Microsoft.NETCore.App framework instead.

@davidfowl

This comment has been minimized.

Show comment
Hide comment
@davidfowl

davidfowl Jul 8, 2018

Member

The exception to that case if there application was affected by an issue addressed by a patch and in which case they should just be saying what is the minimum version. That is what the property in the property group I imagine would set, is the minimum shared runtime version.

Turns out that’s how it works already. If you target 2.1.0 and only 2.1.1 is installed on the target then it’ll work.

Here’s the thing though, nothing is fundamentally broken, we’re trying to match people’s expectations here and that’s the issue. Why are people even updating to patch versions in their development environments in the first place? They could leave it at 2.1.0 right? There are a couple of things I can think of:

  1. It could be because they hit an issue and want to test the fix.
  2. It could be because they want to be on “the latest”.
  3. It could be because we blogged about it.

What we have is a bit more complicated because we have packages that aren’t in the shared framework that depend on patch versions of things in the shared framework. It means you end up with a mix of 2.1.1 and 2.1.0 at development time and runtime.

The reason I disagree with the assertion that the nuget package version is worse is because the reason people update it is because they want to target the patch at dev/build time. IMO that’s the root of all the problems here.

Member

davidfowl commented Jul 8, 2018

The exception to that case if there application was affected by an issue addressed by a patch and in which case they should just be saying what is the minimum version. That is what the property in the property group I imagine would set, is the minimum shared runtime version.

Turns out that’s how it works already. If you target 2.1.0 and only 2.1.1 is installed on the target then it’ll work.

Here’s the thing though, nothing is fundamentally broken, we’re trying to match people’s expectations here and that’s the issue. Why are people even updating to patch versions in their development environments in the first place? They could leave it at 2.1.0 right? There are a couple of things I can think of:

  1. It could be because they hit an issue and want to test the fix.
  2. It could be because they want to be on “the latest”.
  3. It could be because we blogged about it.

What we have is a bit more complicated because we have packages that aren’t in the shared framework that depend on patch versions of things in the shared framework. It means you end up with a mix of 2.1.1 and 2.1.0 at development time and runtime.

The reason I disagree with the assertion that the nuget package version is worse is because the reason people update it is because they want to target the patch at dev/build time. IMO that’s the root of all the problems here.

@normj

This comment has been minimized.

Show comment
Hide comment
@normj

normj Jul 8, 2018

The reason I disagree with your disagree:) is for the shared runtime it conflates the concept of a NuGet package that is included with your package bundle created at build time and a runtime where the version isn't known until deployment time.

For the sake of conversation I'm ignoring the implicit version case. I believe we are in agreement that that is a bad practice since that was why this issue was open and I apologize for the conversation going well passed the original intent of the issue. In our upcoming Lambda tooling for .NET Core we will require a version being set so we can verify compatibility.

Now back to package version being the decider of runtime version. There is nothing about the following section that indicates to to the developer that if you change 2.1.0 to 2.1.1 They need to make sure their deployment target has 2.1.1 prebaked in it.

  <ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore.App" Version="2.1.0"/>
  </ItemGroup>

As a .NET developer I'm trained to think that dotnet publish will create me a portable package bundle that will work in any .NET Core 2.1 execution environment.

What I see happening is .NET developers are going to run into some problem that they think is due to the version of libraries so in Visual Studio they will see that have a half dozen dependencies that are slightly out of date so they go and upgrade them all thinking they now have all of the latest bug fixes. Unwittingly they have now upgrade their runtime version and their deployments are failing for a new reason. Whats worse is often the deployment happens days later so they forget about the reference update. This is what happened to us all of the time in the .NET Core 1.x days and I spent so much time debugging developers project files.

Now luckily the error message in this case is better but you still have to figure out what caused the package bundle to require a newer runtime.

normj commented Jul 8, 2018

The reason I disagree with your disagree:) is for the shared runtime it conflates the concept of a NuGet package that is included with your package bundle created at build time and a runtime where the version isn't known until deployment time.

For the sake of conversation I'm ignoring the implicit version case. I believe we are in agreement that that is a bad practice since that was why this issue was open and I apologize for the conversation going well passed the original intent of the issue. In our upcoming Lambda tooling for .NET Core we will require a version being set so we can verify compatibility.

Now back to package version being the decider of runtime version. There is nothing about the following section that indicates to to the developer that if you change 2.1.0 to 2.1.1 They need to make sure their deployment target has 2.1.1 prebaked in it.

  <ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore.App" Version="2.1.0"/>
  </ItemGroup>

As a .NET developer I'm trained to think that dotnet publish will create me a portable package bundle that will work in any .NET Core 2.1 execution environment.

What I see happening is .NET developers are going to run into some problem that they think is due to the version of libraries so in Visual Studio they will see that have a half dozen dependencies that are slightly out of date so they go and upgrade them all thinking they now have all of the latest bug fixes. Unwittingly they have now upgrade their runtime version and their deployments are failing for a new reason. Whats worse is often the deployment happens days later so they forget about the reference update. This is what happened to us all of the time in the .NET Core 1.x days and I spent so much time debugging developers project files.

Now luckily the error message in this case is better but you still have to figure out what caused the package bundle to require a newer runtime.

@davidfowl

This comment has been minimized.

Show comment
Hide comment
@davidfowl

davidfowl Jul 8, 2018

Member

Yes I'm aware of our history. Here's what I want to figure out; ASP.NET Core 2.1.1 gets released and people want to use it. There are 2 cases here:

  1. I'm using packages
  2. I'm using the shared framework

For arguments sake, assume the package version doesn't imply the runtime version and we have another msbuild property, <AspNetCoreSharedFrameworkRuntimeVersion>. We release 2.1.1 so all some of the nuget packages end up being 2.1.1. In the first case (package based ASP.NET Core app), I manually update my <PackageReference> to 2.1.1 and now I'm using it.

In the second case, what do i do to use 2.1.1? I first have to install the new 2.1.1 runtime (either via the new SDK or via an explicit runtime download). Assume my application looks like this:

<Project Sdk="Microsoft.NET.Sdk.Web">

  <PropertyGroup>
    <TargetFramework>netcoreapp2.1</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore.App" Version="2.1.0" />
    <PackageReference Include="Microsoft.EntityFrameworkCore.Proxies" Version="2.1.0" />
  </ItemGroup>

</Project>

Does it end up looking like this as a result of the upgrade?

<Project Sdk="Microsoft.NET.Sdk.Web">

  <PropertyGroup>
    <TargetFramework>netcoreapp2.1</TargetFramework>
<AspNetCoreSharedFrameworkRuntimeVersion>2.1.1</AspNetCoreSharedFrameworkRuntimeVersion>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore.App" Version="2.1.1" />
    <PackageReference Include="Microsoft.EntityFrameworkCore.Proxies" Version="2.1.1" />
  </ItemGroup>

</Project>

Is there still a 2.1.1 version of Microsoft.AspNetCore.App available on nuget.org or do we stop shipping the meta package completely?

Member

davidfowl commented Jul 8, 2018

Yes I'm aware of our history. Here's what I want to figure out; ASP.NET Core 2.1.1 gets released and people want to use it. There are 2 cases here:

  1. I'm using packages
  2. I'm using the shared framework

For arguments sake, assume the package version doesn't imply the runtime version and we have another msbuild property, <AspNetCoreSharedFrameworkRuntimeVersion>. We release 2.1.1 so all some of the nuget packages end up being 2.1.1. In the first case (package based ASP.NET Core app), I manually update my <PackageReference> to 2.1.1 and now I'm using it.

In the second case, what do i do to use 2.1.1? I first have to install the new 2.1.1 runtime (either via the new SDK or via an explicit runtime download). Assume my application looks like this:

<Project Sdk="Microsoft.NET.Sdk.Web">

  <PropertyGroup>
    <TargetFramework>netcoreapp2.1</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore.App" Version="2.1.0" />
    <PackageReference Include="Microsoft.EntityFrameworkCore.Proxies" Version="2.1.0" />
  </ItemGroup>

</Project>

Does it end up looking like this as a result of the upgrade?

<Project Sdk="Microsoft.NET.Sdk.Web">

  <PropertyGroup>
    <TargetFramework>netcoreapp2.1</TargetFramework>
<AspNetCoreSharedFrameworkRuntimeVersion>2.1.1</AspNetCoreSharedFrameworkRuntimeVersion>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore.App" Version="2.1.1" />
    <PackageReference Include="Microsoft.EntityFrameworkCore.Proxies" Version="2.1.1" />
  </ItemGroup>

</Project>

Is there still a 2.1.1 version of Microsoft.AspNetCore.App available on nuget.org or do we stop shipping the meta package completely?

@dasMulli

This comment has been minimized.

Show comment
Hide comment
@dasMulli

dasMulli Jul 8, 2018

We are investigating ref assemblies for 3.0, but that's going to be a lot of work to get right.

+💯 agree. It's something we plan to address in 3.0.

Looking forward to it. The hard part about the recent changes is mostly that keeping devs up-to-date on the current best practice to configure problems and investigate build issues both in person and over StackOverflow / GH issues / slack etc. is becoming an exercise in futility since it changes constantly.
The way this is currently solved for M.NETCore.App is pretty neat (ref assemblies + conflict resolution with old System.* packages being removed during the build), but there are always N+1 options on how to do it. Only thing people are still a little confused about is that the NuGet package manager shows "blocked by project" since the SO questions about it are still showing high number of views and upvotes.

dasMulli commented Jul 8, 2018

We are investigating ref assemblies for 3.0, but that's going to be a lot of work to get right.

+💯 agree. It's something we plan to address in 3.0.

Looking forward to it. The hard part about the recent changes is mostly that keeping devs up-to-date on the current best practice to configure problems and investigate build issues both in person and over StackOverflow / GH issues / slack etc. is becoming an exercise in futility since it changes constantly.
The way this is currently solved for M.NETCore.App is pretty neat (ref assemblies + conflict resolution with old System.* packages being removed during the build), but there are always N+1 options on how to do it. Only thing people are still a little confused about is that the NuGet package manager shows "blocked by project" since the SO questions about it are still showing high number of views and upvotes.

@davidfowl

This comment has been minimized.

Show comment
Hide comment
@davidfowl

davidfowl Jul 8, 2018

Member

@dasMulli it’s much harder for ASP.NET Core because we ship as both packages and as a shared framwork. Most of Microsoft.NETCore.App isn’t made up of packages so conflict resolution mostly has to deal with old binaries. ASP.NET Core needs to deal with cases where libraries depend on patch versions and can easily lift you out on the shared framework. In essence it’s much easier to create conflicts if they are conflicting ways to represent the same thing.

Member

davidfowl commented Jul 8, 2018

@dasMulli it’s much harder for ASP.NET Core because we ship as both packages and as a shared framwork. Most of Microsoft.NETCore.App isn’t made up of packages so conflict resolution mostly has to deal with old binaries. ASP.NET Core needs to deal with cases where libraries depend on patch versions and can easily lift you out on the shared framework. In essence it’s much easier to create conflicts if they are conflicting ways to represent the same thing.

@normj

This comment has been minimized.

Show comment
Hide comment
@normj

normj Jul 8, 2018

Okay I now understand that at build time some version number for Microsoft.AspNetCore.App is required to resolve dependencies for packages with dependencies included with Microsoft.AspNetCore.App. So defaulting to the minimum 2.1.0 to support rolling forward will not work.

Does using Microsoft.AspNetCore.App without using the shared runtime or self contained have a use case? In a .NET Core 2.1.0 dev environment just simply updating the PackageReference to 2.1.1 won't work in the local dev/build stage.

Given the requirement for a version number I would prefer just the property is set. It basically serves the same purpose as the PackageReference but its difference in nature stands out more.

<Project Sdk="Microsoft.NET.Sdk.Web">

  <PropertyGroup>
    <TargetFramework>netcoreapp2.1</TargetFramework>
    <AspNetCoreSharedFrameworkRuntimeVersion>2.1.1</AspNetCoreSharedFrameworkRuntimeVersion>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.EntityFrameworkCore.Proxies" Version="2.1.1" />
  </ItemGroup>

</Project>

The value can easily be adjust at build as well

dotnet publish /p:AspNetCoreSharedFrameworkRuntimeVersion=2.1.0

I would think it would be possible to build better error messaging as well. If I was using ASP.NET Core runtime 2.1.0 but a dependency like Microsoft.EntityFrameworkCore.Proxies is requiring 2.1.1. The tooling should be able to detect the issue and tell the developer that this package requires a newer version of the ASP.NET Core runtime and tell them to install the newer patch version and update the AspNetCoreSharedFrameworkRuntimeVersion property.

I recognize this is mostly a cosmetic change and I now have a better understanding of the challenge. I do highly prefer the more explicit we can be about how things are working the less frustrated we will make developers when version matching doesn't line up.

normj commented Jul 8, 2018

Okay I now understand that at build time some version number for Microsoft.AspNetCore.App is required to resolve dependencies for packages with dependencies included with Microsoft.AspNetCore.App. So defaulting to the minimum 2.1.0 to support rolling forward will not work.

Does using Microsoft.AspNetCore.App without using the shared runtime or self contained have a use case? In a .NET Core 2.1.0 dev environment just simply updating the PackageReference to 2.1.1 won't work in the local dev/build stage.

Given the requirement for a version number I would prefer just the property is set. It basically serves the same purpose as the PackageReference but its difference in nature stands out more.

<Project Sdk="Microsoft.NET.Sdk.Web">

  <PropertyGroup>
    <TargetFramework>netcoreapp2.1</TargetFramework>
    <AspNetCoreSharedFrameworkRuntimeVersion>2.1.1</AspNetCoreSharedFrameworkRuntimeVersion>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.EntityFrameworkCore.Proxies" Version="2.1.1" />
  </ItemGroup>

</Project>

The value can easily be adjust at build as well

dotnet publish /p:AspNetCoreSharedFrameworkRuntimeVersion=2.1.0

I would think it would be possible to build better error messaging as well. If I was using ASP.NET Core runtime 2.1.0 but a dependency like Microsoft.EntityFrameworkCore.Proxies is requiring 2.1.1. The tooling should be able to detect the issue and tell the developer that this package requires a newer version of the ASP.NET Core runtime and tell them to install the newer patch version and update the AspNetCoreSharedFrameworkRuntimeVersion property.

I recognize this is mostly a cosmetic change and I now have a better understanding of the challenge. I do highly prefer the more explicit we can be about how things are working the less frustrated we will make developers when version matching doesn't line up.

@dasMulli

This comment has been minimized.

Show comment
Hide comment
@dasMulli

dasMulli Jul 8, 2018

ASP.NET Core needs to deal with cases where libraries depend on patch versions and can easily lift you out on the shared framework

@davidfowl Do you actively want to allow this or do these cases happen by accident by library authors?
e.g. if I'm authoring a netstandard lib that uses the dependency injection packages, I may want to use the latest patch to make sure that it ends up being used on .net framework.
If a .NET Core App using a shared framework references this package, is there a need to be lifted out of the shared framework (they would end up with fixes eventually through runtime updates)? Or could it raise the minimum shared framework version required by the application?

dasMulli commented Jul 8, 2018

ASP.NET Core needs to deal with cases where libraries depend on patch versions and can easily lift you out on the shared framework

@davidfowl Do you actively want to allow this or do these cases happen by accident by library authors?
e.g. if I'm authoring a netstandard lib that uses the dependency injection packages, I may want to use the latest patch to make sure that it ends up being used on .net framework.
If a .NET Core App using a shared framework references this package, is there a need to be lifted out of the shared framework (they would end up with fixes eventually through runtime updates)? Or could it raise the minimum shared framework version required by the application?

@bording

This comment has been minimized.

Show comment
Hide comment
@bording

bording Jul 8, 2018

When using an SDK-style project with Microsoft.NET.Sdk.Web, why can't the project implicitly reference the correct packages without needing the package reference in the project at all, much like Microsoft.NET.Sdk does already for Microsoft.NETCore.App and NETStandard.Library? If you try to have an explicit reference to one of those, you get a warning telling you to remove it.

People would be much less inclined to try and incorrectly update the package if they don't see a package reference for it. The NuGet UI already tells you that you should update your SDK if you want to reference a newer version of those packages.

bording commented Jul 8, 2018

When using an SDK-style project with Microsoft.NET.Sdk.Web, why can't the project implicitly reference the correct packages without needing the package reference in the project at all, much like Microsoft.NET.Sdk does already for Microsoft.NETCore.App and NETStandard.Library? If you try to have an explicit reference to one of those, you get a warning telling you to remove it.

People would be much less inclined to try and incorrectly update the package if they don't see a package reference for it. The NuGet UI already tells you that you should update your SDK if you want to reference a newer version of those packages.

@tmds

This comment has been minimized.

Show comment
Hide comment
@tmds

tmds Jul 9, 2018

+1 to have an easy way to build for a specific asp.net core version.

By this I meant, the template could be:

<Project Sdk="Microsoft.NET.Sdk.Web">
  <PropertyGroup>
    <TargetFramework>netcoreapp2.1</TargetFramework>
    <AspNetCoreVersion>2.1.1</AspNetCoreVersion>
  </PropertyGroup>
  <ItemGroup>
    <Folder Include="wwwroot\" />
  </ItemGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore.App" Version="$(AspNetCoreVersion)"/>
  </ItemGroup>
</Project>

Then we can publish for a specific runtime version:

dotnet publish -c Release /p:AspNetCoreVersion=2.1.0

And encourage using that variable elsewhere:

<PackageReference Include="Microsoft.AspNetCore.Mvc.Testing"  Version="$(AspNetCoreVersion)"/>

That made it difficult to use packages which shared dependencies with the shared framework. In 2.1.1, we tried to fix this by easing off the restrictions and allowing package upgrades, but means users unintentionally upgrade out of the baseline (B), and end up with redundant copies of binaries in their publish output.

Looking at the above, I wonder if being able to reference the implicit version on other packages could have helped (no idea if msbuild supports this).
pseudo:

<PackageReference Include="Microsoft.AspNetCore.App"/>
<PackageReference Include="Microsoft.AspNetCore.Mvc.Testing"  Version="$(AspNetCoreAppVersion)"/> <!-- AspNetCoreAppVersion is resolved version of Microsoft.AspNetCore.App -->

tmds commented Jul 9, 2018

+1 to have an easy way to build for a specific asp.net core version.

By this I meant, the template could be:

<Project Sdk="Microsoft.NET.Sdk.Web">
  <PropertyGroup>
    <TargetFramework>netcoreapp2.1</TargetFramework>
    <AspNetCoreVersion>2.1.1</AspNetCoreVersion>
  </PropertyGroup>
  <ItemGroup>
    <Folder Include="wwwroot\" />
  </ItemGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore.App" Version="$(AspNetCoreVersion)"/>
  </ItemGroup>
</Project>

Then we can publish for a specific runtime version:

dotnet publish -c Release /p:AspNetCoreVersion=2.1.0

And encourage using that variable elsewhere:

<PackageReference Include="Microsoft.AspNetCore.Mvc.Testing"  Version="$(AspNetCoreVersion)"/>

That made it difficult to use packages which shared dependencies with the shared framework. In 2.1.1, we tried to fix this by easing off the restrictions and allowing package upgrades, but means users unintentionally upgrade out of the baseline (B), and end up with redundant copies of binaries in their publish output.

Looking at the above, I wonder if being able to reference the implicit version on other packages could have helped (no idea if msbuild supports this).
pseudo:

<PackageReference Include="Microsoft.AspNetCore.App"/>
<PackageReference Include="Microsoft.AspNetCore.Mvc.Testing"  Version="$(AspNetCoreAppVersion)"/> <!-- AspNetCoreAppVersion is resolved version of Microsoft.AspNetCore.App -->
@natemcmaster

This comment has been minimized.

Show comment
Hide comment
@natemcmaster

natemcmaster Jul 9, 2018

Member

@bording: When using an SDK-style project with Microsoft.NET.Sdk.Web, why can't the project implicitly reference the correct packages without needing the package reference in the project at all, much like Microsoft.NET.Sdk does already for Microsoft.NETCore.App and NETStandard.Library?

We decided not to do this because we can't correctly infer user intent. For example, did the user want to use Microsoft.AspNetCore.App or Microsoft.AspNetCore.All? NETStandard.Library and Microsoft.NETCore.App are different because they map 1:1 to a target framework moniker (netstandard2.0 and netcoreapp2.1). We don't have a target framework moniker which represents ASP.NET Core, and that is something we definitely can't introduce in a patch.

@tmds: the template could be....

These are great ideas; it's an approach we use in aspnet source code and something I personally use on my own projects. Unfortunately, Visual Studio and the NuGet UI do not play nice with MSBuild variables as item metadata. You can't use the NuGet UI for updates since updating packages makes the project look like this:

<Project Sdk="Microsoft.NET.Sdk.Web">

  <PropertyGroup>
    <TargetFramework>netcoreapp2.1</TargetFramework>
    <AspNetCoreAppVersion>2.1.0</AspNetCoreAppVersion>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore.App" Version="2.1.1" />
  </ItemGroup>

</Project>
Member

natemcmaster commented Jul 9, 2018

@bording: When using an SDK-style project with Microsoft.NET.Sdk.Web, why can't the project implicitly reference the correct packages without needing the package reference in the project at all, much like Microsoft.NET.Sdk does already for Microsoft.NETCore.App and NETStandard.Library?

We decided not to do this because we can't correctly infer user intent. For example, did the user want to use Microsoft.AspNetCore.App or Microsoft.AspNetCore.All? NETStandard.Library and Microsoft.NETCore.App are different because they map 1:1 to a target framework moniker (netstandard2.0 and netcoreapp2.1). We don't have a target framework moniker which represents ASP.NET Core, and that is something we definitely can't introduce in a patch.

@tmds: the template could be....

These are great ideas; it's an approach we use in aspnet source code and something I personally use on my own projects. Unfortunately, Visual Studio and the NuGet UI do not play nice with MSBuild variables as item metadata. You can't use the NuGet UI for updates since updating packages makes the project look like this:

<Project Sdk="Microsoft.NET.Sdk.Web">

  <PropertyGroup>
    <TargetFramework>netcoreapp2.1</TargetFramework>
    <AspNetCoreAppVersion>2.1.0</AspNetCoreAppVersion>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore.App" Version="2.1.1" />
  </ItemGroup>

</Project>
@natemcmaster

This comment has been minimized.

Show comment
Hide comment
@natemcmaster

natemcmaster Jul 9, 2018

Member

Thank you, everyone, for the feedback. I'm closing the issue as "fixed" because we do not plan to keep supporting PackageReference without a Version, which was the original intention of this thread. We have merged changes to put back the explicit version on the PackageReference in templates, and produce build warnings when you don't.

That said, I don't believe this is a complete solution for how users should be referencing ASP.NET Core in 2.1. I've opened #3307 for further discussion and follow up. Please take a look as we would also like to do this proposal in 2.1.3.

Member

natemcmaster commented Jul 9, 2018

Thank you, everyone, for the feedback. I'm closing the issue as "fixed" because we do not plan to keep supporting PackageReference without a Version, which was the original intention of this thread. We have merged changes to put back the explicit version on the PackageReference in templates, and produce build warnings when you don't.

That said, I don't believe this is a complete solution for how users should be referencing ASP.NET Core in 2.1. I've opened #3307 for further discussion and follow up. Please take a look as we would also like to do this proposal in 2.1.3.

natemcmaster added a commit to aspnet/Docs that referenced this issue Jul 10, 2018

Rick-Anderson added a commit to aspnet/Docs that referenced this issue Jul 10, 2018

Remove instructions about using the implicit PackageReference feature…
… for AspNetCore.App/All (#7540)

* Remove instructions about using the implicit PackageReference feature for AspNetCore.App/All

cref aspnet/Home#3292

* Minor language updates

* Remove extra line space

* Update metapackage-app.md

@natemcmaster natemcmaster modified the milestones: 2.1.3, 2.2.0-preview1 Jul 18, 2018

@natemcmaster

This comment has been minimized.

Show comment
Hide comment
@natemcmaster

natemcmaster Jul 19, 2018

Member

Hey all, status update. I'm reopening this issue because we had to revert some changes. This call was made after lots and lots of discussion with our partner teams and management. We still intend to remove the versionless PackageRef, but we haven't landed on a replacement for it. Until we do, our templates and tools will continue to support the versionless PackageRef for Microsoft.AspNetCore.App.

One candidate for replacement is being discussed here: #3307. There are several other ideas to address the problem, including those mentioned above in this thread. We will continue to explore those for the 2.2 timeframe, and possibly for a 2.1 SDK (no promises.) In the meantime, I'll keep you posted.

Side note: the default implicit of Microsoft.AspNetCore.App stays pinned at 2.1.1, much like how the implicit package ref to Microsoft.NETCore.App stays pinned to 2.1.0. This is intentional to make your app compatible with more deployment environments. To get security patches, we recommend installing the new runtime version to your deployment servers and rely on the patch-rollforward behavior to run your app on the newer framework version.

Thanks,
Nate

Member

natemcmaster commented Jul 19, 2018

Hey all, status update. I'm reopening this issue because we had to revert some changes. This call was made after lots and lots of discussion with our partner teams and management. We still intend to remove the versionless PackageRef, but we haven't landed on a replacement for it. Until we do, our templates and tools will continue to support the versionless PackageRef for Microsoft.AspNetCore.App.

One candidate for replacement is being discussed here: #3307. There are several other ideas to address the problem, including those mentioned above in this thread. We will continue to explore those for the 2.2 timeframe, and possibly for a 2.1 SDK (no promises.) In the meantime, I'll keep you posted.

Side note: the default implicit of Microsoft.AspNetCore.App stays pinned at 2.1.1, much like how the implicit package ref to Microsoft.NETCore.App stays pinned to 2.1.0. This is intentional to make your app compatible with more deployment environments. To get security patches, we recommend installing the new runtime version to your deployment servers and rely on the patch-rollforward behavior to run your app on the newer framework version.

Thanks,
Nate

@natemcmaster natemcmaster reopened this Jul 19, 2018

@poke

This comment has been minimized.

Show comment
Hide comment
@poke

poke Jul 20, 2018

our templates and tools will continue to support the versionless PackageRef

Support is one thing, but the templates revert means that the version-less PackageReference is currently still the recommended way. Is that correct? Should we instruct users to keep using a version-less reference for now, or is there some other guidance?

I’m thinking about this comment of yours which recommended the versioned reference in preparation for the upcoming changes.

poke commented Jul 20, 2018

our templates and tools will continue to support the versionless PackageRef

Support is one thing, but the templates revert means that the version-less PackageReference is currently still the recommended way. Is that correct? Should we instruct users to keep using a version-less reference for now, or is there some other guidance?

I’m thinking about this comment of yours which recommended the versioned reference in preparation for the upcoming changes.

@natemcmaster

This comment has been minimized.

Show comment
Hide comment
@natemcmaster

natemcmaster Jul 20, 2018

Member

version-less PackageReference is currently still the recommended way. Is that correct?

Yes, until we can land SDK changes updates. When/if that happens, we'll update our guidance and templates.

That comment, by the way, is made in the context of a user who was still on the 2.1.300 SDK (implicit package version == 2.1.0) and they were fighting NU1107 errors. NU1107 goes away with the 2.1.1 metapackage, which is a baseline we intend to keep for the lifetime of the 2.1 release.

The scenarios in which I still recommend adding a version to the package ref are:

  • you want to lift the minimum required version of the shared runtime to 2.1.x
  • you are writing a test project and don't want to use Microsoft.NET.Sdk.Web
  • you need to use a patched version of something newer than what is in the 2.1.1 shared runtime, and upgrading would have cause you to get a copy-local version instead of using the one from the shared runtime.
Member

natemcmaster commented Jul 20, 2018

version-less PackageReference is currently still the recommended way. Is that correct?

Yes, until we can land SDK changes updates. When/if that happens, we'll update our guidance and templates.

That comment, by the way, is made in the context of a user who was still on the 2.1.300 SDK (implicit package version == 2.1.0) and they were fighting NU1107 errors. NU1107 goes away with the 2.1.1 metapackage, which is a baseline we intend to keep for the lifetime of the 2.1 release.

The scenarios in which I still recommend adding a version to the package ref are:

  • you want to lift the minimum required version of the shared runtime to 2.1.x
  • you are writing a test project and don't want to use Microsoft.NET.Sdk.Web
  • you need to use a patched version of something newer than what is in the 2.1.1 shared runtime, and upgrading would have cause you to get a copy-local version instead of using the one from the shared runtime.
@jeffschwMSFT

This comment has been minimized.

Show comment
Hide comment
@jeffschwMSFT

jeffschwMSFT commented Jul 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment