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

Adds Visual Studio Filters & Sorts #12704

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

Adds Visual Studio Filters & Sorts #12704

wants to merge 3 commits into from

Conversation

JonDouglas
Copy link
Contributor

@JonDouglas JonDouglas commented Jun 26, 2023

This proposal introduces two new ways to filter and sort packages in Visual Studio's NuGet Package Manager experience.

Rendered Proposal

Please 👍 or 👎 this comment to help us with the direction of this feature & leave as much feedback/questions/concerns as you'd like on this issue itself and we will get back to you shortly.

Thank You 🎉

@JonDouglas JonDouglas requested a review from a team as a code owner June 26, 2023 17:56
Copy link
Member

@nkolev92 nkolev92 left a comment

Choose a reason for hiding this comment

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

This is one of those, I'd use it features. Excited for it.


When a developer is using Visual Studio to search and manage NuGet packages, they will see a new filter and sort experience.

The browse filter experience will allow the developer to filter results based on common concepts such as "frameworks", "prerelease", "owners", "download count", "license", and "created on".
Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing for licenses we'd only support SPDX?

created on => published on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Licenses most likely would need to be SPDX expressed, yes. Maybe a custom textbox for anything outside that but that is a edge case imo.

Copy link
Member

@nkolev92 nkolev92 Jun 27, 2023

Choose a reason for hiding this comment

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

worth clarifying that.

I'm not so convinced files are an edge case though.

let totalEvents = toscalar(NiPackageManifests | summarize count());
NiPackageManifests
| extend LicenseType = iff(isempty(LicenseMetadata) and isempty(LicenseUrl), 
"None", iff(isempty(LicenseMetadata), "LicenseUrl",tostring(LicenseMetadata['Type'])))
| summarize  count() by LicenseType
| extend Total = totalEvents
| extend percetanges = todouble(count_)*100/todouble(Total)

says

None	2473119	7165196	34.515720156154835
File	665275	7165196	9.2848123065998482
LicenseUrl	2255208	7165196	31.474477460211833
Expression	1771594	7165196	24.724990077033482

It's close to 10%. 31% have license url as well which also need to fall in that group.

Granted, I'm looking at all packages, but I think the Expression vs File ratio still applies.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I was referring to the SPDX expressed name as those are typically the same as the file expression. For the edge case, I was referring to atypical licenses. For the sake of this feature, if it is too difficult to parse a file / licenseURL, we should only support expression to encourage best practices.

Copy link
Member

Choose a reason for hiding this comment

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

I don't mind having the filter, I was mostly just trying to clarify behavior for the places where it won't make sense.
It was a very technical approach to reading the spec from my side.


Whenever a filter is applied, it will additionally show up in the Visual Studio search box so that developers get used to the search prefix syntax and can apply filters more quickly in the future.

The sorting experience will allow the developer to sort these options on common concepts such as "alphabetical", "version", "relevance", "downloads", and "recently updated".
Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing this is jsut examples, rather than a concrete proposal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

these are concrete examples 🥲 can go into more details on them if needed.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see :D My feedback on the ones I have concerns with.

  • version - Can you elaborate more? I'm not sure what this is supposed to mean or how is it helpful.
  • relevance - can we define relevance? How would we measure it.
  • downloads, recently updated - these are "network dependent", and what I mean by that is that it needs the client to reach out to the server and potentially change the order of what the customer is, so there's a good chance you'd have a noticeable flicker. This could get weird. Personally, I'd steer clear on any sorting that cannot be done locally, but I know there's been asks.

Another side question, are the filters sticky? Within same session?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

version would sort by the package version in asc/desc order.

relevance would be the default search experience today just as a filter to get back to (like most search experiences)

downloads / recently updated - could we not cache the package details and use that locally?

At the time of writing, filters are not sticky as typical filtering and sorting experiences are not sticky outside the context of the same session. In other words, if a user exits the PM UI, the defaults will be set again. Further customer feedback may change this behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

another thought might be that version could possibly be helpful if you can specify a range to filter as well. that might be helpful for the .NET libraries case and upgrading those in bulk.

Choose a reason for hiding this comment

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

I think versions should allow package versions and assembly versions. The reason is that if there is an assembly not found exception or similar issue, the error returns the actual assembly version rather than the package version. Right now my process is to click Show All Files in Solution Explorer, search for the problematic dll by name, and then look at the Dependencies tree to find the concrete dll version used by clicking into it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now our concept would only support package metadata which would be a package version. If an actual assembly version does not match the package version, that may be better resolved with the package author.

Choose a reason for hiding this comment

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

Right now our concept would only support package metadata which would be a package version. If an actual assembly version does not match the package version, that may be better resolved with the package author.

That is impractical in the case when an author ships a hotfix for an older framework. In any case, completely disagree as standard practice among many top 100 non Microsoft packages is major.0.0.0 precisely because this is a PITA to deal with. Microsoft however refuses this convention for OOB packages.

Please consider this filter.

Copy link
Member

Choose a reason for hiding this comment

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

I think the assembly filter is a reasonable ask, but having it as a later phase might be more practical, based on upvotes etc. I haven't heard this ask on the NuGet.org side recently. Additionally, there can be multiple assemblies per package (both via cross-targeting and with multiple assembly names). These all have different assembly versions. So a filtering model would need to handle that 1-to-many relationship between package version and assembly version. It's not a simple thing unless we take some very heavy handed concessions.

Choose a reason for hiding this comment

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

Understood. I think some people don't know to ask for it, though. It's like the Steve Jobs principle of listening to your customers (we need better searching), but then figuring out what they should really have help searching for, rather than just building the ones they ask for. When you consider that .NET Tree Shaking of assemblies removes classes/methods, the possibility of version conflicts grows and the ultimate debugging details lie in what assembly you are using, not a nuget package version. - When .NET throws an assembly-related exception, the error message is always from the point of view of the assembly, not the package.

I have a pretty convoluted process to work around these problems, personally, and I've mastered it, so it doesn't really affect me any more since I have now become an expert at it, but boy, do I notice a lot of people struggle with this. And the messaging that .NET Core took care of this stuff with deps.json generation is just wrong - netstandard2.0 has an unbounded upper cardinality on what .NET 5+ version it could accept, etc.

Anyway, will table this aspect of the review for now - assuming there is a separate later phase documented so this doesnt get lost.


The installed filter experience will allow the developer to filter results based on all the **browse filter** and additional package status filters such as "deprecated", "vulnerable", and "update".

![](resources/VSFilter/PackageOptionsAndStatus.png)
Copy link
Member

Choose a reason for hiding this comment

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

If we're adding a has updates filter, I think we should consider removing the updates tab.
IMO, it'd inconsistent to have a filter that basically makes the installed tab and updates tab.

The only thing that the updates tab now has is that it has checkboxes that allow you to start the operation on multiple packages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah or we can just omit updates as a filter and bring the filtering/sort experiences to updates tab too.

Copy link
Member

Choose a reason for hiding this comment

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

Fair. Given the amount of work that'd go into this, I'd love for us to consider removing the updates tab as a parallel workstream.

It's one of those, if you do them one after the other, it might take 3 weeks each for a total of 6, but doing them together is probably just 4 weeks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

May need to look again in the actual usage of update tab, but this makes sense to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Updates tab could also be thought of as a shortcut to quickly filter. It's essentially enabling the "show updates" filter, without the option to disable it.

Copy link
Member

Choose a reason for hiding this comment

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

Not part of this spec discussion, but it may be worthwhile to discuss the future of "Updates" tab - removing it or overloading it for quick "attention seeker" tab for all the 3 filters together?
image

Copy link
Member

Choose a reason for hiding this comment

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

Btw, UI for package status seems like "enable" buttons and less of filters. I am assuming these will go through UX scrub.

<!-- What other designs have been considered and why weren't they chosen? -->
<!-- What is the impact of not doing this? -->

One major challenge with search filtering experiences today is knowing the [supported search prefixes](https://learn.microsoft.com/en-us/nuget/consume-packages/finding-and-choosing-packages#search-syntax). Technically one who knows or finds the documentation on this topic could use these in Visual Studio, but it should be more visual and easy to find in the product itself through the means of a user interface that one can interact with.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this is too technical for a high level spec, but I'm confused if this proposal is saying NuGet client should filter on the client side, or if we're going to start making nuget.org style "advanced search" requests?

If we're filtering on the client side, I guess this is an advantage of using an infinite scroll list, as it might keep making more requests to fill the screen with packages. However, if I have some very specific client side filters that filter out everything (say, I request download count over 100 billion), then PM UI is going to keep asking for page after page, until the server says there are no more search results. It obviously depends a bit on the search service technology the server uses, but high page numbers are typically more computationally expensive than low page numbers, and in this scenario PM UI will be automatically requesting these in a very short time period.

On the other hand, if the proposal is to change the search query NuGet sends to servers, then are we're effective imposing a change on the server protocol. I assume we should make servers report to us which filters they support, and then disable the filter controls when a filter is not supported? When the "all" sources is selected, we should use the lowest common denominator? Or is the proposal to just always send an "advanced query", and show the customer low quality results, whatever the server happens to return?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this is too technical for a high level spec, but I'm confused if this proposal is saying NuGet client should filter on the client side, or if we're going to start making nuget.org style "advanced search" requests?

+1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The aim would be to bring browse parity to NuGet.org search experiences first. There are plenty of experiences that work today in VS that users simply don't know about and it would be very helpful to expose that through this proposal. If another registry does not support our search syntax/etc, then this feature will not work for them full stop. If they support it, then they would see the functionality light up.

For local packages (i.e. installed), we want to bring quality of life filters and sorts based on whatever we have cached / available. Some of the browse filters/sorts may just simply not apply if they cost too much to retrieve metadata about / the package does not have an equivalent on nuget.org.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, so this is a request for protocol changes. This can be done, certainly, but we'd need to plan them carefully so that a) other package sources could reasonably implement them in part or whole (e.g. support sort by published date but not filter by owners) and b) the experience is reasonable when other sources do not support the new protocol at all which will be the case forever for some sources, and for a good amount of time for all sources (e.g. waiting for ADO to ship support). This is just expanding on @zivkan's point a bit.

This also brings in the question of upstreaming again. If a lot of folks use ADO upstreaming, is reduces the impact of nuget.org implementing the protocol so perhaps a parallel activity would be getting early buy-in from ADO or even GPR.

Copy link
Contributor

@jeffkl jeffkl left a comment

Choose a reason for hiding this comment

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

We can work on the wording of the options during implementation but I think they should be like "Include Vulnerable Packages" instead of "That are vulnerable" etc.

@zivkan
Copy link
Member

zivkan commented Jul 1, 2023

The wording "Include Vulnerable Packages" sounds like when it's enabled, that packages with known vulnerabilities would be excluded from the results. My guess is that the intention is the opposite, to filter out any packages without known vulnerabilities and therefore only show installed packages with known vulnerabilities, so customers know which ones are most important to upgrade.

@jzabroski
Copy link

Is there a NU warning for deprecated and vulnerable packages? I think having people click through the UI to get this information vs. make it fail-able as part of a build and <TreatWarningsAsErrors>NU99999</TreatWarningsAsErrors> is a non-optimal approach given the amount of work it would require to add these features to the UI. I do see the value in avoiding vulnerable and deprecated packages when searching for them, but honestly, that's not the main thing I want to search for.

According to Anand Gaurov's blog post (from only last year!) the Client experience today is minimal: https://devblogs.microsoft.com/nuget/deprecating-packages-on-nuget-org/#client-experience

You can also run dotnet.exe list package --deprecated from the command line to get a list of deprecated packages in your projects.

What things do I actually want to search for?

What I want to know is:

I am on net48 - how do I find packages that support .net core? Or, can I target netstandard2.0 or do I need to be on net6? This is by far the most time consuming thing I deal with vs. vulnerabilities and deprecated packages.

Can Joel tell us how many packages today even use the deprecated flag? As in, how often is that filter actually going to do anything useful for us as developers vs adding a NUwarning ID to the build system that pops up on Restore? I think, the more powerful analytic would be:

  1. When was this package last updated?
  2. What is the highest framework it explicitly supports using .NET SDK default RollForward of LatestFeature?
  3. Looking-through to GitHub repository if there is a Source Repository URL and determining when the last commit to the main/master branch was.

@nkolev92
Copy link
Member

nkolev92 commented Jul 7, 2023

@jzabroski

Is there a NU warning for deprecated and vulnerable packages?

Reporting of vulnerabilities during restore will be added starting with 17.7. We'll be adding improvements over the next few VS releases and the .NET 8 SDK.

You can read more about it in https://github.com/NuGet/Home/blob/dev/proposed/2022/vulnerabilities-in-restore.md

Some of the docs are already public: https://learn.microsoft.com/en-us/nuget/reference/errors-and-warnings/nu1901-nu1904

@killergege
Copy link

This is not explicitely in this proposal, but it would be nice that the VS Updates tab/client don't report that there are updates if more recent packages than installed are marked as:

  • Deprecated (for instance Microsoft.AspNetCore.Http.Abstractions where 2.2.0 is deprecated but previous version 2.1.1 is not)
  • Unlisted (for instance Microsoft.Rest.ClientRuntime where the VS UI notifies for updates to 3.0.3 which has been unlisted - moreover this is confusing as the unlisted status information appears nowhere, and the version cannot be found on the nuget website, except in full stats).

Those package versions should not be considered as "latest stable".
So ideally those filters should be enabled by default, but could be disabled.

@jzabroski
Copy link

jzabroski commented Jul 7, 2023

Some of the docs are already public: https://learn.microsoft.com/en-us/nuget/reference/errors-and-warnings/nu1901-nu1904

Great.

One last thought would be whether the SDK team envisions using this a lot for Out-of-Box packages. In my experience, the one package I have in our solution that is deprecated is deprecated because we're using net48. It is equally annoying that the Package Manager asks me to upgrade to a package that is only supported on a higher framework version. In this sense, I don't want search filters, I want upgrade intelligence that understands where our team is with managing dependencies. Just my own 2 cents. I also don't want to set a max package version at the PackageReference-level because the max version is only insofar as we are tied to net48.

If you have some time, read the Python blog post The 9 Circles of Dependency Hell. It's one of the best written articles on dependency management out there, and worth lifting ideas for C# / .NET from it. He walks you through the 9 circles and the wisdom on how to approach each one. Right now, .NET doesnt really support "getting through hell." I can talk more on this if this comment flies over your head but I imagine you know what I mean. - One immediately clear challenge is how you could query for transitive dependencies. That strikes me as a complex cross-filter operation where you first compute your existing target framework dependencies, then search for matching packages.

In that sense, a checkbox "Only include target frameworks my solution targets" would probably work for cutting down transitive dependency hell.

@jzabroski
Copy link

jzabroski commented Jul 7, 2023

One last thought not addressed by this proposal.

My main problem with Visual Studio Package Manager is that when I change the Package Source from All to my company-specific one, if I update all, I sometimes miss cross-dependencies with third party packages, so update fails because, for example, CsvHelper gets updated constantly and our Core library references it, and if we publish a new Core version and someone has dropped in a specific direct reference to CsvHelper, Update tab wont work properly when Package Source = Company Internal.

In this sense, I wish the Package Source dropdown were re-envisioned. Might be a big request but I just have always hated how this works. I wonder if others share my hate.

@jzabroski
Copy link

Sorry, one last comment. I think the last issue people have problems with is when they get a DLL load run-time error due to mismatch between version and roll forward behavior. This happens when Microsoft spins out of box packages into the core framework on major version changes, like the Cryptopgraphy changes that I think happened in .net 6. As an open source package maintainer with this stuff as a deep transitive dependency, it is super annoying that my customers of my free project open bugs against me and dont know the Advanced .NET Ninja Math of DLL dependency hell and how to resolve this conflict.

The tl;dr is it would be great to search for specific assembly versions when searching for packages. Like Visual Studio Watson for DLL Exceptions. Honestly dont understand why the Exception for DLL exceptions cant work like a QR code and automatically link to Nuget search results to help resolve.

@JonDouglas
Copy link
Contributor Author

@killergege That's an excellent suggestion, but may better be addressed in a separate issue if one doesn't exist already. In short, how we may provide more flexibility to developers on package statuses and providing update behavior.

@jzabroski Thanks for the comments, but I'm having a hard time understanding what you are commenting on regarding what is proposed here. If you want to see what packages support a specific framework, this proposal is going to introduce framework filters similar to https://devblogs.microsoft.com/nuget/introducing-search-by-target-framework-on-nuget-org/ for your project/solution context. If you simply do not want search filters but want something else entirely, please raise a separate issue and downvote this proposal. This a highly requested feature based on developer interviews and quarterly survey results of n=500+. I will add some further information to this proposal for transparency sake.

@jzabroski
Copy link

@JonDouglas OK, I commented on the most relevant thing to this focused proposal, here: #12704 (comment)

My comments were meant to be, yes, filtering and sorting is good, but why are we searching in the first place? What common tasks do we have? I dont know if I am unique in my tasks, but I thought I would try to list out my user stories and think through which can be satisfied by some combination of the features listed.

I also see from your criticism that I missed the comment that frameworks will be searchable; I was looking at the screenshot in the Rendered Proposal, which doesnt show that ability. If that is there, then I think that meets a good deal of user stories. If version search includes package versions AND assembly versions, then I think most of my user stories are met. The remaining complex search is the transitive dependency search. I often need to know, what package includes/included this package or assembly name. I do these searches manually today.


When a developer is using Visual Studio to search and manage NuGet packages, they will see a new filter and sort experience.

The browse filter experience will allow the developer to filter results based on common concepts such as "frameworks", "prerelease", "owners", "download count", "license", and "created on".
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we know what will happen or what customers expect to happen for embedded licenses?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

license support will only be for properly expressed licenses. anything else would likely not work with the filter. data suggests this is a very rare case too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is good rationale as to why: https://github.com/aaronpowell/dotnet-delice#undetermined-licenses and follows my thinking.


When a developer is using Visual Studio to search and manage NuGet packages, they will see a new filter and sort experience.

The browse filter experience will allow the developer to filter results based on common concepts such as "frameworks", "prerelease", "owners", "download count", "license", and "created on".
Copy link
Contributor

Choose a reason for hiding this comment

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

Would having tags as a keyword filter be useful? I haven't thought through it, but wondering if customers mentioned tags at all, or if we understand how tags are impacting those google searches they referred to as being useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tags are not in scope right now as they are hardly used for even the top 100 packages in helpful ways. The list provided here comes directly from survey results. Tags was not a popular request.

@ghost ghost added the Status:No recent activity No recent activity. label Aug 13, 2023
@ghost
Copy link

ghost commented Aug 13, 2023

This PR has been automatically marked as stale because it has no activity for 30 days. It will be closed if no further activity occurs within another 15 days of this comment, unless it has a "Status:Do not auto close" label. If it is closed, you may reopen it anytime when you're ready again, as long as you don't delete the branch.

@ghost ghost closed this Aug 28, 2023
@JonDouglas JonDouglas reopened this Sep 20, 2023
@ghost ghost removed the Status:No recent activity No recent activity. label Sep 20, 2023
@JonDouglas
Copy link
Contributor Author

Re-opening this proposal to move forward with merging. While there are open questions on various things, I do not see anything suggesting that filters/sorting is something we should not do. If people would like to merge this, we will avoid another auto-close and have this proposal in the repo instead for when we can start work on it.

@nkolev92
Copy link
Member

nkolev92 commented Sep 20, 2023

@JonDouglas
There's been a lot of discussions on this PR. Scrolling through the comments, none of the threads are resolved, so it's not really clear to me what's been addressed and what not.

I think it'd be great to give people the chance to re-review the latest commit if you think the spec is ready.

Please re-request review:

image

@JonDouglas JonDouglas requested review from a team September 20, 2023 16:58

When a developer is using Visual Studio to search and manage NuGet packages, they will see a new filter and sort experience.

The browse filter experience will allow the developer to filter results based on common concepts such as "frameworks", "prerelease", "owners", "download count", "license", and "created on".
Copy link
Member

Choose a reason for hiding this comment

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

The owners filter does not make sense on many non-NuGet.org feeds. I assume this is explicitly NOT the authors field which is metadata and not validated. The owners field on NuGet.org (not the silly .nuspec thing, the real owners) is trustable and good to filter on I imagine. But the concept doesn't translate everywhere. This aligns with @zivkan's point about package sources advertising which fields they support filtering on. So ADO could opt out of filtering on owner since the concept doesn't translate well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

owners specifically represents the entity that currently owns the package on the registry/source. Not to be confused with the owners/authors nuspec fields.

this mostly applies to the central registry (nuget.org).

Copy link
Member

Choose a reason for hiding this comment

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

If there are concepts that apply to nuget.org only, I think we should explicitly call them out in the spec and make sure those are hidden when a source does not support them.


When a developer is using Visual Studio to search and manage NuGet packages, they will see a new filter and sort experience.

The browse filter experience will allow the developer to filter results based on common concepts such as "frameworks", "prerelease", "owners", "download count", "license", and "created on".
Copy link
Member

Choose a reason for hiding this comment

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

Do these filters apply only to the latest package version per package ID? This is how search works today except for the prerelease filter. Said another way, should an older package version of the same ID be returned if it matches one of these filters, instead of having the package ID entirely excluded from the result set? This would be a big change for the server side since we would need to index all package versions, not just the latest.

This is a nuanced space, so let's take a real example:
Consider the package Polly. It has the tag Hedging to the 8.0.0-beta.2 release (this is the latest prerelease version). This tag does not exist on 7.2.4 (this is the latest stable version).

If you search for tag:Hedging with prerelease included, you see Polly at the top.
image

If you uncheck prerelease, Polly disappears entirely. That's the "added metadata" case meaning the latest version matches a filter but the non-latest does not.
image

It's a similar story for the "removed metadata" case. If you search tag:AppFabric + exclude prerelease, you see CacheManager.Core at the top. If you include prerelease, you no longer see CacheManager.Core at all.

All of this is because currently the search service only operates on the latest package version's metadata w.r.t to the "include prerelease" and (this is a small detail) the include SemVer 2.0.0 flag (which all recent clients set to true). So we index up to 4 package versions per package ID. 4 because there are 4 combinations of a prerelease flag and include SemVer 2.0.0 flag.

For some filters, this behavior is probably fine. But what about frameworks? If the latest version of a package does not support net46 for example, the package consumer probably wants to try an older version of the package which still has that support.

I think the spec should call out whether the filtering applies only to the latest package version or to all package versions. If it's the latter then it will be a much bigger ask for the server side to implement since we'll need to index a lot more data (400K package IDs vs. 6 million package versions).

Copy link
Member

Choose a reason for hiding this comment

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

This is my Kusto query for reference

NiPackageVersions
| where IsLatestSemVer2 or IsLatestStableSemVer2
| join kind=inner NiPackageManifests on Identity
| summarize
    LatestVersion = take_anyif(Version, IsLatestSemVer2),
    LatestStableVersion = take_anyif(Version, IsLatestStableSemVer2),
    LatestTags = take_anyif(array_sort_asc(SplitTags), IsLatestSemVer2),
    LatestSplitTags = take_anyif(array_sort_asc(SplitTags), IsLatestStableSemVer2)
    by LowerId
| where isnotempty(LatestVersion) and isnotempty(LatestStableVersion)
| where LatestVersion != LatestStableVersion
| extend Added = set_difference(LatestTags, LatestSplitTags)
| extend Removed = set_difference(LatestSplitTags, LatestTags)
| where array_length(Added) > 0 or array_length(Removed) > 0
| join kind=inner (NiPackageDownloads | distinct LowerId, TotalDownloads) on LowerId
| order by TotalDownloads desc
| project LowerId, LatestStableVersion, LatestVersion, Added, Removed, TotalDownloads

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be consistent with the current experiences as seen on screen. No experience is changing here outside the concept that you can apply additional filters and quickly sort results. So that would be latest version of a package from both the source (browse tab) and locally installed (installed/updates tab)

Frameworks would also behave this way for now. If it isn't found on stable or pre-rel channels, then the functionality just doesn't show it. Mirroring search by tfm behavior on nuget.org.

Choose a reason for hiding this comment

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

All of this is because currently the search service only operates on the latest package version's metadata w.r.t to the "include prerelease" and (this is a small detail) the include SemVer 2.0.0 flag (which all recent clients set to true). So we index up to 4 package versions per package ID. 4 because there are 4 combinations of a prerelease flag and include SemVer 2.0.0 flag.

Wow! That makes a lot of sense as to what behavior I have seen in the past. Unlike some, I have to maintain a lot of legacy code and often feel like Microsoft is almost deliberately tanking my experience to somehow coerce me to upgrade (as if I have not tried - it's taken 6 years to get off Framework and we're finally going to be there in a few months).

I can't tell from your comments if this is something you plan to improve, but I greatly appreciate you explaining it as a limitation.

Copy link
Member

Choose a reason for hiding this comment

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

In the case of NuGet.org, we're not deliberately limiting the search of older version metadata to coerce users. We do it as a way to scope the amount of metadata that we index. And, perhaps most significantly, it was a decision made long ago that resulted in the UI and protocol working a specific way.

The UI and protocol all center around ID-level results, not version-level results. This does lead to smaller and perhaps more understandable results. I don't think we'd want to crowd the top results with a bunch of Newtonsoft.Json versions and I also think it's reasonable to respect the most recent wishes of the package author on what their package metadata is (i.e. the latest version metadata). Packages are immutable so there's no other way to change your metadata than to release a new version. Perhaps there's a middle ground where we only index packages when certain metadata fields change or we somehow allow users to opt into an "all version" search, but that's just not implemented today and we haven't had many complaints about it (or any in recent years that I know of). The cost would be big to change the decision at this point so we'd need a compelling reason for the user experience. Based on @JonDouglas's comment, it seems like this spec isn't the time to change course on the "indexing all vs. latest version" decision.

We have a million things on our backlog so we have to be careful about the big changes we pick up so we don't overwhelm the team. If you feel that this is something we should change, please feel free to open a proposal and gather support with upvotes (perhaps starting with a bug report on NuGet/NuGetGallery).

@joelverhagen
Copy link
Member

@jzabroski sorry I missed your question here. I've been behind on reviewing specs due to some big investments on the services we run.

Can Joel tell us how many packages today even use the deprecated flag?

LatestIsDeprecated LatestIsVulnerable TopDeprecated TopVulnerable
341 2 microsoft.aspnetcore.http.abstractions razorengine

So 341 of the top 5000 packages are deprecated on their latest version (including prerelease). 2 are vulnerable (yay, that's low). So in my opinion it's a reasonable filter to have and could even be defaulted to On (by default filter out deprecated and vulnerable).

Kusto query for internal reference
NiPackageVersions
| where IsLatestSemVer2 or IsLatestStableSemVer2
| project LowerId, Identity, Id, Version, IsLatestSemVer2, IsLatestStableSemVer2
| join kind=inner (NiPackageVulnerabilities | project VulnerableResultType = ResultType, Identity) on Identity
| project-away Identity1
| join kind=inner (NiPackageDeprecations | project DeprecatedResultType = ResultType, Identity) on Identity
| project-away Identity1
| summarize
    LatestIsDeprecated = countif(DeprecatedResultType == "Deprecated" and IsLatestSemVer2) > 0,
    LatestIsVulnerable = countif(VulnerableResultType == "Vulnerable" and IsLatestSemVer2) > 0,
    LatestStableIsDeprecated = countif(DeprecatedResultType == "Deprecated" and IsLatestStableSemVer2) > 0,
    LatestStableIsVulnerable = countif(VulnerableResultType == "Vulnerable" and IsLatestStableSemVer2) > 0
    by LowerId
| join kind=inner (NiPackageDownloads | distinct LowerId, TotalDownloads) on LowerId
| project-away LowerId1
| order by TotalDownloads desc
| take 5000
| summarize
    LatestIsDeprecated = countif(LatestIsDeprecated),
    LatestIsVulnerable = countif(LatestIsVulnerable),
    (_, TopDeprecated) = arg_max(TotalDownloads * iff(LatestIsDeprecated, 1, 0), LowerId),
    (__, TopVulnerable) = arg_max(TotalDownloads * iff(LatestIsVulnerable, 1, 0), LowerId)
| project-away _, __

@jzabroski
Copy link

Can Joel tell us how many packages today even use the deprecated flag?

LatestIsDeprecated LatestIsVulnerable TopDeprecated TopVulnerable
341 2 microsoft.aspnetcore.http.abstractions razorengine

Yikes. If RazorEngine is top vulnerable, then my RazorLight is probably not far behind. Thanks for sharing!


Whenever a filter is applied, it will additionally show up in the Visual Studio search box so that developers get used to the search prefix syntax and can apply filters more quickly in the future.

The sorting experience will allow the developer to sort these options on common concepts such as "alphabetical", "version", "relevance", "downloads", and "recently updated".
Copy link
Member

Choose a reason for hiding this comment

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

I think we're missing the clarity on what exact filters we want to do and the feasibility of them.

We discussed some of those concerns in https://github.com/NuGet/Home/pull/12704/files#r1244465668, so it'd be great to incorporate that here.

I think it might make it easier to review if there are different sections for installed vs updates.

@ghost ghost added the Status:No recent activity No recent activity. label Oct 26, 2023
@ghost
Copy link

ghost commented Oct 26, 2023

This PR has been automatically marked as stale because it has no activity for 30 days. It will be closed if no further activity occurs within another 15 days of this comment, unless it has a "Status:Do not auto close" label. If it is closed, you may reopen it anytime when you're ready again, as long as you don't delete the branch.

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.

9 participants