From e0a1918cf2c15926c343dbf2082cf675778c9f8d Mon Sep 17 00:00:00 2001 From: sanchitlegalai Date: Thu, 29 Feb 2024 15:05:36 +0530 Subject: [PATCH 1/3] test(publish): Show publishing existing version --- tests/testsuite/publish.rs | 52 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index af071ae0877..14a3425018e 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -130,6 +130,58 @@ 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() + // test registry doesn't error on duplicate versions, we need to + .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_status(101) + .with_stderr_data(str![[r#" +[UPDATING] crates.io index +[WARNING] manifest has no documentation, homepage or repository. +See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info. +[PACKAGING] foo v0.0.1 ([ROOT]/foo) +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[VERIFYING] foo v0.0.1 ([ROOT]/foo) +[WARNING] no edition set: defaulting to the 2015 edition while the latest is 2021 +[COMPILING] foo v0.0.1 ([ROOT]/foo/target/package/foo-0.0.1) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s +[UPLOADING] foo v0.0.1 ([ROOT]/foo) +[ERROR] failed to publish to registry at http://127.0.0.1:41463/ + +Caused by: + the remote server responded with an [ERROR] crate version `0.0.1` is already uploaded + +"#]]) + .run(); +} + // Check that the `token` key works at the root instead of under a // `[registry]` table. #[cargo_test] From 0a84f1fff67d8281d1b439b760acfe5625f0668b Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 6 Sep 2024 13:12:29 -0500 Subject: [PATCH 2/3] refactor(registry): Expose the registry source for reuse --- 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 | 2 +- src/cargo/ops/registry/search.rs | 2 +- src/cargo/ops/registry/yank.rs | 2 +- 6 files changed, 12 insertions(+), 14 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 93b48b6f6f6..ea49a5dfa3b 100644 --- a/src/cargo/ops/registry/mod.rs +++ b/src/cargo/ops/registry/mod.rs @@ -120,14 +120,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"); @@ -136,9 +136,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() @@ -170,11 +170,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 527b0a3fb9d..a1e7d70492d 100644 --- a/src/cargo/ops/registry/publish.rs +++ b/src/cargo/ops/registry/publish.rs @@ -115,7 +115,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { // This is only used to confirm that we can create a token before we build the package. // This causes the credential provider to be called an extra time, but keeps the same order of errors. let source_ids = super::get_source_id(opts.gctx, reg_or_index.as_ref())?; - let mut registry = super::registry( + let (mut registry, _) = super::registry( opts.gctx, &source_ids, opts.token.as_ref().map(Secret::as_deref), 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), From 6ede1e2b276eaf93618765a08bd3ec2f0f44d955 Mon Sep 17 00:00:00 2001 From: sanchitlegalai Date: Thu, 29 Feb 2024 15:13:50 +0530 Subject: [PATCH 3/3] fix(publish): Bail on existing version --- src/cargo/ops/registry/publish.rs | 47 ++++++++++++++++++++++++--- tests/testsuite/credential_process.rs | 27 ++++++++++++++- tests/testsuite/publish.rs | 44 +++---------------------- tests/testsuite/registry_auth.rs | 9 ++--- 4 files changed, 78 insertions(+), 49 deletions(-) diff --git a/src/cargo/ops/registry/publish.rs b/src/cargo/ops/registry/publish.rs index a1e7d70492d..8ff92920c16 100644 --- a/src/cargo/ops/registry/publish.rs +++ b/src/cargo/ops/registry/publish.rs @@ -31,11 +31,13 @@ use crate::core::PackageIdSpecQuery; use crate::core::SourceId; use crate::core::Workspace; use crate::ops; +use crate::ops::registry::RegistrySourceIds; use crate::ops::PackageOpts; use crate::ops::Packages; use crate::ops::RegistryOrIndex; use crate::sources::source::QueryKind; use crate::sources::source::Source; +use crate::sources::RegistrySource; use crate::sources::SourceConfigMap; use crate::sources::CRATES_IO_REGISTRY; use crate::util::auth; @@ -45,6 +47,7 @@ use crate::util::toml::prepare_for_publish; use crate::util::Graph; use crate::util::Progress; use crate::util::ProgressStyle; +use crate::util::VersionExt as _; use crate::CargoResult; use crate::GlobalContext; @@ -115,7 +118,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { // This is only used to confirm that we can create a token before we build the package. // This causes the credential provider to be called an extra time, but keeps the same order of errors. 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), @@ -124,9 +127,15 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { Some(Operation::Read).filter(|_| !opts.dry_run), )?; - // Validate all the packages before publishing any of them. - for (pkg, _) in &pkgs { - verify_dependencies(pkg, ®istry, source_ids.original)?; + { + let _lock = opts + .gctx + .acquire_package_cache_lock(CacheLockMode::DownloadExclusive)?; + + for (pkg, _) in &pkgs { + verify_unpublished(pkg, &mut source, &source_ids)?; + verify_dependencies(pkg, ®istry, source_ids.original)?; + } } let pkg_dep_graph = ops::cargo_package::package_with_dep_graph( @@ -355,6 +364,36 @@ fn poll_one_package( Ok(!summaries.is_empty()) } +fn verify_unpublished( + pkg: &Package, + source: &mut RegistrySource<'_>, + source_ids: &RegistrySourceIds, +) -> CargoResult<()> { + let query = Dependency::parse( + pkg.name(), + Some(&pkg.version().to_exact_req().to_string()), + source_ids.replacement, + )?; + 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()?, + } + }; + if !duplicate_query.is_empty() { + bail!( + "crate {}@{} already exists on {}", + pkg.name(), + pkg.version(), + source.describe() + ); + } + + Ok(()) +} + fn verify_dependencies( pkg: &Package, registry: &Registry, diff --git a/tests/testsuite/credential_process.rs b/tests/testsuite/credential_process.rs index bba657fe8c5..391cdb33b26 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) +[UPLOADING] foo v0.1.1 ([ROOT]/foo) +{"v":1,"registry":{"index-url":"[..]","name":"alternative"},"kind":"get","operation":"publish","name":"foo","vers":"0.1.1","cksum":"[..]"} +[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 14a3425018e..01a4cd0a987 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -132,17 +132,8 @@ You may press ctrl-c to skip waiting; the crate should be available shortly. #[cargo_test] fn duplicate_version() { - let registry_dupl = RegistryBuilder::new() - .http_api() - .http_index() - // test registry doesn't error on duplicate versions, we need to - .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 registry_dupl = RegistryBuilder::new().http_api().http_index().build(); + Package::new("foo", "0.0.1").publish(); let p = project() .file( @@ -164,19 +155,7 @@ fn duplicate_version() { .with_status(101) .with_stderr_data(str![[r#" [UPDATING] crates.io index -[WARNING] manifest has no documentation, homepage or repository. -See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info. -[PACKAGING] foo v0.0.1 ([ROOT]/foo) -[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) -[VERIFYING] foo v0.0.1 ([ROOT]/foo) -[WARNING] no edition set: defaulting to the 2015 edition while the latest is 2021 -[COMPILING] foo v0.0.1 ([ROOT]/foo/target/package/foo-0.0.1) -[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s -[UPLOADING] foo v0.0.1 ([ROOT]/foo) -[ERROR] failed to publish to registry at http://127.0.0.1:41463/ - -Caused by: - the remote server responded with an [ERROR] crate version `0.0.1` is already uploaded +[ERROR] crate foo@0.0.1 already exists on crates.io index "#]]) .run(); @@ -3900,22 +3879,7 @@ You may press ctrl-c to skip waiting; the crate should be available shortly. .with_status(101) .with_stderr_data(str![[r#" [UPDATING] crates.io index -[PACKAGING] a v0.0.1 ([ROOT]/foo/a) -[PACKAGED] 3 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) -[PACKAGING] b v0.0.1 ([ROOT]/foo/b) -[PACKAGED] 3 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) -[VERIFYING] a v0.0.1 ([ROOT]/foo/a) -[COMPILING] a v0.0.1 ([ROOT]/foo/target/package/a-0.0.1) -[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s -[VERIFYING] b v0.0.1 ([ROOT]/foo/b) -[UPDATING] crates.io index -[ERROR] failed to verify package tarball - -Caused by: - failed to get `a` as a dependency of package `b v0.0.1 ([ROOT]/foo/target/package/b-0.0.1)` - -Caused by: - found a package in the remote registry and the local overlay: a@0.0.1 +[ERROR] crate a@0.0.1 already exists on crates.io index "#]]) .run(); diff --git a/tests/testsuite/registry_auth.rs b/tests/testsuite/registry_auth.rs index d73d041b507..707761cc43a 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")); }