`cargo add`: Handle paths automatically without requiring `--path`

[joshtriplett]

Problem

$ cargo add ../testlib
error: invalid character `.` in package name: `../testlib`, the first character must be a Unicode XID start character (most letters or `_`)

Ideally, when given an argument containing a path separator (which is never valid in a crate name), cargo add could automatically assume --path if the specified path exists as a directory.

If there's a compatibility reason why we can't do that, then at a minimum cargo add could suggest --path. But I think we could reasonably infer that and proceed.

Proposed Solution

When cargo add receives an argument containing a path separator, rather than erroring, cargo add should infer --path and proceed.

Notes

No response

[weihanglo]

If we are going to do this, should we consider other commands accepting --path all together for consistency? Like cargo remove, cargo run and cargo install?

(-Zscript supports and does that path separator check btw)

[epage]

Didn't cargo install used to do this and we transitioned away?

What happens when a subdirectory and a package name look the same? Its common to reference a directory as foo and I think it'd be confusing to say that ./foo is required. If it isn't, then it seems like we could run into dependency confusion attacks.

[joshtriplett]

@epage I don't remember if cargo install some/path used to work. cargo install with no arguments used to default to the current directory, and that now requires cargo install --path ., but that doesn't seem related.

I'm not proposing to eliminate the --path option. I'm proposing to handle this automatically if and only if the argument contains a path separator, which seems likely to be a common case; for instance, cargo add ../somedep.

I appreciate the possibility that once people are used to the idea that they can run cargo add some/path, they might also think they can run cargo add subdir (which I'm not suggesting we support). However, I don't think that possibility means we need to have cargo add ../path continue to error rather than doing what the user is clearly asking for.

At a minimum, I think there's value in augmenting the error message to tell people they need the --path option, but in general, this seems to fall in the category of "if you knew what I wanted to do..."

[epage]

I'm all for improving the error message.

This again gets into my concern with "if you knew what I wanted to do..." as it creates a sloppiness that makes behavior difficult to predict. This would allow me to do cargo add crates/foo but if I switch to a repo not using crates/, then if I do cargo add foo then it no longer works.

I think it's a useful to keep in mind but its not a principle to make top priority.

[joshtriplett]

@epage I would argue that the case of doing cargo add inside a workspace is less important, as we've made it so cargo new automatically adds something to the workspace. And arguably, if you cargo add a crate with the same name as a crate in your workspace, we should at the very least warn, if not just automatically use the crate in your workspace.

[epage]

Hmm, looks like we do auto-check the name against workspace members https://github.com/rust-lang/cargo/blob/a2d45dc370580220d13fefd790f9e148e55b7e34/src/cargo/ops/cargo_add/mod.rs#L388-L398