warn on netcoreapp{5,6,7...}

[kasperk81]

netcoreapp was replaced with net since v5.

nuget should issue a deprecation warning or error when <TargetFramework>netcoreapp7.0</TargetFramework> etc. are encountered, instead of allowing the wrong tfms forever: https://github.com/NuGet/NuGet.Client/blob/4a6b7bbad3fb31eb6d8c5a4242649419dedd0f29/src/NuGet.Core/NuGet.Frameworks/NuGetFrameworkFactory.cs#L689 the only valid form for modern versions is net<major>.<minor>, all other forms are "wrong" on user-end and easy to fix.

[ghost]

Issue is missing Type label, remember to add a Type label

[jeffkl]

Team Triage: Consider this work for .NET 8 since it would be a breaking change to warn/error when using netcoreapp

[kasperk81]

it would be a breaking change to warn/error when using netcoreapp

here are some github stats:

netcoreapp5.0: https://grep.app/search?q=netcoreapp5&case=true (210)
netcoreapp6.0: https://grep.app/search?q=netcoreapp6&case=true (101)
netcoreapp7.0: https://grep.app/search?q=netcoreapp7&case=true (3)

warning these few people is much better than papering over their mistakes. new c# / roslyn versions introduce these kind of warnings all the time where previous version was fine with code and after the toolchain update, there are warnings and errors. there is no need to "play nice" with silly mistakes made by users.

in addition to netcoreapp7.0 and netcoreapp70, nuget also parses netcoreapp7 (wrong name, without minor version). this is just too much parsing work for no good justification.

the "issue" is that since nuget is walking extra miles nobody asked for, msbuild, sdk and vs are also blindly supporting the wrong forms. that forces third party implementations working with project files to also support all kinds of weird forms. so it actually hurting the ecosystem.

Consider this work for .NET 8

ok. so after parsing .net 7 and warning for bad forms, let it be an error for .net 8+ if tfm does not start with net or starts with netcoreapp or count of period . != (exactly) 1

[zivkan]

I'm not opposed to suggesting customers use the recommended values, but I feel like the last comment escalated in language significantly. Is there something behind this? Kind of like an XY problem?

netcoreapp was replaced with net since v5.

that's not technically true: https://github.com/dotnet/designs/blob/main/accepted/2020/net5/net5.md#why-is-net-50s-tfi-still-mapped-to-netcoreapp

the "issue" is that since nuget is walking extra miles nobody asked for, msbuild, sdk and vs are also blindly supporting the wrong forms. that forces third party implementations working with project files to also support all kinds of weird forms. so it actually hurting the ecosystem.

Claiming "walking extra miles" feels wrong, at least as someone who was there since before the net5.0 TFM spec. It would take more code to prevent (or warn) about netcoreapp5.0 than to leave it as it is.

Anyway, the current design is that TargetFrameworks are just aliases. The .NET Project System's public coding guidelines document explicitly tells contributors not to try to parse TargetFramework, or a few other properties that might just be aliases. There's a design bug in the assets file format that prevents it from working for certain scenarios (wanting to use the same canonical TFM more than once), but there's a feature request to change it, which is needed to unblock a Microsoft internal partner for a feature they want: #5154. So, expect non-parsable TargetFrameworks to be more common in the future.

So, anyone making assumptions about TargetFramework being parsable, or using a specific format, is going to encounter problems. Within MSBuild, anyone wanting to parse values should only use TargetFrameworkIdentifier, TargetFrameworkVersion, TargetFrameworkMoniker, TargetPlatformIdentifier, TargetFrameworkVersion and TargetPlatformMoniker. Those have guaranteed formats. Outside of MSBuild, I'd need to know more about the scenario to know what to suggest. Unfortunately the safest bet is to use Sytem.Reflection.Metadata or something similar (other "decompiling" tools/libraries) to find the TargetFramework and TargetPlatform assembly attributes, then parse those values.

So, today this works just fine and is completely valid:

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

  <PropertyGroup>
    <TargetFrameworks>Prod;Preview</TargetFrameworks>
  </PropertyGroup>

  <PropertyGroup Condition=" '$(TargetFramework)' == 'Prod' ">
    <TargetFrameworkIdentifier>.NETCoreApp</TargetFrameworkIdentifier>
    <TargetFrameworkVersion>6.0</TargetFrameworkVersion>
  </PropertyGroup>

  <PropertyGroup Condition=" '$(TargetFramework)' == 'Preview' ">
    <TargetFrameworkIdentifier>.NETCoreApp</TargetFrameworkIdentifier>
    <TargetFrameworkVersion>7.0</TargetFrameworkVersion>
  </PropertyGroup>

</Project>

This is a multi-targeting net6.0 and net7.0 project, just defined explicitly (since the TargetFramework can't be parsed), rather than implicitly. The build outputs go to bin\Debug\prod\ and bin\Debug\preview\.

I never talked to the BenchmarkDotNet people about this, but I think once NuGet properly supports TFM aliases, it'd be great to be able to do perf benchmarks between two different versions of a package (on the same runtime) in this way. Currently we have to run the benchmark multiple times and then manually merge the results and manually calculate ratios.