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

NuGet pack should catch incorrect omitting of the dot in version numbers #9215

Closed
terrajobst opened this issue Feb 24, 2020 · 6 comments · Fixed by NuGet/NuGet.Client#3692
Closed
Assignees
Labels
Area:NewFrameworks Type:DCR Design Change Request

Comments

@terrajobst
Copy link

For .NET 5, we're going to reuse the existing net moniker. Due to our new versioning scheme (fixed schedule, major bump every year) we're only five years away from .NET 10. Developers might accidentally say net10 when they really meant net10.0. We're planning to follow in the footsteps of .NET Core where project files will generally use a dot, even when it's not necessary (e.g. net5.0) but of course this won't prevent humans from dropping the .0 either inadvertently or for aesthetics.

It was suggested to handle this as part of NuGet pack.

There are several ways to deal with that:

  1. Generally warn when a dot is missing
  2. Only warn on when the TFM is >= net5.0 (regardless of whether there is an OS flavor or not)

The (1) is more correct but will cause a lot of noise, especially in .NET Framework projects where the ambiguity is irrelevant as the platform will remain at 4.x forever. So my preference would be something more targeted. Not sure (2) is the best way to address that but it seems to hit the sweet spot.

@rrelyea
Copy link
Contributor

rrelyea commented Mar 5, 2020

With the first half of net5.0 tfm work (#9089) that was just merged into dev, we treat net5.0 and later as "netcoreapp" internally, which like netstandard, by default adds dots into the version when writing them down in shortfoldername, etc...

Like netcoreapp and netstandard, we don't currently warn for any use of dotless versions (net50).

We could consider doing some sort of warning in these scenarios:

  1. <TargetFramework>net50</TargetFramework>
    (note: dotnet pack will end up with lib\net5.0\foo.dll in this scenario)

  2. package validation tools could complaing about the short form, that manual creation of packages might have created: lib\net50\foo.dll

  3. other scenarios?

@rrelyea rrelyea added Area:NewFrameworks Type:DCR Design Change Request and removed NuGet API labels Mar 5, 2020
@terrajobst
Copy link
Author

1 and 2 sound good to me. The key is not to warn for net1-4, netstandard and netcoreapp IMHO.

@jarenduan
Copy link

jarenduan commented Apr 3, 2020

I would've never omitted the dots in "TargetFrameworks" of net1-4 if I knew that I was able to. I thought that omitting dots was the only correct way to target .Net Framework 1.0 to 4.x. I really feel shamed. I don't know how many people out there have been doing the same thing.

@rrelyea, a question about adding dots for developer: would it might bring problems? For example, how should the pack tool add the dots correctly for the TFM net451 if we have .Net 4.5.1 and .Net 45.1 at the sametime? (I might think too far, but it is possible).

It would be great if we don't warn against the exsiting code targeting net1-4, netstandard and netcoreapp. But the problem seems to be about net1-4, that developers won't get any warnings or errors when they carelessly use the exsiting TFMs to target newer runtime, e.g., using net10 to target net10.0.

IMHO, I suggest that we:

  1. stop developers from omitting the dots for net5.0 or higher by strictly asking them to use the dots, throwing exceptions, not just warning, and don't add the dots for developers from net5.0.

  2. to deal with net1-4, as a flexible way, we could avoid the carelessly misuses by skipping the confusing versions in the future. For example, we ship .Net 10.1 instead of .Net 10.0, and so on. So that developers have to use dotted TFM net10.1, without any chance to make mistakes.

  3. another way to deal with net1-4 is that we do warn them, but we only do it when the misuse is possible. For example, we won't warn net10 until .Net 10.0 ships, and we nerver warn net12, cos' it is definitely refering to .Net 12.0, it should throwing exception for the dot-omitting. In another words, we turn on warnings for TFMs gradually and only necessarily. This might be a good compromise, cos' there would be very few projects targeting .Net 1.0 after 5 years, and fewer projects would target .Net 2.0 after 15 years, and even more fewer projects would target .Net 3.5 after 30 years.

@Nirmal4G
Copy link

Nirmal4G commented Apr 3, 2020

Both netstandard and netcoreapp have dots in them. So we warn for netcoreapp, netstandard and net (>=5.0). We don't need to warn for the rest of them.

@zkat zkat added this to the Sprint 177 - 2020.09.28 milestone Sep 28, 2020
zkat added a commit to NuGet/NuGet.Client that referenced this issue Sep 30, 2020
zkat added a commit to NuGet/NuGet.Client that referenced this issue Oct 6, 2020
@zkat
Copy link
Contributor

zkat commented Oct 8, 2020

current status: I'm working on adding warnings based on all the places in a nuspec where you might have some kind of TFM, which is a bit more than what I expected this to involve, so I'm having to take extra time to get that in.

@terrajobst
Copy link
Author

Awesome. Thanks for getting this done! 💜

zkat added a commit to NuGet/NuGet.Client that referenced this issue Oct 16, 2020
zkat added a commit to NuGet/NuGet.Client that referenced this issue Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment