Throw exception for missing dependencies

[Bouke]

✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)

feature

⤵️ What is the current behavior?

Missing packages are silently ignored when inspecting:

$ nuget inspect
Found 61 packages
Found 61 packages in use, 14 distinct, in 16 projects.
coverlet.msbuild, LibGit2Sharp, McMaster.Extensions.CommandLineUtils, Microsoft.CodeAnalysis.FxCopAnalyzers, Microsoft.NET.Test.Sdk, Newtonsoft.Json, NSubstitute, NuGet.CommandLine, NuGet.Protocol, NUnit, NUnit3TestAdapter, Octokit, SimpleInjector, System.Text.Encoding.CodePages
Found 0 possible updates
Found no package updates

C:\Program Files\dotnet\dotnet.exe (process 14792) exited with code 0.

🆕 What is the new behavior (if this is a feature change)?

An error is returned if there were missing packages during inspection.

$ nuget inspect
Found 61 packages
Found 61 packages in use, 14 distinct, in 16 projects.
coverlet.msbuild, LibGit2Sharp, McMaster.Extensions.CommandLineUtils, Microsoft.CodeAnalysis.FxCopAnalyzers, Microsoft.NET.Test.Sdk, Newtonsoft.Json, NSubstitute, NuGet.CommandLine, NuGet.Protocol, NUnit, NUnit3TestAdapter, Octokit, SimpleInjector, System.Text.Encoding.CodePages
Could not find package coverlet.msbuild.2.7.0 in these sources: file:///C:/Program Files (x86)/Microsoft SDKs/NuGetPackages/, https://dotnet.myget.org/F/roslyn/api/v3/index.json

C:\Program Files\dotnet\dotnet.exe (process 17888) exited with code -1.

💥 Does this PR introduce a breaking change?

Yes / no. inspect would silently ignore the problem. However update would fail, as a Nuget restore would also fail for missing packages. I think this is desirable, as inspect should handle situations where packages are missing.

🐛 Recommendations for testing

Add a non-existing package reference.

📝 Links to relevant issues/docs

🤔 Checklist before submitting

• All projects build
• Relevant documentation was updated

[Bouke]

This fails some tests, due to the change in behaviour. I can change the tests to the new behaviour, but first wanted to know if you'd be willing to take this change?

[MarcBruins]

Yes this looks like a informative change that clearly states if a package is missing to the user. Would be good if we could get this in!

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Bouke]

Why auto-close the PR?

[AnthonySteele]

This a a good change, can you rebase please?

[AnthonySteele]

I wonder if we want to log all of the missing / unreachable packages.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Bouke]

I can rebase the PR. Anything else that you want me to look into?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Bouke]

Why auto-close the PR?

[Bouke]

Ping @MarcBruins @AnthonySteele

[MarcBruins]

Sorry but can you rebase again? After that i'll merge it in

[Bouke]

Happy to oblige! I've rebased the project and updated the relevant tests.

• Please go over the tests as I've updated them to match the expected behaviour (throwing an error for missing packages instead of silently ignoring the package).
• There's a test that keeps failing: ShouldFilterOutNonDotnetRepository. I'm not sure how my PR is affecting that test, please advice.
• Finally there's the Windows build that fails to build. I don't think my changes are related to that build failure, please advice.

[Bouke]

@MarcBruins can you give some pointers?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Bouke]

Still waiting for feedback…

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Bouke]

Still waiting for feedback…

[msallin]

@Bouke can you please rebase the branch on the latest master?

[Bouke]

@msallin sure, I just did.

[msallin]

@skolima fine with this? Then I will merge it.

[skolima]

@msallin I'm offline for the night, I'll have a look tomorrow, if that's ok. I have free time tomorrow.

[msallin]

@msallin I'm offline for the night, I'll have a look tomorrow, if that's ok. I have free time tomorrow.

Sure! Enjoy!

[skolima]

I'm only concerned how this interacts with #311 and #466 . NuKeeper should currently skip unversioned packages (yes, I know those went away now, but the tag without a version is still valid, even if unwise) and should skip packages with ranges specified (also not a particularly good idea when using any automated dependency upgrader, but hey). I realise both of those are quite niche corner cases, but I'm still not keen on breaking them. I haven't re-tested those interactions yet.

[Bouke]

@skolima Are there any unit tests covering those cases?

Conceptually I expect a missing package to be something different from an unversioned package. So we should be able to tell the two apart and only throw in the first case.

[Bouke]

@skolima I'm getting the impression my contributions are in vain. It's been open for close to 2 years now. On this PR but also various bugs the only interactions are me fighting with the stale bot to keep them open. I've already rebased this PR a few times on the master branch, only to see it linger further.