From 4fa9af7217a007cc4c502689504aa1b8ed390527 Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Fri, 24 Mar 2023 16:03:44 -0700 Subject: [PATCH 01/12] Initial WIP. TODO: * Only warn if there isn't an explicit registry defined in the dependency definition. * Roll multiple other sources for the same package into one warning, instead of having a line per source. * Write tests. --- src/cargo/core/package.rs | 79 +++++++++++++++++++++++++++++++ src/cargo/core/source/mod.rs | 17 +++++++ src/cargo/ops/resolve.rs | 10 ++++ src/cargo/sources/directory.rs | 4 ++ src/cargo/sources/git/source.rs | 7 +++ src/cargo/sources/path.rs | 8 ++++ src/cargo/sources/registry/mod.rs | 12 ++++- src/cargo/sources/replaced.rs | 4 ++ 8 files changed, 140 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index 952b7cba26d..cd8796a3f9a 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -6,6 +6,7 @@ use std::hash; use std::mem; use std::path::{Path, PathBuf}; use std::rc::Rc; +use std::task::Poll; use std::time::{Duration, Instant}; use anyhow::Context; @@ -597,6 +598,84 @@ impl<'cfg> PackageSet<'cfg> { } } } + + Ok(()) + } + + pub(crate) fn warn_deps_defined_in_multiple_registries( + &self, + ws: &Workspace<'cfg>, + resolve: &Resolve, + root_ids: &[PackageId], + has_dev_units: HasDevUnits, + requested_kinds: &[CompileKind], + target_data: &RustcTargetData<'_>, + force_all_targets: ForceAllTargets, + ) -> CargoResult<()> { + let _lock = self.config.acquire_package_cache_lock()?; + + let mut results = Vec::new(); + + let mut sources = self.sources_mut(); + let mut pending: Vec<(PackageId, SourceId)> = root_ids + .iter() + .flat_map(|root_id| { + PackageSet::filter_deps( + *root_id, + resolve, + has_dev_units, + requested_kinds, + target_data, + force_all_targets, + ) + .map(|(pid, _)| { + sources.sources().filter_map(move |(sid, _)| { + if sid != &pid.source_id() { + Some((pid, *sid)) + } else { + None + } + }) + }) + }) + .flatten() + .collect(); + + while !pending.is_empty() { + // dbg!(&pending); + + pending.retain(|(pid, sid)| { + if let Some(source) = sources.get_mut(*sid) { + match source.contains(*pid) { + Poll::Ready(result) => { + results.push((*pid, *sid, result)); + } + Poll::Pending => { + return true; + } + } + } + // TODO: Error? + false + }); + + for (_, source) in sources.sources_mut() { + source.block_until_ready()?; + } + } + + // dbg!(&results); + for (pid, sid, exists) in results.into_iter() { + if exists? { + ws.config().shell().warn(&format!( + "package `{}` from {} is also defined in {}", + pid, + pid.source_id(), + sid + ))?; + } + } + Ok(()) } diff --git a/src/cargo/core/source/mod.rs b/src/cargo/core/source/mod.rs index dca71b64e9d..4700ffb7389 100644 --- a/src/cargo/core/source/mod.rs +++ b/src/cargo/core/source/mod.rs @@ -94,6 +94,9 @@ pub trait Source { false } + /// Returns whether a package exists within the source. + fn contains(&mut self, package: PackageId) -> Poll>; + /// Add a number of crates that should be whitelisted for showing up during /// queries, even if they are yanked. Currently only applies to registry /// sources. @@ -197,6 +200,11 @@ impl<'a, T: Source + ?Sized + 'a> Source for Box { (**self).is_replaced() } + /// Forwards to `Source::contains`. + fn contains(&mut self, package: PackageId) -> Poll> { + (**self).contains(package) + } + fn add_to_yanked_whitelist(&mut self, pkgs: &[PackageId]) { (**self).add_to_yanked_whitelist(pkgs); } @@ -268,6 +276,10 @@ impl<'a, T: Source + ?Sized + 'a> Source for &'a mut T { (**self).is_replaced() } + fn contains(&mut self, package: PackageId) -> Poll> { + (**self).contains(package) + } + fn add_to_yanked_whitelist(&mut self, pkgs: &[PackageId]) { (**self).add_to_yanked_whitelist(pkgs); } @@ -324,6 +336,11 @@ impl<'src> SourceMap<'src> { self.map.len() } + /// Like `HashMap::iter`. + pub fn sources<'a>(&'a self) -> impl Iterator { + self.map.iter().map(|(a, b)| (a, &**b)) + } + /// Like `HashMap::iter_mut`. pub fn sources_mut<'a>( &'a mut self, diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index ea5eded4aa2..3a34c7fc3ba 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -224,6 +224,16 @@ pub fn resolve_ws_with_opts<'cfg>( force_all_targets, )?; + pkg_set.warn_deps_defined_in_multiple_registries( + ws, + &resolved_with_overrides, + &member_ids, + has_dev_units, + requested_targets, + target_data, + force_all_targets, + )?; + Ok(WorkspaceResolve { pkg_set, workspace_resolve: resolve, diff --git a/src/cargo/sources/directory.rs b/src/cargo/sources/directory.rs index 46acb9f8630..7702543a3cf 100644 --- a/src/cargo/sources/directory.rs +++ b/src/cargo/sources/directory.rs @@ -156,6 +156,10 @@ impl<'cfg> Source for DirectorySource<'cfg> { Ok(()) } + fn contains(&mut self, package: PackageId) -> Poll> { + Poll::Ready(Ok(self.packages.contains_key(&package))) + } + fn download(&mut self, id: PackageId) -> CargoResult { self.packages .get(&id) diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index 90c47093dce..b297b09b69d 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -230,6 +230,13 @@ impl<'cfg> Source for GitSource<'cfg> { format!("Git repository {}", self.source_id) } + fn contains(&mut self, package: PackageId) -> Poll> { + match &mut self.path_source { + Some(path_source) => path_source.contains(package), + None => Poll::Pending, + } + } + fn add_to_yanked_whitelist(&mut self, _pkgs: &[PackageId]) {} fn is_yanked(&mut self, _pkg: PackageId) -> Poll> { diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index 37e1e1f0f9d..40cff21f734 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -561,6 +561,14 @@ impl<'cfg> Source for PathSource<'cfg> { } } + fn contains(&mut self, pid: PackageId) -> Poll> { + self.update()?; + Poll::Ready(Ok(self + .packages + .iter() + .any(|package| package.package_id() == pid))) + } + fn add_to_yanked_whitelist(&mut self, _pkgs: &[PackageId]) {} fn is_yanked(&mut self, _pkg: PackageId) -> Poll> { diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 38fc1f94559..0efb4ceadc0 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -164,7 +164,7 @@ use std::collections::HashSet; use std::fs::{File, OpenOptions}; use std::io::{self, Write}; use std::path::{Path, PathBuf}; -use std::task::Poll; +use std::task::{ready, Poll}; use anyhow::Context as _; use cargo_util::paths::{self, exclude_from_backups_and_indexing}; @@ -879,6 +879,16 @@ impl<'cfg> Source for RegistrySource<'cfg> { self.source_id.display_index() } + fn contains(&mut self, package: PackageId) -> Poll> { + Poll::Ready(Ok(ready!(self.index.summaries( + package.name(), + &OptVersionReq::Any, + &mut *self.ops + ))? + .next() + .is_some())) + } + fn add_to_yanked_whitelist(&mut self, pkgs: &[PackageId]) { self.yanked_whitelist.extend(pkgs); } diff --git a/src/cargo/sources/replaced.rs b/src/cargo/sources/replaced.rs index 13191d2234f..2489696e541 100644 --- a/src/cargo/sources/replaced.rs +++ b/src/cargo/sources/replaced.rs @@ -121,6 +121,10 @@ impl<'cfg> Source for ReplacedSource<'cfg> { true } + fn contains(&mut self, package: PackageId) -> Poll> { + self.inner.contains(package) + } + fn add_to_yanked_whitelist(&mut self, pkgs: &[PackageId]) { let pkgs = pkgs .iter() From 2a8ede82d646d8ddf49e6a187839d07da91570ea Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Fri, 24 Mar 2023 17:00:07 -0700 Subject: [PATCH 02/12] Don't warn if an explicit registry is provided. --- src/cargo/core/package.rs | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index cd8796a3f9a..2f2719aa4a0 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -628,22 +628,26 @@ impl<'cfg> PackageSet<'cfg> { target_data, force_all_targets, ) - .map(|(pid, _)| { - sources.sources().filter_map(move |(sid, _)| { - if sid != &pid.source_id() { - Some((pid, *sid)) - } else { - None - } - }) + .filter_map(|(pid, deps)| { + if deps.iter().any(|dep| dep.registry_id().is_some()) { + // If an explicit registry was given in the dependency, + // we don't want to warn. + None + } else { + Some(sources.sources().filter_map(move |(sid, _)| { + if sid != &pid.source_id() { + Some((pid, *sid)) + } else { + None + } + })) + } }) }) .flatten() .collect(); while !pending.is_empty() { - // dbg!(&pending); - pending.retain(|(pid, sid)| { if let Some(source) = sources.get_mut(*sid) { match source.contains(*pid) { @@ -664,7 +668,6 @@ impl<'cfg> PackageSet<'cfg> { } } - // dbg!(&results); for (pid, sid, exists) in results.into_iter() { if exists? { ws.config().shell().warn(&format!( From 79a1424d634003c0616476e008967a8b5f063aa8 Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Fri, 24 Mar 2023 17:09:13 -0700 Subject: [PATCH 03/12] Add short circuit. --- src/cargo/core/package.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index 2f2719aa4a0..1d95a1fb231 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -612,6 +612,12 @@ impl<'cfg> PackageSet<'cfg> { target_data: &RustcTargetData<'_>, force_all_targets: ForceAllTargets, ) -> CargoResult<()> { + // There's no point doing this if we don't have multiple registries in + // the first place. + if self.sources().len() < 2 { + return Ok(()); + } + let _lock = self.config.acquire_package_cache_lock()?; let mut results = Vec::new(); From 9c7242e47eeee8fdd1f6494f6b66e6b102de5fda Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Mon, 27 Mar 2023 19:30:21 -0700 Subject: [PATCH 04/12] Improve output. --- src/cargo/core/package.rs | 62 +++++++++++++++++++++++++++++++++------ 1 file changed, 53 insertions(+), 9 deletions(-) diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index 1d95a1fb231..e3df7ad0db2 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -602,6 +602,10 @@ impl<'cfg> PackageSet<'cfg> { Ok(()) } + /// Check if any dependency packages are defined in more than one registry + /// without an explicit registry defined in the dependency definition. If + /// there are, this function will warn the user that they may be at risk of + /// a dependency confusion attack. pub(crate) fn warn_deps_defined_in_multiple_registries( &self, ws: &Workspace<'cfg>, @@ -618,10 +622,13 @@ impl<'cfg> PackageSet<'cfg> { return Ok(()); } + // We may implicitly start a source update, so let's ensure we have the + // appropriate lock before doing so. let _lock = self.config.acquire_package_cache_lock()?; - let mut results = Vec::new(); - + // We need to build a list of package+source combinations to check. In + // essence, this needs to be every package depended upon on every source + // it's _not_ defined in as a dependency. let mut sources = self.sources_mut(); let mut pending: Vec<(PackageId, SourceId)> = root_ids .iter() @@ -637,13 +644,16 @@ impl<'cfg> PackageSet<'cfg> { .filter_map(|(pid, deps)| { if deps.iter().any(|dep| dep.registry_id().is_some()) { // If an explicit registry was given in the dependency, - // we don't want to warn. + // we don't want to warn, and no further work is + // required. None } else { Some(sources.sources().filter_map(move |(sid, _)| { if sid != &pid.source_id() { Some((pid, *sid)) } else { + // There's no point checking if the package + // exists in the registry it was defined in! None } })) @@ -653,19 +663,24 @@ impl<'cfg> PackageSet<'cfg> { .flatten() .collect(); + // Now we iterate over the pending list of checks until all the results + // are available. + let mut multiply_defined = HashMap::new(); while !pending.is_empty() { pending.retain(|(pid, sid)| { if let Some(source) = sources.get_mut(*sid) { match source.contains(*pid) { Poll::Ready(result) => { - results.push((*pid, *sid, result)); + multiply_defined + .entry(*pid) + .or_insert_with(Vec::default) + .push((*sid, result)); } Poll::Pending => { return true; } } } - // TODO: Error? false }); @@ -674,13 +689,42 @@ impl<'cfg> PackageSet<'cfg> { } } - for (pid, sid, exists) in results.into_iter() { - if exists? { + // Now we have a list of multiply defined packages, we can output that + // list, and suggest to the user how they can avoid the warning. + for (pid, others) in multiply_defined.into_iter() { + let other_sources = others + .into_iter() + .filter_map(|(sid, maybe_exists)| match maybe_exists { + Ok(true) => Some(Ok(format!("`{}`", sid.display_registry_name()))), + Ok(false) => None, + Err(e) => Some(Err(e)), + }) + .collect::, _>>()?; + + if !other_sources.is_empty() { ws.config().shell().warn(&format!( - "package `{}` from {} is also defined in {}", + "package `{}` from {} is also defined in {} {}", pid, pid.source_id(), - sid + if other_sources.len() == 1 { + "registry" + } else { + "registries" + }, + other_sources.join(", "), + ))?; + + ws.config().shell().note(&format!( + r#" +To handle this warning, specify the exact registry in use for the +`{}` dependency in Cargo.toml, eg: + +{} = {{ version = "{}", registry = "{}" }} +"#, + pid, + pid.name(), + pid.version(), + pid.source_id().display_registry_name(), ))?; } } From e2be00390bea37c1a66712c52047af686408e503 Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Tue, 28 Mar 2023 12:46:50 -0700 Subject: [PATCH 05/12] Improve fidelity of source checks. --- src/cargo/core/package.rs | 45 ++++++++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index e3df7ad0db2..20f81ac069c 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -616,19 +616,31 @@ impl<'cfg> PackageSet<'cfg> { target_data: &RustcTargetData<'_>, force_all_targets: ForceAllTargets, ) -> CargoResult<()> { - // There's no point doing this if we don't have multiple registries in - // the first place. - if self.sources().len() < 2 { + // We need to build the possible sources that could cause confusion: + // we're only actually interested in registries here, since non-registry + // sources are more explicitly defined in Cargo.toml. + let registry_source_ids = self + .sources() + .sources() + .filter_map(|(sid, _source)| { + if sid.is_registry() { + Some(sid.clone()) + } else { + None + } + }) + .collect::>(); + + // If there are only zero or one registries, then there's nothing to + // check. + if registry_source_ids.len() < 2 { return Ok(()); } - // We may implicitly start a source update, so let's ensure we have the - // appropriate lock before doing so. - let _lock = self.config.acquire_package_cache_lock()?; - - // We need to build a list of package+source combinations to check. In - // essence, this needs to be every package depended upon on every source - // it's _not_ defined in as a dependency. + // We need to build a list of package+source combinations to check. This + // is essentially the Cartesian product of all package dependencies + // combined with all registry sources that are not the actual source + // that dependency comes from. let mut sources = self.sources_mut(); let mut pending: Vec<(PackageId, SourceId)> = root_ids .iter() @@ -647,6 +659,12 @@ impl<'cfg> PackageSet<'cfg> { // we don't want to warn, and no further work is // required. None + } else if !pid.source_id().is_registry() { + // Dependencies that are coming in from non-registry + // sources — such as Git repos, directories, and paths — + // should also be ignored, as the user will have + // specified the source in their Cargo.toml already. + None } else { Some(sources.sources().filter_map(move |(sid, _)| { if sid != &pid.source_id() { @@ -663,6 +681,11 @@ impl<'cfg> PackageSet<'cfg> { .flatten() .collect(); + // We may implicitly start a source update when checking if a source + // contains a specific package, so let's ensure we have the appropriate + // lock before doing so. + let _lock = self.config.acquire_package_cache_lock()?; + // Now we iterate over the pending list of checks until all the results // are available. let mut multiply_defined = HashMap::new(); @@ -689,6 +712,8 @@ impl<'cfg> PackageSet<'cfg> { } } + drop(_lock); + // Now we have a list of multiply defined packages, we can output that // list, and suggest to the user how they can avoid the warning. for (pid, others) in multiply_defined.into_iter() { From f1a488d0d9d471d3fa713b6cf1de06a4bd3c6983 Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Tue, 28 Mar 2023 14:33:33 -0700 Subject: [PATCH 06/12] Split implementation up. --- src/cargo/core/package.rs | 268 +++++++++++++++++++++++++------------- 1 file changed, 174 insertions(+), 94 deletions(-) diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index 20f81ac069c..f1f57c31fa7 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -6,7 +6,6 @@ use std::hash; use std::mem; use std::path::{Path, PathBuf}; use std::rc::Rc; -use std::task::Poll; use std::time::{Duration, Instant}; use anyhow::Context; @@ -619,114 +618,50 @@ impl<'cfg> PackageSet<'cfg> { // We need to build the possible sources that could cause confusion: // we're only actually interested in registries here, since non-registry // sources are more explicitly defined in Cargo.toml. - let registry_source_ids = self - .sources() - .sources() - .filter_map(|(sid, _source)| { - if sid.is_registry() { - Some(sid.clone()) - } else { - None - } - }) - .collect::>(); - - // If there are only zero or one registries, then there's nothing to - // check. - if registry_source_ids.len() < 2 { - return Ok(()); - } + let mut check = match warn_multiple::DependencyConfusionChecker::new( + self.sources().sources().map(|(sid, _source)| sid), + ) { + Some(check) => check, + None => { + return Ok(()); + } + }; // We need to build a list of package+source combinations to check. This // is essentially the Cartesian product of all package dependencies // combined with all registry sources that are not the actual source // that dependency comes from. - let mut sources = self.sources_mut(); - let mut pending: Vec<(PackageId, SourceId)> = root_ids - .iter() - .flat_map(|root_id| { - PackageSet::filter_deps( - *root_id, - resolve, - has_dev_units, - requested_kinds, - target_data, - force_all_targets, - ) - .filter_map(|(pid, deps)| { - if deps.iter().any(|dep| dep.registry_id().is_some()) { - // If an explicit registry was given in the dependency, - // we don't want to warn, and no further work is - // required. - None - } else if !pid.source_id().is_registry() { - // Dependencies that are coming in from non-registry - // sources — such as Git repos, directories, and paths — - // should also be ignored, as the user will have - // specified the source in their Cargo.toml already. - None - } else { - Some(sources.sources().filter_map(move |(sid, _)| { - if sid != &pid.source_id() { - Some((pid, *sid)) - } else { - // There's no point checking if the package - // exists in the registry it was defined in! - None - } - })) - } - }) - }) - .flatten() - .collect(); - - // We may implicitly start a source update when checking if a source - // contains a specific package, so let's ensure we have the appropriate - // lock before doing so. - let _lock = self.config.acquire_package_cache_lock()?; - - // Now we iterate over the pending list of checks until all the results - // are available. - let mut multiply_defined = HashMap::new(); - while !pending.is_empty() { - pending.retain(|(pid, sid)| { - if let Some(source) = sources.get_mut(*sid) { - match source.contains(*pid) { - Poll::Ready(result) => { - multiply_defined - .entry(*pid) - .or_insert_with(Vec::default) - .push((*sid, result)); - } - Poll::Pending => { - return true; - } - } - } - false - }); - - for (_, source) in sources.sources_mut() { - source.block_until_ready()?; - } + for (pid, deps) in root_ids.iter().flat_map(|root_id| { + PackageSet::filter_deps( + *root_id, + resolve, + has_dev_units, + requested_kinds, + target_data, + force_all_targets, + ) + }) { + check.check_package_deps(pid, deps); } - drop(_lock); + let multiply_defined = check.find_packages_defined_in_multiple_registries( + self.config.acquire_package_cache_lock()?, + self.sources_mut(), + )?; // Now we have a list of multiply defined packages, we can output that // list, and suggest to the user how they can avoid the warning. for (pid, others) in multiply_defined.into_iter() { let other_sources = others .into_iter() - .filter_map(|(sid, maybe_exists)| match maybe_exists { - Ok(true) => Some(Ok(format!("`{}`", sid.display_registry_name()))), - Ok(false) => None, - Err(e) => Some(Err(e)), - }) - .collect::, _>>()?; + .map(|sid| format!("`{}`", sid.display_registry_name())) + .collect::>(); if !other_sources.is_empty() { + // There's no technical reason to sort, but it keeps the test + // output stable. + other_sources.sort(); + ws.config().shell().warn(&format!( "package `{}` from {} is also defined in {} {}", pid, @@ -1335,3 +1270,148 @@ mod tls { }) } } + +mod warn_multiple { + use std::{ + cell::RefMut, + collections::{HashMap, HashSet}, + task::Poll, + }; + + use crate::{ + core::{Dependency, PackageId, SourceId, SourceMap}, + util::config::PackageCacheLock, + CargoResult, + }; + + /// Checks if packages are defined in more than one registry source. + pub(super) struct DependencyConfusionChecker { + registry_source_ids: Vec, + pending_checks: Vec<(PackageId, SourceId)>, + } + + impl DependencyConfusionChecker { + /// Instantiates a new checker based on the given sources. Only sources + /// that are actual registries will be used. + /// + /// If there are fewer than two registry sources, this function returns + /// `None`, and no further action is required to check for dependency + /// confusion. + pub(super) fn new<'a>(sources_iter: impl Iterator) -> Option { + let registry_source_ids = sources_iter + .filter_map(|sid| { + // We're only interested in registry sources, since other + // sources are always explicitly used in dependencies. + if sid.is_registry() { + Some(sid.clone()) + } else { + None + } + }) + .collect::>(); + + // If there are only zero or one registries, then there's nothing to + // check, since no dependency confusion can occur if there is no + // other possible source. + if registry_source_ids.len() > 1 { + Some(Self { + registry_source_ids, + pending_checks: Vec::new(), + }) + } else { + None + } + } + + /// Enqueues a package dependency to be checked for possible dependency + /// confusion, if required. + pub(super) fn check_package_deps(&mut self, pid: PackageId, deps: &HashSet) { + // If an explicit registry was given in a dependency, we don't want + // to warn, and no further work is required. + if deps.iter().any(|dep| dep.registry_id().is_some()) { + return; + } + + // Dependencies that are coming in from non-registry sources — such + // as Git repos, directories, and paths — should also be ignored, as + // the user will have specified the source in their Cargo.toml + // already. + let package_sid = pid.source_id(); + if !package_sid.is_registry() { + return; + } + + // Add a check for the package to the pending list for all registry + // sources that are not the actual source the package is coming + // from. + for sid in self.registry_source_ids.iter() { + if sid != &package_sid { + self.pending_checks.push((pid, *sid)); + } + } + } + + /// Returns packages that are available in multiple registry sources. + /// + /// Note that this requires package cache to be locked, as checking if a + /// source contains a particular package may trigger a pull from that + /// source. + pub(super) fn find_packages_defined_in_multiple_registries( + mut self, + _lock: PackageCacheLock<'_>, + mut sources: RefMut<'_, SourceMap<'_>>, + ) -> CargoResult>> { + // Basically, we want to iterate over the pending checks until we've + // ascertained what the result of each one is. The result may be a + // package+source combination that could cause dependency confusion, + // an error generated when checking if the source contains a + // package, or nothing if there's no confusion possible. + let mut results: Vec> = Vec::new(); + while !self.pending_checks.is_empty() { + self.pending_checks.retain(|(pid, sid)| { + if let Some(source) = sources.get_mut(*sid) { + match source.contains(*pid) { + Poll::Ready(Ok(exists)) => { + if exists { + results.push(Ok((*pid, *sid))); + } + } + Poll::Ready(Err(e)) => { + results.push(Err(e)); + } + Poll::Pending => { + // This is the only scenario where we retain the + // pending check. + return true; + } + } + } else { + // This shouldn't happen in practice unless the source + // map was modified after `Self::new` was invoked, in + // which case there are probably bigger problems anyway. + results.push(Err(anyhow::format_err!( + "cannot find source from ID {:?}", + sid + ))); + } + false + }); + + for (_sid, source) in sources.sources_mut() { + source.block_until_ready()?; + } + } + + // Now we can turn the raw results into a neat HashMap keyed by each + // potentially affected package, which can then be used to report + // back to the user. + let mut by_package = HashMap::new(); + for result in results.into_iter() { + let (pid, sid) = result?; + by_package.entry(pid).or_insert_with(Vec::default).push(sid); + } + + Ok(by_package) + } + } +} From ad7b126c6a1530b7566f9bff07a4a45496601264 Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Tue, 28 Mar 2023 14:57:41 -0700 Subject: [PATCH 07/12] Make package name checks consistent. --- src/cargo/core/package.rs | 2 +- src/cargo/core/source/mod.rs | 12 ++++++------ src/cargo/sources/directory.rs | 4 ++-- src/cargo/sources/git/source.rs | 4 ++-- src/cargo/sources/path.rs | 4 ++-- src/cargo/sources/registry/mod.rs | 4 ++-- src/cargo/sources/replaced.rs | 4 ++-- 7 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index f1f57c31fa7..cc46708f1ba 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -1370,7 +1370,7 @@ mod warn_multiple { while !self.pending_checks.is_empty() { self.pending_checks.retain(|(pid, sid)| { if let Some(source) = sources.get_mut(*sid) { - match source.contains(*pid) { + match source.contains_package_name(&pid.name()) { Poll::Ready(Ok(exists)) => { if exists { results.push(Ok((*pid, *sid))); diff --git a/src/cargo/core/source/mod.rs b/src/cargo/core/source/mod.rs index 4700ffb7389..6ef2c48cc39 100644 --- a/src/cargo/core/source/mod.rs +++ b/src/cargo/core/source/mod.rs @@ -94,8 +94,8 @@ pub trait Source { false } - /// Returns whether a package exists within the source. - fn contains(&mut self, package: PackageId) -> Poll>; + /// Returns whether a specific package is defined within the source. + fn contains_package_name(&mut self, name: &str) -> Poll>; /// Add a number of crates that should be whitelisted for showing up during /// queries, even if they are yanked. Currently only applies to registry @@ -201,8 +201,8 @@ impl<'a, T: Source + ?Sized + 'a> Source for Box { } /// Forwards to `Source::contains`. - fn contains(&mut self, package: PackageId) -> Poll> { - (**self).contains(package) + fn contains_package_name(&mut self, name: &str) -> Poll> { + (**self).contains_package_name(name) } fn add_to_yanked_whitelist(&mut self, pkgs: &[PackageId]) { @@ -276,8 +276,8 @@ impl<'a, T: Source + ?Sized + 'a> Source for &'a mut T { (**self).is_replaced() } - fn contains(&mut self, package: PackageId) -> Poll> { - (**self).contains(package) + fn contains_package_name(&mut self, name: &str) -> Poll> { + (**self).contains_package_name(name) } fn add_to_yanked_whitelist(&mut self, pkgs: &[PackageId]) { diff --git a/src/cargo/sources/directory.rs b/src/cargo/sources/directory.rs index 7702543a3cf..a0d5f6fb893 100644 --- a/src/cargo/sources/directory.rs +++ b/src/cargo/sources/directory.rs @@ -156,8 +156,8 @@ impl<'cfg> Source for DirectorySource<'cfg> { Ok(()) } - fn contains(&mut self, package: PackageId) -> Poll> { - Poll::Ready(Ok(self.packages.contains_key(&package))) + fn contains_package_name(&mut self, name: &str) -> Poll> { + Poll::Ready(Ok(self.packages.keys().any(|id| id.name() == name))) } fn download(&mut self, id: PackageId) -> CargoResult { diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index b297b09b69d..b478faea229 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -230,9 +230,9 @@ impl<'cfg> Source for GitSource<'cfg> { format!("Git repository {}", self.source_id) } - fn contains(&mut self, package: PackageId) -> Poll> { + fn contains_package_name(&mut self, name: &str) -> Poll> { match &mut self.path_source { - Some(path_source) => path_source.contains(package), + Some(path_source) => path_source.contains_package_name(name), None => Poll::Pending, } } diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index 40cff21f734..f038885330d 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -561,12 +561,12 @@ impl<'cfg> Source for PathSource<'cfg> { } } - fn contains(&mut self, pid: PackageId) -> Poll> { + fn contains_package_name(&mut self, name: &str) -> Poll> { self.update()?; Poll::Ready(Ok(self .packages .iter() - .any(|package| package.package_id() == pid))) + .any(|package| package.name() == name))) } fn add_to_yanked_whitelist(&mut self, _pkgs: &[PackageId]) {} diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 0efb4ceadc0..d692fedefe7 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -879,9 +879,9 @@ impl<'cfg> Source for RegistrySource<'cfg> { self.source_id.display_index() } - fn contains(&mut self, package: PackageId) -> Poll> { + fn contains_package_name(&mut self, name: &str) -> Poll> { Poll::Ready(Ok(ready!(self.index.summaries( - package.name(), + name.into(), &OptVersionReq::Any, &mut *self.ops ))? diff --git a/src/cargo/sources/replaced.rs b/src/cargo/sources/replaced.rs index 2489696e541..cfd3945dda9 100644 --- a/src/cargo/sources/replaced.rs +++ b/src/cargo/sources/replaced.rs @@ -121,8 +121,8 @@ impl<'cfg> Source for ReplacedSource<'cfg> { true } - fn contains(&mut self, package: PackageId) -> Poll> { - self.inner.contains(package) + fn contains_package_name(&mut self, name: &str) -> Poll> { + self.inner.contains_package_name(name) } fn add_to_yanked_whitelist(&mut self, pkgs: &[PackageId]) { From 8985fc9770c0bea651bb2dadd5130c708c57f300 Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Mon, 27 Mar 2023 18:42:50 -0700 Subject: [PATCH 08/12] Initial round of test updates. --- src/cargo/core/package.rs | 5 ++--- tests/testsuite/patch.rs | 12 ++++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index cc46708f1ba..4e93ba107f0 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -652,7 +652,7 @@ impl<'cfg> PackageSet<'cfg> { // Now we have a list of multiply defined packages, we can output that // list, and suggest to the user how they can avoid the warning. for (pid, others) in multiply_defined.into_iter() { - let other_sources = others + let mut other_sources = others .into_iter() .map(|sid| format!("`{}`", sid.display_registry_name())) .collect::>(); @@ -675,8 +675,7 @@ impl<'cfg> PackageSet<'cfg> { ))?; ws.config().shell().note(&format!( - r#" -To handle this warning, specify the exact registry in use for the + r#"To handle this warning, specify the exact registry in use for the `{}` dependency in Cargo.toml, eg: {} = {{ version = "{}", registry = "{}" }} diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs index 681c02416bb..c56ebc96a45 100644 --- a/tests/testsuite/patch.rs +++ b/tests/testsuite/patch.rs @@ -2412,6 +2412,12 @@ fn can_update_with_alt_reg() { [UPDATING] `dummy-registry` index [DOWNLOADING] crates ... [DOWNLOADED] bar v0.1.1 (registry `alternative`) +[WARNING] package `bar v0.1.1 (registry `alternative`)` from registry `alternative` is also defined in registry `crates-io` +[NOTE] To handle this warning, specify the exact registry in use for the +`bar v0.1.1 (registry `alternative`)` dependency in Cargo.toml, eg: + +bar = { version = \"0.1.1\", registry = \"alternative\" } + [CHECKING] bar v0.1.1 (registry `alternative`) [CHECKING] foo v0.1.0 ([..]/foo) [FINISHED] [..] @@ -2457,6 +2463,12 @@ fn can_update_with_alt_reg() { [UPDATING] `dummy-registry` index [DOWNLOADING] crates ... [DOWNLOADED] bar v0.1.2 (registry `alternative`) +[WARNING] package `bar v0.1.2 (registry `alternative`)` from registry `alternative` is also defined in registry `crates-io` +[NOTE] To handle this warning, specify the exact registry in use for the +`bar v0.1.2 (registry `alternative`)` dependency in Cargo.toml, eg: + +bar = { version = \"0.1.2\", registry = \"alternative\" } + [CHECKING] bar v0.1.2 (registry `alternative`) [CHECKING] foo v0.1.0 ([..]/foo) [FINISHED] [..] From acccc5787daf9fa067d652598b745e639a6f78f4 Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Tue, 28 Mar 2023 16:41:19 -0700 Subject: [PATCH 09/12] Add new, specific tests. --- tests/testsuite/main.rs | 1 + tests/testsuite/warn_multiple_registries.rs | 280 ++++++++++++++++++++ 2 files changed, 281 insertions(+) create mode 100644 tests/testsuite/warn_multiple_registries.rs diff --git a/tests/testsuite/main.rs b/tests/testsuite/main.rs index a1e293acd81..58c0a02a0c9 100644 --- a/tests/testsuite/main.rs +++ b/tests/testsuite/main.rs @@ -134,6 +134,7 @@ mod update; mod vendor; mod verify_project; mod version; +mod warn_multiple_registries; mod warn_on_failure; mod weak_dep_features; mod workspaces; diff --git a/tests/testsuite/warn_multiple_registries.rs b/tests/testsuite/warn_multiple_registries.rs new file mode 100644 index 00000000000..8f07f148607 --- /dev/null +++ b/tests/testsuite/warn_multiple_registries.rs @@ -0,0 +1,280 @@ +//! Tests for the warnings issued when packages are available in multiple +//! registries without being disambiguated. + +use cargo_test_support::{ + basic_manifest, git, project, + registry::{self, Package}, +}; + +#[cargo_test] +fn same_version_in_two_registries() { + // The basic test: what happens if a package is available in two registries? + // Note that both registries have to actually be used in the dependencies + // for the warning to fire. + + registry::alt_init(); + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = "0.1.2" + in-alternative = { version = "0.1.0", registry = "alternative" } + in-default = "0.1.1" + "#, + ) + .file("src/lib.rs", "") + .build(); + + Package::new("in-alternative", "0.1.0") + .alternative(true) + .publish(); + Package::new("in-default", "0.1.1").publish(); + Package::new("bar", "0.1.2").alternative(true).publish(); + Package::new("bar", "0.1.2").publish(); + + // Note one small piece of weirdness here: if a registry isn't defined in + // the dependency, the default is `crates-io` even if `crates-io` has been + // replaced by another source. + p.cargo("check") + .with_stderr_unordered( + "\ +[UPDATING] `dummy-registry` index +[UPDATING] `alternative` index +[DOWNLOADING] crates ... +[DOWNLOADED] in-default v0.1.1 (registry `dummy-registry`) +[DOWNLOADED] in-alternative v0.1.0 (registry `alternative`) +[DOWNLOADED] bar v0.1.2 (registry `dummy-registry`) +[WARNING] package `bar v0.1.2` from registry `crates-io` is also defined in registry `alternative` +[NOTE] To handle this warning, specify the exact registry in use for the +`bar v0.1.2` dependency in Cargo.toml, eg: + +bar = { version = \"0.1.2\", registry = \"crates-io\" } + +[CHECKING] bar v0.1.2 +[CHECKING] in-alternative v0.1.0 (registry `alternative`) +[CHECKING] in-default v0.1.1 +[CHECKING] foo v0.0.1 ([..]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]s +", + ) + .run(); +} + +#[cargo_test] +fn different_versions_in_two_registries() { + // Checks should take place on package names only, not versions. + + registry::alt_init(); + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = "0.1.2" + in-alternative = { version = "0.1.0", registry = "alternative" } + in-default = "0.1.1" + "#, + ) + .file("src/lib.rs", "") + .build(); + + Package::new("in-alternative", "0.1.0") + .alternative(true) + .publish(); + Package::new("in-default", "0.1.1").publish(); + Package::new("bar", "1.2.3").alternative(true).publish(); + Package::new("bar", "0.1.2").publish(); + + p.cargo("check") + .with_stderr_unordered( + "\ +[UPDATING] `dummy-registry` index +[UPDATING] `alternative` index +[DOWNLOADING] crates ... +[DOWNLOADED] in-default v0.1.1 (registry `dummy-registry`) +[DOWNLOADED] in-alternative v0.1.0 (registry `alternative`) +[DOWNLOADED] bar v0.1.2 (registry `dummy-registry`) +[WARNING] package `bar v0.1.2` from registry `crates-io` is also defined in registry `alternative` +[NOTE] To handle this warning, specify the exact registry in use for the +`bar v0.1.2` dependency in Cargo.toml, eg: + +bar = { version = \"0.1.2\", registry = \"crates-io\" } + +[CHECKING] bar v0.1.2 +[CHECKING] in-alternative v0.1.0 (registry `alternative`) +[CHECKING] in-default v0.1.1 +[CHECKING] foo v0.0.1 ([..]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]s +", + ) + .run(); +} + +#[cargo_test] +fn explicit_dep_registry() { + // Dependencies with explicit registries should not generate warnings. + + registry::alt_init(); + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = { version = "0.1.2", registry = "alternative" } + in-alternative = { version = "0.1.0", registry = "alternative" } + in-default = "0.1.1" + "#, + ) + .file("src/lib.rs", "") + .build(); + + Package::new("in-alternative", "0.1.0") + .alternative(true) + .publish(); + Package::new("in-default", "0.1.1").publish(); + Package::new("bar", "0.1.2").alternative(true).publish(); + Package::new("bar", "0.1.2").publish(); + + p.cargo("check") + .with_stderr_unordered( + "\ +[UPDATING] `dummy-registry` index +[UPDATING] `alternative` index +[DOWNLOADING] crates ... +[DOWNLOADED] in-default v0.1.1 (registry `dummy-registry`) +[DOWNLOADED] in-alternative v0.1.0 (registry `alternative`) +[DOWNLOADED] bar v0.1.2 (registry `alternative`) +[CHECKING] bar v0.1.2 (registry `alternative`) +[CHECKING] in-alternative v0.1.0 (registry `alternative`) +[CHECKING] in-default v0.1.1 +[CHECKING] foo v0.0.1 ([..]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]s +", + ) + .run(); +} + +#[cargo_test] +fn path_dep() { + // Dependencies defined as path deps should never warn. + + registry::alt_init(); + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = { path = "bar" } + in-alternative = { version = "0.1.0", registry = "alternative" } + in-default = "0.1.1" + "#, + ) + .file("src/lib.rs", "") + .file("bar/Cargo.toml", &basic_manifest("bar", "1.1.2")) + .file("bar/src/lib.rs", "") + .build(); + + Package::new("in-alternative", "0.1.0") + .alternative(true) + .publish(); + Package::new("in-default", "0.1.1").publish(); + Package::new("bar", "0.1.2").alternative(true).publish(); + Package::new("bar", "0.1.2").publish(); + + p.cargo("check") + .with_stderr_unordered( + "\ +[UPDATING] `dummy-registry` index +[UPDATING] `alternative` index +[DOWNLOADING] crates ... +[DOWNLOADED] in-default v0.1.1 (registry `dummy-registry`) +[DOWNLOADED] in-alternative v0.1.0 (registry `alternative`) +[CHECKING] bar v1.1.2 ([CWD]/bar) +[CHECKING] in-alternative v0.1.0 (registry `alternative`) +[CHECKING] in-default v0.1.1 +[CHECKING] foo v0.0.1 ([..]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]s +", + ) + .run(); +} + +#[cargo_test] +fn git_dep() { + // Dependencies defined as Git deps should never warn. + + let (dep_project, _dep_repo) = git::new_repo("bar", |p| { + p.file("Cargo.toml", &basic_manifest("bar", "1.1.2")) + .file("src/lib.rs", "") + }); + + registry::alt_init(); + let p = project() + .file( + "Cargo.toml", + &format!( + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = {{ git = "{}" }} + in-alternative = {{ version = "0.1.0", registry = "alternative" }} + in-default = "0.1.1" + "#, + dep_project.url() + ), + ) + .file("src/lib.rs", "") + .file("bar/Cargo.toml", &basic_manifest("bar", "0.1.2")) + .file("bar/src/lib.rs", "") + .build(); + + Package::new("in-alternative", "0.1.0") + .alternative(true) + .publish(); + Package::new("in-default", "0.1.1").publish(); + Package::new("bar", "0.1.2").alternative(true).publish(); + Package::new("bar", "0.1.2").publish(); + + p.cargo("check") + .with_stderr_unordered( + "\ +[UPDATING] `dummy-registry` index +[UPDATING] `alternative` index +[DOWNLOADING] crates ... +[DOWNLOADED] in-default v0.1.1 (registry `dummy-registry`) +[DOWNLOADED] in-alternative v0.1.0 (registry `alternative`) +[UPDATING] git repository `[..]/bar` +[CHECKING] bar v1.1.2 ([..]/bar#[..]) +[CHECKING] in-alternative v0.1.0 (registry `alternative`) +[CHECKING] in-default v0.1.1 +[CHECKING] foo v0.0.1 ([..]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]s +", + ) + .run(); +} From d2bfa7d27f5a6703e5989afd8ea934093844163a Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Tue, 28 Mar 2023 16:51:29 -0700 Subject: [PATCH 10/12] Moar test. --- tests/testsuite/warn_multiple_registries.rs | 144 ++++++++++++++++++++ 1 file changed, 144 insertions(+) diff --git a/tests/testsuite/warn_multiple_registries.rs b/tests/testsuite/warn_multiple_registries.rs index 8f07f148607..291ce12c7fa 100644 --- a/tests/testsuite/warn_multiple_registries.rs +++ b/tests/testsuite/warn_multiple_registries.rs @@ -278,3 +278,147 @@ fn git_dep() { ) .run(); } + +#[cargo_test] +fn other_dep_types() { + // Ensure we generate warnings on build and dev dependencies as well. + + registry::alt_init(); + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + main = "0.1.2" + in-alternative = { version = "0.1.0", registry = "alternative" } + in-default = "0.1.1" + + [build-dependencies] + build = "0.1.3" + + [dev-dependencies] + dev = "0.1.4" + "#, + ) + .file("src/lib.rs", "") + .build(); + + Package::new("in-alternative", "0.1.0") + .alternative(true) + .publish(); + Package::new("in-default", "0.1.1").publish(); + + for (package, version) in [("main", "0.1.2"), ("build", "0.1.3"), ("dev", "0.1.4")] { + Package::new(package, version).alternative(true).publish(); + Package::new(package, version).publish(); + } + + // Note one small piece of weirdness here: if a registry isn't defined in + // the dependency, the default is `crates-io` even if `crates-io` has been + // replaced by another source. + p.cargo("check --tests") + .with_stderr_unordered( + "\ +[UPDATING] `dummy-registry` index +[UPDATING] `alternative` index +[DOWNLOADING] crates ... +[DOWNLOADED] in-default v0.1.1 (registry `dummy-registry`) +[DOWNLOADED] in-alternative v0.1.0 (registry `alternative`) +[DOWNLOADED] main v0.1.2 (registry `dummy-registry`) +[DOWNLOADED] build v0.1.3 (registry `dummy-registry`) +[DOWNLOADED] dev v0.1.4 (registry `dummy-registry`) +[WARNING] package `main v0.1.2` from registry `crates-io` is also defined in registry `alternative` +[NOTE] To handle this warning, specify the exact registry in use for the +`main v0.1.2` dependency in Cargo.toml, eg: + +main = { version = \"0.1.2\", registry = \"crates-io\" } + +[WARNING] package `build v0.1.3` from registry `crates-io` is also defined in registry `alternative` +[NOTE] To handle this warning, specify the exact registry in use for the +`build v0.1.3` dependency in Cargo.toml, eg: + +build = { version = \"0.1.3\", registry = \"crates-io\" } + +[WARNING] package `dev v0.1.4` from registry `crates-io` is also defined in registry `alternative` +[NOTE] To handle this warning, specify the exact registry in use for the +`dev v0.1.4` dependency in Cargo.toml, eg: + +dev = { version = \"0.1.4\", registry = \"crates-io\" } + +[CHECKING] main v0.1.2 +[CHECKING] dev v0.1.4 +[CHECKING] in-alternative v0.1.0 (registry `alternative`) +[CHECKING] in-default v0.1.1 +[CHECKING] foo v0.0.1 ([..]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]s +", + ) + .run(); +} + +#[cargo_test] +fn other_dep_types_explicit_registries() { + // Ensure we don't generate warnings on build and dev dependencies when they + // have explicit registries defined. + + registry::alt_init(); + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + main = { version = "0.1.2", registry = "alternative" } + in-alternative = { version = "0.1.0", registry = "alternative" } + in-default = "0.1.1" + + [build-dependencies] + build = { version = "0.1.3", registry = "alternative" } + + [dev-dependencies] + dev = { version = "0.1.4", registry = "crates-io" } + "#, + ) + .file("src/lib.rs", "") + .build(); + + Package::new("in-alternative", "0.1.0") + .alternative(true) + .publish(); + Package::new("in-default", "0.1.1").publish(); + + for (package, version) in [("main", "0.1.2"), ("build", "0.1.3"), ("dev", "0.1.4")] { + Package::new(package, version).alternative(true).publish(); + Package::new(package, version).publish(); + } + + p.cargo("check --tests") + .with_stderr_unordered( + "\ +[UPDATING] `dummy-registry` index +[UPDATING] `alternative` index +[DOWNLOADING] crates ... +[DOWNLOADED] in-default v0.1.1 (registry `dummy-registry`) +[DOWNLOADED] in-alternative v0.1.0 (registry `alternative`) +[DOWNLOADED] main v0.1.2 (registry `alternative`) +[DOWNLOADED] build v0.1.3 (registry `alternative`) +[DOWNLOADED] dev v0.1.4 (registry `dummy-registry`) +[CHECKING] main v0.1.2 (registry `alternative`) +[CHECKING] dev v0.1.4 +[CHECKING] in-alternative v0.1.0 (registry `alternative`) +[CHECKING] in-default v0.1.1 +[CHECKING] foo v0.0.1 ([..]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]s +", + ) + .run(); +} From 62c00913cf3bb49a9db149ae6e9734d5d923371e Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Tue, 28 Mar 2023 17:04:42 -0700 Subject: [PATCH 11/12] lowercase things, add docs --- src/cargo/core/package.rs | 2 +- src/doc/src/reference/specifying-dependencies.md | 8 ++++++++ tests/testsuite/patch.rs | 4 ++-- tests/testsuite/warn_multiple_registries.rs | 10 +++++----- 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index 4e93ba107f0..b187b8896a0 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -675,7 +675,7 @@ impl<'cfg> PackageSet<'cfg> { ))?; ws.config().shell().note(&format!( - r#"To handle this warning, specify the exact registry in use for the + r#"to handle this warning, specify the exact registry in use for the `{}` dependency in Cargo.toml, eg: {} = {{ version = "{}", registry = "{}" }} diff --git a/src/doc/src/reference/specifying-dependencies.md b/src/doc/src/reference/specifying-dependencies.md index 8d9eac308b8..c6862693939 100644 --- a/src/doc/src/reference/specifying-dependencies.md +++ b/src/doc/src/reference/specifying-dependencies.md @@ -119,6 +119,14 @@ to the name of the registry to use. some-crate = { version = "1.0", registry = "my-registry" } ``` +To explicitly depend on a package on [crates.io], the `registry` key can be set +to `crates-io`. + +```toml +[dependencies] +some-crate = { version = "1.0", registry = "crates-io" } +``` + > **Note**: [crates.io] does not allow packages to be published with > dependencies on other registries. diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs index c56ebc96a45..ca25ec74a40 100644 --- a/tests/testsuite/patch.rs +++ b/tests/testsuite/patch.rs @@ -2413,7 +2413,7 @@ fn can_update_with_alt_reg() { [DOWNLOADING] crates ... [DOWNLOADED] bar v0.1.1 (registry `alternative`) [WARNING] package `bar v0.1.1 (registry `alternative`)` from registry `alternative` is also defined in registry `crates-io` -[NOTE] To handle this warning, specify the exact registry in use for the +[NOTE] to handle this warning, specify the exact registry in use for the `bar v0.1.1 (registry `alternative`)` dependency in Cargo.toml, eg: bar = { version = \"0.1.1\", registry = \"alternative\" } @@ -2464,7 +2464,7 @@ bar = { version = \"0.1.1\", registry = \"alternative\" } [DOWNLOADING] crates ... [DOWNLOADED] bar v0.1.2 (registry `alternative`) [WARNING] package `bar v0.1.2 (registry `alternative`)` from registry `alternative` is also defined in registry `crates-io` -[NOTE] To handle this warning, specify the exact registry in use for the +[NOTE] to handle this warning, specify the exact registry in use for the `bar v0.1.2 (registry `alternative`)` dependency in Cargo.toml, eg: bar = { version = \"0.1.2\", registry = \"alternative\" } diff --git a/tests/testsuite/warn_multiple_registries.rs b/tests/testsuite/warn_multiple_registries.rs index 291ce12c7fa..bd6c682ca9c 100644 --- a/tests/testsuite/warn_multiple_registries.rs +++ b/tests/testsuite/warn_multiple_registries.rs @@ -51,7 +51,7 @@ fn same_version_in_two_registries() { [DOWNLOADED] in-alternative v0.1.0 (registry `alternative`) [DOWNLOADED] bar v0.1.2 (registry `dummy-registry`) [WARNING] package `bar v0.1.2` from registry `crates-io` is also defined in registry `alternative` -[NOTE] To handle this warning, specify the exact registry in use for the +[NOTE] to handle this warning, specify the exact registry in use for the `bar v0.1.2` dependency in Cargo.toml, eg: bar = { version = \"0.1.2\", registry = \"crates-io\" } @@ -106,7 +106,7 @@ fn different_versions_in_two_registries() { [DOWNLOADED] in-alternative v0.1.0 (registry `alternative`) [DOWNLOADED] bar v0.1.2 (registry `dummy-registry`) [WARNING] package `bar v0.1.2` from registry `crates-io` is also defined in registry `alternative` -[NOTE] To handle this warning, specify the exact registry in use for the +[NOTE] to handle this warning, specify the exact registry in use for the `bar v0.1.2` dependency in Cargo.toml, eg: bar = { version = \"0.1.2\", registry = \"crates-io\" } @@ -333,19 +333,19 @@ fn other_dep_types() { [DOWNLOADED] build v0.1.3 (registry `dummy-registry`) [DOWNLOADED] dev v0.1.4 (registry `dummy-registry`) [WARNING] package `main v0.1.2` from registry `crates-io` is also defined in registry `alternative` -[NOTE] To handle this warning, specify the exact registry in use for the +[NOTE] to handle this warning, specify the exact registry in use for the `main v0.1.2` dependency in Cargo.toml, eg: main = { version = \"0.1.2\", registry = \"crates-io\" } [WARNING] package `build v0.1.3` from registry `crates-io` is also defined in registry `alternative` -[NOTE] To handle this warning, specify the exact registry in use for the +[NOTE] to handle this warning, specify the exact registry in use for the `build v0.1.3` dependency in Cargo.toml, eg: build = { version = \"0.1.3\", registry = \"crates-io\" } [WARNING] package `dev v0.1.4` from registry `crates-io` is also defined in registry `alternative` -[NOTE] To handle this warning, specify the exact registry in use for the +[NOTE] to handle this warning, specify the exact registry in use for the `dev v0.1.4` dependency in Cargo.toml, eg: dev = { version = \"0.1.4\", registry = \"crates-io\" } From 9fb4f4119b08db8b29784f0d3cc78da2e3f7448a Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Tue, 28 Mar 2023 17:24:42 -0700 Subject: [PATCH 12/12] Another wording tweak. --- src/cargo/core/package.rs | 2 +- tests/testsuite/patch.rs | 4 ++-- tests/testsuite/warn_multiple_registries.rs | 10 +++++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index b187b8896a0..2709a480247 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -675,7 +675,7 @@ impl<'cfg> PackageSet<'cfg> { ))?; ws.config().shell().note(&format!( - r#"to handle this warning, specify the exact registry in use for the + r#"you can specify the exact registry to use for the `{}` dependency in Cargo.toml, eg: {} = {{ version = "{}", registry = "{}" }} diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs index ca25ec74a40..67a6e7b683c 100644 --- a/tests/testsuite/patch.rs +++ b/tests/testsuite/patch.rs @@ -2413,7 +2413,7 @@ fn can_update_with_alt_reg() { [DOWNLOADING] crates ... [DOWNLOADED] bar v0.1.1 (registry `alternative`) [WARNING] package `bar v0.1.1 (registry `alternative`)` from registry `alternative` is also defined in registry `crates-io` -[NOTE] to handle this warning, specify the exact registry in use for the +[NOTE] you can specify the exact registry to use for the `bar v0.1.1 (registry `alternative`)` dependency in Cargo.toml, eg: bar = { version = \"0.1.1\", registry = \"alternative\" } @@ -2464,7 +2464,7 @@ bar = { version = \"0.1.1\", registry = \"alternative\" } [DOWNLOADING] crates ... [DOWNLOADED] bar v0.1.2 (registry `alternative`) [WARNING] package `bar v0.1.2 (registry `alternative`)` from registry `alternative` is also defined in registry `crates-io` -[NOTE] to handle this warning, specify the exact registry in use for the +[NOTE] you can specify the exact registry to use for the `bar v0.1.2 (registry `alternative`)` dependency in Cargo.toml, eg: bar = { version = \"0.1.2\", registry = \"alternative\" } diff --git a/tests/testsuite/warn_multiple_registries.rs b/tests/testsuite/warn_multiple_registries.rs index bd6c682ca9c..532e4b2dfdd 100644 --- a/tests/testsuite/warn_multiple_registries.rs +++ b/tests/testsuite/warn_multiple_registries.rs @@ -51,7 +51,7 @@ fn same_version_in_two_registries() { [DOWNLOADED] in-alternative v0.1.0 (registry `alternative`) [DOWNLOADED] bar v0.1.2 (registry `dummy-registry`) [WARNING] package `bar v0.1.2` from registry `crates-io` is also defined in registry `alternative` -[NOTE] to handle this warning, specify the exact registry in use for the +[NOTE] you can specify the exact registry to use for the `bar v0.1.2` dependency in Cargo.toml, eg: bar = { version = \"0.1.2\", registry = \"crates-io\" } @@ -106,7 +106,7 @@ fn different_versions_in_two_registries() { [DOWNLOADED] in-alternative v0.1.0 (registry `alternative`) [DOWNLOADED] bar v0.1.2 (registry `dummy-registry`) [WARNING] package `bar v0.1.2` from registry `crates-io` is also defined in registry `alternative` -[NOTE] to handle this warning, specify the exact registry in use for the +[NOTE] you can specify the exact registry to use for the `bar v0.1.2` dependency in Cargo.toml, eg: bar = { version = \"0.1.2\", registry = \"crates-io\" } @@ -333,19 +333,19 @@ fn other_dep_types() { [DOWNLOADED] build v0.1.3 (registry `dummy-registry`) [DOWNLOADED] dev v0.1.4 (registry `dummy-registry`) [WARNING] package `main v0.1.2` from registry `crates-io` is also defined in registry `alternative` -[NOTE] to handle this warning, specify the exact registry in use for the +[NOTE] you can specify the exact registry to use for the `main v0.1.2` dependency in Cargo.toml, eg: main = { version = \"0.1.2\", registry = \"crates-io\" } [WARNING] package `build v0.1.3` from registry `crates-io` is also defined in registry `alternative` -[NOTE] to handle this warning, specify the exact registry in use for the +[NOTE] you can specify the exact registry to use for the `build v0.1.3` dependency in Cargo.toml, eg: build = { version = \"0.1.3\", registry = \"crates-io\" } [WARNING] package `dev v0.1.4` from registry `crates-io` is also defined in registry `alternative` -[NOTE] to handle this warning, specify the exact registry in use for the +[NOTE] you can specify the exact registry to use for the `dev v0.1.4` dependency in Cargo.toml, eg: dev = { version = \"0.1.4\", registry = \"crates-io\" }