From b87ae285ce86cc5212e200e6862a2d15dccda39b Mon Sep 17 00:00:00 2001 From: petr-tik Date: Wed, 30 Mar 2022 20:07:25 +0100 Subject: [PATCH 1/8] Fix #10485 Heuristically identify git URL by matching the pattern "git" in the domain. Add a special case error with a fixit hint to help users rerun the correct cargo install command --- .../ops/common_for_install_and_uninstall.rs | 43 ++++++++++++++++--- tests/testsuite/install.rs | 13 ++++++ 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/src/cargo/ops/common_for_install_and_uninstall.rs b/src/cargo/ops/common_for_install_and_uninstall.rs index fd456ae90ad..9817266fc8f 100644 --- a/src/cargo/ops/common_for_install_and_uninstall.rs +++ b/src/cargo/ops/common_for_install_and_uninstall.rs @@ -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}; @@ -520,6 +522,25 @@ pub fn path_source(source_id: SourceId, config: &Config) -> CargoResult 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( source: &mut T, @@ -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 {}`", + 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(), + ) + } } } } diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs index 99d4c0b163e..bf9bb797c62 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -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") From 174c80ec6fceeed732bd4d3f95190e18bb63efc8 Mon Sep 17 00:00:00 2001 From: petr-tik Date: Fri, 1 Apr 2022 22:59:01 +0100 Subject: [PATCH 2/8] Refactor select_dep_pkg error handling Create a local Error type that that wraps existing error handling code Giving the three error variants names improves readability and makes it easier to recognise duplicate code Move all string operations to create errors from 3 separate bail! calls into anyhow::anyhow! calls that are encapsulated in the From<> impl --- .../ops/common_for_install_and_uninstall.rs | 157 ++++++++++++------ 1 file changed, 103 insertions(+), 54 deletions(-) diff --git a/src/cargo/ops/common_for_install_and_uninstall.rs b/src/cargo/ops/common_for_install_and_uninstall.rs index 9817266fc8f..ac7ebaf0d92 100644 --- a/src/cargo/ops/common_for_install_and_uninstall.rs +++ b/src/cargo/ops/common_for_install_and_uninstall.rs @@ -17,7 +17,7 @@ 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::{Config, OptVersionReq}; use crate::util::{FileLock, Filesystem}; /// On-disk tracking for which package installed which binary. @@ -522,25 +522,6 @@ pub fn path_source(source_id: SourceId, config: &Config) -> CargoResult 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( source: &mut T, @@ -566,47 +547,115 @@ where Poll::Pending => source.block_until_ready()?, } }; - match deps.iter().map(|p| p.package_id()).max() { - Some(pkgid) => { - let pkg = Box::new(source).download_now(pkgid, config)?; - Ok(pkg) + if let Some(pkgid) = deps.iter().map(|p| p.package_id()).max() { + let pkg = Box::new(source).download_now(pkgid, config)?; + Ok(pkg) + } else { + let spe = SelectPackageError::new(dep, source); + bail!(anyhow::Error::from(spe)) + } +} + +struct YankedInfo { + package_name: InternedString, + source_id: SourceId, +} + +struct PackageNotFound { + package_name: InternedString, + source_id: SourceId, + // REVIEW make it a reference or is cloning ok? + version_req: OptVersionReq, +} + +enum SelectPackageError { + /// Return when the Package had been yanked from the Registry + Yanked(YankedInfo), + // Return when someone accidentally passed a git(hub|lab|ea) URL as the package name + UrlTreatedAsPackageName(PackageNotFound), + // Catch-all variant for all other errors + CouldntFindInRegistry(PackageNotFound), +} + +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"); } - None => { - let is_yanked: bool = if dep.version_req().is_exact() { - let version: String = dep.version_req().to_string(); - PackageId::new(dep.package_name(), &version[1..], source.source_id()) - .map_or(false, |pkg_id| source.is_yanked(pkg_id).unwrap_or(false)) - } else { - false - }; - if is_yanked { - bail!( - "cannot install package `{}`, it has been yanked from {}", - dep.package_name(), - source.source_id() - ) + } + 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()); +} + +impl SelectPackageError { + fn new(dep: Dependency, source: &mut T) -> Self + where + T: Source, + { + let is_yanked: bool = if dep.version_req().is_exact() { + let version: String = dep.version_req().to_string(); + PackageId::new(dep.package_name(), &version[1..], source.source_id()) + .map_or(false, |pkg_id| source.is_yanked(pkg_id).unwrap_or(false)) + } else { + false + }; + if is_yanked { + SelectPackageError::Yanked(YankedInfo { + package_name: dep.package_name(), + source_id: source.source_id(), + }) + } else { + if was_git_url_miscategorised_as_a_registry_dep(&dep) { + SelectPackageError::UrlTreatedAsPackageName(PackageNotFound { + package_name: dep.package_name(), + source_id: source.source_id(), + version_req: dep.version_req().clone(), + }) } else { - if was_git_url_miscategorised_as_a_registry_dep(&dep) { - bail!( - "could not find `{}` in {} with version `{}`. Try adding `--git {}`", - 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(), - ) - } + SelectPackageError::CouldntFindInRegistry(PackageNotFound { + package_name: dep.package_name(), + source_id: source.source_id(), + version_req: dep.version_req().clone(), + }) } } } } +impl From for anyhow::Error { + fn from(err: SelectPackageError) -> Self { + match err { + SelectPackageError::Yanked(info) => anyhow::anyhow!( + "cannot install package `{}`, it has been yanked from {}", + info.package_name, + info.source_id + ), + SelectPackageError::UrlTreatedAsPackageName(pkg_not_found) => anyhow::anyhow!( + "could not find `{}` in {} with version `{}`. Try adding `--git {}`", + pkg_not_found.package_name, + pkg_not_found.source_id, + pkg_not_found.version_req, + pkg_not_found.package_name, + ), + SelectPackageError::CouldntFindInRegistry(pkg_not_found) => anyhow::anyhow!( + "could not find `{}` in {} with version `{}`", + pkg_not_found.package_name, + pkg_not_found.source_id, + pkg_not_found.version_req, + ), + } + } +} + pub fn select_pkg( source: &mut T, dep: Option, From 869973fc17ec3c9caedd13bede2e8ddd48a01141 Mon Sep 17 00:00:00 2001 From: petr-tik Date: Tue, 12 Apr 2022 18:10:39 +0100 Subject: [PATCH 3/8] Revert "Refactor select_dep_pkg error handling" This reverts commit 174c80ec6fceeed732bd4d3f95190e18bb63efc8. --- .../ops/common_for_install_and_uninstall.rs | 157 ++++++------------ 1 file changed, 54 insertions(+), 103 deletions(-) diff --git a/src/cargo/ops/common_for_install_and_uninstall.rs b/src/cargo/ops/common_for_install_and_uninstall.rs index ac7ebaf0d92..9817266fc8f 100644 --- a/src/cargo/ops/common_for_install_and_uninstall.rs +++ b/src/cargo/ops/common_for_install_and_uninstall.rs @@ -17,7 +17,7 @@ use crate::ops::{self, CompileFilter, CompileOptions}; use crate::sources::PathSource; use crate::util::errors::CargoResult; use crate::util::interning::InternedString; -use crate::util::{Config, OptVersionReq}; +use crate::util::Config; use crate::util::{FileLock, Filesystem}; /// On-disk tracking for which package installed which binary. @@ -522,6 +522,25 @@ pub fn path_source(source_id: SourceId, config: &Config) -> CargoResult 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( source: &mut T, @@ -547,115 +566,47 @@ where Poll::Pending => source.block_until_ready()?, } }; - if let Some(pkgid) = deps.iter().map(|p| p.package_id()).max() { - let pkg = Box::new(source).download_now(pkgid, config)?; - Ok(pkg) - } else { - let spe = SelectPackageError::new(dep, source); - bail!(anyhow::Error::from(spe)) - } -} - -struct YankedInfo { - package_name: InternedString, - source_id: SourceId, -} - -struct PackageNotFound { - package_name: InternedString, - source_id: SourceId, - // REVIEW make it a reference or is cloning ok? - version_req: OptVersionReq, -} - -enum SelectPackageError { - /// Return when the Package had been yanked from the Registry - Yanked(YankedInfo), - // Return when someone accidentally passed a git(hub|lab|ea) URL as the package name - UrlTreatedAsPackageName(PackageNotFound), - // Catch-all variant for all other errors - CouldntFindInRegistry(PackageNotFound), -} - -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"); + match deps.iter().map(|p| p.package_id()).max() { + Some(pkgid) => { + let pkg = Box::new(source).download_now(pkgid, config)?; + Ok(pkg) } - } - 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()); -} - -impl SelectPackageError { - fn new(dep: Dependency, source: &mut T) -> Self - where - T: Source, - { - let is_yanked: bool = if dep.version_req().is_exact() { - let version: String = dep.version_req().to_string(); - PackageId::new(dep.package_name(), &version[1..], source.source_id()) - .map_or(false, |pkg_id| source.is_yanked(pkg_id).unwrap_or(false)) - } else { - false - }; - if is_yanked { - SelectPackageError::Yanked(YankedInfo { - package_name: dep.package_name(), - source_id: source.source_id(), - }) - } else { - if was_git_url_miscategorised_as_a_registry_dep(&dep) { - SelectPackageError::UrlTreatedAsPackageName(PackageNotFound { - package_name: dep.package_name(), - source_id: source.source_id(), - version_req: dep.version_req().clone(), - }) + None => { + let is_yanked: bool = if dep.version_req().is_exact() { + let version: String = dep.version_req().to_string(); + PackageId::new(dep.package_name(), &version[1..], source.source_id()) + .map_or(false, |pkg_id| source.is_yanked(pkg_id).unwrap_or(false)) + } else { + false + }; + if is_yanked { + bail!( + "cannot install package `{}`, it has been yanked from {}", + dep.package_name(), + source.source_id() + ) } else { - SelectPackageError::CouldntFindInRegistry(PackageNotFound { - package_name: dep.package_name(), - source_id: source.source_id(), - version_req: dep.version_req().clone(), - }) + if was_git_url_miscategorised_as_a_registry_dep(&dep) { + bail!( + "could not find `{}` in {} with version `{}`. Try adding `--git {}`", + 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(), + ) + } } } } } -impl From for anyhow::Error { - fn from(err: SelectPackageError) -> Self { - match err { - SelectPackageError::Yanked(info) => anyhow::anyhow!( - "cannot install package `{}`, it has been yanked from {}", - info.package_name, - info.source_id - ), - SelectPackageError::UrlTreatedAsPackageName(pkg_not_found) => anyhow::anyhow!( - "could not find `{}` in {} with version `{}`. Try adding `--git {}`", - pkg_not_found.package_name, - pkg_not_found.source_id, - pkg_not_found.version_req, - pkg_not_found.package_name, - ), - SelectPackageError::CouldntFindInRegistry(pkg_not_found) => anyhow::anyhow!( - "could not find `{}` in {} with version `{}`", - pkg_not_found.package_name, - pkg_not_found.source_id, - pkg_not_found.version_req, - ), - } - } -} - pub fn select_pkg( source: &mut T, dep: Option, From 1c9c53b9b7f37ca99a47e4c95824cb71dc2782ab Mon Sep 17 00:00:00 2001 From: petr-tik Date: Wed, 13 Apr 2022 19:40:27 +0100 Subject: [PATCH 4/8] Experiment with GitUrl to generate the suggestion Add a test case for a git@ repo and refactor the method to return Option instead of bool. Reword the suggestion to follow rustc guidelines --- Cargo.toml | 1 + .../ops/common_for_install_and_uninstall.rs | 48 +++++++++++-------- tests/testsuite/install.rs | 17 ++++++- 3 files changed, 45 insertions(+), 21 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d1143aa15ae..3957be94da7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -60,6 +60,7 @@ termcolor = "1.1" toml_edit = { version = "0.13.4", features = ["serde", "easy"] } unicode-xid = "0.2.0" url = "2.2.2" +git-url-parse = "0.4.0" walkdir = "2.2" clap = "3.1.0" unicode-width = "0.1.5" diff --git a/src/cargo/ops/common_for_install_and_uninstall.rs b/src/cargo/ops/common_for_install_and_uninstall.rs index 9817266fc8f..0acc044e830 100644 --- a/src/cargo/ops/common_for_install_and_uninstall.rs +++ b/src/cargo/ops/common_for_install_and_uninstall.rs @@ -7,9 +7,9 @@ use std::rc::Rc; use std::task::Poll; use anyhow::{bail, format_err, Context as _}; +use git_url_parse::{GitUrl, Scheme}; 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}; @@ -522,23 +522,33 @@ pub fn path_source(source_id: SourceId, config: &Config) -> CargoResult 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"); +/// 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) -> Option { + if dep.source_id().is_registry() { + if let Ok(git_url) = GitUrl::parse(&dep.package_name()) { + let final_git_url: InternedString = match git_url.scheme { + Scheme::Ssh | Scheme::Git | Scheme::GitSsh => { + if let (Some(host), Some(owner)) = (git_url.host, git_url.owner) { + let https_git_url = format!( + "https://{host}/{owner}/{repo_name}", + repo_name = git_url.name + ); + InternedString::from(https_git_url) + } else { + dep.package_name() + } + } + _ => dep.package_name(), + }; + + return Some(format!( + "To install a package from a git repository, use `--git {final_git_url}`", + )); } } - 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()); + return None; } /// Gets a Package based on command-line requirements. @@ -586,13 +596,13 @@ where source.source_id() ) } else { - if was_git_url_miscategorised_as_a_registry_dep(&dep) { + if let Some(suggestion) = was_git_url_miscategorised_as_a_registry_dep(&dep) { bail!( - "could not find `{}` in {} with version `{}`. Try adding `--git {}`", + "could not find `{}` in {} with version `{}`. {}", dep.package_name(), source.source_id(), dep.version_req(), - dep.package_name(), + suggestion, ) } else { bail!( diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs index bf9bb797c62..316141588de 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -1419,14 +1419,27 @@ fn test_install_git_cannot_be_a_base_url() { } #[cargo_test] -fn test_install_from_git_generate_suggestion() { +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 `*`. Try adding `--git https://github.com/rust-lang/cargo`", +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 git@bitbucket.org:jcmoyer/rust-tictactoe.git") + .with_status(101) + .with_stderr( + "\ +[UPDATING] `[..]` index +error: could not find `git@bitbucket.org: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(); } From ef7ee2c7ee9e1373ec66f04460dca1db1da7f828 Mon Sep 17 00:00:00 2001 From: petr-tik Date: Wed, 13 Apr 2022 22:38:33 +0100 Subject: [PATCH 5/8] Fix the tests Make GitUrl parsing and normalisation defensive. Create a type for InstallSuggestion and pass it to the anyhow error message --- .../ops/common_for_install_and_uninstall.rs | 40 ++++++++++++++----- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/src/cargo/ops/common_for_install_and_uninstall.rs b/src/cargo/ops/common_for_install_and_uninstall.rs index 0acc044e830..f6356b86ceb 100644 --- a/src/cargo/ops/common_for_install_and_uninstall.rs +++ b/src/cargo/ops/common_for_install_and_uninstall.rs @@ -522,33 +522,51 @@ pub fn path_source(source_id: SourceId, config: &Config) -> CargoResult Option { +fn was_git_url_miscategorised_as_a_registry_dep(dep: &Dependency) -> InstallSuggestion { if dep.source_id().is_registry() { if let Ok(git_url) = GitUrl::parse(&dep.package_name()) { - let final_git_url: InternedString = match git_url.scheme { + let final_git_url: Option = match git_url.scheme { + // cargo doesn't support cargo install git@ urls, so Scheme::Ssh | Scheme::Git | Scheme::GitSsh => { if let (Some(host), Some(owner)) = (git_url.host, git_url.owner) { let https_git_url = format!( "https://{host}/{owner}/{repo_name}", repo_name = git_url.name ); - InternedString::from(https_git_url) + Some(InternedString::from(https_git_url)) } else { - dep.package_name() + None } } - _ => dep.package_name(), + Scheme::Http | Scheme::Https => Some(dep.package_name()), + // otherwise cargo install bar will interpret bar as a file-based + // git repo + _ => None, }; - return Some(format!( - "To install a package from a git repository, use `--git {final_git_url}`", - )); + 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 None; + return InstallSuggestion::None; } /// Gets a Package based on command-line requirements. @@ -596,7 +614,9 @@ where source.source_id() ) } else { - if let Some(suggestion) = was_git_url_miscategorised_as_a_registry_dep(&dep) { + if let InstallSuggestion::NormalisedGitUrl(suggestion) = + was_git_url_miscategorised_as_a_registry_dep(&dep) + { bail!( "could not find `{}` in {} with version `{}`. {}", dep.package_name(), From 283c6e8f3ebf65a14c002b262364cc3535b111b1 Mon Sep 17 00:00:00 2001 From: petr-tik Date: Sun, 20 Aug 2023 21:04:48 +0100 Subject: [PATCH 6/8] Fix the build and update Cargo.lock --- Cargo.lock | 150 +++++++++++++++++++++++++++++++++++++++++++++++++++++ Cargo.toml | 1 + 2 files changed, 151 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 37150413810..486552832ba 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,6 +2,15 @@ # It is not intended for manual editing. version = 3 +[[package]] +name = "addr2line" +version = "0.20.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f4fa78e18c64fce05e902adecd7a5eed15a5e0a3439f7b0e169f0252214865e3" +dependencies = [ + "gimli", +] + [[package]] name = "adler" version = "1.0.2" @@ -117,6 +126,21 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa" +[[package]] +name = "backtrace" +version = "0.3.68" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4319208da049c43661739c5fade2ba182f09d1dc2299b32298d3a31692b17e12" +dependencies = [ + "addr2line", + "cc", + "cfg-if", + "libc", + "miniz_oxide", + "object", + "rustc-demangle", +] + [[package]] name = "base16ct" version = "0.2.0" @@ -277,6 +301,7 @@ dependencies = [ "filetime", "flate2", "fwdansi", + "git-url-parse", "git2", "git2-curl", "gix", @@ -540,6 +565,33 @@ version = "0.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b8191fa7302e03607ff0e237d4246cc043ff5b3cb9409d995172ba3bea16b807" +[[package]] +name = "color-eyre" +version = "0.6.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a667583cca8c4f8436db8de46ea8233c42a7d9ae424a82d338f2e4675229204" +dependencies = [ + "backtrace", + "color-spantrace", + "eyre", + "indenter", + "once_cell", + "owo-colors", + "tracing-error", +] + +[[package]] +name = "color-spantrace" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1ba75b3d9449ecdccb27ecbc479fdc0b87fa2dd43d2f8298f9bf0e59aacc8dce" +dependencies = [ + "once_cell", + "owo-colors", + "tracing-core", + "tracing-error", +] + [[package]] name = "colorchoice" version = "1.0.0" @@ -869,6 +921,16 @@ dependencies = [ "serde_json", ] +[[package]] +name = "eyre" +version = "0.6.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4c2b6b5a29c02cdc822728b7d7b8ae1bab3e3b05d44522770ddd49722eeac7eb" +dependencies = [ + "indenter", + "once_cell", +] + [[package]] name = "fastrand" version = "1.9.0" @@ -987,6 +1049,26 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "gimli" +version = "0.27.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b6c80984affa11d98d1b88b66ac8853f143217b399d3c74116778ff8fdb4ed2e" + +[[package]] +name = "git-url-parse" +version = "0.4.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b037f7449dd4a8b711e660301ff1ff28aa00eea09698421fb2d78db51a7b7a72" +dependencies = [ + "color-eyre", + "regex", + "strum", + "strum_macros", + "tracing", + "url", +] + [[package]] name = "git2" version = "0.17.2" @@ -1720,6 +1802,12 @@ version = "0.14.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2c6201b9ff9fd90a5a3bac2e56a830d0caa509576f0e503818ee82c181b3437a" +[[package]] +name = "heck" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "95505c38b4572b2d910cecb0281560f54b440a19336cbbcb27bf6ce6adc6f5a8" + [[package]] name = "hermit-abi" version = "0.2.6" @@ -1841,6 +1929,12 @@ dependencies = [ "hashbrown 0.12.3", ] +[[package]] +name = "indenter" +version = "0.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce23b50ad8242c51a442f3ff322d56b02f08852c77e4c0b4d3fd684abc89c683" + [[package]] name = "indexmap" version = "2.0.0" @@ -2204,6 +2298,15 @@ dependencies = [ "libc", ] +[[package]] +name = "object" +version = "0.31.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8bda667d9f2b5051b8833f59f3bf748b28ef54f850f4fcb389a252aa383866d1" +dependencies = [ + "memchr", +] + [[package]] name = "once_cell" version = "1.18.0" @@ -2318,6 +2421,12 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b15813163c1d831bf4a13c3610c05c0d03b39feb07f7e09fa234dac9b15aaf39" +[[package]] +name = "owo-colors" +version = "3.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c1b04fb49957986fdce4d6ee7a65027d55d4b6d2265e5848bbb507b58ccfdb6f" + [[package]] name = "p384" version = "0.13.0" @@ -2745,6 +2854,12 @@ dependencies = [ "subtle", ] +[[package]] +name = "rustc-demangle" +version = "0.1.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d626bb9dae77e28219937af045c257c28bfd3f69333c512553507f5f9798cb76" + [[package]] name = "rustc-hash" version = "1.1.0" @@ -2790,6 +2905,12 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "rustversion" +version = "1.0.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7ffc183a10b4478d04cbbbfc96d0873219d962dd5accaff2ffbd4ceb7df837f4" + [[package]] name = "rusty-fork" version = "0.3.0" @@ -3107,6 +3228,25 @@ version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623" +[[package]] +name = "strum" +version = "0.24.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "063e6045c0e62079840579a7e47a355ae92f60eb74daaf156fb1e84ba164e63f" + +[[package]] +name = "strum_macros" +version = "0.24.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e385be0d24f186b4ce2f9982191e7101bb737312ad61c1f2f984f34bcf85d59" +dependencies = [ + "heck", + "proc-macro2", + "quote", + "rustversion", + "syn 1.0.109", +] + [[package]] name = "subtle" version = "2.5.0" @@ -3340,6 +3480,16 @@ dependencies = [ "valuable", ] +[[package]] +name = "tracing-error" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d686ec1c0f384b1277f097b2f279a2ecc11afe8c133c1aabf036a27cb4cd206e" +dependencies = [ + "tracing", + "tracing-subscriber", +] + [[package]] name = "tracing-log" version = "0.1.3" diff --git a/Cargo.toml b/Cargo.toml index 79c9ac1320e..8738e90bd4e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -135,6 +135,7 @@ curl = { workspace = true, features = ["http2"] } curl-sys.workspace = true filetime.workspace = true flate2.workspace = true +git-url-parse.workspace = true git2.workspace = true git2-curl.workspace = true gix.workspace = true From ddea2580aaefb2b01d8563f01ca6fed931367805 Mon Sep 17 00:00:00 2001 From: petr-tik Date: Mon, 21 Aug 2023 22:46:15 +0100 Subject: [PATCH 7/8] Replace git-url-parse with gix::url::parse Keep the dependency try the same by leveraging already imported gix crate to try parsing a git url from a string --- Cargo.lock | 150 ------------------ Cargo.toml | 2 - .../ops/common_for_install_and_uninstall.rs | 12 +- 3 files changed, 7 insertions(+), 157 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 486552832ba..37150413810 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,15 +2,6 @@ # It is not intended for manual editing. version = 3 -[[package]] -name = "addr2line" -version = "0.20.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f4fa78e18c64fce05e902adecd7a5eed15a5e0a3439f7b0e169f0252214865e3" -dependencies = [ - "gimli", -] - [[package]] name = "adler" version = "1.0.2" @@ -126,21 +117,6 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa" -[[package]] -name = "backtrace" -version = "0.3.68" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4319208da049c43661739c5fade2ba182f09d1dc2299b32298d3a31692b17e12" -dependencies = [ - "addr2line", - "cc", - "cfg-if", - "libc", - "miniz_oxide", - "object", - "rustc-demangle", -] - [[package]] name = "base16ct" version = "0.2.0" @@ -301,7 +277,6 @@ dependencies = [ "filetime", "flate2", "fwdansi", - "git-url-parse", "git2", "git2-curl", "gix", @@ -565,33 +540,6 @@ version = "0.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b8191fa7302e03607ff0e237d4246cc043ff5b3cb9409d995172ba3bea16b807" -[[package]] -name = "color-eyre" -version = "0.6.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5a667583cca8c4f8436db8de46ea8233c42a7d9ae424a82d338f2e4675229204" -dependencies = [ - "backtrace", - "color-spantrace", - "eyre", - "indenter", - "once_cell", - "owo-colors", - "tracing-error", -] - -[[package]] -name = "color-spantrace" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1ba75b3d9449ecdccb27ecbc479fdc0b87fa2dd43d2f8298f9bf0e59aacc8dce" -dependencies = [ - "once_cell", - "owo-colors", - "tracing-core", - "tracing-error", -] - [[package]] name = "colorchoice" version = "1.0.0" @@ -921,16 +869,6 @@ dependencies = [ "serde_json", ] -[[package]] -name = "eyre" -version = "0.6.8" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4c2b6b5a29c02cdc822728b7d7b8ae1bab3e3b05d44522770ddd49722eeac7eb" -dependencies = [ - "indenter", - "once_cell", -] - [[package]] name = "fastrand" version = "1.9.0" @@ -1049,26 +987,6 @@ dependencies = [ "wasm-bindgen", ] -[[package]] -name = "gimli" -version = "0.27.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b6c80984affa11d98d1b88b66ac8853f143217b399d3c74116778ff8fdb4ed2e" - -[[package]] -name = "git-url-parse" -version = "0.4.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b037f7449dd4a8b711e660301ff1ff28aa00eea09698421fb2d78db51a7b7a72" -dependencies = [ - "color-eyre", - "regex", - "strum", - "strum_macros", - "tracing", - "url", -] - [[package]] name = "git2" version = "0.17.2" @@ -1802,12 +1720,6 @@ version = "0.14.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2c6201b9ff9fd90a5a3bac2e56a830d0caa509576f0e503818ee82c181b3437a" -[[package]] -name = "heck" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "95505c38b4572b2d910cecb0281560f54b440a19336cbbcb27bf6ce6adc6f5a8" - [[package]] name = "hermit-abi" version = "0.2.6" @@ -1929,12 +1841,6 @@ dependencies = [ "hashbrown 0.12.3", ] -[[package]] -name = "indenter" -version = "0.3.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ce23b50ad8242c51a442f3ff322d56b02f08852c77e4c0b4d3fd684abc89c683" - [[package]] name = "indexmap" version = "2.0.0" @@ -2298,15 +2204,6 @@ dependencies = [ "libc", ] -[[package]] -name = "object" -version = "0.31.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8bda667d9f2b5051b8833f59f3bf748b28ef54f850f4fcb389a252aa383866d1" -dependencies = [ - "memchr", -] - [[package]] name = "once_cell" version = "1.18.0" @@ -2421,12 +2318,6 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b15813163c1d831bf4a13c3610c05c0d03b39feb07f7e09fa234dac9b15aaf39" -[[package]] -name = "owo-colors" -version = "3.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c1b04fb49957986fdce4d6ee7a65027d55d4b6d2265e5848bbb507b58ccfdb6f" - [[package]] name = "p384" version = "0.13.0" @@ -2854,12 +2745,6 @@ dependencies = [ "subtle", ] -[[package]] -name = "rustc-demangle" -version = "0.1.23" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d626bb9dae77e28219937af045c257c28bfd3f69333c512553507f5f9798cb76" - [[package]] name = "rustc-hash" version = "1.1.0" @@ -2905,12 +2790,6 @@ dependencies = [ "windows-sys", ] -[[package]] -name = "rustversion" -version = "1.0.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7ffc183a10b4478d04cbbbfc96d0873219d962dd5accaff2ffbd4ceb7df837f4" - [[package]] name = "rusty-fork" version = "0.3.0" @@ -3228,25 +3107,6 @@ version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623" -[[package]] -name = "strum" -version = "0.24.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "063e6045c0e62079840579a7e47a355ae92f60eb74daaf156fb1e84ba164e63f" - -[[package]] -name = "strum_macros" -version = "0.24.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e385be0d24f186b4ce2f9982191e7101bb737312ad61c1f2f984f34bcf85d59" -dependencies = [ - "heck", - "proc-macro2", - "quote", - "rustversion", - "syn 1.0.109", -] - [[package]] name = "subtle" version = "2.5.0" @@ -3480,16 +3340,6 @@ dependencies = [ "valuable", ] -[[package]] -name = "tracing-error" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d686ec1c0f384b1277f097b2f279a2ecc11afe8c133c1aabf036a27cb4cd206e" -dependencies = [ - "tracing", - "tracing-subscriber", -] - [[package]] name = "tracing-log" version = "0.1.3" diff --git a/Cargo.toml b/Cargo.toml index 8738e90bd4e..703e84ce31d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,7 +37,6 @@ curl-sys = "0.4.65" filetime = "0.2.21" flate2 = { version = "1.0.26", default-features = false, features = ["zlib"] } fwdansi = "1.1.0" -git-url-parse = "0.4.0" git2 = "0.17.2" git2-curl = "0.18.0" gix = { version = "0.45.1", default-features = false, features = ["blocking-http-transport-curl", "progress-tree"] } @@ -135,7 +134,6 @@ curl = { workspace = true, features = ["http2"] } curl-sys.workspace = true filetime.workspace = true flate2.workspace = true -git-url-parse.workspace = true git2.workspace = true git2-curl.workspace = true gix.workspace = true diff --git a/src/cargo/ops/common_for_install_and_uninstall.rs b/src/cargo/ops/common_for_install_and_uninstall.rs index 08f3f95281f..139bfea2bfa 100644 --- a/src/cargo/ops/common_for_install_and_uninstall.rs +++ b/src/cargo/ops/common_for_install_and_uninstall.rs @@ -7,7 +7,8 @@ use std::rc::Rc; use std::task::Poll; use anyhow::{bail, format_err, Context as _}; -use git_url_parse::{GitUrl, Scheme}; +use gix::bstr::BStr; +use gix::url::{parse, Scheme}; use ops::FilterRule; use serde::{Deserialize, Serialize}; @@ -538,14 +539,15 @@ enum InstallSuggestion { /// 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) = GitUrl::parse(&dep.package_name()) { + if let Ok(git_url) = parse(BStr::new(dep.package_name().as_bytes())) { let final_git_url: Option = match git_url.scheme { // cargo doesn't support cargo install git@ urls, so - Scheme::Ssh | Scheme::Git | Scheme::GitSsh => { - if let (Some(host), Some(owner)) = (git_url.host, git_url.owner) { + 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 = git_url.name() + repo_name = "TODO" ); Some(InternedString::from(https_git_url)) } else { From f08e95bf0029d32979375a00d3ea5e5754b98f48 Mon Sep 17 00:00:00 2001 From: petr-tik Date: Mon, 21 Aug 2023 23:43:33 +0100 Subject: [PATCH 8/8] Limit the actionable error to install https inputs Since I opened the PR, cargo started supporting `crate@version` command-line inputs https://github.com/rust-lang/cargo/commit/2806270ef63152f9ddf80ea10fad4444c2293f7c and now the test input `git@bitbucket.org:jcmoyer/rust-tictactoe.git` is parsed as a version string, which fails 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. Delete the failing test --- .../ops/common_for_install_and_uninstall.rs | 16 +--------------- tests/testsuite/install.rs | 13 ------------- 2 files changed, 1 insertion(+), 28 deletions(-) diff --git a/src/cargo/ops/common_for_install_and_uninstall.rs b/src/cargo/ops/common_for_install_and_uninstall.rs index 139bfea2bfa..8eca4c920c9 100644 --- a/src/cargo/ops/common_for_install_and_uninstall.rs +++ b/src/cargo/ops/common_for_install_and_uninstall.rs @@ -541,22 +541,8 @@ fn was_git_url_miscategorised_as_a_registry_dep(dep: &Dependency) -> InstallSugg if dep.source_id().is_registry() { if let Ok(git_url) = parse(BStr::new(dep.package_name().as_bytes())) { let final_git_url: Option = 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" - ); - 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 + // cargo doesn't support cargo install git@ urls _ => None, }; diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs index 564a13a5678..9235a19a9b5 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -1655,19 +1655,6 @@ error: could not find `https://github.com/rust-lang/cargo` in registry `crates-i .run(); } -#[cargo_test] -fn test_install_from_git_generate_suggestion() { - registry::init(); - cargo_process("install git@bitbucket.org:jcmoyer/rust-tictactoe.git") - .with_status(101) - .with_stderr( - "\ -[UPDATING] `[..]` index -error: could not find `git@bitbucket.org: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")