From 8bea1b078f8c940f526f5d62f124694ee4e9846d Mon Sep 17 00:00:00 2001 From: bishtpawan Date: Thu, 30 Jul 2020 16:53:26 +0530 Subject: [PATCH 1/7] Add yank check for package that needs to be download --- .../ops/common_for_install_and_uninstall.rs | 26 ++++++++++++++----- 1 file changed, 20 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 78e53af5c4f..9d3e63bd63e 100644 --- a/src/cargo/ops/common_for_install_and_uninstall.rs +++ b/src/cargo/ops/common_for_install_and_uninstall.rs @@ -544,12 +544,26 @@ where let pkg = Box::new(source).download_now(pkgid, config)?; Ok(pkg) } - None => bail!( - "could not find `{}` in {} with version `{}`", - dep.package_name(), - source.source_id(), - dep.version_req(), - ), + None => { + // check whether the package is yanked or not + if source.is_yanked( + PackageId::new(dep.package_name(), dep.version_req(), source.source_id()).into_ok(), + ) { + bail!( + "provided package has been yanked `{}` in {} with version `{}`", + dep.package_name(), + source.source_id(), + dep.version_req(), + ) + } else { + bail!( + "could not find `{}` in {} with version `{}`", + dep.package_name(), + source.source_id(), + dep.version_req(), + ) + } + } } } From c180eb499082c15725880ddb5ae71e064ddeb153 Mon Sep 17 00:00:00 2001 From: bishtpawan Date: Fri, 31 Jul 2020 11:52:54 +0530 Subject: [PATCH 2/7] Update approach to check for yanked version --- .../ops/common_for_install_and_uninstall.rs | 27 +++++++------------ 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/src/cargo/ops/common_for_install_and_uninstall.rs b/src/cargo/ops/common_for_install_and_uninstall.rs index 9d3e63bd63e..cb18e6a38d0 100644 --- a/src/cargo/ops/common_for_install_and_uninstall.rs +++ b/src/cargo/ops/common_for_install_and_uninstall.rs @@ -545,24 +545,17 @@ where Ok(pkg) } None => { - // check whether the package is yanked or not - if source.is_yanked( - PackageId::new(dep.package_name(), dep.version_req(), source.source_id()).into_ok(), - ) { - bail!( - "provided package has been yanked `{}` in {} with version `{}`", - dep.package_name(), - source.source_id(), - dep.version_req(), - ) - } else { - bail!( - "could not find `{}` in {} with version `{}`", - dep.package_name(), - source.source_id(), - dep.version_req(), - ) + for pkg_id in deps.iter() { + if source.is_yanked(pkg_id.package_id()).unwrap_or(false) { + bail!("provided package has been yanked `{}`", pkg_id.package_id()) + } } + bail!( + "could not find `{}` in {} with version `{}`", + dep.package_name(), + source.source_id(), + dep.version_req(), + ) } } } From 844cde203fdd3150c1b3eb29a6b267d7cf1dd1d6 Mon Sep 17 00:00:00 2001 From: bishtpawan Date: Sat, 1 Aug 2020 16:53:58 +0530 Subject: [PATCH 3/7] Add test case for yank validation check --- .../ops/common_for_install_and_uninstall.rs | 26 ++++++++++++------- tests/testsuite/install.rs | 9 +++++++ 2 files changed, 25 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 cb18e6a38d0..131b152fa29 100644 --- a/src/cargo/ops/common_for_install_and_uninstall.rs +++ b/src/cargo/ops/common_for_install_and_uninstall.rs @@ -545,17 +545,23 @@ where Ok(pkg) } None => { - for pkg_id in deps.iter() { - if source.is_yanked(pkg_id.package_id()).unwrap_or(false) { - bail!("provided package has been yanked `{}`", pkg_id.package_id()) - } + let version: String = dep.version_req().clone().to_string(); + let pkg_id: PackageId = + PackageId::new(dep.package_name(), &version[1..], source.source_id())?; + if source.is_yanked(pkg_id).unwrap_or(false) { + bail!( + "cannot install package `{}`, it has been yanked from {}", + pkg_id.name(), + pkg_id.source_id() + ) + } else { + bail!( + "could not find `{}` in {} with version `{}`", + dep.package_name(), + source.source_id(), + dep.version_req(), + ) } - 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 9c5a1e72651..8dcd44daf1e 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -1555,3 +1555,12 @@ fn install_git_with_symlink_home() { ) .run(); } + +#[cargo_test] +fn install_yanked_cargo_package() { + Package::new("baz", "0.0.1").yanked(true).publish(); + cargo_process("install baz --version 0.0.1") + .with_status(101) + .with_stderr("error: cannot install package `baz`, it has been yanked from registry `https://github.com/rust-lang/crates.io-index`") + .run(); +} From a50b10af27e1ecc8bd391a3956f90ccb6a6da8f0 Mon Sep 17 00:00:00 2001 From: bishtpawan Date: Sat, 1 Aug 2020 16:57:35 +0530 Subject: [PATCH 4/7] Refactor code --- src/cargo/ops/common_for_install_and_uninstall.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/ops/common_for_install_and_uninstall.rs b/src/cargo/ops/common_for_install_and_uninstall.rs index 131b152fa29..23f8909521a 100644 --- a/src/cargo/ops/common_for_install_and_uninstall.rs +++ b/src/cargo/ops/common_for_install_and_uninstall.rs @@ -545,7 +545,7 @@ where Ok(pkg) } None => { - let version: String = dep.version_req().clone().to_string(); + let version: String = dep.version_req().to_string(); let pkg_id: PackageId = PackageId::new(dep.package_name(), &version[1..], source.source_id())?; if source.is_yanked(pkg_id).unwrap_or(false) { From dd654d5576f8cf880429425da38c98e08f2c01cf Mon Sep 17 00:00:00 2001 From: bishtpawan Date: Sat, 1 Aug 2020 20:53:24 +0530 Subject: [PATCH 5/7] Add pattern match for yank check and updated corresponding test cases --- .../ops/common_for_install_and_uninstall.rs | 30 ++++++++++++------- tests/testsuite/install.rs | 2 +- tests/testsuite/install_upgrade.rs | 7 ++--- 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/src/cargo/ops/common_for_install_and_uninstall.rs b/src/cargo/ops/common_for_install_and_uninstall.rs index 23f8909521a..67e8b11140e 100644 --- a/src/cargo/ops/common_for_install_and_uninstall.rs +++ b/src/cargo/ops/common_for_install_and_uninstall.rs @@ -546,21 +546,29 @@ where } None => { let version: String = dep.version_req().to_string(); - let pkg_id: PackageId = - PackageId::new(dep.package_name(), &version[1..], source.source_id())?; - if source.is_yanked(pkg_id).unwrap_or(false) { - bail!( - "cannot install package `{}`, it has been yanked from {}", - pkg_id.name(), - pkg_id.source_id() - ) - } else { - bail!( + match PackageId::new(dep.package_name(), &version[1..], source.source_id()) { + Ok(pkg_id) => { + if source.is_yanked(pkg_id).unwrap_or(false) { + bail!( + "cannot install package `{}`, it has been yanked from {}", + pkg_id.name(), + pkg_id.source_id() + ) + } else { + bail!( + "could not find `{}` in {} with version `{}`", + dep.package_name(), + source.source_id(), + dep.version_req(), + ) + } + } + Err(_) => 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 8dcd44daf1e..96cf34fa536 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -1561,6 +1561,6 @@ fn install_yanked_cargo_package() { Package::new("baz", "0.0.1").yanked(true).publish(); cargo_process("install baz --version 0.0.1") .with_status(101) - .with_stderr("error: cannot install package `baz`, it has been yanked from registry `https://github.com/rust-lang/crates.io-index`") + .with_stderr_contains("error: cannot install package `baz`, it has been yanked from registry `https://github.com/rust-lang/crates.io-index`") .run(); } diff --git a/tests/testsuite/install_upgrade.rs b/tests/testsuite/install_upgrade.rs index 41f00087126..e46695044fd 100644 --- a/tests/testsuite/install_upgrade.rs +++ b/tests/testsuite/install_upgrade.rs @@ -799,11 +799,8 @@ fn already_installed_updates_yank_status_on_upgrade() { cargo_process("install foo --version=1.0.1") .with_status(101) - .with_stderr( - "\ -[UPDATING] `[..]` index -[ERROR] could not find `foo` in registry `[..]` with version `=1.0.1` -", + .with_stderr_contains( + "error: cannot install package `foo`, it has been yanked from registry `https://github.com/rust-lang/crates.io-index`", ) .run(); From 3952fdb223f4b7a50b26bcb887b0ade9bcad9b56 Mon Sep 17 00:00:00 2001 From: bishtpawan Date: Wed, 5 Aug 2020 17:33:48 +0530 Subject: [PATCH 6/7] Refactor code and resolve PR comments --- .../ops/common_for_install_and_uninstall.rs | 36 +++++++++---------- tests/testsuite/install.rs | 14 +++++--- tests/testsuite/install_upgrade.rs | 3 +- 3 files changed, 28 insertions(+), 25 deletions(-) diff --git a/src/cargo/ops/common_for_install_and_uninstall.rs b/src/cargo/ops/common_for_install_and_uninstall.rs index 67e8b11140e..8207e423a2a 100644 --- a/src/cargo/ops/common_for_install_and_uninstall.rs +++ b/src/cargo/ops/common_for_install_and_uninstall.rs @@ -546,29 +546,27 @@ where } None => { let version: String = dep.version_req().to_string(); - match PackageId::new(dep.package_name(), &version[1..], source.source_id()) { - Ok(pkg_id) => { - if source.is_yanked(pkg_id).unwrap_or(false) { - bail!( - "cannot install package `{}`, it has been yanked from {}", - pkg_id.name(), - pkg_id.source_id() - ) - } else { - bail!( - "could not find `{}` in {} with version `{}`", - dep.package_name(), - source.source_id(), - dep.version_req(), - ) - } - } - Err(_) => bail!( + let pkg_id; + if dep.version_req().is_exact() { + pkg_id = PackageId::new(dep.package_name(), &version[1..], source.source_id()); + } else { + pkg_id = PackageId::new(dep.package_name(), &version[..], source.source_id()); + } + let is_yanked = + pkg_id.map_or(false, |pkg_id| source.is_yanked(pkg_id).unwrap_or(false)); + if is_yanked { + bail!( + "cannot install package `{}`, it has been yanked from {}", + dep.package_name(), + source.source_id() + ) + } 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 96cf34fa536..d5fe04194a6 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -5,15 +5,16 @@ use std::io::prelude::*; use cargo_test_support::cross_compile; use cargo_test_support::git; -use cargo_test_support::install::{ - assert_has_installed_exe, assert_has_not_installed_exe, cargo_home, -}; -use cargo_test_support::paths; use cargo_test_support::registry::{registry_path, registry_url, Package}; use cargo_test_support::{ basic_manifest, cargo_process, no_such_file_err_msg, project, symlink_supported, t, }; +use cargo_test_support::install::{ + assert_has_installed_exe, assert_has_not_installed_exe, cargo_home, +}; +use cargo_test_support::paths; + fn pkg(name: &str, vers: &str) { Package::new(name, vers) .file("src/lib.rs", "") @@ -1561,6 +1562,9 @@ fn install_yanked_cargo_package() { Package::new("baz", "0.0.1").yanked(true).publish(); cargo_process("install baz --version 0.0.1") .with_status(101) - .with_stderr_contains("error: cannot install package `baz`, it has been yanked from registry `https://github.com/rust-lang/crates.io-index`") + .with_stderr_contains( + "error: cannot install package `baz`, it has been yanked from registry \ + `https://github.com/rust-lang/crates.io-index`", + ) .run(); } diff --git a/tests/testsuite/install_upgrade.rs b/tests/testsuite/install_upgrade.rs index e46695044fd..6de8277d62b 100644 --- a/tests/testsuite/install_upgrade.rs +++ b/tests/testsuite/install_upgrade.rs @@ -800,7 +800,8 @@ fn already_installed_updates_yank_status_on_upgrade() { cargo_process("install foo --version=1.0.1") .with_status(101) .with_stderr_contains( - "error: cannot install package `foo`, it has been yanked from registry `https://github.com/rust-lang/crates.io-index`", + "error: cannot install package `foo`, it has been yanked from registry \ + `https://github.com/rust-lang/crates.io-index`", ) .run(); From 81687e7926800b54aee0c4754f8ec2137c951aea Mon Sep 17 00:00:00 2001 From: bishtpawan Date: Wed, 5 Aug 2020 20:56:56 +0530 Subject: [PATCH 7/7] Resolve PR comments --- src/cargo/ops/common_for_install_and_uninstall.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/cargo/ops/common_for_install_and_uninstall.rs b/src/cargo/ops/common_for_install_and_uninstall.rs index 8207e423a2a..9f0c5dd206e 100644 --- a/src/cargo/ops/common_for_install_and_uninstall.rs +++ b/src/cargo/ops/common_for_install_and_uninstall.rs @@ -545,15 +545,13 @@ where Ok(pkg) } None => { - let version: String = dep.version_req().to_string(); - let pkg_id; - if dep.version_req().is_exact() { - pkg_id = PackageId::new(dep.package_name(), &version[1..], source.source_id()); + 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 { - pkg_id = PackageId::new(dep.package_name(), &version[..], source.source_id()); - } - let is_yanked = - pkg_id.map_or(false, |pkg_id| source.is_yanked(pkg_id).unwrap_or(false)); + false + }; if is_yanked { bail!( "cannot install package `{}`, it has been yanked from {}",