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

Add initial design document for parsing net10 #322

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

terrajobst
Copy link
Member

@terrajobst terrajobst commented Oct 2, 2024

We want to steer people towards a proper version syntax, that is

<TargetFramework>netX.Y.Z</TargetFramework>

Examples include net9.0, net10.0, net4.5.1. This syntax makes it unambiguous.

Proposal:

  • Fail the build if the TFM is net10 or net11 with an error message

    This framework syntax is unsupported because it's ambiguous. If you mean .NET Framework 1.0, use net1.0. If you mean .NET 10, use net10.0.

  • If the TFM is neither net10 nor net11, produce a warning if it doesn't contain a period:

    This framework syntax is obsolete. You should use periods to separate version components.

The reason to fail the build is because in practice someone trying to target .NET Framework 1.0 will most likely fail anyway because the either don't have the .NET Framework 3.5 targeting pack -- or worse -- they do, but the code doesn't compile because it's meant for modern .NET Core, not a 25 year old .NET Framework 1.0. By failing early it turns an unintelligible error message into something actionable.

The reason to issue the warning is to avoid any ambiguities moving forward.

Note

Please note that this change only applies to the .NET Core SDK, i.e. SDK-style projects. Old school .NET Framework CSPROJ files will be unaffected by this change because the representation of the target framework is already a proper version string.

@WeihanLi
Copy link
Contributor

WeihanLi commented Oct 3, 2024

currently, the SDK supports net8, would the SDK drop the support? Since net10 may conflict between net10.0 and netfx 1.0

This relates to my question before

dotnet/sdk#42920 (comment)

Since we're going to use net10.0 in the previous design proposal, thanks @huoyaoyuan sharing the info

https://github.com/dotnet/designs/blob/main/accepted/2020/net5/net5.md#what-about-net-10

Why not just treat the net10 as the dotnet framework 1.0, and net10.0 as dotnet 10, think it would also keep it simpler

It would also introduce the complexity and inconsistency across SDKs versions

@terrajobst
Copy link
Member Author

To be clear, net10.0 already means .NET 10 and is the preferred syntax (because it's unambiguous).

This proposal is about how we should address the problem that some people will use net10, thinking it will mean .NET 10 when in fact it already means .NET Framework 1.0.

@Varorbc
Copy link

Varorbc commented Oct 3, 2024

I think we shouldn’t support this non-standard target framework, and someone’s already brought up issues with it.
dotnet/sdk#43773

@reflectronic
Copy link

Is this change just for TargetFramework in MSBuild? There are net10 and net11 packages on NuGet.org--would they magically become .NET 10 packages?

@GrabYourPitchforks
Copy link
Member

GrabYourPitchforks commented Oct 3, 2024

If even first-party tools msbuild and nuget don't handle this properly, shouldn't this sound the alarm that third-party ecosystem tools also won't handle it properly? It seems like us special-casing net10 to try to make it work might end up causing harm by falsely lulling developers into believing that something works, when in reality it's potentially masking a failure elsewhere. Seems like this - not net12 - might be the right time to make it an error. With an improved message, of course.

(Edit: This is just a random opinion. Not a hill I'm willing to spend much effort defending. 🙂)

@terrajobst
Copy link
Member Author

terrajobst commented Oct 3, 2024

If even first-party tools msbuild and nuget don't handle this properly, shouldn't this sound the alarm that third-party ecosystem tools also won't handle it properly?

What do you mean by this? net10 has been supported forever because that was the old syntax.

Seems like this - not net12 - might be the right time to make it an error.

So what about net472? Should this be an error too?

accepted/2024/net10.md Outdated Show resolved Hide resolved
INDEX.md Outdated Show resolved Hide resolved
terrajobst and others added 2 commits October 2, 2024 19:15
Co-authored-by: Kexy Biscuit <kexybiscuit@outlook.com>
Co-authored-by: Kexy Biscuit <kexybiscuit@outlook.com>
@huoyaoyuan
Copy link
Member

A retrocompute question - is it ever possible to build for .NET Framework 1.0 with current .NET SDK?

I'd suggest to keep net10 meaning .NET Framework 1.0 with more dedicated warning/error message.

@terrajobst
Copy link
Member Author

A retrocompute question - is it ever possible to build for .NET Framework 1.0 with current .NET SDK?

I don't believe so because .NET Framework 1.x doesn't even have a targeting pack; those came to be with .NET Framework 2.0.

@GrabYourPitchforks
Copy link
Member

What do you mean by this? net10 has been supported forever because that was the old syntax.

Depends on what you mean. ".NET 10" has never been supported by virtue of the .NET 10 product and SDK never existing. Unless I misread your doc, ".NET Framework 1.0" has not been supported by modern toolchains for years. This seems like a case where net10 is ambiguous, and in the case of ambiguity, the typical preference is for APIs / tools to throw rather than guess.

So what about net472? Should this be an error too?

I only suggested that the target framework net10 be considered an error. It's a modification to your current proposal, where you contemplate that net10 be a warning and net12 be an error. Per your proposal, net20 and beyond (which presumably includes net472) retain their current meaning. I think this part of your proposal is sound and you should keep it as-is.

@terrajobst
Copy link
Member Author

terrajobst commented Oct 3, 2024

@GrabYourPitchforks

Depends on what you mean. ".NET 10" has never been supported by virtue of the .NET 10 product and SDK never existing.

The project file/SDK didn't invent the framework syntax, we simply made it the NuGet syntax.

As a result, you can put <TargetFramework>net10</TargetFramework> in the project file today (or net10 as a folder in a NuGet package) and it means .NET Framework 1.0. See https://nugettools.azurewebsites.net/6.11.1/parse-framework?framework=net10.

I only suggested that the target framework net10 be considered an error. It's a modification to your current proposal, where you contemplate that net10 be a warning and net12 be an error

Gotcha. I just updated the PR description. Does the last sentence represent your proposal then? If so, I'd be OK with that too.

@GrabYourPitchforks
Copy link
Member

GrabYourPitchforks commented Oct 3, 2024

Does the last sentence represent your proposal then? If so, I'd be OK with that too.

Yup! Thanks. :)

Again - I'm not terribly passionate one way or another. It's just a proposal for others' consideration. Your doc is very well thought through.

@rolfbjarne
Copy link
Member

Would it possible / make sense to make nuget.org reject packages that use the old format without periods? In particular for net1X that might be doable, since it would only be a breaking change for anyone trying to submit a .NET Framework v1.0 package.

Also this page should be updated to use periods: https://learn.microsoft.com/en-us/dotnet/standard/frameworks (and probably other doc pages too).

Here is the proposal:

* Make `net10` mean .NET 10 and `net11` mean .NET 11 but keep all higher
versions as-is (`net20` still means .NET Framework 2.0).
Copy link
Member

@akoeplinger akoeplinger Oct 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems backwards to me. if the goal is to eventually guide people to always use periods then I don't think we should complicate our lives by allowing net10 and potentially cause more issues in downstream tools that parse the version.

I'd change the rule to:

  1. Warn about missing dot for (currently) non-ambigious versions like net20, net472
  2. Error for ambigious versions like net10, net11.

And change to error everywhere in net12 like you suggested (or wait until net20 when that becomes ambigious?).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Warn about missing dot for (currently) non-ambigious versions like net20, net472

We don't have any alternatives to net20 or net472 to fix the warning, do we? If not, we shouldn't warn for those.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can use net2.0 and net4.7.2 instead.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I've changed the proposal.

@sundhaug92
Copy link

Could it be an idea to make it a warning by default but an opt-in error?

@goswinr
Copy link

goswinr commented Oct 3, 2024

How about dotnet10 ?

@WeihanLi
Copy link
Contributor

WeihanLi commented Oct 3, 2024

thinking it will mean .NET 10 when in fact it already means .NET Framework 1.0.

That's it, it would introduce the complexity and inconsistency across SDKs versions.

Do not think it's worth making these changes for a usage that should not be used

If we want to produce a warning before producing an error directly, would it be applicable to produce a warning for net9 target and an error for net10 target usage since net9.0 has not GA

@terrajobst
Copy link
Member Author

@rolfbjarne

Would it possible / make sense to make nuget.org reject packages that use the old format without periods?

That's a good point. It should be easy because NuGet tooling could already normalize the syntax.

@terrajobst
Copy link
Member Author

@WeihanLi

If we want to produce a warning before producing an error directly, would it be applicable to produce a warning for net9 target and an error for net10 target usage since net9.0 has not GA

I don't think it's practical to make such a change before .NET 9 GA. We could, however, likely make that change to the SDK post GA (because the SDK versions gets shipped more frequently). It's probably considered a source breaking change, but I'd consider that acceptable.

@akoeplinger
Copy link
Member

I'm not sure we need to do anything for nuget.org or nuget packages, net10 can continue to mean net1.0 there since nobody is creating these by hand. I thought the main motivation for this is the TFM property in .csproj.

@terrajobst
Copy link
Member Author

@akoeplinger

That's a fair point actually.

@AustinWise
Copy link
Contributor

The "GitHub Usage of a Framework" does not include .NET Framework IDs. The percent of occurrences of IDs without dots is much higher:

GitHub Usage of a Framework Without . With . Percent
net20 vs net2.0 3.8k 37 99%
net472 vs net4.7.2 33.9k 259 99%

I still think the design is reasonable, as the number of occurrences of .NET Framework target framework IDs is much lower compared to .NET and .NET Standard IDs.

accepted/2024/net10.md Outdated Show resolved Hide resolved
accepted/2024/net10.md Outdated Show resolved Hide resolved
Co-authored-by: Igor Velikorossov <RussKie@users.noreply.github.com>
Copy link
Member

@akoeplinger akoeplinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but the issue description needs to be updated to reflect the current proposal.

accepted/2024/net10.md Outdated Show resolved Hide resolved
accepted/2024/net10.md Outdated Show resolved Hide resolved
accepted/2024/net10.md Outdated Show resolved Hide resolved
terrajobst and others added 2 commits October 4, 2024 11:14
Co-authored-by: Rolf Bjarne Kvinge <rolf@xamarin.com>

## Q & A

### What about `net10.0`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this q&a still relevant? The answer seems to be referring to the original proposal.

nil4 added a commit to royalapplications/royalapps-community-freerdp that referenced this pull request Oct 5, 2024
ref. dotnet/designs#322:

> We want to steer people towards a proper version syntax, that is
>
> ```xml
> <TargetFramework>netX.Y.Z</TargetFramework>
> ```
>
> Examples include `net9.0`, `net10.0`, `net4.5.1`. This syntax makes it unambiguous.
nil4 added a commit to royalapplications/royalapps-community-externalapps that referenced this pull request Oct 5, 2024
ref. dotnet/designs#322:

> We want to steer people towards a proper version syntax, that is
>
> ```xml
> <TargetFramework>netX.Y.Z</TargetFramework>
> ```
>
> Examples include `net9.0`, `net10.0`, `net4.5.1`. This syntax makes it unambiguous.
@kasperk81
Copy link

dotnet/sdk#43773

I've been busy submitting PRs to enforce correct framework versioning across projects, promoting the only documented net8.0 (since net5.0) over variants like net8, net80, netcoreapp8, netcoreapp80, and netcoreapp8.0. Going forward, only allowing net10.0 and showing errors for other variants via SDK commands would be ideal.

Here’s why:

  • Consistency: Aligns with what dotnet new and Visual Studio's New Project feature generates.
  • Simplicity: Eliminates the need for complex pattern matching in tools like NuGet (which will only keep growing).
  • Ease of Use: The version string can be as simple as $"net{Environment.Version.Major}.{Environment.Version.Minor}".
  • Scalability: The current parsing logic is already showing signs of complexity (see NuGet's framework parsing logic), and it’s getting worse over time.

Given that only about 0.01% of GitHub projects use these non-standard variants, updating to net10.0 should be easy for those few projects. No big deal!

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

Successfully merging this pull request may close these issues.