Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add readme to resolver-tests crate #13319

Closed
ShuiRuTian opened this issue Jan 18, 2024 · 8 comments · Fixed by #13977
Closed

Add readme to resolver-tests crate #13319

ShuiRuTian opened this issue Jan 18, 2024 · 8 comments · Fixed by #13977
Labels
A-documenting-cargo-itself Area: Cargo's documentation C-bug Category: bug S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review

Comments

@ShuiRuTian
Copy link

Problem

It's surprised for me to find there is a SAT resolver in resolver-tests crate.

It looks strange,

  • this resolver is not used anywhere
  • rust allows install different versions of same package. But in my understanding, SAT only allows one version for each package.

So, it feels like it's useless? What is the purpose of this resolver? Another strategy which might be enabled in the future?

Steps

No response

Possible Solution(s)

No response

Notes

No response

Version

No response

@ShuiRuTian ShuiRuTian added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Jan 18, 2024
@epage
Copy link
Contributor

epage commented Jan 18, 2024

this resolver is not used anywhere

The SAT resolver is a re-implementation of our resolver for us to compare results against like with

pub fn resolve_and_validated(

which is used in tests like
pub fn resolve_and_validated(

But in my understanding, SAT only allows one version for each package.

I'm not one to be able to speak to those kind of details but the fact that the two resolvers match behavior means we've modeled it in SAT in a way that works.

@Eh2406
Copy link
Contributor

Eh2406 commented Jan 21, 2024

@epage has summarize things pretty well, although he's pasted the same link twice. I am happy to answer any follow-up question you may have! The challenge will be getting me to stop talking. If you have specific questions feel free to ask them here, or come to my office hours. I would appreciate if you could turn your new understanding into documentation. @ShuiRuTian, when you are surprised to see this code, which documentation did you look at? Were there places you looked where there wasn't documentation? Were there confusing things about the documentation you did find?

Let's make (or move) answers to your questions, and put them where you would have found them!

@ShuiRuTian
Copy link
Author

@Eh2406 Hi.

I read the comments in the file and some third-party documents:

NOTE: here is some more docs, but not as important as the above ones

And I found most of SAT resolvers work for the environment that does not allow duplicated packages(.i.e, you could not have a package with different versions).

It feels like reasonable, if you could download different versions, SAT should be able to always give an answer, then it does not make much of sense to use SAT.

Finally, when I read the resolver of cargo, I wanted to have a look at test files firstly, then I am surprised to find cargo has a SAT resolver only for test. Especially the comment says "The SAT library dose not optimize for the newer version, so the selected packages may not match the real resolver."

Thanks for the following up!

@Eh2406
Copy link
Contributor

Eh2406 commented Jan 23, 2024

I read the comments in the file

Looking around, you make a good point. We don't seem to have general documentation about what the resolver-tests crates are for. Would you be up for adding a reame.md to https://github.com/rust-lang/cargo/tree/master/crates/resolver-tests ?

pub solver, how dart's package manager works with SAT

If you enjoyed this link, you may also be interested in the https://github.com/pubgrub-rs/pubgrub project and its longform documentation at https://pubgrub-rs-guide.netlify.app/ we would love help.

It feels like reasonable, if you could download different versions, SAT should be able to always give an answer, then it does not make much of sense to use SAT.

In order to always be able to find a solution you either need to not have "these two versions are not compatible" rules (like npm) or not have "these two version constraints do not overlap" (like go). Otherwise you're resolver should be NP-complete and a SAT library might be useful.

SAT is a remarkably general framework. It consists of variables and clauses. If there is a solution each variable must be set to either true or false. Similarly if there is a solution one literal in each clause needs to be true. This is sufficient to model any binary operation. Each bit is variable, and there's a clause ensuring each connection. Considering modern programs are "just" operations on bits in theory SAT can solve anything. Alternatively, you can model circuits directly with the variable for each wire which either has power (true) or does not have power (false) then all connections get modeled with clauses. So any circuit can be modeled in SAT and computers are "just" circuits. So almost anything can be modeled as an SAT problem. Whether SAT solvers can find a solution to that problem is a different story.

So how do we model dependency resolution as an SAT problem.

  1. We create a variables that represents each package version which is set to "true" if the packages in the lock file and "false" if it is unused.
    let var_for_is_packages_used: HashMap<PackageId, varisat::Var> = registry
  2. At this point is possible to select every version of every package. So we need to mark certain versions as incompatible with each other. We could add a clause not A, not B for all A and B that are incompatible, but there are more efficient ways to do it for large numbers of versions.
    fn sat_at_most_one(solver: &mut impl varisat::ExtendFormula, vars: &[varisat::Var]) {
    if you want "at most one version per package" then you have these constraints for all versions of the package. But for cargo we want "at most one version per compatibility range per package" so we do
    sat_at_most_one_by_key(
  3. Now different versions can conflict, but dependencies might not be selected. So we need to add clauses that ensure that if a package is selected for each dependency a version that satisfies that dependency is selected.
    // active packages need each of there `deps` to be satisfied

I hope that helps you understand what is going on. Feel free to turn this into comments in the code to help answer questions going forward! And if you have follow-up questions please ask!

@ShuiRuTian
Copy link
Author

Cooooool, especially about https://pubgrub-rs-guide.netlify.app/. Looks really interesting.

Need some time to read the doc/code to figure out how it works. Would like to contribute a README if I am ready in future!

@Eh2406 Eh2406 changed the title [Question-like] SAT resolver in resolver-tests crate Add readme to resolver-tests crate Jan 24, 2024
@Eh2406 Eh2406 added the A-documenting-cargo-itself Area: Cargo's documentation label Jan 24, 2024
@Eh2406
Copy link
Contributor

Eh2406 commented Jan 24, 2024

@epage , what is correct tag for "help wanted" these days?

@epage
Copy link
Contributor

epage commented Jan 24, 2024

S-accepted

@weihanglo weihanglo added S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review and removed S-triage Status: This issue is waiting on initial triage. labels Mar 28, 2024
@weihanglo
Copy link
Member

Assumed Eh2406 was going to tag this as S-accepted for documenting things discussed above, so I went ahead and did that.

bors added a commit that referenced this issue May 31, 2024
doc: Add README for resolver-tests

### What does this PR try to resolve?

This collect the comments from `@Eh2406` about the resolver-tests and add a README to it.

### How should we test and review this PR?

### Additional information

Maybe Fixed #13319
See  #11938 (comment)
bors added a commit that referenced this issue May 31, 2024
doc: Add README for resolver-tests

### What does this PR try to resolve?

This collect the comments from `@Eh2406` about the resolver-tests and add a README to it.

### How should we test and review this PR?

### Additional information

Maybe Fixed #13319
See  #11938 (comment)
@bors bors closed this as completed in d503f74 Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documenting-cargo-itself Area: Cargo's documentation C-bug Category: bug S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants