diff --git a/CHANGELOG.md b/CHANGELOG.md index a8eace4..d3e4a60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,13 @@ served it. and uv do.** A final release in range is still preferred. Some resolutions that used to fail or backtrack now pick a pre-release or a newer version instead. +### Changed + +- **An extra a version does not declare is now ignored, as pip and uv do, instead of excluding + that version.** A resolution can move to a newer version, and a misspelled extra no longer + fails. Each ignored extra is reported in the new `Resolution.MissingExtras` field + (`MissingExtra`, `Requester`). + ## [0.11.0] - 2026-09-18 ### Breaking diff --git a/CLAUDE.md b/CLAUDE.md index 190bfd4..9dfb5ef 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -7,7 +7,7 @@ Phase 3. > **Status: implemented end to end.** `pypirsf/`, `index/`, `pep440set/`, > `candidate/`, `provider/` and `resolver/` all carry code. `resolver.Resolve` > is the entry point; keep its exported surface to `Resolve`, `Options`, -> `Resolution` and `ResolutionError`. +> `Resolution`, `ResolutionError`, `MissingExtra` and `Requester`. > > ⚠️ `resolver` is **no longer the only package Package Manager imports**. PPM > implements its own `MetadataIndex` (`src/pyindex`) and imports `index/` for the diff --git a/provider/dependencies.go b/provider/dependencies.go index 7223441..0cd27e1 100644 --- a/provider/dependencies.go +++ b/provider/dependencies.go @@ -7,6 +7,7 @@ import ( "fmt" "slices" "strconv" + "strings" "github.com/posit-dev/go-pubgrub/solver" "github.com/posit-dev/go-pyresolver/index" @@ -89,6 +90,7 @@ func (p *Provider) rootDependencies() ([]dependency, error) { // aborts the resolve rather than excluding anything. return nil, fmt.Errorf("provider: the requested requirements cannot be resolved: %s", reason) } + p.recordExtraRequests(Root(), version.Version{}, expanded) return append(deps, expanded...), nil } @@ -182,22 +184,20 @@ func (p *Provider) dependenciesFrom( deps = append(deps, pyDep) } } else { - // An extra nobody declared must fail loudly. Without this check - // pkg[tests], where the extra is spelled test, resolves happily and - // installs nothing extra -- which looks like success. Reporting it as - // "no candidate version" is what lets the solver explain it through - // the derivation graph for free. - if !slices.Contains(meta.ProvidesExtra, pkg.Extra) { - return nil, fmt.Sprintf("it does not provide the extra %q", pkg.Extra), nil + // An extra nobody declared is ignored, as pip and uv do, rather than + // excluding the version. It is reported in Resolution.MissingExtras. + if slices.Contains(meta.ProvidesExtra, pkg.Extra) { + active = []string{pkg.Extra} + reqs = extraOnly(meta.RequiresDist, p.opts.Environment, active) + } else { + reqs = nil + p.recordUndeclaredExtra(pkg.Name, v, pkg.Extra) } - active = []string{pkg.Extra} - // The same-version link. Without it the extra could resolve to a - // version other than the base package it is an extra OF, and the - // installed set would be incoherent. + // The same-version link, whether or not the extra is declared. Without + // it the extra could resolve to a version other than the base package + // it is an extra OF, and the installed set would be incoherent. deps = append(deps, dependency{Package: Project(pkg.Name), Allowed: pep440set.Exactly(v)}) - - reqs = extraOnly(meta.RequiresDist, p.opts.Environment, active) } expanded, reason, err := expandRequirements(reqs, p.opts.Environment, active) @@ -207,6 +207,11 @@ func (p *Provider) dependenciesFrom( if reason != "" { return nil, reason, nil } + // pkg itself, not Project(pkg.Name): when pkg is an extra, the request came + // from THAT extra, not from the base with no extra active. Dropping the + // extra here would let resolver.missingExtras attribute the request to a + // base that survives while the extra that actually asked was abandoned. + p.recordExtraRequests(pkg, v, expanded) // Only now is the version definitely offered, so only now is an // Offered:true record truthful. @@ -329,3 +334,92 @@ func expandRequirements(reqs []requirement.Requirement, env marker.Environment, return deps, "", nil } + +// ExtraRequest is one requester -> package[extra] edge, recorded as +// expandRequirements builds a dependency onto an extra virtual package. It +// says who asked, independent of whether the target version turns out to +// declare the extra. +// +// This is what was ASKED for, not what the resolution settled on: the solver +// can visit this edge on a branch it later backtracks past. resolver.Resolve +// keeps only the edges the final solution still contains. +type ExtraRequest struct { + // Requester is Root() or a Package identifying who asked. When the + // requester is itself an extra (e.g. base[extraA] asking for + // other[extraB]), Requester.Extra carries that -- resolver.missingExtras + // needs it to tell an abandoned base[extraA] apart from a base that + // survived with no extra active. + Requester Package + + // RequesterVersion is the requester's pinned version. Meaningless when + // Requester is Root(). + RequesterVersion version.Version + + // Package is the project whose extra was requested. + Package index.PackageName + + // Extra is the PEP 685-normalized extra requested. + Extra string +} + +// ExtraRequests returns every requester -> package[extra] edge this Provider +// has seen, including ones on a branch the solver later backtracked past. +func (p *Provider) ExtraRequests() []ExtraRequest { + return p.extraRequests +} + +// recordExtraRequests scans deps for edges onto an extra virtual package. +// rootDependencies and dependenciesFrom call this once they have expanded +// their own requirements, because they are the ones who know their requester +// identity -- the solver's Dependencies method is never told who is asking. +func (p *Provider) recordExtraRequests(requester Package, requesterVersion version.Version, deps []dependency) { + for _, d := range deps { + if d.Package.Kind != KindProject || d.Package.Extra == "" { + continue + } + p.recordExtraRequest(requester, requesterVersion, d.Package.Name, d.Package.Extra) + } +} + +// recordExtraRequest adds one edge, ignoring a repeat of one already held. See +// Provider.record for why the dedupe key is built from strings. +func (p *Provider) recordExtraRequest( + requester Package, requesterVersion version.Version, name index.PackageName, extra string, +) { + key := strings.Join([]string{requester.String(), requesterVersion.String(), string(name), extra}, "\x00") + if p.extraRequestsSeen[key] { + return + } + p.extraRequestsSeen[key] = true + p.extraRequests = append(p.extraRequests, ExtraRequest{ + Requester: requester, + RequesterVersion: requesterVersion, + Package: name, + Extra: extra, + }) +} + +// UndeclaredExtra records that one project's specific version does not +// declare an extra the resolution asked about. +type UndeclaredExtra struct { + Package index.PackageName + Version version.Version + Extra string +} + +// UndeclaredExtras returns every (package, version, extra) this Provider found +// not declared, including versions the solver later backtracked past. +func (p *Provider) UndeclaredExtras() []UndeclaredExtra { + return p.undeclaredExtras +} + +// recordUndeclaredExtra adds one record, ignoring a repeat of one already +// held. +func (p *Provider) recordUndeclaredExtra(name index.PackageName, v version.Version, extra string) { + key := strings.Join([]string{string(name), v.String(), extra}, "\x00") + if p.undeclaredExtrasSeen[key] { + return + } + p.undeclaredExtrasSeen[key] = true + p.undeclaredExtras = append(p.undeclaredExtras, UndeclaredExtra{Package: name, Version: v, Extra: extra}) +} diff --git a/provider/extras_test.go b/provider/extras_test.go index 75e8e0e..115962d 100644 --- a/provider/extras_test.go +++ b/provider/extras_test.go @@ -77,11 +77,11 @@ func TestExtraDependsOnItsBaseAtExactlyTheSameVersion(t *testing.T) { } } -// PackageMetadata.ProvidesExtra exists precisely so pkg[tests] where the extra -// is spelled test does not resolve happily and install nothing. Asserted -// through Candidates, because found == false is exactly the signal the solver -// reads as "no such thing" and turns into an explanation. -func TestUnknownExtraHasNoCandidates(t *testing.T) { +// An undeclared extra is ignored, as pip and uv do, rather than excluding the +// version: pkg[tests], where the extra is spelled test, still resolves to the +// newest version. Asserted through Candidates, since found == true and best +// unchanged is the signal the solver reads as "usable". +func TestUnknownExtraHasCandidatesAndPicksTheNewestVersion(t *testing.T) { idx := index.NewMockIndex("test"). SetMetadata("flask", "3.0.0", index.PackageMetadata{ ProvidesExtra: []string{"async"}, @@ -89,8 +89,12 @@ func TestUnknownExtraHasNoCandidates(t *testing.T) { p := provider.New(context.Background(), idx, testOptions(t)) - if _, found, _, err := p.Candidates(provider.WithExtra("flask", "asynk"), pep440set.All()); err != nil || found { - t.Errorf("misspelled extra: found = %v, err = %v; want false, nil", found, err) + best, found, _, err := p.Candidates(provider.WithExtra("flask", "asynk"), pep440set.All()) + if err != nil || !found { + t.Fatalf("misspelled extra: found = %v, err = %v; want true, nil", found, err) + } + if got := bestVersion(t, best); got.String() != "3.0.0" { + t.Errorf("misspelled extra: best = %s, want 3.0.0", got) } if _, found, _, err := p.Candidates(provider.WithExtra("flask", "async"), pep440set.All()); err != nil || !found { t.Errorf("declared extra: found = %v, err = %v; want true, nil", found, err) @@ -101,18 +105,12 @@ func TestUnknownExtraHasNoCandidates(t *testing.T) { } } -// Only the versions that declare the extra are SELECTABLE for it, which is what -// makes "this package has that extra only from 3.0 on" resolvable rather than a -// silent no-op. -// -// ⚠️ Note what rank does and does not say here. Three versions are in range and -// only two provide the extra, and rank reports 3 — it counts what is in range -// before usability is tested, deliberately, because testing usability is the cost -// this provider exists to avoid. Over-counting is what this provider chooses -- -// go-pubgrub requires no bound either way -- and this is that gap in action. What -// must still be exact is best (the newest version actually providing the extra) -// and found. -func TestCandidatesForAnExtraSelectOnlyVersionsThatProvideIt(t *testing.T) { +// Every version is SELECTABLE for an extra, whether or not it declares one: +// an undeclared extra is ignored rather than excluding the version. best is +// the newest version overall, not the newest that happens to provide the +// extra -- 2.0 (which does not declare "async") is exactly as usable as 3.0 +// and 4.0 here. +func TestCandidatesForAnExtraAreEveryVersionRegardlessOfWhatItProvides(t *testing.T) { idx := index.NewMockIndex("test"). SetMetadata("flask", "2.0", index.PackageMetadata{}). SetMetadata("flask", "3.0", index.PackageMetadata{ProvidesExtra: []string{"async"}}). @@ -125,14 +123,13 @@ func TestCandidatesForAnExtraSelectOnlyVersionsThatProvideIt(t *testing.T) { t.Fatalf("Candidates: %v", err) } if !found { - t.Fatal("found = false, want true: 3.0 and 4.0 both provide the extra") + t.Fatal("found = false, want true: all three versions are usable") } if got := bestVersion(t, best); got.String() != "4.0" { - t.Errorf("best = %s, want 4.0 — the newest version that actually provides the extra, "+ - "which is the part that must NOT be approximate", got) + t.Errorf("best = %s, want 4.0 — the newest version overall", got) } - if rank < 2 { - t.Errorf("rank = %d, want at least 2: rank may over-count but must never under-count "+ + if rank < 3 { + t.Errorf("rank = %d, want at least 3: rank may over-count but must never under-count "+ "the usable versions, or the heuristic would prefer this package over one that "+ "genuinely has fewer", rank) } diff --git a/provider/provider.go b/provider/provider.go index 41cf846..b36cff5 100644 --- a/provider/provider.go +++ b/provider/provider.go @@ -96,6 +96,20 @@ type Provider struct { unusable []Unusable recorded map[string]bool + // extraRequests holds every requester -> package[extra] edge seen while + // expanding requirements, and extraRequestsSeen is its dedupe key set. See + // recordExtraRequest. This is what was ASKED for, independent of whether + // the target version ends up declaring the extra -- resolver.Resolve + // filters it against the final solution to build Resolution.MissingExtras. + extraRequests []ExtraRequest + extraRequestsSeen map[string]bool + + // undeclaredExtras holds each (package, version, extra) this Provider found + // not declared, and undeclaredExtrasSeen is its dedupe key set. See + // recordUndeclaredExtra. + undeclaredExtras []UndeclaredExtra + undeclaredExtrasSeen map[string]bool + // ranked memoizes candidate.Rank over a package's FULL version list, so the // sort is paid once per package per resolution rather than once per // Candidates call. See rankedVersions. @@ -120,12 +134,14 @@ func New(ctx context.Context, idx index.MetadataIndex, opts Options) *Provider { opts.RootVersion = version.MustParse("0") } return &Provider{ - ctx: ctx, - index: idx, - opts: opts, - recorded: make(map[string]bool), - ranked: make(map[index.PackageName][]version.Version), - tagFilter: tagFilteringEnabled(idx, opts.WheelTags), + ctx: ctx, + index: idx, + opts: opts, + recorded: make(map[string]bool), + extraRequestsSeen: make(map[string]bool), + undeclaredExtrasSeen: make(map[string]bool), + ranked: make(map[index.PackageName][]version.Version), + tagFilter: tagFilteringEnabled(idx, opts.WheelTags), } } diff --git a/provider/solve_test.go b/provider/solve_test.go index 8749c53..e38fa69 100644 --- a/provider/solve_test.go +++ b/provider/solve_test.go @@ -239,25 +239,27 @@ func TestSolveWithoutTheExtraLeavesItsRequirementOut(t *testing.T) { // A misspelled extra must fail the resolve rather than install nothing and // report success. This is the end-to-end form of the ProvidesExtra check. -func TestSolveMisspelledExtraFails(t *testing.T) { +// A misspelled extra is ignored, as pip and uv do, rather than failing the +// resolve: it does not backtrack away from the newest version, and it does +// not pull in the real extra's requirements. See the paired resolver test, +// TestResolveReportsAMissingExtra, for Resolution.MissingExtras. +func TestSolveMisspelledExtraIsIgnoredAndPicksTheNewestVersion(t *testing.T) { idx := index.NewMockIndex("test"). - SetMetadata("flask", "3.0", index.PackageMetadata{ProvidesExtra: []string{"async"}}) - - _, _, err := solve(t, idx, "flask[asynk]") + AddVersion("flask", "2.0"). + SetMetadata("flask", "3.0", index.PackageMetadata{ + RequiresDist: mustRequirements(t, `asgiref>=3.2; extra == "async"`), + ProvidesExtra: []string{"async"}, + }). + AddVersion("asgiref", "3.7") - var unsolvable *solver.Unsolvable[provider.Package, pep440set.Set] - if !errors.As(err, &unsolvable) { - t.Fatalf("err = %v, want *solver.Unsolvable", err) + got, _, err := solve(t, idx, "flask[asynk]") + if err != nil { + t.Fatalf("Solve: %v", err) } - // Asserting only the error TYPE would keep this test green if flask became - // unresolvable for some unrelated reason, which is precisely the failure it - // exists to distinguish. Pin the derivation to the misspelled extra. - if !causeMentions(unsolvable.RootCause, func(pkg provider.Package) bool { - return pkg == provider.WithExtra("flask", "asynk") - }) { - t.Errorf("root cause does not mention flask[asynk]; the resolve failed for some other reason: %v", - unsolvable.RootCause) + assertSelected(t, got, map[string]string{"flask": "3.0"}) + if _, ok := got["asgiref"]; ok { + t.Errorf("the misspelled extra pulled in asgiref, which only the real async extra declares: %v", got) } } diff --git a/resolver/packse_test.go b/resolver/packse_test.go index 23dd5af..295f48f 100644 --- a/resolver/packse_test.go +++ b/resolver/packse_test.go @@ -88,12 +88,6 @@ var knownFail = map[string]string{ "PEP 592 (and uv) still allow it when a requirement pins it exactly, which this scenario's root does for b==1.0.0", "yanked/transitive-yanked-and-unyanked-dependency-opt-in": "FilteredIndex.ExcludeYanked drops a yanked version outright; " + "PEP 592 (and uv) still allow it when a requirement pins it exactly, which this scenario's root does for c==2.0.0", - - "extras/missing-extra": "go-pyresolver models name[extra] as a virtual package requiring a candidate that " + - "declares the extra, so a version that omits it is excluded rather than the extra being silently dropped; " + - "uv ignores an extra no candidate provides", - "extras/extra-does-not-exist-backtrack": "same gap as extras/missing-extra: the newest version (3.0.0) does not " + - "provide the extra, so go-pyresolver backtracks to the one that does (1.0.0) instead of dropping the extra", } // divergence is one scenario's entry on intentionalDivergence: the reason and @@ -304,6 +298,29 @@ func TestPackse(t *testing.T) { return } + if name == "extras/missing-extra" { + // The harness can assert the warning here, so it does: the root + // asked for a[extra], and 1.0.0 (the only version) does not + // declare it. + res, resolveErr := resolvePackseScenario(t, s) + matched, detail := matchOutcome(t, res, resolveErr, s.Expected.Satisfiable, s.Expected.Packages) + if !matched { + t.Errorf("%s: %s", name, detail) + } + if resolveErr == nil { + want := []resolver.MissingExtra{{ + Package: index.NewPackageName("a"), + Version: version.MustParse("1.0.0"), + Extra: "extra", + RequestedBy: resolver.Requester{Root: true}, + }} + if !reflect.DeepEqual(res.MissingExtras, want) { + t.Errorf("%s: MissingExtras = %+v, want %+v", name, res.MissingExtras, want) + } + } + return + } + matched, detail := runPackseScenario(t, s) switch { case isKnownFail && matched: diff --git a/resolver/resolver.go b/resolver/resolver.go index a89ec27..43199af 100644 --- a/resolver/resolver.go +++ b/resolver/resolver.go @@ -6,6 +6,7 @@ import ( "context" "fmt" "slices" + "strings" "github.com/posit-dev/go-pubgrub/solver" "github.com/posit-dev/go-pyresolver/candidate" @@ -25,8 +26,9 @@ import ( // requires_dist is exactly that: arbitrary text published by third parties. // A resolution that hits this bound fails loudly instead of hanging. // -// Unexported: this package's supported surface is Resolve, Options, Resolution -// and ResolutionError, and a consumer that wants a specific bound sets one. +// Unexported: this package's supported surface is Resolve, Options, Resolution, +// ResolutionError, MissingExtra and Requester, and a consumer that wants a +// specific bound sets one. const defaultMaxRounds = 10_000 // Options configures one resolution. @@ -179,6 +181,49 @@ type Resolution struct { // detect a downgrade, either leave Policy nil or account for the ordering you // imposed. Unusable []provider.Unusable + + // MissingExtras lists each requested package[extra] whose pinned version + // does not declare the extra. go-pyresolver ignores it rather than + // excluding that version, matching pip and uv, and this is the warning a + // caller shows for it. + // + // It describes the FINAL solution only: an extra requested on a branch the + // solver later backtracked past is not reported, the same way Extras never + // names a virtual package that was not ultimately selected. + // + // One entry per (requester, package, extra), sorted by (package, extra, + // requester). + MissingExtras []MissingExtra +} + +// MissingExtra is a requested extra that the pinned version does not declare, +// so it was ignored (pip and uv do the same). +type MissingExtra struct { + // Package is the project whose extra is missing. + Package index.PackageName + + // Version is Package's pinned version -- the one that does not declare + // Extra. + Version version.Version + + // Extra is the PEP 685-normalized extra that was requested. + Extra string + + // RequestedBy is who asked for Package[Extra]. + RequestedBy Requester +} + +// Requester is the root (the caller's own requirements) or one pinned +// package. +type Requester struct { + // Root is true when the caller's own requirements asked for the extra. + Root bool + + // Package is empty when Root. + Package index.PackageName + + // Version is the zero value when Root. + Version version.Version } // rootVersion is the synthetic version of the root package. Nothing in a @@ -231,9 +276,132 @@ func Resolve( // Read from the same provider the failure path reads, so a release set aside // is reported identically whether the resolution went on to succeed or not. res.Unusable = p.Unusable() + filterUndeclaredExtras(res, p.UndeclaredExtras()) + res.MissingExtras = missingExtras(res, p.ExtraRequests(), p.UndeclaredExtras()) return res, nil } +// undeclaredKey identifies one (package, version, extra) triple so +// filterUndeclaredExtras and missingExtras can both test membership in the +// provider's undeclared-extra set without version.Version's Equal, which +// cannot key a map. +func undeclaredKey(name index.PackageName, v version.Version, extra string) string { + return name.String() + "\x00" + v.String() + "\x00" + extra +} + +// filterUndeclaredExtras removes an extra from res.Extras when the pinned +// version does not declare it. Resolution.Extras documents itself as "what a +// caller needs to reproduce the same install", and a version that never +// declared the extra cannot be reproduced by asking for it. +func filterUndeclaredExtras(res *Resolution, undeclared []provider.UndeclaredExtra) { + if len(undeclared) == 0 { + return + } + bad := make(map[string]bool, len(undeclared)) + for _, u := range undeclared { + bad[undeclaredKey(u.Package, u.Version, u.Extra)] = true + } + for name, list := range res.Extras { + v, ok := res.Pinned[name] + if !ok { + continue + } + kept := list[:0] + for _, e := range list { + if !bad[undeclaredKey(name, v, e)] { + kept = append(kept, e) + } + } + if len(kept) == 0 { + delete(res.Extras, name) + } else { + res.Extras[name] = kept + } + } +} + +// requesterKey renders a Requester so missingExtras can sort and dedupe on it +// without version.Version's Equal, which cannot key a map or a sort compare +// directly. +func requesterKey(r Requester) string { + if r.Root { + return "" + } + return r.Package.String() + "@" + r.Version.String() +} + +// missingExtras turns the provider's raw requester -> package[extra] edges +// into MissingExtra values, keeping only what the FINAL solution still +// contains: a requester that is pinned (Root always is) at the recorded +// version, and a target whose pinned version does not declare the extra. +// +// The provider's edges include ones from a branch the solver later +// backtracked past -- see the trap on warnings from versions the solver left +// behind. Filtering against res.Pinned, rather than trusting the edge's own +// recorded facts, is what excludes them. +func missingExtras( + res *Resolution, requests []provider.ExtraRequest, undeclared []provider.UndeclaredExtra, +) []MissingExtra { + bad := make(map[string]bool, len(undeclared)) + for _, u := range undeclared { + bad[undeclaredKey(u.Package, u.Version, u.Extra)] = true + } + + var out []MissingExtra + for _, req := range requests { + var requester Requester + var requesterPinned bool + switch req.Requester.Kind { + case provider.KindRoot: + requester = Requester{Root: true} + requesterPinned = true + case provider.KindProject: + requester = Requester{Package: req.Requester.Name, Version: req.RequesterVersion} + v, ok := res.Pinned[req.Requester.Name] + requesterPinned = ok && v.Equal(req.RequesterVersion) + // When the requester is itself an extra, its own extra must have + // survived into the final solution too -- a name+version match + // alone cannot tell an abandoned base[extra] apart from the base + // that survived with no extra active. + if requesterPinned && req.Requester.Extra != "" { + requesterPinned = slices.Contains(res.Extras[req.Requester.Name], req.Requester.Extra) + } + default: + continue // the interpreter never requests an extra + } + if !requesterPinned { + continue + } + + v, ok := res.Pinned[req.Package] + if !ok || !bad[undeclaredKey(req.Package, v, req.Extra)] { + continue + } + out = append(out, MissingExtra{ + Package: req.Package, + Version: v, + Extra: req.Extra, + RequestedBy: requester, + }) + } + + slices.SortFunc(out, func(a, b MissingExtra) int { + if c := strings.Compare(a.Package.String(), b.Package.String()); c != 0 { + return c + } + if c := strings.Compare(a.Extra, b.Extra); c != 0 { + return c + } + return strings.Compare(requesterKey(a.RequestedBy), requesterKey(b.RequestedBy)) + }) + return slices.CompactFunc(out, func(a, b MissingExtra) bool { + return a.Package == b.Package && a.Extra == b.Extra && a.Version.Equal(b.Version) && + a.RequestedBy.Root == b.RequestedBy.Root && + a.RequestedBy.Package == b.RequestedBy.Package && + a.RequestedBy.Version.Equal(b.RequestedBy.Version) + }) +} + // validate checks that the options describe ONE interpreter. // // This runs before the index is touched. A mismatch caught after a solve would diff --git a/resolver/resolver_test.go b/resolver/resolver_test.go index 5950397..19a763d 100644 --- a/resolver/resolver_test.go +++ b/resolver/resolver_test.go @@ -558,3 +558,168 @@ func TestResolveAcceptsAnEquivalentPythonVersionSpelling(t *testing.T) { t.Fatalf("Resolve rejected an equivalent spelling of the same version: %v", err) } } + +// A misspelled extra is ignored, as pip and uv do: the resolve still picks the +// newest flask, Extras carries no entry for it, and the warning names it in +// MissingExtras instead. Paired with provider's +// TestSolveMisspelledExtraIsIgnoredAndPicksTheNewestVersion, which cannot see +// MissingExtras -- that lives in this package. +func TestResolveReportsAMissingExtra(t *testing.T) { + idx := index.NewMockIndex("test"). + SetMetadata("flask", "3.0", index.PackageMetadata{ + RequiresDist: mustRequirements(t, `asgiref>=3.2; extra == "async"`), + ProvidesExtra: []string{"async"}, + }). + AddVersion("asgiref", "3.7") + + res, err := resolve(t, idx, "flask[asynk]") + if err != nil { + t.Fatalf("Resolve: %v", err) + } + + want := map[string]string{"flask": "3.0"} + if got := pins(t, res); !reflect.DeepEqual(got, want) { + t.Errorf("Pinned = %v, want %v", got, want) + } + if got, ok := res.Extras["flask"]; ok { + t.Errorf("Extras[flask] = %v, want no entry: asynk is not a real extra", got) + } + + wantMissing := []resolver.MissingExtra{{ + Package: index.NewPackageName("flask"), + Version: version.MustParse("3.0"), + Extra: "asynk", + RequestedBy: resolver.Requester{Root: true}, + }} + if !reflect.DeepEqual(res.MissingExtras, wantMissing) { + t.Errorf("MissingExtras = %+v, want %+v", res.MissingExtras, wantMissing) + } +} + +// The transitive case: x, not the root, is the one who asked for a[extra]. +// The newest a (2.0) does not declare it, so b -- which only the extra would +// have pulled in -- must be absent, and the warning must name x as the +// requester, not the root. +func TestResolveReportsATransitiveMissingExtra(t *testing.T) { + idx := index.NewMockIndex("test"). + AddVersion("x", "1.0", "a[extra]"). + SetMetadata("a", "1.0", index.PackageMetadata{ + RequiresDist: mustRequirements(t, `b==1.0.0; extra == "extra"`), + ProvidesExtra: []string{"extra"}, + }). + SetMetadata("a", "2.0", index.PackageMetadata{ + // The marker-gated requirement is still published, but 2.0 does not + // list "extra" in ProvidesExtra -- the real-world shape of an + // undeclared extra, and what makes the "b absent" assertion below + // meaningful rather than vacuous. + RequiresDist: mustRequirements(t, `b==1.0.0; extra == "extra"`), + }). + AddVersion("b", "1.0.0") + + res, err := resolve(t, idx, "x") + if err != nil { + t.Fatalf("Resolve: %v", err) + } + + want := map[string]string{"x": "1.0", "a": "2.0"} + if got := pins(t, res); !reflect.DeepEqual(got, want) { + t.Errorf("Pinned = %v, want %v", got, want) + } + if _, ok := res.Pinned["b"]; ok { + t.Errorf("b is pinned, but a 2.0 does not declare the extra that would pull it in: %v", pins(t, res)) + } + + wantMissing := []resolver.MissingExtra{{ + Package: index.NewPackageName("a"), + Version: version.MustParse("2.0"), + Extra: "extra", + RequestedBy: resolver.Requester{ + Package: index.NewPackageName("x"), + Version: version.MustParse("1.0"), + }, + }} + if !reflect.DeepEqual(res.MissingExtras, wantMissing) { + t.Errorf("MissingExtras = %+v, want %+v", res.MissingExtras, wantMissing) + } +} + +// The backtracked-requester trap: x 2.0 asks for z[extra] (which z does not +// declare) on its way to a conflict the solver can only see once it has +// decided z's own requirement on "bad" -- and the solver then reverts to +// x 1.0, which never asks for z at all. The abandoned request must not +// appear; MissingExtras describes the final solution only. +func TestResolveDoesNotReportAMissingExtraFromABacktrackedRequester(t *testing.T) { + idx := index.NewMockIndex("test"). + AddVersion("x", "1.0"). + AddVersion("x", "2.0", "z[extra]"). + AddVersion("z", "1.0", "bad>=1.0"). + AddVersion("bad", "0.5") + + res, err := resolve(t, idx, "x", "bad<1.0") + if err != nil { + t.Fatalf("Resolve: %v", err) + } + + want := map[string]string{"x": "1.0", "bad": "0.5"} + if got := pins(t, res); !reflect.DeepEqual(got, want) { + t.Errorf("Pinned = %v, want %v", got, want) + } + if len(res.MissingExtras) != 0 { + t.Errorf("MissingExtras = %+v, want none: the request came from x 2.0, which the solver abandoned", + res.MissingExtras) + } +} + +// The extra-requests-extra trap: x 2.0 asks for z[declared], a REAL declared +// extra of z, and z[declared] itself asks for other[missing] (which other does +// not declare). recordExtraRequests attributes that ask to plain z, not to +// z[declared] -- the requester identity it is given already dropped the extra +// (dependencies.go, dependenciesFrom's call to recordExtraRequests). x then +// backtracks to 1.0, which never asks for z at all, while root's own separate, +// unconditional requirement on z pins it at the SAME version regardless. A +// requester check keyed on (name, version) alone cannot tell the abandoned +// z[declared] apart from the z that survived, so the warning must not appear +// unless it also confirms "declared" is still in res.Extras["z"]. +func TestResolveDoesNotReportAMissingExtraFromAnAbandonedExtraOfAPinnedBase(t *testing.T) { + idx := index.NewMockIndex("test"). + AddVersion("x", "1.0"). + AddVersion("x", "2.0", "z[declared]"). + SetMetadata("z", "1.0", index.PackageMetadata{ + ProvidesExtra: []string{"declared"}, + RequiresDist: mustRequirements(t, + `other[missing]; extra == "declared"`, + `shared>=2.0; extra == "declared"`), + }). + AddVersion("other", "1.0"). + AddVersion("shared", "1.5"). + AddVersion("shared", "2.5"). + // y has many versions, all requiring shared<2.0, so the solver's + // fewest-candidates-first heuristic decides it LAST among root's direct + // requirements -- after z[declared] has already committed to + // shared>=2.0. That is what makes the eventual conflict a real + // post-commit backjump instead of an instant pre-commit rejection: the + // latter would reject z[declared] before its OTHER edge (other[missing]) + // is ever explored, which is a weaker, uninteresting test. + AddVersion("y", "1.0", "shared<2.0"). + AddVersion("y", "2.0", "shared<2.0"). + AddVersion("y", "3.0", "shared<2.0"). + AddVersion("y", "4.0", "shared<2.0"). + AddVersion("y", "5.0", "shared<2.0") + + res, err := resolve(t, idx, "x", "z", "other", "y") + if err != nil { + t.Fatalf("Resolve: %v", err) + } + + want := map[string]string{"x": "1.0", "z": "1.0", "other": "1.0", "y": "5.0", "shared": "1.5"} + if got := pins(t, res); !reflect.DeepEqual(got, want) { + t.Errorf("Pinned = %v, want %v", got, want) + } + if got, ok := res.Extras["z"]; ok { + t.Errorf("Extras[z] = %v, want no entry: x 1.0 never activated \"declared\"", got) + } + if len(res.MissingExtras) != 0 { + t.Errorf("MissingExtras = %+v, want none: \"declared\" was never active on the pinned z", + res.MissingExtras) + } +} diff --git a/resolver/satoracle_test.go b/resolver/satoracle_test.go index 98fa0c0..d3293df 100644 --- a/resolver/satoracle_test.go +++ b/resolver/satoracle_test.go @@ -143,10 +143,9 @@ func extraVar(pkg, extra, ver string) string { return pkg + "[" + extra + "]@" + // one per (package[extra], version); at-most-one per project; the root // requirements; and implications of the form x(p,v) -> OR(admissible // versions of each dependency). Extras are modeled the way go-pyresolver -// models them (a version must itself declare an extra for a request naming it -// to admit that version) so the oracle agrees with the resolver on the -// extras/ knownFail scenarios rather than adding a second, unrelated -// disagreement. +// models them: a version that does not declare a requested extra still +// admits the requirement, just without the extra clause -- an undeclared +// extra is ignored rather than excluding the version. func buildOracleModel(t *testing.T, s tomlScenario, py pythonSpec, env marker.Environment, matcher *tags.Matcher) oracleModel { t.Helper() @@ -181,19 +180,13 @@ func buildOracleModel(t *testing.T, s tomlScenario, py pythonSpec, env marker.En if r.Specifiers.String() != "" && !r.Specifiers.Check(vi.parsed) { continue } - ok := true - for _, e := range r.Extras { - if !vi.provides[e] { - ok = false - break - } - } - if !ok { - continue - } + // An extra vi does not declare is ignored rather than excluding vi: + // no term references its (nonexistent) extra variable. term := []bf.Formula{bf.Var(projVar(name, vi.str))} for _, e := range r.Extras { - term = append(term, bf.Var(extraVar(name, e, vi.str))) + if vi.provides[e] { + term = append(term, bf.Var(extraVar(name, e, vi.str))) + } } terms = append(terms, bf.And(term...)) }