dotnet tool install skip unlisted version

[JiaqiWang18]

Fixes #24037
Use the NugetToolSearchApiRequest to get the latest listed package version in dotnet tool install <PACKAGE_ID> when --version is not supplied.

Manual Testing

Created a test tool jackywang118.botsay with two versions where the most recent version 2.0.0 is unlisted.
Run dotnet tool install jackywang118.botsay, version 1.0.0 should be installed
A request to https://azuresearch-usnc.nuget.org/query?q=jackywang118.botsay&packageType=dotnettool&semVerLevel=2.0.0 is made to get the most recent listed package version

[dotnet-issue-labeler]

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

[JiaqiWang18]

@baronfel

[baronfel]

This is great! I'd love to see a test validating the behavior though - I think that would require getting known packages with the expected unlisted/listed statuses added to the internal feeds.

Scheduling-wise, @marcpopMSFT this rides the line between a fix and an feature - I think it's what users expect out of NuGet so I'd be happy to see this in 7.0.2xx rather than waiting for 8.0.1xx (which is what main is targeted for).

[JiaqiWang18]

This is great! I'd love to see a test validating the behavior though - I think that would require getting known packages with the expected unlisted/listed statuses added to the internal feeds.

Scheduling-wise, @marcpopMSFT this rides the line between a fix and an feature - I think it's what users expect out of NuGet so I'd be happy to see this in 7.0.2xx rather than waiting for 8.0.1xx (which is what main is targeted for).

Thanks for the feedback! Could you point me to where these internal feeds are located? Is there an example test that is using this internal feed you are talking about?

[baronfel]

The feeds I mention are defined in the nuget.config in the root of the repo, so if you had sample packages that you wanted to use for a test, we could coordinate to get those versions downloaded to an appropriate feed (we have to communicate with an internal team for this), and then your test would be good to go. @dotnet/domestic-cat is this an acceptable plan?

[JiaqiWang18]

The feeds I mention are defined in the nuget.config in the root of the repo, so if you had sample packages that you wanted to use for a test, we could coordinate to get those versions downloaded to an appropriate feed (we have to communicate with an internal team for this), and then your test would be good to go. @dotnet/domestic-cat is this an acceptable plan?

I did create this package jackywang118.botsay to test.
It has two versions where 2.0.0 is unlisted and 1.0.0 is not. Maybe this package could be added to the feed so that new test cases can be created under ToolInstallLocalCommandTests and ToolInstallGlobalOrToolPathCommandTests to verify that only 1.0.0 will be installed if running dotnet tool install jackywang118.botsay

[JiaqiWang18]

@baronfel sounds like I can't test it by myself. Would it be reasonable to get this PR in and file another issue for the testing part?

[knocte]

ping? I'm affected by issue #24037 and would prefer this reviewed/merged instead of having to write a workaround.
Thanks!

[baronfel]

Thanks for the ping, @knocte - I dropped the ball on this one. Need to get a listed and unlisted package to test with into MS internal feeds, but we can't just take random packages. I'll pick this up again and work with the internal team to get tool packages created and pushed.

[baronfel]

@JiaqiWang18 is there source for jackywant118.botsay available?

[knocte]

@JiaqiWang18 ping?

[JiaqiWang18]

@JiaqiWang18 is there source for jackywant118.botsay available?

@baronfel Yes, I do have the source code.

[knocte]

ping? :)

[JL03-Yue]

Investigating

[knocte]

Why was author information not preserved?

[marcpopMSFT]

Why was author information not preserved?

Not sure what you mean. GH still considers jiaqiwang18 the author of this PR. It lists Annie as the merger as someone from the team has to merge.

BTW, we chatted offline and investigated ways to test this using our internal AzDO feeds but were not able to do so and decided it wasn't worth investigating further for a regression test.

[knocte]

GH still considers jiaqiwang18 the author of this PR.

Right, but that's just GH, I'm referring to git.

It lists Annie as the merger as someone from the team has to merge.

Normally when a PR is merged, there are 2 commits: the one from the merger and the real commit of the PR. In this case it has been squashed to one, but when you select this option in GH, it normally preserves author information (the one you extract with git tools, such as git log), why didn't it preserve it this way? I guess you used another tool to merge. I don't think this is an acceptable policy.

[marcpopMSFT]

Interesting. That seems like a bug in GH or maybe just unintended behavior of which option @JL03-Yue chose in the dropdown menu (GH includes three options when merging).

[elinor-fung]

I expect this was merged with 'Create a merge commit'. All the original commits - with correct author information - are there in the actual history:
https://github.com/dotnet/sdk/commits/main/src/Cli/dotnet/commands/dotnet-tool/install/ParseResultExtension.cs

As an aside - does the sdk repo have a general policy on which option to use? In runtime, we actually only allow squash and merge as we find that makes for simpler / easier to understand history.

[marcpopMSFT]

SDK has to use the merge commit option for the inter-branch codeflow as otherwise it gets confused about commits that were modified/reverted so I've left the default as that. You're right though that squash and merge is better for history most of the time.

[pokdeep]

I think this change caused an issue I saw whilst building net8 rc1 base images in my companies build infrastructure.
We use private nuget feeds and all traffic to nuget.org is blocked, our base images tries to install the latest version of certain dotnet tools e.g. coverlet.console, so we don't specify as version, but with this change it tries to connect to nuget.org to get latest versions causing the build to fail.

A workaround is to specify the version flag, e.g. dotnet tool install coverlet.console --version 6.0.0 or dotnet tool install coverlet.console --version 6 as a partial pin
stacktrace.txt

[marcpopMSFT]

@pokdeep please file a separate issue with the details and we'll take a look.