Improve the error message when running cargo add cubing @v0.5.0

[lgarron]

This has now been rescoped to providing a good error message if a v prefix is present, see #12331 (comment)

---

Problem

I keep doing something the following:

cargo add [email protected]

This causes the following error:

error: invalid version requirement `v0.5.0`

Caused by:
  unexpected character 'v' while parsing major version number

I do this because I have a strong habit of prefixing my version numbers with v where possible, as vX.Y.Z tends to be a more unmistakable way to specify a version. And although it is not universal, it is also very common to prefix project release tags with v — to the point that the canonical cargo test for for a tag dependency even uses a value like this: https://github.com/rust-lang/cargo/blob/5b377cece0e0dd0af686cf53ce4637d5d85c2a10/tests/testsuite/cargo_add/git_tag/mod.rs#L22C1-L22C1
(I know that doesn't have a semantic meaning in this case, but it illustrates how widespread it is to indicate a version using a prefix of v.)

Further, other tools like npm allow a prefix of v (e.g. npm install [email protected]), which is an inconsistency that causes a small stumbling block when working with multiple web technologies.

Proposed Solution

In order of preference:

• Accept an extra v before a semantic version number.
• Recognize the extra v and provide a helpful error message that includes something like: If you want to install a specific version, please run: cargo add [email protected]

Notes

No response

[epage]

From the angle of prior art (see #10472)

• npm and go support a leading v
• poetry, yarn, pnpm, bundle, and pub do not

[epage]

If we supported v here, we should also evaluate whether to support it in cargo install and all commands that accept pkgid.

[epage]

Personally, I think a v being supported for cargo add would be incorrect. cargo add accepts a version requirement and not a version and we would not want to confuse that further by having a "sigil" that implies its a version. For the other commands, there would be more room for consideration with the drawback that it would make the commands more consistent in accepted syntax while adding support for @ in all commands was a drive for consistency (#10582).

[weihanglo]

cargo install --version <ver-req> also falls into the same category where cargo add is. That mean if cargo is going to support v, there will be a inconsistentcy between cargo add/cargo install and other commands. I lean towards not support such thing for consistency.

[lgarron]

It sounds like there are good reasons not to support this, as it could produce other inconsistencies.

In that case, I would advocate for the "friendly error message" in the vein that Rust is known for:

• Recognize the extra v and provide a helpful error message that includes something like: If you want to install a specific version, please run: cargo add [email protected]

[lgarron]

Personally, I think a v being supported for cargo add would be incorrect. cargo add accepts a version requirement and not a version and we would not want to confuse that further by having a "sigil" that implies its a version.

Just to note: npm also supports e.g. npm add cubing@latest. It would be similarly nice to apply the same solution as a v prefix (either support it or show a helpful message suggesting how to force an install of the latest version — particularly, a command that will pull a version even if it was just published moments ago).

[epage]

@latest is being discussed at #10741

[weihanglo]

• Recognize the extra v and provide a helpful error message that includes something like: If you want to install a specific version, please run: cargo add [email protected]

The tricky part of this error message is that the suggestion “please run: cargo add [email protected]” might not be the correct one. People run with toolchain override cargo +nightly add … could also get the same error.

If we want this, suggesting [email protected] instead of the full command line args is preferable I guess?

[DeveloperC286]

@epage I was having this issue with install I see yank also takes a version. Is someone working on this? I am assuming you want all three commands changed at once and the behaviour across all three the same?

[epage]

Good point in updating all of them. It would be nice though maybe not required. cargo update --precise can also take a version though that is tricky because it can also be a git ref.

I am not aware of anyone else working on this.

I'd recommend

• Making each unrelated command update its own commit or even PR
• Split each change into two commits: we ask that people add a test case showing the bad behavior in its own commit (ie it will pass showing the current error message) and then the follow up commit changes the behavior and updates the test

[DeveloperC286]

@epage You were added to the PR, but I created this draft #15259. I need to tidy up the commits and implement it for install, add and version(and ensure consistency across them). However, is the draft along the lines of acceptable quality/testing?

[DeveloperC286]

While a version prefixed with a v is not strictly a semantic version it is commonly used, e.g. output by automated release tooling etc, so not supporting it just makes it harder.

My philosophy on errors is along the lines of that in the chapter 'Define Errors Out Of Existence' from A Philosophy of Software Design. If the intent is clear I don't see a need on being pedantic. E.g. if I run cargo install [email protected] and I get back the error message did you mean to run 'cargo... @0.5.0'? I don't see the point in erroring if the intent is that clear you can give me back the command to run.

I would also apply this to cargo install [email protected] --version 0.5.0, if the versions match why error 🤷‍♂ I would only error if the versions differ.

[epage]

That is addressing cargo install. My concern earlier is with cargo add. We regularly have people confused between versions and version requirements and I feel it would make things worse.

Now, we could make things different between commands. However, this would move us away from a consistency we specifically went to when we added cargo add (make name/version separator the same despite slightly different meanings of "version").

[DeveloperC286]

Ah okay I see what you mean now, I haven't been doing much Rust the past few years so reread https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#version-requirement-syntax.

Personally I think Rust's version requirement syntax is completely unintuitive that is probably why loads of people are confused and asking questions. Yeah maybe accepting cargo add [email protected] would add more confusion but cargo add [email protected] already doesn't do what I would expect it to(nor the entry in the Cargo.toml so not just the commands fault). 🤷‍♂

[Pyr0de]

For cargo yank, it is not checked whether the package name and version are valid or not (on the cargo side). So should that be checked along with if the version starts with v?
@epage

[epage]

It would be helpful to cover more cases but we should also be careful to avoid continually extending the scope of the issue such that it makes ti hard to know when its "done".

Something that will help is that most commands accept a PackageSpecId, allowing it to be handled in one place. It looks like cargo yank manually handles it, like cargo add and cargo install.

[Pyr0de]

I was wondering if it would it make sense to add an error/help message for just this one case when the package name/version is not checked when cargo yank is run?

As you mentioned, a PackageSpecId would be good since there are many different help messages between install and add. Is there any issue related to adding something like PackageSpecId?

[epage]

I was wondering if it would it make sense to add an error/help message for just this one case when the package name/version is not checked when cargo yank is run?

I feel there is a distinction here that I'm missing.

. Is there any issue related to adding something like PackageSpecId?

There is not.

[Pyr0de]

Oh sorry about that. What I meant to say is that cargo yank does not check if package name/version is valid or not. So if I were to try something like cargo yank foo@bar, I get an error from the registry rather than from cargo itself,

    Updating crates.io index
        Yank foo@bar
error: failed to yank from the registry at https://crates.io

Caused by:
  the remote server responded with an error (status 400 Bad Request): Invalid URL: unexpected character 'b' while parsing major version number

So adding just this one error message to cargo about removing the 'v' from v0.0.1 would make cargo give an error but any other invalid package name/version would cause the registry to give an error.

[epage]

So cargo yank has the following failure modes which talk to the registry

• bad version
• unknown version
• unknown name

The API spec says those should be crate names and versions. I'm fine with validating the version. I wouldn't bother verifying unknown versions or names as we'll just have to talk to the registry to check that and we might as well do that in one call.