From bf4a71b9be05dcbb5d700ca82f80108d51a929db Mon Sep 17 00:00:00 2001 From: sanchitlegalai Date: Thu, 29 Feb 2024 15:05:36 +0530 Subject: [PATCH 1/4] add test | publishing existing version --- tests/testsuite/publish.rs | 51 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index b6a29741a4b..bd252b17341 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -130,6 +130,57 @@ You may press ctrl-c to skip waiting; the crate should be available shortly. validate_upload_foo(); } +#[cargo_test] +fn duplicate_version() { + let registry_dupl = RegistryBuilder::new() + .http_api() + .http_index() + .add_responder("/api/v1/crates/new", move |_req, _server| Response { + code: 200, + headers: vec![], + body: br#"{"errors": [{"detail": "crate version `0.0.1` is already uploaded"}]}"# + .to_vec(), + }) + .build(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + license = "MIT" + description = "foo" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + p.cargo("publish") + .replace_crates_io(registry_dupl.index_url()) + .with_stderr( + "\ +[UPDATING] crates.io index +[WARNING] [..] +See [..] +[PACKAGING] foo v0.0.1 ([CWD]) +[VERIFYING] foo v0.0.1 ([CWD]) +[..] +[..] +[..] +[UPLOADING] foo v0.0.1 ([CWD]) +error: failed to publish [..] + +Caused by: +[..] is already uploaded +", + ) + .with_status(101) + .run(); +} + // Check that the `token` key works at the root instead of under a // `[registry]` table. #[cargo_test] From e17f2b1ba7ceb9c0ef1a2d600e0bf686e70dc5f6 Mon Sep 17 00:00:00 2001 From: sanchitlegalai Date: Thu, 29 Feb 2024 15:13:50 +0530 Subject: [PATCH 2/4] bail on existing version | modify test --- src/cargo/ops/registry/publish.rs | 29 ++++++++++++++++++++++++++++ tests/testsuite/publish.rs | 32 +++++++++++++++---------------- 2 files changed, 44 insertions(+), 17 deletions(-) diff --git a/src/cargo/ops/registry/publish.rs b/src/cargo/ops/registry/publish.rs index 7fd3acfbec7..727deb78019 100644 --- a/src/cargo/ops/registry/publish.rs +++ b/src/cargo/ops/registry/publish.rs @@ -139,6 +139,35 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { )?; verify_dependencies(pkg, ®istry, source_ids.original)?; + // Bail before packaging and uploading if same version already exists in the registry + + let mut source = SourceConfigMap::empty(opts.gctx)?.load(reg_ids.original, &HashSet::new())?; + + let query = Dependency::parse(pkg.name(), Some(&ver), reg_ids.original)?; + + let _lock = opts + .gctx + .acquire_package_cache_lock(CacheLockMode::DownloadExclusive)?; + + let duplicate_query = loop { + match source.query_vec(&query, QueryKind::Exact) { + std::task::Poll::Ready(res) => { + break res?; + } + std::task::Poll::Pending => source.block_until_ready()?, + } + }; + + drop(_lock); + + if !duplicate_query.is_empty() { + bail!( + "crate {} already has version {}. Aborting publish.", + pkg.name(), + pkg.version() + ); + } + // Prepare a tarball, with a non-suppressible warning if metadata // is missing since this is being put online. let tarball = ops::package_one( diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index bd252b17341..33adabf8982 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -132,14 +132,18 @@ You may press ctrl-c to skip waiting; the crate should be available shortly. #[cargo_test] fn duplicate_version() { + let arc: Arc> = Arc::new(Mutex::new(0)); let registry_dupl = RegistryBuilder::new() .http_api() .http_index() - .add_responder("/api/v1/crates/new", move |_req, _server| Response { - code: 200, - headers: vec![], - body: br#"{"errors": [{"detail": "crate version `0.0.1` is already uploaded"}]}"# - .to_vec(), + .add_responder("/index/3/f/foo", move |req, server| { + let mut lock = arc.lock().unwrap(); + *lock += 1; + if *lock <= 1 { + server.not_found(req) + } else { + server.index(req) + } }) .build(); @@ -158,23 +162,17 @@ fn duplicate_version() { .file("src/main.rs", "fn main() {}") .build(); + p.cargo("publish") + .replace_crates_io(registry_dupl.index_url()) + .without_status() + .run(); + p.cargo("publish") .replace_crates_io(registry_dupl.index_url()) .with_stderr( "\ [UPDATING] crates.io index -[WARNING] [..] -See [..] -[PACKAGING] foo v0.0.1 ([CWD]) -[VERIFYING] foo v0.0.1 ([CWD]) -[..] -[..] -[..] -[UPLOADING] foo v0.0.1 ([CWD]) -error: failed to publish [..] - -Caused by: -[..] is already uploaded +error: crate foo already has version 0.0.1. Aborting publish. ", ) .with_status(101) From d24d542ab77e68ca66754f81200696f22e225d55 Mon Sep 17 00:00:00 2001 From: Tor Hovland <55164+torhovland@users.noreply.github.com> Date: Thu, 1 Aug 2024 13:07:02 +0200 Subject: [PATCH 3/4] Fix duplicate updating messages when using alt registries by reusing the RegistrySource. --- src/cargo/ops/registry/login.rs | 2 +- src/cargo/ops/registry/mod.rs | 16 +++++++--------- src/cargo/ops/registry/owner.rs | 2 +- src/cargo/ops/registry/publish.rs | 7 +++---- src/cargo/ops/registry/search.rs | 2 +- src/cargo/ops/registry/yank.rs | 2 +- tests/testsuite/publish.rs | 9 ++++----- 7 files changed, 18 insertions(+), 22 deletions(-) diff --git a/src/cargo/ops/registry/login.rs b/src/cargo/ops/registry/login.rs index 4087317bb28..34e78e1edc3 100644 --- a/src/cargo/ops/registry/login.rs +++ b/src/cargo/ops/registry/login.rs @@ -34,7 +34,7 @@ pub fn registry_login( false, None, ) { - Ok(registry) => Some(format!("{}/me", registry.host())), + Ok((registry, _)) => Some(format!("{}/me", registry.host())), Err(e) if e.is::() => e .downcast::() .unwrap() diff --git a/src/cargo/ops/registry/mod.rs b/src/cargo/ops/registry/mod.rs index f45947566b7..3aeecaeb8ee 100644 --- a/src/cargo/ops/registry/mod.rs +++ b/src/cargo/ops/registry/mod.rs @@ -118,14 +118,14 @@ impl RegistryCredentialConfig { /// `registry`, or `index` are set, then uses `crates-io`. /// * `force_update`: If `true`, forces the index to be updated. /// * `token_required`: If `true`, the token will be set. -fn registry( - gctx: &GlobalContext, +fn registry<'gctx>( + gctx: &'gctx GlobalContext, source_ids: &RegistrySourceIds, token_from_cmdline: Option>, reg_or_index: Option<&RegistryOrIndex>, force_update: bool, token_required: Option>, -) -> CargoResult { +) -> CargoResult<(Registry, RegistrySource<'gctx>)> { let is_index = reg_or_index.map(|v| v.is_index()).unwrap_or_default(); if is_index && token_required.is_some() && token_from_cmdline.is_none() { bail!("command-line argument --index requires --token to be specified"); @@ -134,9 +134,9 @@ fn registry( auth::cache_token_from_commandline(gctx, &source_ids.original, token); } + let mut src = RegistrySource::remote(source_ids.replacement, &HashSet::new(), gctx)?; let cfg = { let _lock = gctx.acquire_package_cache_lock(CacheLockMode::DownloadExclusive)?; - let mut src = RegistrySource::remote(source_ids.replacement, &HashSet::new(), gctx)?; // Only update the index if `force_update` is set. if force_update { src.invalidate_cache() @@ -168,11 +168,9 @@ fn registry( None }; let handle = http_handle(gctx)?; - Ok(Registry::new_handle( - api_host, - token, - handle, - cfg.auth_required, + Ok(( + Registry::new_handle(api_host, token, handle, cfg.auth_required), + src, )) } diff --git a/src/cargo/ops/registry/owner.rs b/src/cargo/ops/registry/owner.rs index a5ff86594c3..7c8246fdfbf 100644 --- a/src/cargo/ops/registry/owner.rs +++ b/src/cargo/ops/registry/owner.rs @@ -36,7 +36,7 @@ pub fn modify_owners(gctx: &GlobalContext, opts: &OwnersOptions) -> CargoResult< let operation = Operation::Owners { name: &name }; let source_ids = super::get_source_id(gctx, opts.reg_or_index.as_ref())?; - let mut registry = super::registry( + let (mut registry, _) = super::registry( gctx, &source_ids, opts.token.as_ref().map(Secret::as_deref), diff --git a/src/cargo/ops/registry/publish.rs b/src/cargo/ops/registry/publish.rs index 727deb78019..ca7ea1bf82f 100644 --- a/src/cargo/ops/registry/publish.rs +++ b/src/cargo/ops/registry/publish.rs @@ -28,6 +28,7 @@ use crate::ops; use crate::ops::PackageOpts; use crate::ops::Packages; use crate::sources::source::QueryKind; +use crate::sources::source::Source; use crate::sources::SourceConfigMap; use crate::sources::CRATES_IO_REGISTRY; use crate::util::auth; @@ -129,7 +130,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { val => val, }; let source_ids = super::get_source_id(opts.gctx, reg_or_index.as_ref())?; - let mut registry = super::registry( + let (mut registry, mut source) = super::registry( opts.gctx, &source_ids, opts.token.as_ref().map(Secret::as_deref), @@ -141,9 +142,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { // Bail before packaging and uploading if same version already exists in the registry - let mut source = SourceConfigMap::empty(opts.gctx)?.load(reg_ids.original, &HashSet::new())?; - - let query = Dependency::parse(pkg.name(), Some(&ver), reg_ids.original)?; + let query = Dependency::parse(pkg.name(), Some(&ver), source_ids.replacement)?; let _lock = opts .gctx diff --git a/src/cargo/ops/registry/search.rs b/src/cargo/ops/registry/search.rs index 41e12e36e46..652730f1fad 100644 --- a/src/cargo/ops/registry/search.rs +++ b/src/cargo/ops/registry/search.rs @@ -21,7 +21,7 @@ pub fn search( limit: u32, ) -> CargoResult<()> { let source_ids = super::get_source_id(gctx, reg_or_index.as_ref())?; - let mut registry = + let (mut registry, _) = super::registry(gctx, &source_ids, None, reg_or_index.as_ref(), false, None)?; let (crates, total_crates) = registry.search(query, limit).with_context(|| { format!( diff --git a/src/cargo/ops/registry/yank.rs b/src/cargo/ops/registry/yank.rs index 523c21bf84f..f46b9332f6b 100644 --- a/src/cargo/ops/registry/yank.rs +++ b/src/cargo/ops/registry/yank.rs @@ -47,7 +47,7 @@ pub fn yank( } }; let source_ids = super::get_source_id(gctx, reg_or_index.as_ref())?; - let mut registry = super::registry( + let (mut registry, _) = super::registry( gctx, &source_ids, token.as_ref().map(Secret::as_deref), diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index 33adabf8982..cce66f35fb0 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -169,12 +169,11 @@ fn duplicate_version() { p.cargo("publish") .replace_crates_io(registry_dupl.index_url()) - .with_stderr( - "\ + .with_stderr_data(str![[r#" [UPDATING] crates.io index -error: crate foo already has version 0.0.1. Aborting publish. -", - ) +[ERROR] crate foo already has version 0.0.1. Aborting publish. + +"#]]) .with_status(101) .run(); } From b5be434fdf7499238bf54596e127c3135b19e8f7 Mon Sep 17 00:00:00 2001 From: stupendoussuperpowers Date: Wed, 7 Aug 2024 12:09:34 +0530 Subject: [PATCH 4/4] fix broken tests - token_caching token_not_logged --- src/cargo/ops/registry/publish.rs | 5 +++-- tests/testsuite/credential_process.rs | 27 ++++++++++++++++++++++++++- tests/testsuite/publish.rs | 17 ++--------------- tests/testsuite/registry_auth.rs | 9 +++++---- 4 files changed, 36 insertions(+), 22 deletions(-) diff --git a/src/cargo/ops/registry/publish.rs b/src/cargo/ops/registry/publish.rs index ca7ea1bf82f..61473a0fa66 100644 --- a/src/cargo/ops/registry/publish.rs +++ b/src/cargo/ops/registry/publish.rs @@ -161,9 +161,10 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { if !duplicate_query.is_empty() { bail!( - "crate {} already has version {}. Aborting publish.", + "crate {}@{} already exists on {}", pkg.name(), - pkg.version() + pkg.version(), + source.describe() ); } diff --git a/tests/testsuite/credential_process.rs b/tests/testsuite/credential_process.rs index a07006d549b..ef637a87d8f 100644 --- a/tests/testsuite/credential_process.rs +++ b/tests/testsuite/credential_process.rs @@ -544,6 +544,31 @@ You may press ctrl-c [..] .with_stderr_data(output) .run(); + let output_non_independent = r#"[UPDATING] `alternative` index +{"v":1,"registry":{"index-url":"[..]","name":"alternative"},"kind":"get","operation":"read"} +[PACKAGING] foo v0.1.1 ([ROOT]/foo) +[PACKAGED] 3 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +{"v":1,"registry":{"index-url":"[..]","name":"alternative"},"kind":"get","operation":"publish","name":"foo","vers":"0.1.1","cksum":"[..]"} +[UPLOADING] foo v0.1.1 ([ROOT]/foo) +[UPLOADED] foo v0.1.1 to registry `alternative` +[NOTE] waiting [..] +You may press ctrl-c [..] +[PUBLISHED] foo v0.1.1 at registry `alternative` +"#; + + p.change_file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.1" + edition = "2015" + description = "foo" + license = "MIT" + homepage = "https://example.com/" + "#, + ); + p.change_file( ".cargo/config.toml", &format!( @@ -557,7 +582,7 @@ You may press ctrl-c [..] ); p.cargo("publish --registry alternative --no-verify") - .with_stderr_data(output) + .with_stderr_data(output_non_independent) .run(); } diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index cce66f35fb0..f9fdd3f6354 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -132,20 +132,7 @@ You may press ctrl-c to skip waiting; the crate should be available shortly. #[cargo_test] fn duplicate_version() { - let arc: Arc> = Arc::new(Mutex::new(0)); - let registry_dupl = RegistryBuilder::new() - .http_api() - .http_index() - .add_responder("/index/3/f/foo", move |req, server| { - let mut lock = arc.lock().unwrap(); - *lock += 1; - if *lock <= 1 { - server.not_found(req) - } else { - server.index(req) - } - }) - .build(); + let registry_dupl = RegistryBuilder::new().http_api().http_index().build(); let p = project() .file( @@ -171,7 +158,7 @@ fn duplicate_version() { .replace_crates_io(registry_dupl.index_url()) .with_stderr_data(str![[r#" [UPDATING] crates.io index -[ERROR] crate foo already has version 0.0.1. Aborting publish. +[ERROR] crate foo@0.0.1 already exists on [..] "#]]) .with_status(101) diff --git a/tests/testsuite/registry_auth.rs b/tests/testsuite/registry_auth.rs index 49ff7efa2c9..158d3fda11f 100644 --- a/tests/testsuite/registry_auth.rs +++ b/tests/testsuite/registry_auth.rs @@ -565,9 +565,10 @@ fn token_not_logged() { // 2. config.json again for verification // 3. /index/3/b/bar // 4. /dl/bar/1.0.0/download - // 5. /api/v1/crates/new - // 6. config.json for the "wait for publish" - // 7. /index/3/f/foo for the "wait for publish" - assert_eq!(authorizations.len(), 7); + // 5. /index/3/f/foo for checking duplicate version + // 6. /api/v1/crates/new + // 7. config.json for the "wait for publish" + // 8. /index/3/f/foo for the "wait for publish" + assert_eq!(authorizations.len(), 8); assert!(!log.contains("a-unique_token")); }