fix(bundler): resolve catalog search at highest-precedence source before filtering#3331
Open
Quratulain-bilal wants to merge 1 commit into
Open
fix(bundler): resolve catalog search at highest-precedence source before filtering#3331Quratulain-bilal wants to merge 1 commit into
Quratulain-bilal wants to merge 1 commit into
Conversation
…ore filtering
CatalogStack.search() claimed a bundle id in `seen` only when the entry matched
the query. so when the highest-precedence entry for an id did NOT match, a
lower-precedence entry with the same id could match and be returned instead --
even though resolve()/install always use the highest-precedence entry. search
advertised a bundle (name, version, source) the user could never actually get,
contradicting the method's own docstring ("resolved at its highest-precedence
source").
resolve every id to its highest-precedence entry first, then filter the
resolved set by the query. search now agrees with resolve(): a query that only
a shadowed lower-precedence copy matches returns nothing.
add a regression test covering the shadowed-entry case.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #3330
what
CatalogStack.search()claimed a bundle id inseenonly when the entry matched the query. so when the highest-precedence entry for an id did not match, a lower-precedence entry with the same id could match and be returned instead — even thoughresolve()/install always use the highest-precedence entry. search advertised a bundle the user could never actually install, contradicting the method's own docstring ("resolved at its highest-precedence source").how
resolve every id to its highest-precedence entry first, then filter the resolved set by the query. search now agrees with
resolve().reproduction (fixed)
id
sharedin two catalogs — high-precedence "Alpha Tool", low-precedence "Searchable Widget":search("widget")returned the shadowed low-precedence "Searchable Widget"search("widget")returns nothing (that entry is not installable);search("alpha tool")returns the high-precedence entrytests
added
test_search_does_not_surface_a_shadowed_lower_precedence_entry. verified it fails on the current code (returns the shadowed entry) and passes with the fix; the existingtest_search_dedupes_by_precedence_and_filtersand the rest of the file still pass.