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

Error when a PackageReference specified in a project is missing a version #13796

Open
nkolev92 opened this issue Sep 20, 2024 · 4 comments
Open
Labels

Comments

@nkolev92
Copy link
Member

NuGet Product(s) Affected

NuGet.exe, MSBuild.exe, dotnet.exe

Current Behavior

Take a project like below:

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

  <PropertyGroup>
    <TargetFramework>net9.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>
  
  <ItemGroup>
    <PackageReference Include="NuGet.Commands" />
  </ItemGroup>
</Project>

Currently, at restore time, this leads to a restore happening, and a NU1604, and then whatever warnings resolving to the lowest available version may do.
In this case, loads of NU1701.

Note that NU1604 doesn't even say missing version, but missing lower bound, hence inconsistent restore.

I can't imagine there's any scenario where not adding a version to a PackageReference is wanted.

Desired Behavior

Elevate this to a warning.

NU1111: Missing version for NuGet.Commands

Additional Context

This is a warn change, so it needs to be hidden behind SDKAnalysisLevel.

@msherms2
Copy link

My item in the VS community was closed as a duplicate and subsequently locked (which means I can't reply...): here

It seems more or less the same except that I don't see that this item specifically called out that the restore is "successful" in that it still pulled packages despite the error. In our specific case, we have nuget.org as an upstream feed due to logging requirements, and our upstream feed suddenly had the latest version of these packages. I would expect no pull to occur nor any upstream feed to get triggered, this should merely be a syntax error and halt of the build until the version is properly supplied (via CPM or not)

@nkolev92
Copy link
Member Author

@msherms2
The NU1604 you got is a warning. When you get a warning like that, restore continues and it may or may not fail.

I think that's the reason why it was deduped to this issue. If this was an error in the first place, you would for sure not get a successful restore.

@msherms2
Copy link

@msherms2 The NU1604 you got is a warning. When you get a warning like that, restore continues and it may or may not fail.

I think that's the reason why it was deduped to this issue. If this was an error in the first place, you would for sure not get a successful restore.

With WarningsAsErrors set, it still fails. Regardless, I get your point, but also regardless, I would never want this as a capability. If i forgot the version, I would want an absolutely critical failure, not to just decide to pull the latest version. I think we're on the same page, the text that said "elevate to a warning" just threw me off. I'd prefer it be an error, so we don't have to rely on WarningsAsErrors setting. It seems critical enough.

@nkolev92
Copy link
Member Author

Thank you for sharing your feedback!
I agree, feels like something that should've been an error from the beginning.

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

No branches or pull requests

3 participants