Skip to content

Commit

Permalink
fix(publish): Bail on existing version
Browse files Browse the repository at this point in the history
  • Loading branch information
stupendoussuperpowers authored and dingxiangfei2009 committed Sep 17, 2024
1 parent 8204e61 commit cd8cd33
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 49 deletions.
47 changes: 43 additions & 4 deletions src/cargo/ops/registry/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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),
Expand All @@ -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(
Expand Down Expand Up @@ -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,
Expand Down
27 changes: 26 additions & 1 deletion tests/testsuite/credential_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand All @@ -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();
}

Expand Down
44 changes: 4 additions & 40 deletions tests/testsuite/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
9 changes: 5 additions & 4 deletions tests/testsuite/registry_auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}

0 comments on commit cd8cd33

Please sign in to comment.