Skip to content
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

Enable projects to use the SDK TargetFramework #29949

Open
richlander opened this issue Jan 14, 2023 · 17 comments
Open

Enable projects to use the SDK TargetFramework #29949

richlander opened this issue Jan 14, 2023 · 17 comments
Assignees
Milestone

Comments

@richlander
Copy link
Member

richlander commented Jan 14, 2023

The .NET SDK comes with a targeting pack and a runtime (in addition to build tools). If you compile an app for the same version as the SDK, then you can use the targeting pack and runtime that comes with the SDK. Yeahh! If you compile an app for an older version, then you need an older version of the runtime and targeting pack. You can either install those or update the project file. There is no way to that via the commandline. You can change the RID via the commandline but not the TFM.

The .NET SDK offers a variety of controls for controlling compile-time vs runtime framework versions. The model, however, could be better. This is particularly obvious if you are wanting to test existing/old apps on a newer SDK.

FYI: I'm using older .NET versions for my examples, only because they were convenient. This experience hasn't changed recently.

This is the base experience today, when SDK and TargetFramework don't match:

root@3ed29927af5a:/core/samples/dotnet-runtimeinfo# dotnet --version
6.0.113
root@3ed29927af5a:/core/samples/dotnet-runtimeinfo# dotnet run
You must install or update .NET to run this application.

App: /core/samples/dotnet-runtimeinfo/bin/Debug/netcoreapp3.1/dotnet-runtimeinfo
Architecture: x64
Framework: 'Microsoft.NETCore.App', version '3.1.0' (x64)
.NET location: /usr/lib/dotnet

The following frameworks were found:
  6.0.13 at [/usr/lib/dotnet/shared/Microsoft.NETCore.App]

You cannot updated TargetFramework from the commandline, AFAICT.

root@3ed29927af5a:/core/samples/dotnet-runtimeinfo# cat dotnet-runtimeinfo.csproj | grep TargetFramework
    <TargetFramework>netcoreapp3.1</TargetFramework>
root@3ed29927af5a:/core/samples/dotnet-runtimeinfo# dotnet run -p:TargetFramework=net6.0
/usr/lib/dotnet/sdk/6.0.113/Sdks/Microsoft.NET.Sdk/targets/Microsoft.PackageDependencyResolution.targets(267,5): error NETSDK1005: Assets file '/core/samples/dotnet-runtimeinfo/obj/project.assets.json' doesn't have a target for 'net6.0'. Ensure that restore has run and that you have included 'net6.0' in the TargetFrameworks for your project. [/core/samples/dotnet-runtimeinfo/dotnet-runtimeinfo.csproj]

The build failed. Fix the build errors and run again.

The RollForward property works pretty well. It can be added to a project file or injected via ENV or the CLI (via -p:RollFoward). RollForward model has the downside that an old target framework needs to be downloaded, at least in the case that you want to use a new SDK for both compiling and running the app.

Instead, it would nice if there was a way to say "roll forward by using the SDK target framework". That would result in using SDK-provided assets and to naturally using the SDK runtime version.

We could offer a way to specify what the minimum target framework is for a project and infer the actual target framework from the SDK (assuming it is same or higher). That way, you wouldn't ever get weird compile-time failures due to missing APIs.

Perhaps via the following property:

<MinimumTargetFramework>net6.0</MinimumTargetFramework>

or possibly:

<TargetFramework>net6.0</TargetFramework>
<UseSdkTargetFramework>true</UseSdkTargetFramework>

The second property could also be UseSdkDefaults if we wanted something more general.

This approach would have a few benefits:

  • The SDK version would become the primary version concept to reason about (for folks that adopted this model).
  • The (minimum) TargetFramework property would solely describe API surface area, as opposed to also determining minimum runtime version.
  • The SDK-provided targeting pack would be used, skipping an extra download.
  • The RollForward property wouldn't need to be used as part of development, leaving it only for deployed apps.

Some apps would break with this model, either due to breaking changes, warning waves or something else. It's less about the apps breaking and more about the care factor of the apps breaking. There are two solutions for that: don't use the feature, or use global.json. For people that use global.json, this feature would still have significant value.

This model would be more complicated for compound TFMs, like the ones we have for Android and iOS. That would require some more thought. I thought we should start out with the base premise before looking at those TFMs.

This model wouldn't work (at least not naturally) with multi-targeting. That seems fine, both because multi-targeting is uncommon with apps and because multi-targeting is an intentional model that isn't amenable to policy solutions like this.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-NetSDK untriaged Request triage from a team member labels Jan 14, 2023
@richlander
Copy link
Member Author

I noticed that I've raised this previously.

#11613

@shanselman
Copy link
Contributor

:shipit: why don't we do this? I love user-delightful stuff like this. I hit this daily.

@baronfel baronfel self-assigned this Jan 19, 2023
@richlander
Copy link
Member Author

An alternative (and likely better) design is to leave TargetFramework as-is and to introduce a boolean property like:UseSdkTargetFramework, UseSdkRuntimeVersion, or UseRuntimeVersion.

@baronfel
Copy link
Member

I definitely think we should do something. There are several different user experiences we could provide, on a sliding scale of potential work/disruption:

  • Provide the SDK TFM as a property that's read very early on in the build process, then let the user interpolate it as an MSBuild property: <TargetFramework>$(SDKTargetFramework)</TargetFramework>

    • Pro: very easy to implement on our side, minimal new concepts, easyish to add tooling for (MSBuild XSD could add the new property)
    • Con: exposes potentially-new users to MSBuild property syntax very early, may not be the easiest hurdle. More concepts to understand to get started.
  • Provide the UseSdkTargetFramework boolean property:

    • Pro: very user-friendly. easy-ish to add tooling support for (MSBuild XSD could add the new property and valid boolean values relatively easily). would likely imply doing solution 1 as well from a technical perspective. the new property is 'narrow' - allows to infer intent.
    • Con: discovery question - properties are already not that discoverable to users. unclear guidance - a user that's used to TFMs has to learn when to use this new property
  • Provide a more general UseSdkDefaults boolean property:

    • Pro: clearly expresses intent. easy-ish to add tooling support for (MSBuild XSD could add the new property and valid boolean values relatively easily). allows for changing the meaning of 'SDK Defaults' over time without impacting user projects.
    • Con: unclear semantics across SDK versions - would need clear documentation/editor guidance.

My preference is the second option - the third, while tempting, is very broad and I would worry about giving users clarity about what it means version-to-version, as well as having firm guidance for folks contributing to the SDK to prevent it becoming a 'kitchen sink' property.

@dsplaisted
Copy link
Member

  • Provide the SDK TFM as a property that's read very early on in the build process, then let the user interpolate it as an MSBuild property: <TargetFramework>$(SDKTargetFramework)</TargetFramework>

We have a NETCoreAppMaximumVersion property which is defined in Microsoft.NETCoreSdk.BundledVersions.props. So it's not pretty, but you could do this:

<TargetFramework>net$(NETCoreAppMaximumVersion)</TargetFramework>

We should think twice about offering something like this though. Projects which roll forward the TargetFramework will break way more often when we update the SDK. Lots of times we put new behavior behind a TargetFramework version check to avoid breaking projects until they are specifically retargeted. Having projects that roll forward might make people more hesitant to update to a newer SDK, when we really would like them to update regardless of the TargetFramework they're using.

@richlander
Copy link
Member Author

I agree with you @dsplaisted although it is odd to gate upgrades on both global.json / SDK version and TargetFramework. This is being proposed only as an opt-in. In fact, it makes testing a new SDK much easier, both with n and n-1 TFMs.

<TargetFramework>net$(NETCoreAppMaximumVersion)</TargetFramework>

That's not actually that bad, however, it requires the project to be written that way. I was hoping for a way to augment this value outside of the project.

It may not be obvious, but we'd equally need to support the following flows:

  • net7.0 -> net8.0
  • net7.0-android -> net8.0-android

The NETCoreAppMaximumVersion works with that. If we went with UseSdkTargetFramework, we'd need to support replacing just the first part of the TFM.

@KalleOlaviNiemitalo
Copy link

We have a NETCoreAppMaximumVersion property which is defined in Microsoft.NETCoreSdk.BundledVersions.props.

IIRC this is defined after Directory.Build.props is imported, which makes it harder to do solution-wide conditional logic on SDK versions.

@richlander
Copy link
Member Author

richlander commented Jan 27, 2023

That's a good point.

It isn't written down yet, but we're moving to model (I think) where:

  • Convenience experiences like the one I'm proposing are primarily for project build.
  • Solution builds may require a D.b.p.

@richlander
Copy link
Member Author

richlander commented Jan 31, 2023

Perhaps we should put a version number at the top:

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

  <PropertyGroup>
    <Nullable>enable</Nullable>
    <ImplicitUsings>enable</ImplicitUsings>
  </PropertyGroup>

</Project>

That completes the pivot. The 8.0 would be a minimum.

Or better yet:

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

</Project>

Obviously, there might be an ItemGroup here.

@mhutch @DamianEdwards

@DamianEdwards
Copy link
Member

So there's already a versioning mechanism for SDKs, it just isn't used widely today. Are you suggesting that specifying the SDK version in this way should default the TFM (and other properties potentially) to "current SDK defaults"?

@richlander
Copy link
Member Author

Yes. Certainly, whether it is opt-in min or exact is a detail.

There are really two suggestions here:

  • Simplify project files by making SDK type the key concept.
  • Enable the min version idea through some variation of that.

@ViktorHofer
Copy link
Member

ViktorHofer commented Jan 22, 2024

Here's a slightly different look at this. To build our own repositories with Arcade and avoid manually updating TFMs, we added floating TFMs properties, i.e. NetCurrent: https://github.com/dotnet/arcade/blob/abddd0bd5145578246dcadda264c7557f2a935a9/src/Microsoft.DotNet.Arcade.Sdk/tools/TargetFrameworkDefaults.props#L14

I would find it useful if we would publicly expose those in the SDK. They would get updated when the SDK revs to the next version of .NET.

Examples

  1. Single-targeting project
<Project>

  <PropertyGroup>
    <TargetFramework>$(NetCurrent)</TargetFramework>
  <PropertyGroup>

</Project>
  1. Multi-targeting project
<Project>

  <PropertyGroup>
    <TargetFrameworks>$(NetCurrent);$(NetMinimum)</TargetFrameworks>
  <PropertyGroup>

</Project>

@ViktorHofer
Copy link
Member

cc @mmitche @jaredpar @ericstj

@jaredpar
Copy link
Member

I don't have any real concerns with exposing $(NetCurrent). The semantics for that property and when it changes are very clear to me and I imagine pretty much all of our customers.

I'd have concerns with $(NetMinimum) cause it comes with a number of questions. If it's the minimum currently supported runtime then that comes with some questions. Consider as a concrete example that at the time of this writing net7.0 is the effective minimum runtime: lowest runtime that is in active service. That goes out of service in May but when would we change $(NetMinumum) to be net8.0:

  1. Immediately: that would mean say 8.0.300 changes $(NetMinimum) to be net8.0 which feels very much like breaking change territory in a minor update
  2. At the next major .NET SDK release: that happens in November so there would be ~5 months where $(NetMinimum) was incorrect.

@baronfel
Copy link
Member

I think we should scope this to $(NetCurrent) for the first pass and focus on making that work well with tooling. There's IMO enough value there to be useful.

To address concerns about breaking projects:

  • users that don't explicitly set a TFM are opting in to this behavior
  • users that want to control their SDK can set a global.json (and indeed many CI/CD pipelines do this already)
  • users have a really easy way to control this - set a TFM

@baronfel baronfel added this to the Discussion milestone Jan 30, 2024
@baronfel baronfel removed the untriaged Request triage from a team member label Jan 30, 2024
@WeihanLi
Copy link
Contributor

Any update? Seemed the $(NetCurrent) doesn't working so far

@ViktorHofer
Copy link
Member

Not implemented yet but it would be great to get this in for .NET 10. It would simplify things a lot.

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

No branches or pull requests

9 participants