Skip to content

Commit 1d43c19

Browse files
Eh2406Eh2406
committed
move public_dependencys code to methods for better organization
Note: this commit does not change code, just moves it
1 parent 3cd05e2 commit 1d43c19

File tree

2 files changed

+115
-79
lines changed

2 files changed

+115
-79
lines changed

‎src/cargo/core/resolver/context.rs

Lines changed: 109 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@ pub struct Context {
3030
pub links: im_rc::HashMap<InternedString, PackageId>,
3131
/// for each package the list of names it can see,
3232
/// then for each name the exact version that name represents and weather the name is public.
33-
pub public_dependency:
34-
Option<im_rc::HashMap<PackageId, im_rc::HashMap<InternedString, (PackageId, bool)>>>,
33+
pub public_dependency: Option<PublicDependency>,
3534

3635
/// a way to look up for a package in activations what packages required it
3736
/// and all of the exact deps that it fulfilled.
@@ -86,7 +85,7 @@ impl Context {
8685
resolve_features: im_rc::HashMap::new(),
8786
links: im_rc::HashMap::new(),
8887
public_dependency: if check_public_visible_dependencies {
89-
Some(im_rc::HashMap::new())
88+
Some(PublicDependency::new())
9089
} else {
9190
None
9291
},
@@ -240,3 +239,110 @@ impl Context {
240239
graph
241240
}
242241
}
242+
243+
#[derive(Clone, Debug, Default)]
244+
pub struct PublicDependency {
245+
inner: im_rc::HashMap<PackageId, im_rc::HashMap<InternedString, (PackageId, bool)>>,
246+
}
247+
248+
impl PublicDependency {
249+
fn new() -> Self {
250+
PublicDependency {
251+
inner: im_rc::HashMap::new(),
252+
}
253+
}
254+
pub fn add_edge(
255+
&mut self,
256+
candidate_pid: PackageId,
257+
parent_pid: PackageId,
258+
dep: &Dependency,
259+
parents: &Graph<PackageId, Rc<Vec<Dependency>>>,
260+
) {
261+
// one tricky part is that `candidate_pid` may already be active and
262+
// have public dependencies of its own. So we not only need to mark
263+
// `candidate_pid` as visible to its parents but also all of its existing
264+
// public dependencies.
265+
let existing_public_deps: Vec<PackageId> = self
266+
.inner
267+
.get(&candidate_pid)
268+
.iter()
269+
.flat_map(|x| x.values())
270+
.filter_map(|x| if x.1 { Some(&x.0) } else { None })
271+
.chain(&Some(candidate_pid))
272+
.cloned()
273+
.collect();
274+
for c in existing_public_deps {
275+
// for each (transitive) parent that can newly see `t`
276+
let mut stack = vec![(parent_pid, dep.is_public())];
277+
while let Some((p, public)) = stack.pop() {
278+
match self.inner.entry(p).or_default().entry(c.name()) {
279+
im_rc::hashmap::Entry::Occupied(mut o) => {
280+
// the (transitive) parent can already see something by `c`s name, it had better be `c`.
281+
assert_eq!(o.get().0, c);
282+
if o.get().1 {
283+
// The previous time the parent saw `c`, it was a public dependency.
284+
// So all of its parents already know about `c`
285+
// and we can save some time by stopping now.
286+
continue;
287+
}
288+
if public {
289+
// Mark that `c` has now bean seen publicly
290+
o.insert((c, public));
291+
}
292+
}
293+
im_rc::hashmap::Entry::Vacant(v) => {
294+
// The (transitive) parent does not have anything by `c`s name,
295+
// so we add `c`.
296+
v.insert((c, public));
297+
}
298+
}
299+
// if `candidate_pid` was a private dependency of `p` then `p` parents can't see `c` thru `p`
300+
if public {
301+
// if it was public, then we add all of `p`s parents to be checked
302+
for &(grand, ref d) in parents.edges(&p) {
303+
stack.push((grand, d.iter().any(|x| x.is_public())));
304+
}
305+
}
306+
}
307+
}
308+
}
309+
pub fn can_add_edge(
310+
&self,
311+
b_id: PackageId,
312+
parent: PackageId,
313+
dep: &Dependency,
314+
parents: &Graph<PackageId, Rc<Vec<Dependency>>>,
315+
) -> Result<(), (PackageId, ConflictReason)> {
316+
let existing_public_deps: Vec<PackageId> = self
317+
.inner
318+
.get(&b_id)
319+
.iter()
320+
.flat_map(|x| x.values())
321+
.filter_map(|x| if x.1 { Some(&x.0) } else { None })
322+
.chain(&Some(b_id))
323+
.cloned()
324+
.collect();
325+
for t in existing_public_deps {
326+
// for each (transitive) parent that can newly see `t`
327+
let mut stack = vec![(parent, dep.is_public())];
328+
while let Some((p, public)) = stack.pop() {
329+
// TODO: dont look at the same thing more then once
330+
if let Some(o) = self.inner.get(&p).and_then(|x| x.get(&t.name())) {
331+
if o.0 != t {
332+
// the (transitive) parent can already see a different version by `t`s name.
333+
// So, adding `b` will cause `p` to have a public dependency conflict on `t`.
334+
return Err((p, ConflictReason::PublicDependency));
335+
}
336+
}
337+
// if `b` was a private dependency of `p` then `p` parents can't see `t` thru `p`
338+
if public {
339+
// if it was public, then we add all of `p`s parents to be checked
340+
for &(grand, ref d) in parents.edges(&p) {
341+
stack.push((grand, d.iter().any(|x| x.is_public())));
342+
}
343+
}
344+
}
345+
}
346+
Ok(())
347+
}
348+
}

‎src/cargo/core/resolver/mod.rs

Lines changed: 6 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -603,52 +603,7 @@ fn activate(
603603
// and associate dep with that edge
604604
.push(dep.clone());
605605
if let Some(public_dependency) = cx.public_dependency.as_mut() {
606-
// one tricky part is that `candidate_pid` may already be active and
607-
// have public dependencies of its own. So we not only need to mark
608-
// `candidate_pid` as visible to its parents but also all of its existing
609-
// public dependencies.
610-
let existing_public_deps: Vec<PackageId> = public_dependency
611-
.get(&candidate_pid)
612-
.iter()
613-
.flat_map(|x| x.values())
614-
.filter_map(|x| if x.1 { Some(&x.0) } else { None })
615-
.chain(&Some(candidate_pid))
616-
.cloned()
617-
.collect();
618-
for c in existing_public_deps {
619-
// for each (transitive) parent that can newly see `t`
620-
let mut stack = vec![(parent_pid, dep.is_public())];
621-
while let Some((p, public)) = stack.pop() {
622-
match public_dependency.entry(p).or_default().entry(c.name()) {
623-
im_rc::hashmap::Entry::Occupied(mut o) => {
624-
// the (transitive) parent can already see something by `c`s name, it had better be `c`.
625-
assert_eq!(o.get().0, c);
626-
if o.get().1 {
627-
// The previous time the parent saw `c`, it was a public dependency.
628-
// So all of its parents already know about `c`
629-
// and we can save some time by stopping now.
630-
continue;
631-
}
632-
if public {
633-
// Mark that `c` has now bean seen publicly
634-
o.insert((c, public));
635-
}
636-
}
637-
im_rc::hashmap::Entry::Vacant(v) => {
638-
// The (transitive) parent does not have anything by `c`s name,
639-
// so we add `c`.
640-
v.insert((c, public));
641-
}
642-
}
643-
// if `candidate_pid` was a private dependency of `p` then `p` parents can't see `c` thru `p`
644-
if public {
645-
// if it was public, then we add all of `p`s parents to be checked
646-
for &(grand, ref d) in cx.parents.edges(&p) {
647-
stack.push((grand, d.iter().any(|x| x.is_public())));
648-
}
649-
}
650-
}
651-
}
606+
public_dependency.add_edge(candidate_pid, parent_pid, dep, &cx.parents);
652607
}
653608
}
654609

@@ -762,7 +717,7 @@ impl RemainingCandidates {
762717
dep: &Dependency,
763718
parent: PackageId,
764719
) -> Option<(Summary, bool)> {
765-
'main: for b in self.remaining.by_ref() {
720+
for b in self.remaining.by_ref() {
766721
let b_id = b.package_id();
767722
// The `links` key in the manifest dictates that there's only one
768723
// package in a dependency graph, globally, with that particular
@@ -801,35 +756,10 @@ impl RemainingCandidates {
801756
// activated and have public dependants of its own,
802757
// all of witch also need to be checked the same way.
803758
if let Some(public_dependency) = cx.public_dependency.as_ref() {
804-
let existing_public_deps: Vec<PackageId> = public_dependency
805-
.get(&b_id)
806-
.iter()
807-
.flat_map(|x| x.values())
808-
.filter_map(|x| if x.1 { Some(&x.0) } else { None })
809-
.chain(&Some(b_id))
810-
.cloned()
811-
.collect();
812-
for t in existing_public_deps {
813-
// for each (transitive) parent that can newly see `t`
814-
let mut stack = vec![(parent, dep.is_public())];
815-
while let Some((p, public)) = stack.pop() {
816-
// TODO: dont look at the same thing more then once
817-
if let Some(o) = public_dependency.get(&p).and_then(|x| x.get(&t.name())) {
818-
if o.0 != t {
819-
// the (transitive) parent can already see a different version by `t`s name.
820-
// So, adding `b` will cause `p` to have a public dependency conflict on `t`.
821-
conflicting_prev_active.insert(p, ConflictReason::PublicDependency);
822-
continue 'main;
823-
}
824-
}
825-
// if `b` was a private dependency of `p` then `p` parents can't see `t` thru `p`
826-
if public {
827-
// if it was public, then we add all of `p`s parents to be checked
828-
for &(grand, ref d) in cx.parents.edges(&p) {
829-
stack.push((grand, d.iter().any(|x| x.is_public())));
830-
}
831-
}
832-
}
759+
if let Err((p, c)) = public_dependency.can_add_edge(b_id, parent, dep, &cx.parents)
760+
{
761+
conflicting_prev_active.insert(p, c);
762+
continue;
833763
}
834764
}
835765

0 commit comments

Comments
 (0)