Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bail before packaging on same version #14448

Merged
merged 3 commits into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/cargo/ops/registry/login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<AuthorizationError>() => e
.downcast::<AuthorizationError>()
.unwrap()
Expand Down
16 changes: 7 additions & 9 deletions src/cargo/ops/registry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Secret<&str>>,
reg_or_index: Option<&RegistryOrIndex>,
force_update: bool,
token_required: Option<Operation<'_>>,
) -> CargoResult<Registry> {
) -> 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");
Expand All @@ -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()
Expand Down Expand Up @@ -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,
))
}

Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/registry/owner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
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
2 changes: 1 addition & 1 deletion src/cargo/ops/registry/search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/registry/yank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
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
48 changes: 32 additions & 16 deletions tests/testsuite/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,37 @@ 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().build();
Package::new("foo", "0.0.1").publish();

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
[ERROR] crate foo@0.0.1 already exists on crates.io index

"#]])
.run();
}

// Check that the `token` key works at the root instead of under a
// `[registry]` table.
#[cargo_test]
Expand Down Expand Up @@ -3848,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"));
}