[ListCommand] Fix --source filter not restricting list output to specified source#6159
Conversation
…ified source ## Problem - `winget list --source <name>` displayed ALL installed packages (~270+) instead of only those correlated with the specified source - The `ReportListResult` workflow task had no source-filtering logic; it relied solely on `m_onlyShowUpgrades` to decide whether to skip rows, which is always false for the `list` command - Users could not determine which packages were installed from a specific source ## Solution - Add a `sourceFilterProvided` check using `context.Args.Contains(Execution::Args::Type::Source)` to detect when `--source` is explicitly passed - Skip packages where `latestVersion` is null when source filtering is active, since null `latestVersion` means the installed package has no correlation with the specified source's available packages - The fix leverages the existing CompositeSource correlation mechanism -- packages that match the specified source already receive a non-null `latestVersion` from the composite search ## Impact - `winget list --source devsource` now returns only packages correlated with that source (e.g., 2 instead of 270+) - `winget list` (no source filter) behavior is unchanged -- all installed packages still appear - `winget upgrade --source` was already correct (guarded by `m_onlyShowUpgrades`) and is unaffected ## How Validated - Built and deployed to wingetdev - Verified `wingetdev list` returns 283 packages (no regression) - Verified `wingetdev list --source winget` returns 92 packages (filtered correctly) - Verified `wingetdev list --source devsource` returns 2 packages (was 270+ before fix) - Verified `wingetdev upgrade --source devsource` shows 1 upgrade (7zip 25.01->26.00)
JohnMcPMS
left a comment
There was a problem hiding this comment.
The root cause
This suggests that it is a bug, when any issue was actually a difference in expectations between some users and the original developer intent. Given that, it is important that we clearly define the new expectations and preferably have some amount of confidence that this is in line with more of the users than previously (or we provide options rather than only a single new behavior).
A package in multiple sources correctly appears in results for either source.
Is that behavior "correct"? There is no linked issue, and thus no indication of the goal of the change.
I could see two options:
- Source filtering answers "Is there a correlated package in the target source?" :: The goal is simply to filter the packages based on what is available, almost like a
winget update -s winget --show-packages-without-update-too. This is sort of a forward-facing view in that you are looking for what is possible. This seems like the goal of the code presented here (although it misses in some ways). - Source filtering answers "Which packages were installed from the target source?" :: The goal is to get a view into our tracking data / local history. The user would be wanting to see packages that came from the source. This is the rear-facing view; what has happened.
The current GA code is simply a selector on which source we use to find available packages, just like it is everywhere else.
I feel like option 1 is more targeted at update scenarios and is already covered by the update command except that it won't show packages without updates. But for that case the user would need to want to answer "Which packages are available from the target source, but are not currently an update?" That seems like a narrow use case.
Option 2 is something that isn't currently easily accessible and is what I have believed the complaint about the current behavior to be. If this is the goal, there are then further questions about the behavior that should be asked. For instance, if I install from source A, then later from source B, should the package show up in the list when I target source A?
| // The only time we don't want to output a line is when filtering and no update is available. | ||
| // When --source is given, only show packages that have a correlation (available version) | ||
| // in the specified source. Packages with no available match are not from that source. | ||
| if (sourceFilterProvided && !latestVersion) |
There was a problem hiding this comment.
latestVersion is only about potential upgrades; it has nothing to do with the origin on the package that is installed.
latestVersion will be empty if the package is pinned and cannot be upgraded. I suppose depending on the goal of the change, that could make sense.
There was a problem hiding this comment.
You're right that latestVersion is semantically about upgrades, not about source correlation directly. Here's why it works as a reliable proxy in this specific code path:
When the user runs "winget list --source X", the composite source is constructed with only source X as the available side. OpenSource (line 685-687) opens only the named source, and OpenCompositeSource (line 796) pairs it with the installed source. So all downstream version queries are scoped to that one source.
For the "winget list" path specifically, pinBehavior is set to IgnorePins (line 1116). This clears m_database in the PinStateEvaluator constructor (PinningData.cpp, lines 118-124), which causes GetLatestAvailableVersionForPins to skip all pin logic and fall through to package->GetLatestVersion() (PinningData.cpp, lines 150-152). In this path, latestVersion being null means "no available version was found in the composite" -- which in turn means the package did not correlate with the specified source.
For the "--upgrade-available" path, where ConsiderPins is active, pinned packages are already handled at lines 1153-1177 before reaching this check, so the pin concern does not apply there either.
If you feel a more direct check would be clearer -- for example, checking whether availableVersions has any version keys rather than relying on latestVersion being null -- I am happy to make that change. Let me know how you would prefer this to read.
This is also what I interpreted the request to be, since there are ways to get the list of packages that are installed and available from the WinGet source with other means. However, those other means are also listed in some of the linked issues and explicitly called out that it would be nice to have a way to easily filter that. It does seem like a majority of the requests are geared towards option 1 as opposed to option 2 |
|
From just #4236 :
The OP uses "available in that source" which points to Option 1 to me. Comments use "winget sourced packages" and "the ones from the [...] source", which are not clear. This feels like it is @denelon filing a "I have received comments" issue and he needs to set the record on what we should do here. Responding to the comment that I see in email but is gone here: For option 2 you can definitely determine install source as each source has its own database. It doesn't need to be in the schema. The determination is already done, it would just be a different filtering mechanism. This was what I was referring to in our hallway chat and I would suggest that we add a new search behavior to implement it. |
Fixes #4236
Summary
winget list --source <name>displayed ALL installed packages instead of only those correlated with the specified source. The root cause was thatReportListResulthad no source-filtering logic -- the only row-skipping guard (m_onlyShowUpgrades) is always false for thelistcommand, so every installed package was unconditionally displayed. This PR adds a source-correlation check that skips packages with no available match in the specified source.Root Cause
When
--sourceis provided,OpenCompositeSource(Installed)creates a composite of installed (ARP) packages correlated against the specified source. Correlated packages get a non-nulllatestVersion; uncorrelated ones get null. However,ReportListResultnever used this signal for filtering. The only skip logic at line 1186 (if (updateAvailable || !m_onlyShowUpgrades)) is always true forlistbecausem_onlyShowUpgrades=false.latestVersionwas only checked for display formatting, never for row filtering.What Changed
sourceFilterProvidedflag that checkscontext.Args.Contains(Execution::Args::Type::Source). When the flag is set andlatestVersionis null (meaning the installed package has no correlation with the specified source's available packages), the package is skipped withcontinue.updateAvailable || !m_onlyShowUpgradesguard. This ensures non-correlated packages are filtered out early, regardless of them_onlyShowUpgradesstate.Design Notes
ReportListResult), leveraging the existing CompositeSource correlation mechanism without modifying search semantics.--upgrade-available-- non-correlated packages were already excluded byupdateAvailable=false, but the new check adds an explicit early exit.--sourcefilters by correlation (does the source's index contain this package ID?), not by install origin. A package in multiple sources correctly appears in results for either source.Impact
winget list --source <name>now returns only packages correlated with the specified sourcewinget list(no source filter) is unchanged -- all installed packages still appearwinget list --upgrade-available --source <name>correctly intersects both filters (source correlation AND upgrade availability)winget list --upgrade-available(no source filter) is unchanged -- all upgradable packages still appearHow Validated
wingetdevdeployed with fix, using custom dev source (devsource):wingetdev list- all installed packages shown (no regression)wingetdev list --source winget- only winget-correlated packages shownwingetdev list --source devsource- only devsource-correlated packages shown (was showing all before fix)wingetdev list --upgrade-available- all upgradable packages shown (no regression)wingetdev list --upgrade-available --source devsource- only upgradable packages from devsource shownCompositeSourcetests validate correlation; no directReportListResultsource filter tests exist.Microsoft Reviewers: Open in CodeFlow