Skip to content

Commit

Permalink
Fix duplicate updating messages when using alt registries by reusing …
Browse files Browse the repository at this point in the history
…the RegistrySource.
  • Loading branch information
torhovland committed Aug 1, 2024
1 parent ba03f16 commit a6aca5f
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 22 deletions.
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 @@ -118,14 +118,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 @@ -134,9 +134,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 @@ -168,11 +168,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
7 changes: 3 additions & 4 deletions src/cargo/ops/registry/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use crate::ops;
use crate::ops::PackageOpts;
use crate::ops::Packages;
use crate::sources::source::QueryKind;
use crate::sources::source::Source;
use crate::sources::SourceConfigMap;
use crate::sources::CRATES_IO_REGISTRY;
use crate::util::auth;
Expand Down Expand Up @@ -129,7 +130,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> {
val => val,
};
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 @@ -141,9 +142,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> {

// Bail before packaging and uploading if same version already exists in the registry

let mut source = SourceConfigMap::empty(opts.gctx)?.load(reg_ids.original, &HashSet::new())?;

let query = Dependency::parse(pkg.name(), Some(&ver), reg_ids.original)?;
let query = Dependency::parse(pkg.name(), Some(&ver), source_ids.replacement)?;

let _lock = opts
.gctx
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
9 changes: 4 additions & 5 deletions tests/testsuite/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,12 +169,11 @@ fn duplicate_version() {

p.cargo("publish")
.replace_crates_io(registry_dupl.index_url())
.with_stderr(
"\
.with_stderr_data(str![[r#"
[UPDATING] crates.io index
error: crate foo already has version 0.0.1. Aborting publish.
",
)
[ERROR] crate foo already has version 0.0.1. Aborting publish.
"#]])
.with_status(101)
.run();
}
Expand Down

0 comments on commit a6aca5f

Please sign in to comment.