Skip to content

Commit 0be926d

Browse files
committed
Merge remote-tracking branch 'origin/master' into PeekAtRemainingCandidates
# Conflicts: # src/cargo/core/resolver/mod.rs
2 parents ee8eade + fe0c18b commit 0be926d

File tree

16 files changed

+200
-84
lines changed

16 files changed

+200
-84
lines changed

src/cargo/core/manifest.rs

+1
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ pub struct ManifestMetadata {
7878
pub repository: Option<String>, // url
7979
pub documentation: Option<String>, // url
8080
pub badges: BTreeMap<String, BTreeMap<String, String>>,
81+
pub links: Option<String>,
8182
}
8283

8384
#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]

src/cargo/core/resolver/mod.rs

+98-38
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@
2323
//! * Never try to activate a crate version which is incompatible. This means we
2424
//! only try crates which will actually satisfy a dependency and we won't ever
2525
//! try to activate a crate that's semver compatible with something else
26-
//! activated (as we're only allowed to have one).
26+
//! activated (as we're only allowed to have one) nor try to activate a crate
27+
//! that has the same links attribute as something else
28+
//! activated.
2729
//! * Always try to activate the highest version crate first. The default
2830
//! dependency in Cargo (e.g. when you write `foo = "0.1.2"`) is
2931
//! semver-compatible, so selecting the highest version possible will allow us
@@ -325,11 +327,12 @@ enum GraphNode {
325327
// possible.
326328
#[derive(Clone)]
327329
struct Context<'a> {
328-
// TODO: Both this and the map below are super expensive to clone. We should
330+
// TODO: Both this and the two maps below are super expensive to clone. We should
329331
// switch to persistent hash maps if we can at some point or otherwise
330332
// make these much cheaper to clone in general.
331333
activations: Activations,
332334
resolve_features: HashMap<PackageId, HashSet<String>>,
335+
links: HashMap<String, PackageId>,
333336

334337
// These are two cheaply-cloneable lists (O(1) clone) which are effectively
335338
// hash maps but are built up as "construction lists". We'll iterate these
@@ -354,9 +357,10 @@ pub fn resolve(summaries: &[(Summary, Method)],
354357
let cx = Context {
355358
resolve_graph: RcList::new(),
356359
resolve_features: HashMap::new(),
360+
links: HashMap::new(),
357361
resolve_replacements: RcList::new(),
358362
activations: HashMap::new(),
359-
replacements: replacements,
363+
replacements,
360364
warnings: RcList::new(),
361365
};
362366
let _p = profile::start("resolving");
@@ -416,13 +420,13 @@ fn activate(cx: &mut Context,
416420
candidate.summary.package_id().clone()));
417421
}
418422

419-
let activated = cx.flag_activated(&candidate.summary, method);
423+
let activated = cx.flag_activated(&candidate.summary, method)?;
420424

421425
let candidate = match candidate.replace {
422426
Some(replace) => {
423427
cx.resolve_replacements.push((candidate.summary.package_id().clone(),
424428
replace.package_id().clone()));
425-
if cx.flag_activated(&replace, method) && activated {
429+
if cx.flag_activated(&replace, method)? && activated {
426430
return Ok(None);
427431
}
428432
trace!("activating {} (replacing {})", replace.package_id(),
@@ -456,7 +460,7 @@ impl<T> RcVecIter<T> {
456460
fn new(vec: Rc<Vec<T>>) -> RcVecIter<T> {
457461
RcVecIter {
458462
rest: 0..vec.len(),
459-
vec: vec,
463+
vec,
460464
}
461465
}
462466
}
@@ -528,6 +532,21 @@ impl Ord for DepsFrame {
528532
}
529533
}
530534

535+
#[derive(Clone, PartialOrd, Ord, PartialEq, Eq)]
536+
enum ConflictReason {
537+
Semver,
538+
Links(String),
539+
}
540+
541+
impl ConflictReason {
542+
fn is_links(&self) -> bool {
543+
match self {
544+
&ConflictReason::Semver => false,
545+
&ConflictReason::Links(_) => true,
546+
}
547+
}
548+
}
549+
531550
#[derive(Clone)]
532551
struct BacktrackFrame<'a> {
533552
cur: usize,
@@ -543,7 +562,7 @@ struct BacktrackFrame<'a> {
543562
struct RemainingCandidates {
544563
remaining: RcVecIter<Candidate>,
545564
// note: change to RcList or something if clone is to expensive
546-
conflicting_prev_active: HashSet<PackageId>,
565+
conflicting_prev_active: HashMap<PackageId, ConflictReason>,
547566
// This is a inlined peekable generator
548567
has_another: Option<Candidate>,
549568
}
@@ -552,31 +571,40 @@ impl RemainingCandidates {
552571
fn new(candidates: &Rc<Vec<Candidate>>) -> RemainingCandidates {
553572
RemainingCandidates {
554573
remaining: RcVecIter::new(Rc::clone(candidates)),
555-
conflicting_prev_active: HashSet::new(),
574+
conflicting_prev_active: HashMap::new(),
556575
has_another: None,
557576
}
558577
}
559578

560-
fn next(&mut self, prev_active: &[Summary]) -> Result<(Candidate, bool), HashSet<PackageId>> {
579+
fn next(&mut self, prev_active: &[Summary], links: &HashMap<String, PackageId>) -> Result<(Candidate, bool), HashMap<PackageId, ConflictReason>> {
561580
// Filter the set of candidates based on the previously activated
562581
// versions for this dependency. We can actually use a version if it
563582
// precisely matches an activated version or if it is otherwise
564583
// incompatible with all other activated versions. Note that we
565584
// define "compatible" here in terms of the semver sense where if
566585
// the left-most nonzero digit is the same they're considered
567-
// compatible.
586+
// compatible unless we have a `*-sys` crate (defined by having a
587+
// linked attribute) then we can only have one version.
568588
//
569589
// When we are done we return the set of previously activated
570590
// that conflicted with the ones we tried. If any of these change
571591
// then we would have considered different candidates.
572592
use std::mem::replace;
573593
for (_, b) in self.remaining.by_ref() {
594+
if let Some(link) = b.summary.links() {
595+
if let Some(a) = links.get(link) {
596+
if a != b.summary.package_id() {
597+
self.conflicting_prev_active.insert(a.clone(), ConflictReason::Links(link.to_owned()));
598+
continue
599+
}
600+
}
601+
}
574602
if let Some(a) = prev_active
575603
.iter()
576604
.find(|a| compatible(a.version(), b.summary.version()))
577605
{
578606
if *a != b.summary {
579-
self.conflicting_prev_active.insert(a.package_id().clone());
607+
self.conflicting_prev_active.insert(a.package_id().clone(), ConflictReason::Semver);
580608
continue;
581609
}
582610
}
@@ -675,15 +703,15 @@ fn activate_deps_loop<'a>(
675703
trace!("{}[{}]>{} {} candidates", parent.name(), cur, dep.name(), candidates.len());
676704
trace!("{}[{}]>{} {} prev activations", parent.name(), cur, dep.name(), cx.prev_active(&dep).len());
677705
let mut remaining_candidates = RemainingCandidates::new(&candidates);
678-
let next = remaining_candidates.next(cx.prev_active(&dep));
706+
let next = remaining_candidates.next(cx.prev_active(&dep), &cx.links);
679707

680708
// Alright, for each candidate that's gotten this far, it meets the
681709
// following requirements:
682710
//
683711
// 1. The version matches the dependency requirement listed for this
684712
// package
685713
// 2. There are no activated versions for this package which are
686-
// semver-compatible, or there's an activated version which is
714+
// semver/links-compatible, or there's an activated version which is
687715
// precisely equal to `candidate`.
688716
//
689717
// This means that we're going to attempt to activate each candidate in
@@ -768,15 +796,15 @@ fn find_candidate<'a>(
768796
dep: &mut Dependency,
769797
features: &mut Rc<Vec<String>>,
770798
remaining_candidates: &mut RemainingCandidates,
771-
conflicting_activations: &mut HashSet<PackageId>,
799+
conflicting_activations: &mut HashMap<PackageId, ConflictReason>,
772800
) -> Option<(Candidate, bool)> {
773801
while let Some(mut frame) = backtrack_stack.pop() {
774-
let next= frame.remaining_candidates.next(frame.context_backup.prev_active(&frame.dep));
802+
let next= frame.remaining_candidates.next(frame.context_backup.prev_active(&frame.dep), &frame.context_backup.links);
775803
if frame.context_backup.is_active(parent.package_id())
776804
&& conflicting_activations
777805
.iter()
778806
// note: a lot of redundant work in is_active for similar debs
779-
.all(|con| frame.context_backup.is_active(con))
807+
.all(|(con, _)| frame.context_backup.is_active(con))
780808
{
781809
continue;
782810
}
@@ -798,7 +826,7 @@ fn activation_error(cx: &Context,
798826
registry: &mut Registry,
799827
parent: &Summary,
800828
dep: &Dependency,
801-
conflicting_activations: HashSet<PackageId>,
829+
conflicting_activations: HashMap<PackageId, ConflictReason>,
802830
candidates: &[Candidate],
803831
config: Option<&Config>) -> CargoError {
804832
let graph = cx.graph();
@@ -814,26 +842,53 @@ fn activation_error(cx: &Context,
814842
dep_path_desc
815843
};
816844
if !candidates.is_empty() {
817-
let mut msg = format!("failed to select a version for `{}`.\n\
818-
all possible versions conflict with \
819-
previously selected packages.\n",
820-
dep.name());
821-
msg.push_str("required by ");
845+
let mut msg = format!("failed to select a version for `{}`.", dep.name());
846+
msg.push_str("\n ... required by ");
822847
msg.push_str(&describe_path(parent.package_id()));
823-
let mut conflicting_activations: Vec<_> = conflicting_activations.iter().collect();
824-
conflicting_activations.sort_unstable();
825-
for v in conflicting_activations.iter().rev() {
826-
msg.push_str("\n previously selected ");
827-
msg.push_str(&describe_path(v));
828-
}
829848

830-
msg.push_str("\n possible versions to select: ");
849+
msg.push_str("\nversions that meet the requirements `");
850+
msg.push_str(&dep.version_req().to_string());
851+
msg.push_str("` are: ");
831852
msg.push_str(&candidates.iter()
832853
.map(|v| v.summary.version())
833854
.map(|v| v.to_string())
834855
.collect::<Vec<_>>()
835856
.join(", "));
836857

858+
let mut conflicting_activations: Vec<_> = conflicting_activations.iter().collect();
859+
conflicting_activations.sort_unstable();
860+
let (links_errors, other_errors): (Vec<_>, Vec<_>) = conflicting_activations.drain(..).rev().partition(|&(_, r)| r.is_links());
861+
862+
for &(p, r) in &links_errors {
863+
match r {
864+
&ConflictReason::Links(ref link) => {
865+
msg.push_str("\n\nthe package `");
866+
msg.push_str(dep.name());
867+
msg.push_str("` links to the native library `");
868+
msg.push_str(&link);
869+
msg.push_str("`, but it conflicts with a previous package which links to `");
870+
msg.push_str(&link);
871+
msg.push_str("` as well:\n");
872+
},
873+
_ => (),
874+
}
875+
msg.push_str(&describe_path(p));
876+
}
877+
878+
if links_errors.is_empty() {
879+
msg.push_str("\n\nall possible versions conflict with \
880+
previously selected packages.");
881+
}
882+
883+
for &(p, _) in &other_errors {
884+
msg.push_str("\n\n previously selected ");
885+
msg.push_str(&describe_path(p));
886+
}
887+
888+
msg.push_str("\n\nfailed to select a version for `");
889+
msg.push_str(dep.name());
890+
msg.push_str("` which could resolve this conflict");
891+
837892
return format_err!("{}", msg)
838893
}
839894

@@ -1044,12 +1099,12 @@ fn build_requirements<'a, 'b: 'a>(s: &'a Summary, method: &'b Method)
10441099
}
10451100

10461101
impl<'a> Context<'a> {
1047-
// Activate this summary by inserting it into our list of known activations.
1048-
//
1049-
// Returns if this summary with the given method is already activated.
1102+
/// Activate this summary by inserting it into our list of known activations.
1103+
///
1104+
/// Returns true if this summary with the given method is already activated.
10501105
fn flag_activated(&mut self,
10511106
summary: &Summary,
1052-
method: &Method) -> bool {
1107+
method: &Method) -> CargoResult<bool> {
10531108
let id = summary.package_id();
10541109
let prev = self.activations
10551110
.entry(id.name().to_string())
@@ -1058,26 +1113,31 @@ impl<'a> Context<'a> {
10581113
.or_insert(Vec::new());
10591114
if !prev.iter().any(|c| c == summary) {
10601115
self.resolve_graph.push(GraphNode::Add(id.clone()));
1116+
if let Some(link) = summary.links() {
1117+
ensure!(self.links.insert(link.to_owned(), id.clone()).is_none(),
1118+
"Attempting to resolve a with more then one crate with the links={}. \n\
1119+
This will not build as is. Consider rebuilding the .lock file.", link);
1120+
}
10611121
prev.push(summary.clone());
1062-
return false
1122+
return Ok(false)
10631123
}
10641124
debug!("checking if {} is already activated", summary.package_id());
10651125
let (features, use_default) = match *method {
10661126
Method::Required { features, uses_default_features, .. } => {
10671127
(features, uses_default_features)
10681128
}
1069-
Method::Everything => return false,
1129+
Method::Everything => return Ok(false),
10701130
};
10711131

10721132
let has_default_feature = summary.features().contains_key("default");
1073-
match self.resolve_features.get(id) {
1133+
Ok(match self.resolve_features.get(id) {
10741134
Some(prev) => {
10751135
features.iter().all(|f| prev.contains(f)) &&
10761136
(!use_default || prev.contains("default") ||
10771137
!has_default_feature)
10781138
}
10791139
None => features.is_empty() && (!use_default || !has_default_feature)
1080-
}
1140+
})
10811141
}
10821142

10831143
fn build_deps(&mut self,
@@ -1189,7 +1249,7 @@ impl<'a> Context<'a> {
11891249
.unwrap_or(&[])
11901250
}
11911251

1192-
fn is_active(&mut self, id: &PackageId) -> bool {
1252+
fn is_active(&self, id: &PackageId) -> bool {
11931253
self.activations.get(id.name())
11941254
.and_then(|v| v.get(id.source_id()))
11951255
.map(|v| v.iter().any(|s| s.package_id() == id))

src/cargo/core/summary.rs

+10-4
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,14 @@ struct Inner {
2222
dependencies: Vec<Dependency>,
2323
features: BTreeMap<String, Vec<String>>,
2424
checksum: Option<String>,
25+
links: Option<String>,
2526
}
2627

2728
impl Summary {
2829
pub fn new(pkg_id: PackageId,
2930
dependencies: Vec<Dependency>,
30-
features: BTreeMap<String, Vec<String>>) -> CargoResult<Summary> {
31+
features: BTreeMap<String, Vec<String>>,
32+
links: Option<String>) -> CargoResult<Summary> {
3133
for dep in dependencies.iter() {
3234
if features.get(dep.name()).is_some() {
3335
bail!("Features and dependencies cannot have the \
@@ -66,9 +68,10 @@ impl Summary {
6668
Ok(Summary {
6769
inner: Rc::new(Inner {
6870
package_id: pkg_id,
69-
dependencies: dependencies,
70-
features: features,
71+
dependencies,
72+
features,
7173
checksum: None,
74+
links,
7275
}),
7376
})
7477
}
@@ -82,6 +85,9 @@ impl Summary {
8285
pub fn checksum(&self) -> Option<&str> {
8386
self.inner.checksum.as_ref().map(|s| &s[..])
8487
}
88+
pub fn links(&self) -> Option<&str> {
89+
self.inner.links.as_ref().map(|s| &s[..])
90+
}
8591

8692
pub fn override_id(mut self, id: PackageId) -> Summary {
8793
Rc::make_mut(&mut self.inner).package_id = id;
@@ -94,7 +100,7 @@ impl Summary {
94100
}
95101

96102
pub fn map_dependencies<F>(mut self, f: F) -> Summary
97-
where F: FnMut(Dependency) -> Dependency {
103+
where F: FnMut(Dependency) -> Dependency {
98104
{
99105
let slot = &mut Rc::make_mut(&mut self.inner).dependencies;
100106
let deps = mem::replace(slot, Vec::new());

src/cargo/ops/registry.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ fn transmit(config: &Config,
159159
let ManifestMetadata {
160160
ref authors, ref description, ref homepage, ref documentation,
161161
ref keywords, ref readme, ref repository, ref license, ref license_file,
162-
ref categories, ref badges,
162+
ref categories, ref badges, ref links,
163163
} = *manifest.metadata();
164164
let readme_content = match *readme {
165165
Some(ref readme) => Some(paths::read(&pkg.root().join(readme))?),
@@ -194,6 +194,7 @@ fn transmit(config: &Config,
194194
license: license.clone(),
195195
license_file: license_file.clone(),
196196
badges: badges.clone(),
197+
links: links.clone(),
197198
}, tarball);
198199

199200
match publish {

0 commit comments

Comments
 (0)