Skip to content

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

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

75 changes: 69 additions & 6 deletions src/cargo/ops/common_for_install_and_uninstall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ use std::rc::Rc;
use std::task::Poll;

use anyhow::{bail, format_err, Context as _};
use gix::bstr::BStr;
use gix::url::{parse, Scheme};
use ops::FilterRule;
use serde::{Deserialize, Serialize};

Expand All @@ -16,6 +18,7 @@ use crate::core::{Dependency, FeatureValue, Package, PackageId, QueryKind, Sourc
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 @@ -521,6 +524,54 @@ pub fn path_source(source_id: SourceId, config: &Config) -> CargoResult<PathSour
Ok(PathSource::new(&path, source_id, config))
}

/// A holder for a user-facing string that helps them run
/// cargo install --git with correct arguments
enum InstallSuggestion {
/// Hint to append to the error message that the user can
/// run to install a package from a Git Repo
NormalisedGitUrl(String),
/// Failed to generate an install suggestion.
None,
}

/// Returns None if the Registry Dependency was not a GitUrl, otherwise
/// return a suggestion with the https-normalised GitUrl for user
/// to rerun cargo install with
fn was_git_url_miscategorised_as_a_registry_dep(dep: &Dependency) -> InstallSuggestion {
if dep.source_id().is_registry() {
if let Ok(git_url) = parse(BStr::new(dep.package_name().as_bytes())) {
let final_git_url: Option<InternedString> = match git_url.scheme {
// cargo doesn't support cargo install git@ urls, so
Scheme::Ssh | Scheme::Git => {
if let (Some(host), Some(owner)) = (git_url.host(), git_url.user()) {
let https_git_url = format!(
"https://{host}/{owner}/{repo_name}",
// repo_name = git_url.name()
repo_name = "TODO"
Copy link
Author

@petr-tik petr-tik Aug 21, 2023

Choose a reason for hiding this comment

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

the reason this repo_name = "TODO" doesn't cause test_install_from_https_git_generate_suggestion to fail is the same reason why test_install_from_git_generate_suggestion is now failing with

error: the --version provided, bitbucket.org:jcmoyer/rust-tictactoe.git, is not a valid semver version

Since I opened the PR, cargo started supporting crate@version command-line arguments
2806270
and now the test input [email protected]:jcmoyer/rust-tictactoe.git is parsed as a version string (and fails with the error in the CI logs).

the code path I added only deals with strings that weren't erroneously parsed as version strings, so the section I highlighted above won't be traversed, because git@ strings now fail to parse as version strings before they get here.

I propose removing this chunk of code and removing the failing test to focus on supporting actionable errors for https git URLs only.

Please let me know what you think

Copy link
Author

Choose a reason for hiding this comment

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

actioned here
f08e95b

);
Some(InternedString::from(https_git_url))
} else {
None
}
}
Scheme::Http | Scheme::Https => Some(dep.package_name()),
// otherwise cargo install bar will interpret bar as a file-based
// git repo
_ => None,
};

if let Some(final_git_url) = final_git_url {
return InstallSuggestion::NormalisedGitUrl(format!(
"To install a package from a git repository, use `--git {final_git_url}`",
));
} else {
return InstallSuggestion::None;
}
}
}
return InstallSuggestion::None;
}

/// Gets a Package based on command-line requirements.
pub fn select_dep_pkg<T>(
source: &mut T,
Expand Down Expand Up @@ -578,12 +629,24 @@ where
source.source_id()
)
} else {
bail!(
"could not find `{}` in {} with version `{}`",
dep.package_name(),
source.source_id(),
dep.version_req(),
)
if let InstallSuggestion::NormalisedGitUrl(suggestion) =
was_git_url_miscategorised_as_a_registry_dep(&dep)
{
bail!(
"could not find `{}` in {} with version `{}`. {}",
dep.package_name(),
source.source_id(),
dep.version_req(),
suggestion,
)
} else {
bail!(
"could not find `{}` in {} with version `{}`",
dep.package_name(),
source.source_id(),
dep.version_req(),
)
}
}
}
}
Expand Down
26 changes: 26 additions & 0 deletions tests/testsuite/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1642,6 +1642,32 @@ fn test_install_git_cannot_be_a_base_url() {
.run();
}

#[cargo_test]
fn test_install_from_https_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 `*`. To install a package from a git repository, use `--git https://github.com/rust-lang/cargo`",
)
.run();
}

#[cargo_test]
fn test_install_from_git_generate_suggestion() {
registry::init();
cargo_process("install [email protected]:jcmoyer/rust-tictactoe.git")
.with_status(101)
.with_stderr(
"\
[UPDATING] `[..]` index
error: could not find `[email protected]:jcmoyer/rust-tictactoe.git` in registry `crates-io` with version `*`. To install a package from a git repository, use `--git https://bitbucket.org/jcmoyer/rust-tictactoe`",
)
.run();
}

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