Let constraints and requirements name the file they came from - #122
Conversation
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
|
You've got some CI failures here. Want to have your 🤖 fix them or shall I have mine do it? |
|
I did it |
StefanKarpinski
left a comment
There was a problem hiding this comment.
Thanks, this is a feature Pkg needs and the shape of it is mostly right. One design change, which I'm pushing to the branch as a fix commit rather than describing in the abstract; this review is the explanation.
What's right
The test I applied is genericness: would this make sense for any package manager that aggregates several user-editable files into one resolve (Cargo and npm workspaces, go.work, pip's requirements plus constraints files)? "A fix is an edit, so name the file" is a universal need, so the feature passes.
The constraint side models it well. Two files' compat entries are two independently editable things, so two kinds is the right model, not a hack: nothing new reaches the solver, and the existing minimal-lift search finds which files' entries actually need relaxing (your "only the one excluding the version needed" test shows this). Packing the source into the Symbol after @ is a compromise, but kinds are Symbols everywhere and Pkg rebuilds reports positionally, so I'd rather that than widen the kind type.
The change: a requirement is one fact, and dropping it is one decision
The requirement side doesn't pass the same test. A requirement is one fact however many files list it, and satisfiability never asks where it came from. The PR instead expands its drop into one action per file (drop@src kinds), which leaks rendering data into everything that counts actions. Concretely, on this branch, a dependency listed in five workspace files turns
Note: relaxing your compat on P would not help unless you also dropped dependency Q.
into
Note: relaxing your compat on P would not help without 5 other changes.
because the elision threshold counts actions. The "do both" / "do all of them" wording in blocked verdicts flattens bundles the same way.
The fix commit keeps Action(:drop, p) as the one action it was and treats the places as an annotation the printer consults. The map lives once on Diagnosis (d.sources) rather than on every Conflict: the requirements an alternative drops aren't always ones a heading names, and it saves the ninth positional argument that named in Pkg would have to mirror. drop_kind, drop_source, is_drop_kind and drop_actions go away, along with the test oracle's withdrawal change (which was also stricter than the Diagnostics-side withdrawal, which read any partial drop as a full one).
Rendering
Places print in parentheses after what they're on, as given, so that a caller passing path:line gets something an editor opens:
Conflict 1: DataFrames (Sub1/Project.toml:5)
• your compat (Sub1/Project.toml:12) restricts DataFrames to ≥1.7.0
...
Fix it by any one of:
1. relax your compat on PrettyTables (Sub2/Project.toml:9)
3. drop dependency DataFrames (Sub1/Project.toml:5)
and several constraints of one kind on one package become one phrase naming every place, relax your compat on P (A/Project.toml:3, B/Project.toml:4), where before they were one full clause per place joined by "and".
API
Any kind may carry a source now, not just compat: sourced_kind(base, source) replaces compat_kind, with kind_base and kind_source reading the halves back, and check_constraints takes a sourced pin's dictionary the way it takes a sourced compat's (it rejected pin@src before while accepting compat@src).
For JuliaLang/Pkg.jl#4812 this means: Resolver.compat_kind(file) becomes Resolver.sourced_kind(:compat, file), Resolver.compat_source(kind) becomes Resolver.kind_source(kind), the Conflict rebuild in named loses its last argument, and Diagnosis gains a sources map to carry over. Pass path:line as the source if the TOML parse gives you line numbers; plain paths work as before.
Follow-up, not this PR
lift_actions enumerates subsets of the kinds excluding a version. Per-file compat kinds make that reachable: a package every workspace member holds to the same range, with a needed version outside it, costs (one conflict, one package)
| members | time |
|---|---|
| 16 | 0.05 s |
| 20 | 1.3 s |
| 22 | 4.8 s |
| 24 | 21 s |
quadrupling per two members, so a ~30-member monorepo hangs in the error path. The minimal lift is always a union of the exclusion sets of the dead versions, so enumerating subfamilies of those (usually one set) instead of subsets of kinds fixes it. Worth its own issue once this lands.
🤖 Generated with Claude Code
|
The |
A workspace's compat on a package is the intersection of the entries of several Project.toml files, and a report that says "relax your compat on X" does not say which one. `sourced_kind(base, source)` makes a kind that a `Problem` takes exactly like `base`: `sourced_kind(:compat, "a/Project.toml")` is that file's compat, and a pin from a named place is taken the same way. `kind_base` and `kind_source` read the halves back. Each place is a constraint of its own, so where several files exclude the version needed the fix names all of them, and where only one does, only that one. The place prints in parentheses after what it is on, as the query gave it, so a caller that passes `path:line` gets what an editor opens: "your compat (Sub/Project.toml:12) restricts X to …" and "relax your compat on X (Sub/Project.toml:12)". Several constraints of one kind on one package are one phrase naming every place. The source rides inside the kind symbol after an `@`, since a kind is a symbol wherever it goes: a report is plain data, and a caller rebuilding one over its own package names keeps the kinds as they are. The bare kinds are answered by identity before the symbol is read as a string, so an unconstrained `Problem` still costs its vector and its struct and nothing else. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hyi2aKkUnkd9HA2hS9YChk
`Problem([pkg => sources, ...]; kinds...)` records where each requirement is declared, the workspace members whose Project.toml lists it, and the report names them after it: "Conflict 1: X (Sub1/Project.toml:5)", "no version of X (Sub1/Project.toml:5) is available." and the fix "drop dependency X (Sub1/Project.toml:5, Sub2/Project.toml:7)". A requirement is one fact and dropping it one decision, however many places list it, so the fix stays the one `Action(:drop, p)` and the places are an annotation the printer consults. One action per place would make everything that counts actions count places: a requirement five files list would turn "would not help unless you also dropped dependency Q" into "would not help without 5 other changes". The places ride on the `Diagnosis` rather than on each `Conflict`: the requirements an alternative drops are not always ones a heading names, and the map is the query's, not a conflict's. Satisfiability never asks for it, and a query that says nothing shares one empty map, as it does for constraints. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hyi2aKkUnkd9HA2hS9YChk
`lift_actions` found the smallest set of kinds to lift by trying every subset of the kinds excluding a version of the package, smallest first. With a kind per workspace file that is exponential in the workspace: a package every member holds to the same range, with a needed version outside it, needs every file's kind lifted, which was the last subset of the last size. Measured on one conflict, one package: 16 members 0.05 s, 20 members 1.3 s, 24 members 21 s, quadrupling per two members. A class comes back once every kind excluding some member of it is lifted, so what restores it is one of its members' exclusion sets, and a lift restoring every class is a union of one such choice per class. A smallest lift is such a union, and so is the tie-break winner among the smallest, since a kind outside every exclusion set a lift covers could be dropped from it. So the search is over those unions: one choice per dead class, classes that leave no choice first, and a branch cut once its union is already larger than the best. The workspace case has one exclusion set and one branch. 40 members take a millisecond. Same answer as before on every input: checked by running both searches side by side over three thousand random problems mixing compat, pin, sourced and predicate kinds, and by the test suite's expected outputs. The `subsets` helper had one other user, the decompose tests, which now build their k-subsets themselves. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hyi2aKkUnkd9HA2hS9YChk
7288359 to
943f1ee
Compare
|
Refactored and rebased but this is good to go now. I'll merge and we can adjust the Pkg callers (PRs only atm). |
A query's constraints can come from several places at once: in a Julia workspace, every member
Project.tomlcontributes compat entries and requirements, and Pkg intersects them into one spec before resolving. A report could then only say "your compat restricts X" and "drop dependency X", leaving the reader to find which file to edit. This lets the query say where each constraint and requirement came from, and the report names it. Nothing changes for a query that says nothing.compat_kind(source)makes a constraint kind aProblemtakes exactly likecompat, passed asProblem(reqs; compat_kind("a/Project.toml") => dict). The report reads "your compat in a/Project.toml restricts X to …" and "relax your compat on X in a/Project.toml". Kinds are lifted per kind already, so where two files both exclude the version needed, the fix names both; where only one does, only that one.Problem([pkg => sources, ...]; kinds...)records where each requirement is required from. A fix that drops it emits onedrop_kind(source)action per source ("drop dependency X from a/Project.toml"), since a requirement is gone only once every place has dropped it.Conflictcarries the sources of its requirements, so the heading reads "Conflict 1: X (required by a/Project.toml)".The source rides inside the kind symbol (
compat@a/Project.toml,drop@a/Project.toml) because a report is plain data and a caller rebuilding one over its own package names keeps the kinds as they are;kind_base/kind_sourceread the halves back. The test oracle'swithdrawalhonours drop sources the same way.Used by JuliaLang/Pkg.jl#4812, whose description shows the before and after reports for a workspace.
🤖 Generated with Claude Code