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

Resolvers error messages should include the version requirements. #6199

Closed
Eh2406 opened this issue Oct 22, 2018 · 22 comments · Fixed by #9827
Closed

Resolvers error messages should include the version requirements. #6199

Eh2406 opened this issue Oct 22, 2018 · 22 comments · Fixed by #9827
Assignees
Labels
A-dependency-resolution Area: dependency resolution and the resolver A-diagnostics Area: Error and warning messages generated by Cargo itself.

Comments

@Eh2406
Copy link
Contributor

Eh2406 commented Oct 22, 2018

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

cargo/tests/testsuite/build.rs

Lines 1356 to 1358 in 0b530c3

previously selected package `bad v1.0.0`
... which is depended on by `bar v0.1.0`
... which is depended on by `incompatible_dependencies v0.0.1 ([..])`

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 Eh2406 added A-diagnostics Area: Error and warning messages generated by Cargo itself. E-help-wanted A-dependency-resolution Area: dependency resolution and the resolver labels Oct 22, 2018
@Eh2406
Copy link
Contributor Author

Eh2406 commented Oct 22, 2018

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

@Dylan-DPC-zz
Copy link

@Eh2406 I can work on this

@Eh2406
Copy link
Contributor Author

Eh2406 commented Oct 24, 2018

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
Copy link

@Eh2406 will ping you on discord

@Eh2406
Copy link
Contributor Author

Eh2406 commented Oct 25, 2018

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
Copy link
Contributor Author

Eh2406 commented Nov 13, 2018

@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
Copy link

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

@jaredonline
Copy link

@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
Copy link
Contributor Author

Eh2406 commented Aug 13, 2019

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
Copy link

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

@Eh2406
Copy link
Contributor Author

Eh2406 commented Nov 6, 2019

I believe it is open.

@zmitchell
Copy link

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
Copy link

necaris commented Nov 11, 2019

@zmitchell don't see why not put things here

@Eh2406
Copy link
Contributor Author

Eh2406 commented Nov 11, 2019

Here works grate!

@zmitchell
Copy link

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
Copy link
Contributor Author

Eh2406 commented Nov 13, 2019

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
Copy link

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

  • A Context has a Resolve
  • A Resolve has a Graph<N, E>
  • The Graph<N, E> is really a HashMap<N, HashMap<N, E>>
  • In the context of Resolve this is actually HashMap<PackageId, HashMap<PackageId, Vec<Dependency>>>

So, here are my questions:

  • Conceptually, what is the difference between a Dependency and a package referred to by a PackageId?
  • How do I interpret the Graph structure? (I'm familiar with graphs with nodes and edges, I mean specifically this implementation):
    • If I iterate over the nodes in Graph, I'm given a tuple that looks like this: (node, edges) = (PackageId, HashMap<PackageId, Vec<Dependency>>).
    • Are the keys in that HashMap the PackageIds of packages that can be reached from node?

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

@Eh2406
Copy link
Contributor Author

Eh2406 commented Nov 21, 2019

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:

  • serde_derive v1.0.99 was selected to
    • satisfy serde v1.0.99 Dev-Dependence that looked like serde_derive ="^1.0" and also was selected to
    • satisfy serde v1.0.99 normal Dependence that looked like serde_derive ="^1.0" and
  • quote v1.0.1 was selected to satisfy serde_derive v1.0.99 normal Dependence that looked like quote="^1.0".

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
Copy link

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.

  • What information does self.nodes[pkg] (a HashMap<PackageId, Vec<Dependency>>) contain?
  • What do the keys in that HashMap represent?
  • How do I obtain the nodes adjacent to pkg (below, actually, since this is a directed graph)?

@Eh2406
Copy link
Contributor Author

Eh2406 commented Nov 22, 2019

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
Copy link
Contributor

kellda commented Jun 7, 2020

Did #7934 solved this issue ?

@Eh2406
Copy link
Contributor Author

Eh2406 commented Jun 7, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dependency-resolution Area: dependency resolution and the resolver A-diagnostics Area: Error and warning messages generated by Cargo itself.
Projects
None yet
6 participants