fix(publish): Bail on existing version

Author: stupendoussuperpowers

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, &registry, 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, &registry, 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,

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();
 }
 

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 [email protected] 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: [email protected]
+[ERROR] crate [email protected] already exists on crates.io index
 
 "#]])
         .run();

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"));
 }