search

rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.76k stars 2.42k forks source link

Resolvers error messages should include the version requirements. #6199

Closed Eh2406 closed 3 years ago

Eh2406 commented 6 years ago

Currently, if the resolver fails the error messages give lots of information on the paths that caused the problems, like:

https://github.com/rust-lang/cargo/blob/0b530c30867da26a4b59146f490c9f1d5377c20a/tests/testsuite/build.rs#L1356-L1358

It would be nice to have the version requirements like: (wording needs improvement)

  previously selected package `bad v1.0.0`
    ... which is depended on by `bar v0.1.0` which requires `bad: "=1.0.0"`
    ... which is depended on by `incompatible_dependencies v0.0.1 ([..])` which requires `bar: "0.1.0"`

The data is easily available thanks to #5428 we just need to get it into the correct place, and find good wording. We almost had this working in #5452 before it bit-roted.

I am willing to mentor someone on getting this done. It may be a good place to start looking at the resolver without having to stare into the void at its core. If it would be useful I can rebase #5452 so that only the wording is left.

cc @necaris would you be interested?

Eh2406 commented 6 years ago

Do to the github outage this got opened several times. Sorry for the spam.

Dylan-DPC-zz commented 6 years ago

@Eh2406 I can work on this

Eh2406 commented 6 years ago

Wonderful! How can I help you get started? What are your opening questions? Should I try and rebase/rewrite or do you want to? Do you want to schedule a time to chat on discord or hangouts?

Dylan-DPC-zz commented 6 years ago

@Eh2406 will ping you on discord

Eh2406 commented 6 years ago

The #5452 is not seeing it for some reason, but the two logick commits have been rebased on eh2406/master and pass the tests localy for me.

Eh2406 commented 5 years ago

@Dylan-DPC Did you get a chance to work on this? How is it going? Is there anything we can do to help?

Dylan-DPC-zz commented 5 years ago

@Eh2406 Hey. Got busy with some stuff. Will work on it this week. Will ping on discord if I need anything :)

jaredonline commented 5 years ago

@Eh2406 I see that a couple of pull requests were opened for this but never merged. I've got some time and would love to get involved if code still needs to be written.

Eh2406 commented 5 years ago

Help is welcome! The pull requests have bit-roteded, but I don't thing there is much to do to bring them up to date. The harder part was suggesting a better wording.

zmitchell commented 5 years ago

Is anyone still working on this? If not I can take a stab at it!

Eh2406 commented 5 years ago

I believe it is open.

zmitchell commented 5 years ago

Is this the appropriate place to ask questions while I work on this? I don't have much experience contributing to projects other my own.

necaris commented 5 years ago

@zmitchell don't see why not put things here

Eh2406 commented 5 years ago

Here works grate!

zmitchell commented 4 years ago

I'm trying to understand the pre-bitrot way of doing things so that I can understand what changes were made in the PR. After that the plan is to understand how things work post-bitrot so I know how to translate the changes in the PR.

First question: What does the links field on Manifest represent? It's just an Option<String>, which doesn't tell me much.

Eh2406 commented 4 years ago

Good plan!

The links holds the value of the "links field" https://doc.rust-lang.org/cargo/reference/manifest.html#the-links-field-optional. There will be only one crate in the dependency tree with each links value. This is primarily used to avoid linking errors.

zmitchell commented 4 years ago

I'm still trying to build a mental model of the different pieces that fit together here.

So, here are my questions:

I have a couple more questions, but I'll wait until I know what a Dependency is before asking.

Eh2406 commented 4 years ago

The mental model seems almost right. One quip is that A Context is more a work in progress version of a Resolve.

Conceptually, what is the difference between a Dependency and a package referred to by a PackageId?

A Dependency conceptually is a statement in some Cargo.toml like serde='^1.0.69'. A PackageId conceptually is a unique version of a crate like https://crates.io/crates/serde/1.0.99 . A Package can satisfy a Dependency, like how we can pick serde v1.0.99 if your Cargo.toml asked for serde='^1.0.69'. Similarly, the Summary (The part of the Cargo.toml that gets stored in the index) associated with a PackageId can have a list of Dependencies that will need to be satisfied if we use that Package.

How do I interpret the Graph structure? (I'm familiar with graphs with nodes and edges, I mean specifically this implementation):

It is a directed graph. Also to confuse things Context.parents and Resolve.graph think of the edges as going in opposite directions. So from Context.parents perspective (as that is how activation_error access it at this time), an edge goes from a child to the parent that asked for it and it keeps track of the Dependencys the child satisfied.

{
    'serde_derive v1.0.99': {
        'serde v1.0.99':  ['Dev-Dependencies serde_derive="^1.0"', 'Dependencies serde_derive="^1.0"'],
    },
    'quote v1.0.1': {
        'serde_derive v1.0.99':  ['Dependencies quote="^1.0"',],
    },
}

Witch can be read as:

I don't think I have made sense, so keep asking! If I ever manage to make sence, please add some comments in your PR to make it easier for the next user!

zmitchell commented 4 years ago

Thank you, I definitely plan to add some comments to make this all clear for the next complete n00b. I think most of that makes sense, but I think a better way to phrase my question would have been "how do I navigate Graph" rather than "how do I interpret Graph".

Let's say I'm looking at a particular node, pkg.

Eh2406 commented 4 years ago

So from Context.parents perspective, you are looking at pkg so self.nodes[pkg] is all the ways pkg is needed. So the keys of the HashMap are the Packages that depend on pkg. If you continue down you will eventually get to the root package. See path_to_bottom for an example, that gets used in the describe_path part of the error messages. So if we get a self.nodes[pkg][parent] we will get a &Vec<Dependency> is the ways that this parent depended on pkg

kellda commented 4 years ago

Did #7934 solved this issue ?

Eh2406 commented 4 years ago

I dont think so. If a fails to load then #7934 will show the path foo->c->b->a like the errors discussed above. But it does not include what versions of b would be acceptable.