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

Suggest cargo install --git when missing registry package looks like a git* URL #10522

43 changes: 37 additions & 6 deletions src/cargo/ops/common_for_install_and_uninstall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@ use std::task::Poll;
use anyhow::{bail, format_err, Context as _};
use serde::{Deserialize, Serialize};
use toml_edit::easy as toml;
use url::Url;

use crate::core::compiler::Freshness;
use crate::core::{Dependency, FeatureValue, Package, PackageId, Source, SourceId};
use crate::ops::{self, CompileFilter, CompileOptions};
use crate::sources::PathSource;
use crate::util::errors::CargoResult;
use crate::util::interning::InternedString;
use crate::util::Config;
use crate::util::{FileLock, Filesystem};

Expand Down Expand Up @@ -520,6 +522,25 @@ pub fn path_source(source_id: SourceId, config: &Config) -> CargoResult<PathSour
Ok(PathSource::new(&path, source_id, config))
}

fn is_package_name_a_git_url(package_name: &InternedString) -> bool {
if let Ok(url) = Url::parse(package_name) {
if let Some(domain) = url.domain() {
// REVIEW
// Are there any other git services without "git"
// in the domain?
// bitbucket?
// Is it possible to ask the cargo/crates team for
// some stats where crates projects are currently hosted on
return domain.contains("git");
}
}
false
}

fn was_git_url_miscategorised_as_a_registry_dep(dep: &Dependency) -> bool {
return dep.source_id().is_registry() && is_package_name_a_git_url(&dep.package_name());
}

/// Gets a Package based on command-line requirements.
pub fn select_dep_pkg<T>(
source: &mut T,
Expand Down Expand Up @@ -565,12 +586,22 @@ where
source.source_id()
)
} else {
bail!(
"could not find `{}` in {} with version `{}`",
dep.package_name(),
source.source_id(),
dep.version_req(),
)
if was_git_url_miscategorised_as_a_registry_dep(&dep) {
bail!(
"could not find `{}` in {} with version `{}`. Try adding `--git {}`",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I don't think I've seen cargo strictly follow rustc's error conventions, I think it'd be helpful for us to consider them with our error messages. See https://rustc-dev-guide.rust-lang.org/diagnostics.html#suggestion-style-guide.

While this isn't quite a question, it still falls under the "be more explicit" part of questions where it says

Sometimes, it's unclear why a particular suggestion is being made

Maybe the "further instruction" section can also provide ideas on how to work this.

The message may contain further instruction such as "to do xyz, use" or "to do xyz, use abc".

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out. I will rewrite the error message to follow rustc conventions for the suggestion along the lines of

To install a package from a git repository, use --git {}

I think the original first part of the error message is a) good enough b) familiar to users by now, so will only rewrite the suggestion

dep.package_name(),
source.source_id(),
dep.version_req(),
dep.package_name(),
)
} else {
bail!(
"could not find `{}` in {} with version `{}`",
dep.package_name(),
source.source_id(),
dep.version_req(),
)
}
}
}
}
Expand Down
13 changes: 13 additions & 0 deletions tests/testsuite/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1418,6 +1418,19 @@ fn test_install_git_cannot_be_a_base_url() {
.run();
}

#[cargo_test]
fn test_install_from_git_generate_suggestion() {
registry::init();
cargo_process("install https://github.com/rust-lang/cargo")
.with_status(101)
.with_stderr(
"\
[UPDATING] `[..]` index
error: could not find `https://github.com/rust-lang/cargo` in registry `crates-io` with version `*`. Try adding `--git https://github.com/rust-lang/cargo`",
)
.run();
}

#[cargo_test]
fn uninstall_multiple_and_specifying_bin() {
cargo_process("uninstall foo bar --bin baz")
Expand Down