Fix assertion failure when a package constrains itself#230
Conversation
|
This is admittedly a strange thing to do, but here was the rationale: https://gitlab.com/redhat/centos-stream/rpms/centos-stream-release/-/commit/ce231c10b48cb4ddde860bfc9d6f79c76ba92e0a I believe the semantics here are basically "if this package is installed, you must not install another provider, and if another provider is installed, you cannot install this package". |
|
Let me know the ideal way to provide tests, unless you want to do it |
baszalmstra
left a comment
baszalmstra
left a comment
There was a problem hiding this comment.
The constraint introduces a tautology. I think a more technically correct approach would be to (not crash and) block the package from being installable using an assertion. This would be the same as when this is done through a transitive dependency (e.g. A depending on B, which constrains A). I don't think skipping the clause is the right approach. But maybe I misunderstand the use-case?
|
So, it's a package that is installed and can be updated (on my CentOS Stream dev container it's at release 21)
I haven't tracked down exactly what libsolv is doing to handle this scenario, but libsolv / DNF must be handling it somehow. Probably it's doing something similar. |
dralley
force-pushed
the
constrains-itself
branch
2 times, most recently
from
ab34b25 to
c16fc12
Compare
|
Ah, so here's some libsolv documentation: So, libsolv does indeed do basically the same thing as this patch, but it's behind a solver configuration flag, because the desired behavior varies by ecosystem. Would you want to do something similar (or is there already a mechanism for solver config options)? Unrelated: looks like the multiversion code is right there as well. And that might need a similar solution as well |
|
Is that then something we should not just solve in the dependency provider itself? Simply dont return self-referential constraints? We should still solve the crash of course, but I think we should backtrack in that case. |
|
How would that work exactly? The "conflicts" is on any provider of |
dralley
force-pushed
the
constrains-itself
branch
from
c16fc12 to
6b790f3
Compare
|
We can continue discussing the implementation, but I added a It's a bit sparse right now but maybe it would make sense to add "activity params" to the config? (sidenote: the method is public but the actual function of those params doesn't appear in the public documentation, only in the solver internals) |
|
Conceptually the DependencyProvider provides the ecosystem implementation. Whether to allow self referential constraints or not seems like an ecosystem specific implementation, so I think this should be part of that. We can either model this as a seperate function to enable disable globally, we can allow this per nameid when returning candidates, or even per solvable when returning dependency information, or even per versionset with a new function. WDYT? |
You mean a trait method on If I can suggest a tweak on that, going w/ the And the (if that was the case I would probably drop that 2nd commit w/ the activity params change, it wouldn't make much sense anymore).
That could probably work. Do you prefer that over option 1 (+tweaks)? I could spit out both for comparison purposes.
I think that's probably overkill. At least at the moment I have no example of a versioned self-conflicts, only this example of a conflict over a global virtual provide. |
dralley
force-pushed
the
constrains-itself
branch
2 times, most recently
from
95a411e to
829c580
Compare
|
@baszalmstra Ok, this passes locally (the C++ side is broken of course). Once you give me a final design direction I'll clean it up. The multi-version change ended up being a very similar implementation so I include that commit as well, but I can split that off into a different PR if you want. |
|
I'm still a bit on the fence; please help me understand this better. I could not understand how self-referential constraints make any sense, because the constraint literally says that it cannot install itself; a tautology. But in combination with multi-version, I can see how it makes sense. But it's basically saying: you can have multiple versions of package A, but if you select version A@42 (the version that adds the self-referential constraints), that's the only possible version. Right? I am wondering if there is not a more elegant way of modeling that specifically. Resolvo takes a lot of care to try to minimize the number of clauses that it has to maintain. I wonder if it would take fewer clauses if we model this particular behavior specifically rather than just adding general constraint clauses. |
|
The two aren't directly related, I'm only thinking about both because I would eventually want to have both and because the approach you suggested was essentially the same as what I was working on for multi-version, not because there's a direct relationship. I agree with you that from a purely logical perspective it doesn't make a ton of sense. What I've been told (from Slack) is:
and then another response:
Specifically this package seems to include things like distro public GPG keys for package signing, secure boot certificates, and the list of distro repo configs. In other words it's a package that you don't want to be able to remove or replace easily lest the system become unusable, & kind of defines what the distro is in some sense (the root of trust and such). Fedora also has a similar pattern: https://src.fedoraproject.org/rpms/fedora-release/blob/f44/f/fedora-release.spec#_156 Otherwise all I know that it seems to be something DNF / libsolv explicitly supports. And really it seems to just be standard behavior across an entire solve and necessarily something that is per-package configurable. |
When a package both provides and conflicts with the same capability (example: in CentOS Stream, "centos-stream-release" provides "system-release" and also conflicts with "system-release"), the constrains clause would try to forbid the package from coexisting with itself. This caused WatchedLiterals::constrains to panic with "both literals cannot be false" because both watched literals referred to the same variable. Skip self-referential constrains entries: a package that is already in the solution obviously cannot constrain itself out of it. Assisted-By: Claude Opus 4.6 <noreply@anthropic.com>
Self-conflict policy is ecosystem-specific (e.g. RPM allows it, others don't), so it belongs on the DependencyProvider's per-package Candidates rather than on a solver-level config struct. This removes SolverConfig entirely and adds allow_self_conflicts as a field on Candidates alongside allow_multiple. The per-package flag is tracked on SolverState (not the Encoder) because candidates and constraints may be processed by different encoder invocations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add `allow_multiple: bool` to `Candidates`, allowing dependency providers to indicate that multiple versions of a package may coexist in the solution (e.g. kernel packages on RPM systems). When set, the solver skips the at-most-one constraint encoding for that package name. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
dralley
force-pushed
the
constrains-itself
branch
from
829c580 to
bab305a
Compare
|
If you want to / have time to play with it yourself, you can. Code is here: https://github.com/prefix-dev/resolvo-rpm/ Here's the repo metadata I'm using: https://drive.google.com/file/d/12e_lwXdLfykQn0r3fHL7qlX886wJXnF4/view?usp=sharing Extract the files and dump them in Example: I need to clean up this branch a bit though FYI. That example no longer reproduces the original issue exactly, it hits a related one first. |
2 participants
When a package both provides and conflicts with the same capability (example: in CentOS Stream, "centos-stream-release" provides "system-release" and also conflicts with "system-release"), the constrains clause would try to forbid the package from coexisting with itself. This caused WatchedLiterals::constrains to panic with "both literals cannot be false" because both watched literals referred to the same variable.
Skip self-referential constrains entries: a package that is already in the solution obviously cannot constrain itself out of it.
Assisted-By: Claude Opus 4.6 noreply@anthropic.com