diff --git a/src/cargo/ops/registry/publish.rs b/src/cargo/ops/registry/publish.rs index d19da9f612c..6e181100b1a 100644 --- a/src/cargo/ops/registry/publish.rs +++ b/src/cargo/ops/registry/publish.rs @@ -28,6 +28,7 @@ use crate::core::Package; use crate::core::PackageId; use crate::core::PackageIdSpecQuery; use crate::core::SourceId; +use crate::core::Summary; use crate::core::Workspace; use crate::core::dependency::DepKind; use crate::core::manifest::ManifestMetadata; @@ -85,15 +86,17 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { .into_iter() .partition(|(pkg, _)| pkg.publish() == &Some(vec![])); // If `--workspace` is passed, - // the intent is more like "publish all publisable packages in this workspace", - // so skip `publish=false` packages. - let allow_unpublishable = match &opts.to_publish { + // the intent is more like "publish all publisable packages in this workspace". + // Hence, + // * skip `publish=false` packages + // * skip already published packages + let is_workspace_publish = match &opts.to_publish { Packages::Default => ws.is_virtual(), Packages::All(_) => true, Packages::OptOut(_) => true, Packages::Packages(_) => false, }; - if !unpublishable.is_empty() && !allow_unpublishable { + if !unpublishable.is_empty() && !is_workspace_publish { bail!( "{} cannot be published.\n\ `package.publish` must be set to `true` or a non-empty list in Cargo.toml to publish.", @@ -105,7 +108,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { } if pkgs.is_empty() { - if allow_unpublishable { + if is_workspace_publish { let n = unpublishable.len(); let plural = if n == 1 { "" } else { "s" }; ws.gctx().shell().warn(format_args!( @@ -154,13 +157,30 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { Some(Operation::Read).filter(|_| !opts.dry_run), )?; + // `maybe_published` tracks package versions that already exist in the registry, + // meaning they might have been published before. + // Later, we verify the tarball checksum to see + // if the local package matches the registry. + // This helps catch cases where the local version + // wasn’t bumped but files changed. + let mut maybe_published = HashMap::new(); + { let _lock = opts .gctx .acquire_package_cache_lock(CacheLockMode::DownloadExclusive)?; for (pkg, _) in &pkgs { - verify_unpublished(pkg, &mut source, &source_ids, opts.dry_run, opts.gctx)?; + if let Some(summary) = verify_unpublished( + pkg, + &mut source, + &source_ids, + opts.dry_run, + is_workspace_publish, + opts.gctx, + )? { + maybe_published.insert(pkg.package_id(), summary); + } verify_dependencies(pkg, ®istry, source_ids.original).map_err(|err| { ManifestError::new( err.context(format!( @@ -213,15 +233,38 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { let mut ready = plan.take_ready(); while let Some(pkg_id) = ready.pop_first() { let (pkg, (_features, tarball)) = &pkg_dep_graph.packages[&pkg_id]; - opts.gctx.shell().status("Uploading", pkg.package_id())?; - - if !opts.dry_run { - let ver = pkg.version().to_string(); + if opts.dry_run { + opts.gctx.shell().status("Uploading", pkg.package_id())?; + } else { tarball.file().seek(SeekFrom::Start(0))?; let hash = cargo_util::Sha256::new() .update_file(tarball.file())? .finish_hex(); + + if let Some(summary) = maybe_published.get(&pkg.package_id()) { + if summary.checksum() == Some(hash.as_str()) { + opts.gctx.shell().warn(format_args!( + "skipping upload for crate {}@{}: already exists on {}", + pkg.name(), + pkg.version(), + source.describe() + ))?; + plan.mark_confirmed([pkg.package_id()]); + continue; + } + bail!( + "crate {}@{} already exists on {} but tarball checksum mismatched\n\ + perhaps local files have changed but forgot to bump the version?", + pkg.name(), + pkg.version(), + source.describe() + ); + } + + opts.gctx.shell().status("Uploading", pkg.package_id())?; + + let ver = pkg.version().to_string(); let operation = Operation::Publish { name: pkg.name().as_str(), vers: &ver, @@ -273,6 +316,12 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { } } + if to_confirm.is_empty() { + // nothing to confirm because some are already uploaded before + // this cargo invocation. + continue; + } + let confirmed = if opts.dry_run { to_confirm.clone() } else { @@ -440,13 +489,18 @@ fn poll_one_package( Ok(!summaries.is_empty()) } +/// Checks if a package is already published. +/// +/// Returns a [`Summary`] for computing the tarball checksum +/// to compare with the registry index later, if needed. fn verify_unpublished( pkg: &Package, source: &mut RegistrySource<'_>, source_ids: &RegistrySourceIds, dry_run: bool, + skip_already_publish: bool, gctx: &GlobalContext, -) -> CargoResult<()> { +) -> CargoResult> { let query = Dependency::parse( pkg.name(), Some(&pkg.version().to_exact_req().to_string()), @@ -460,28 +514,36 @@ fn verify_unpublished( std::task::Poll::Pending => source.block_until_ready()?, } }; - if !duplicate_query.is_empty() { - // Move the registry error earlier in the publish process. - // Since dry-run wouldn't talk to the registry to get the error, we downgrade it to a - // warning. - if dry_run { - gctx.shell().warn(format!( - "crate {}@{} already exists on {}", - pkg.name(), - pkg.version(), - source.describe() - ))?; - } else { - bail!( - "crate {}@{} already exists on {}", - pkg.name(), - pkg.version(), - source.describe() - ); - } + if duplicate_query.is_empty() { + return Ok(None); } - Ok(()) + // Move the registry error earlier in the publish process. + // Since dry-run wouldn't talk to the registry to get the error, + // we downgrade it to a warning. + if skip_already_publish || dry_run { + gctx.shell().warn(format!( + "crate {}@{} already exists on {}", + pkg.name(), + pkg.version(), + source.describe() + ))?; + } else { + bail!( + "crate {}@{} already exists on {}", + pkg.name(), + pkg.version(), + source.describe() + ); + } + + assert_eq!( + duplicate_query.len(), + 1, + "registry must not have duplicat versions", + ); + let summary = duplicate_query.into_iter().next().unwrap().into_summary(); + Ok(skip_already_publish.then_some(summary)) } fn verify_dependencies( diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index 846b0689328..5261cea818d 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -3954,10 +3954,28 @@ Caused by: // Publishing the whole workspace now will fail, as `a` is already published. p.cargo("publish") .replace_crates_io(registry.index_url()) - .with_status(101) .with_stderr_data(str![[r#" [UPDATING] crates.io index -[ERROR] crate a@0.0.1 already exists on crates.io index +[WARNING] crate a@0.0.1 already exists on crates.io index +[PACKAGING] a v0.0.1 ([ROOT]/foo/a) +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[PACKAGING] b v0.0.1 ([ROOT]/foo/b) +[UPDATING] crates.io index +[PACKAGED] 4 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) +[UNPACKING] a v0.0.1 (registry `[ROOT]/foo/target/package/tmp-registry`) +[COMPILING] a v0.0.1 +[COMPILING] b v0.0.1 ([ROOT]/foo/target/package/b-0.0.1) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s +[WARNING] skipping upload for crate a@0.0.1: already exists on crates.io index +[UPLOADING] b v0.0.1 ([ROOT]/foo/b) +[UPLOADED] b v0.0.1 to registry `crates-io` +[NOTE] waiting for b v0.0.1 to be available at registry `crates-io` +[HELP] you may press ctrl-c to skip waiting; the crate should be available shortly +[PUBLISHED] b v0.0.1 at registry `crates-io` "#]]) .run(); @@ -4108,7 +4126,7 @@ fn virtual_ws_with_multiple_unpublishable_package() { } #[cargo_test] -fn workspace_flag_with_unpublishable_packages() { +fn workspace_flag_with_nonpublishable_packages() { let registry = RegistryBuilder::new().http_api().http_index().build(); let p = project() @@ -4328,6 +4346,120 @@ fn all_unpublishable_packages() { .run(); } +#[cargo_test] +fn all_published_packages() { + let registry = RegistryBuilder::new().http_api().http_index().build(); + + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["foo", "bar"] + "#, + ) + .file( + "foo/Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.0" + edition = "2015" + license = "MIT" + description = "foo" + repository = "foo" + "#, + ) + .file("foo/src/lib.rs", "") + .file( + "bar/Cargo.toml", + r#" + [package] + name = "bar" + version = "0.0.0" + edition = "2015" + license = "MIT" + description = "foo" + repository = "foo" + "#, + ) + .file("bar/src/lib.rs", "") + .build(); + + // First, publish all members + p.cargo("publish --workspace --no-verify") + .replace_crates_io(registry.index_url()) + .with_stderr_data(str![[r#" +[UPDATING] crates.io index +[PACKAGING] bar v0.0.0 ([ROOT]/foo/bar) +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[PACKAGING] foo v0.0.0 ([ROOT]/foo/foo) +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[UPLOADING] bar v0.0.0 ([ROOT]/foo/bar) +[UPLOADED] bar v0.0.0 to registry `crates-io` +[UPLOADING] foo v0.0.0 ([ROOT]/foo/foo) +[UPLOADED] foo v0.0.0 to registry `crates-io` +[NOTE] waiting for bar v0.0.0 or foo v0.0.0 to be available at registry `crates-io` +[HELP] you may press ctrl-c to skip waiting; the crates should be available shortly +[PUBLISHED] bar v0.0.0 and foo v0.0.0 at registry `crates-io` + +"#]]) + .run(); + + // Publishing all members again works + p.cargo("publish --workspace --no-verify") + .replace_crates_io(registry.index_url()) + .with_stderr_data(str![[r#" +[UPDATING] crates.io index +[WARNING] crate foo@0.0.0 already exists on crates.io index +[WARNING] crate bar@0.0.0 already exists on crates.io index +[PACKAGING] bar v0.0.0 ([ROOT]/foo/bar) +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[PACKAGING] foo v0.0.0 ([ROOT]/foo/foo) +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[WARNING] skipping upload for crate bar@0.0.0: already exists on crates.io index +[WARNING] skipping upload for crate foo@0.0.0: already exists on crates.io index + +"#]]) + .run(); + + // Without `--workspace` works as it is a virtual workspace + p.cargo("publish --no-verify") + .replace_crates_io(registry.index_url()) + .with_stderr_data(str![[r#" +[UPDATING] crates.io index +[WARNING] crate foo@0.0.0 already exists on crates.io index +[WARNING] crate bar@0.0.0 already exists on crates.io index +[PACKAGING] bar v0.0.0 ([ROOT]/foo/bar) +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[PACKAGING] foo v0.0.0 ([ROOT]/foo/foo) +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[WARNING] skipping upload for crate bar@0.0.0: already exists on crates.io index +[WARNING] skipping upload for crate foo@0.0.0: already exists on crates.io index + +"#]]) + .run(); + + // Change a file. It should fail due to checksum verification failure. + p.change_file("bar/src/lib.rs", "//! foo"); + p.cargo("publish --no-verify") + .replace_crates_io(registry.index_url()) + .with_status(101) + .with_stderr_data(str![[r#" +[UPDATING] crates.io index +[WARNING] crate foo@0.0.0 already exists on crates.io index +[WARNING] crate bar@0.0.0 already exists on crates.io index +[PACKAGING] bar v0.0.0 ([ROOT]/foo/bar) +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[PACKAGING] foo v0.0.0 ([ROOT]/foo/foo) +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[ERROR] crate bar@0.0.0 already exists on crates.io index but tarball checksum mismatched +perhaps local files have changed but forgot to bump the version? + +"#]]) + .run(); +} + #[cargo_test] fn checksum_changed() { let registry = RegistryBuilder::new().http_api().http_index().build();