net5.0 TFM defines both NET5_0 and NETCOREAPP3_1

[Tratcher]

Description

When building a project that cross compiles between net5.0 and netcoreapp3.1 there's an ordering problem with the #if's. If the NETCOREAPP3_1 condition is put first then the net5.0 TFM will compile using it, but if the NET5_0 condition is first then that is choosen.

Configuration

5.0.100-preview.8.20417.9 SDK

Windows 10 2004 (19041.450) x64

Regression?

Yes. This was working in prior preview SDKs when using NETCOREAPP5_0.

Other information

Program.cs:

using System;
using System.Runtime.CompilerServices;

namespace TfmConsole
{
    class Program
    {
        static void Main(string[] args)
        {
            Broken();
            Fixed();
        }

        static void Broken()
        {
            Console.WriteLine("Hello Broken World! " + Environment.Version);
#if NETCOREAPP3_1
            Console.WriteLine("NETCOREAPP3_1");
#elif NET5_0
            Console.WriteLine("NET5_0");
#elif NETCOREAPP5_0
            Console.WriteLine("NETCOREAPP5_0");
#else
#error A target framework was added to the project and needs to be added to this condition.
#endif
        }
        static void Fixed()
        {
            Console.WriteLine("Hello Fixed World! " + Environment.Version);
#if NET5_0
            Console.WriteLine("NET5_0");
#elif NETCOREAPP5_0
            Console.WriteLine("NETCOREAPP5_0");
#elif NETCOREAPP3_1
            Console.WriteLine("NETCOREAPP3_1");
#else
#error A target framework was added to the project and needs to be added to this condition.
#endif
        }
    }
}

csproj

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

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFrameworks>net5.0;netcoreapp3.1</TargetFrameworks>
  </PropertyGroup>

</Project>

Output:

C:\temp\TfmConsole\TfmConsole>dotnet run --framework netcoreapp3.1
Hello Broken World! 3.1.7
NETCOREAPP3_1
Hello Fixed World! 3.1.7
NETCOREAPP3_1

C:\temp\TfmConsole\TfmConsole>dotnet run --framework net5.0
Hello Broken World! 5.0.0
NETCOREAPP3_1
Hello Fixed World! 5.0.0
NET5_0

C:\temp\TfmConsole\TfmConsole>dotnet run --framework netcoreapp5.0
Hello Broken World! 5.0.0
NETCOREAPP3_1
Hello Fixed World! 5.0.0
NET5_0

[Dotnet-GitSync-Bot]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[Tratcher]

Simpler repro:

            Console.WriteLine("Hello Broken World! " + Environment.Version);
#if NETCOREAPP3_1
            Console.WriteLine("NETCOREAPP3_1");
#endif
#if NET5_0
            Console.WriteLine("NET5_0");
#endif

Output:

C:\temp\TfmConsole\TfmConsole>dotnet run --framework netcoreapp3.1
Hello Broken World! 3.1.7
NETCOREAPP3_1

C:\temp\TfmConsole\TfmConsole>dotnet run --framework net5.0
Hello Broken World! 5.0.0
NETCOREAPP3_1
NET5_0

It looks like both NETCOREAPP3_1 and NET5_0 are being defined by the net5.0 TFM.

[Tratcher]

This also reproduces with the 5.0.100-rc.1.20452.6 SDK.

[sebastienros]

Note that there is no need to multi target to repro the issue. Any app with <TargetFramework>net5.0</TargetFramework> will have NETCOREAPP3_1 defined.

[benaadams]

Is is so apps with NETCOREAPP3_1 defined that haven't been updated for .NET 5 don't revert to a pre-core 3.1 behaviour? (e.g. is NETCOREAPP2_0 also defined?

[sebastienros]

NETCOREAPP is defined, not NETCOREAPP2_0

[Pilchie]

Tag @marcpopMSFT

[dougbu]

Wow, it looks like this behaviour (which violates the Principle of Least Surprise) is intentional ☹️ See

sdk/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.BeforeCommon.targets

Lines 214 to 221 in 9b383d6

	<!-- Add conditional compilation symbols for target platform backwards compatibility. -->
	<Target Name="GeneratePlatformCompatibleDefineConstants"
	Condition=" '$(DisableImplicitFrameworkDefines)' != 'true' and '$(TargetPlatformIdentifier)' != '' and '$(TargetFrameworkIdentifier)' == '.NETCoreApp' and $([MSBuild]::VersionGreaterThanOrEquals($(TargetFrameworkVersion), 5.0)) " >
	<ItemGroup>
	<_SupportedPlatformCompatibleVersions Include="@(SdkSupportedTargetPlatform)" Condition=" %(Identity) != '' and $([MSBuild]::VersionLessThan(%(Identity), $(TargetPlatformVersion))) " />
	<_ImplicitDefineConstant Include="@(_SupportedPlatformCompatibleVersions->'$(TargetPlatformIdentifier.ToUpper())%(Identity)'->Replace('.', '_'))" />
	</ItemGroup>
	</Target>

Why❔ And why isn't there an msbuild property that can disable the extra defines (and only the extra defines)❔

[Tratcher]

PR: #11236
Spec: https://github.com/dotnet/designs/blob/master/accepted/2020/net5/net5.md#preprocessor-symbols

"In order to make it easier to update code, especially when doing multi-targeting, we should make them additive, so that when targeting net6.0 both NET6_0 and NET5_0 are defined." Surprise!

[sharwell]

It's fine if the SDK defines both NETCOREAPP and NET5_0 (the former is allowed to be renamed to just NET but not required). However, the SDK must not define both NETCOREAPP3_1 and NET5_0. Doing so would break the entire mental model of the feature and we'll be stuck with the ugly workaround of writing custom build tooling that fails the build if a user fails to manually remove the extraneous definition.

[dougbu]

ae54a52 introduced this behaviour.

[dougbu]

As @jaredpar asked, how should we detect the TFM is exactly netcoreapp3.1 in C#❔

The options aren't pretty:

<ItemGroup>
   <SdkSupportedTargetPlatform Remove=".NETCoreApp,Version=v3.1" Condition=" '$(TargetFramework)' !- 'netcoreapp3.1' " />
</ItemGroup>

or use

#if !NET5_0 && NETCOREAPP3_1

everywhere you'd previously use

#if NETCOREAPP3_1

and be very careful when attempting to detect new TFMs e.g. the following won't error out from here on

#if NETCOREAPP5_0
internal static readonly Version Http2Version = HttpVersion.Version20;
internal static readonly Version Http11Version = HttpVersion.Version11;
#elif NETCOREAPP3_1
internal static readonly Version Http2Version = new Version(2, 0);
internal static readonly Version Http11Version = new Version(1, 1);
#else
#error A target framework was added to the project and needs to be added to this condition.
#endif

[dsplaisted]

To clarify why we thought this was a good idea:

We think that having compilation constants that mean “version X or greater” will lead to more maintainable code than having constants that are exact matches.

If you write conditional code today for .NET 5:

#if NET5
    // Call .NET 5 API
#else
    // Fall back to .NET Core 3.1 API
#endif

Then you should expect that the .NET 5 code would continue to work for .NET 6. You shouldn’t need to write an #if today that targets exactly .NET 5 but no later versions. When .NET 6 comes out, you should be able to retarget your project to it (or add it as a new target) without having to audit all of your conditional compilation code. With the old way of defining these constants, if you compiled the code for .NET 6 then you would have gotten the .NET Core 3.1 behavior unless you updated the #if statements.

We recognize that a lot of developers won’t expect the new behavior because it is a change. However, we aren’t changing what gets defined for existing target frameworks such as .NET Core 3.1 or .NET Framework 4.8. So they will encounter the new behavior when they are adding support for .NET 5, rather than us breaking their existing code.

[BrennanConroy]

We think that having compilation constants that mean “version X or greater” will lead to more maintainable code

And the previous iteration of the spec did just that, defined a 5.0 or greater definition.
dotnet/designs@8cd5bc4#diff-662c01e21df49c7dc04d82f25fbcf9c6L571

[davidfowl]

This looks like a bad idea. If we introduce this it should be done with a different pattern.

[sharwell]

We think that having compilation constants that mean “version X or greater” will lead to more maintainable code than having constants that are exact matches.

This isn't a problem in theory, but it only works if applied from the beginning. Since the pattern did not apply to NETSTANDARD1_1 and NET451, it can't be changed now to have completely different semantics.

[jaredpar]

Given the new model how can I use an #if that checks for the exact version of the TFM beginning with netcoreapp3.1? It seems I can only do this by referencing TFM that don't actually exist yet. Consider that at the moment we release the .NET 5 SDK the only way I can write a future proof #if for checking for exactly net50 is to do the following:

#if !NET6_0 && NET5_0

Is this the recommendation that we are giving to our customers? That seems odd because it's essentially getting us to commit to future releases and future TFM at the moment we release a given SDK.

This point has been raised a few times but I haven't seen a good answer to it.

[benaadams]

Is this the recommendation that we are giving to our customers? That seems odd because it's essentially getting us to commit to future releases and future TFM at the moment we release a given SDK.

Presumably the #if check is against the <TargetFrameworks> rather than the sdk so it won't kick in for NET6_0 until that's added as a target?

So the question is when adding a newer TFM should it undo all everything that was applied for the previous latest and you need to revisit all the code to add the newer framework e.g.

#if NET5_0 => #if NET5_0 || NET6_0 => #if NET5_0 || NET6_0 || NET7_0 => #if NET5_0 || NET6_0 || NET7_0 || NET8_0

Or to only disapply those ones which no longer apply?

#if NET5_0 => #if !NET6_0 && NET5_0

Which would be the higher count?

[dougbu]

#if !NET6_0 && NET5_0

Woe be unto our customers if we release a .1 version again.

[benaadams]

#if !NET6_0 && NET5_0

Woe be unto our customers if we release a .1 version again.

Or from the alternative approach would #if NET5_0 not apply on NET5_1?

[paul1956]

VB supports MATH in #If and #Constants have values beyond True and False but I don't know what value NET5_0 actuals has. If its True and False writing reasonable code is extremely ugly. For the record at least for VB 3.x is not a superset of 4.X. nor is it a subset, 3.x has features not in Framework and is missing a lot that is in Framework.

[ericsampson]

davidfowl wrote:
This looks like a bad idea. If we introduce this it should be done with a different pattern.

This has got to be handled in a different way, somehow. It's a breaking change in people's expectations of how things work @dsplaisted - even though the new behavior is better, that's not sufficient to change how it has behaved from the start without addressing the concerns listed by people above. If we had a time machine, it would be a different story : )

I'd advocate for defining a new convention like NET5_0+ (generically TFM+), as included in an earlier version of the spec.
That would allow people to choose between targeting exact versions like the scenarios Jared mentioned, and roll-forward behavior.

[paul1956]

@ericsampson I love the concept assuming that NET5_0 means all the . releases of .NET5_0 including 5.01 but not NET5_1. Maybe there is an option for NET5_*+ and NET5_0+. Or for VB define NET5_1 as a Number and let developers/compiler do the Math. I am assuming that something in the toolchain is doing the Math and what is in Source it is NET5_1 but I do think that also will lead to confusion as that would be invisible to most developers unless it was exposed by Visual Studio in the Framework selection.

[marcpopMSFT]

We've reviewed the feedback both from external and internal customers and for net5.0, we will remove the change that defines NETCOREAPP3_1 for this release. @terrajobst will own developing a new proposal for a future SDK release. For now, customers can use #if NET to mean .net 5 and higher.

Based on our analysis of existing projects, most customers use these symbols as >= so we want to provide that option. However, we recognize that changing the previous behavior that existed for 3.1 and before will impact some customers and cause confusion.

Thanks all for the feedback.

[ericsampson]

Thanks @marcpopMSFT, definitely appreciate that the feedback was listened and taken into account.

The goal is laudable, and I look forward to the new proposal.

Cheers

[marcpopMSFT]

Fixed in #13615

[dotMorten]

Not sure I like this change. I can't think of a single line of code in my huge multi-targeted project where NETCOREAPP3_1 code isn't meant to be compiled for NET5_0 as well. This change really wreaks havoc on my code base.
Perhaps it's about time we get the ability to do what C++ can do with preprocessors and write something like:
#if NETVERSION>=5

For now, customers can use #if NET to mean .net 5 and higher.

That just doesn't make any sense what so ever.

[tmds]

I put code in the #else case that is for the highest .NET Core version. So far this has worked well for me.
When I update my projects from .NET Core 3.1 to .NET 5, I don't need to make any changes.

[dsplaisted]

Not sure I like this change. I can't think of a single line of code in my huge multi-targeted project where NETCOREAPP3_1 code isn't meant to be compiled for NET5_0 as well. This change really wreaks havoc on my code base.
Perhaps it's about time we get the ability to do what C++ can do with preprocessors and write something like:
#if NETVERSION>=5

For now, customers can use #if NET to mean .net 5 and higher.

That just doesn't make any sense what so ever.

We got strong feedback that the change wasn't what people expected and did not work for some coding patterns.

We still want to offer something like NET5_OR_HIGHER. @terrajobst is supposed to drive that design.

[bgrainger]

Not sure I like this change. I can't think of a single line of code in my huge multi-targeted project where NETCOREAPP3_1 code isn't meant to be compiled for NET5_0 as well.

I'm grateful that this problem was fixed. As a opposite anecdote, I had multiple lines of code where I was able to simplify #if (NETCOREAPP3_1 && !NET5_0) (that I had to introduce in RC 1) back to #if NETCOREAPP3_1. I greatly appreciate the consistency with previous behaviour (for NETSTANDARD2_0, NETSTANDARD2_1, etc.).

Based on the behaviour of previous SDKs I had landed on a solution similar to the one @tmds described: an #if block to special-case for old frameworks and #else for all future frameworks; for example:

#if NETSTANDARD2_0 || NETCOREAPP2_1
	public Task DisposeAsync() => ...
#else
	public override ValueTask DisposeAsync() => ...
#endif

...

#if NETSTANDARD2_0 || NETSTANDARD2_1 || NETCOREAPP2_1 || NETCOREAPP3_1
	public Task<DataTable?> GetSchemaTableAsync(CancellationToken cancellationToken = default)
#else
	public override Task<DataTable?> GetSchemaTableAsync(CancellationToken cancellationToken = default)
#endif
{ ... }

This is highly compatible with all future TFMs and (now) doesn't break when NETCOREAPP3_1 suddenly also means NET5_0.