include version requirements in path for resolver error messages

[Eh2406]

This is a follow up to #5428 (comment). It changes the error messages from:

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 ([..])`

to:

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

Sorry for the big diff, this includes a cargo +stable fmt commit before my changes.

[rust-highfive]

r? @matklad

(rust_highfive has picked a reviewer for you, use r? to override)

[alexcrichton]

This looks pretty awesome, a great idea!

I wonder though if we may want to tweak the wording slightly? It may be slightly redundant to have the name of the package like foo = "0.1" because it's repeated on every line, and I think it's also not necessarily guaranteed to match what's written in the manifest as well.

It may also be worth avoiding printing a * dependency requirement for things like git/path dependencies, but I'm not acutally sure how that could cause a conflict so it may not be too relevant!

[bors]

☔ The latest upstream changes (presumably #5432) made this pull request unmergeable. Please resolve the merge conflicts.

[Eh2406]

I am open to changing the wording.

Repeating the name is definitely redundant, what frazing is unambiguous and clearer?

You are correct the semver::Version will canonicalized the requirement. If the manifest has bar = "0.1.0" the output will be bar = "^0.1.0". I don't think we keep the pre parsed string anyware. How big a deal do you think this is?

Is there an easy way to distinguish git/path dependencies, so as to remove "*" that is nowhere in the manifest?

A fix for CI and a rebase on its way.

[alexcrichton]

Oh sorry when I meant "written in the manifest" I meant moreso foo = { version = "0.1" } vs foo = "0.1" (maybe sometimes with optional = true). I think canonicalizing "0.1" to "^0.1" is fine to do in the error message.

Oh for sure yeah, the SourceId of the dependency will return true for is_path or is_git, and you can also use !is_registry as well for printing the vesion perhaps

[Eh2406]

Rebase took a couple of tries, but doing it manually worked.

I removed the redundant use of name. Do you have a less verbose/stilted way of saying this? And, a preferred wording for git/path?

[alexcrichton]

Nah this looks reasonable to me! If the version requirement is * (or if it's a git/path dep) I'd be fine just omitting the 'for this version requirement' text as well (aka leave as-is with today)

@matklad wanted to cc you as well, you think the new output is alright?

[bors]

☔ The latest upstream changes (presumably #5473) made this pull request unmergeable. Please resolve the merge conflicts.

[matklad]

I definitely like the information that is presented, however all these ... selected to fulfill are hard to parse for me. I would say that there are two problems with this presentation:

• package and it's version requirement are on adjacent lines, so you have to read this stuff diagonally to make sense
• we try hard to present a tabular data as a single English sentence, and that feels clunky.

As alternative, what about this more line oriented and tabular presentation:

BEFORE:
error: failed to select a version for `bad`.
    ... required by package `incompatible_dependencies v0.0.1 ([..])`
versions that meet the requirements `>= 1.0.1, <= 2.0.0` are: 2.0.0, 1.0.1
all possible versions conflict with previously selected packages.
  previously selected package `bad v2.0.1`
    ... selected to fulfill the requirement \">= 2.0.1\" from package `baz v0.1.0`
    ... selected to fulfill the requirement \"^0.1.0\" from package `incompatible_dependencies v0.0.1 ([..])`
  previously selected package `bad v1.0.0`
    ... selected to fulfill the requirement \"= 1.0.0\" from package `bar v0.1.0`
    ... selected to fulfill the requirement \"^0.1.0\" from package `incompatible_dependencies v0.0.1 ([..])`
failed to select a version for `bad` which could resolve this conflict

AFTER:
error: failed to select a version for `bad`.
   `bad >= 1.0.1, <= 2.0.0` required by `incompatible_dependencies v0.0.1 ([..])`

versions that meet the requirements `bad >= 1.0.1, <= 2.0.0` are: 2.0.0, 1.0.1
all possible versions conflict with previously selected packages.

previously selected package `bad v2.0.1`
  `bad >= 2.0.1` required by `baz v0.1.0`, picked  `bad v2.0.1`
  `baz ^0.1.0` required by `incompatible_dependencies v0.0.1 ([..])`, picked `baz v0.1.0`

previously selected package `bad v1.0.0`
  `bad  =1.0.0` required by `bar v0.1.0`, picked  `bad v1.0.0`
  `bar ^0.1.0` required by `incompatible_dependencies v0.0.1 ([..])`,  picked `bar v0.1.0`

failed to select a version for `bad` which could resolve this conflict",

I am not really attached to this format, but I like some of its properties:

• it purposefully duplicates version info, so as to make each line complete in isolation
• version requirements (which are the underling cause of the conflict) are in the first column.
• simpler and shorter English words
• blank lines ❤️ ❤️ ❤️

[matklad]

As this is hard to rebase, I suggest to merge this ASAP, and tweak rendering details in subsequent PR though, if we want to tweak them (I actually quite like the current rendering, I just think it could be even better).

[Eh2406]

as pointed out in #5484, we should mention that precise requirements may be caused by lock files and suggest using update or generate lock file to fix it.

[alexcrichton]

Oh dear it looks like this may have fallen off the radar by accident! @Eh2406 was there any other blockers on this or did you feel this was good to go?

[Eh2406]

It fell off my radar as well. What was left to do:

1. Agree on desired wording.
2. Special case git/path dependencies without breaking the structure of the output.
3. Special case pind by a lock file and not a "=1.1.2" in a dependency. Also add a "note: try cargo update" to the end.

[alexcrichton]

Ah ok, no worries!

[bors]

☔ The latest upstream changes (presumably #5691) made this pull request unmergeable. Please resolve the merge conflicts.

[alexcrichton]

Ok I'm gonna close this for now due to inactivity, but we can of course reopen and/or resubmit!