Auto merge of #6653 - Eh2406:min-pub-dep, r=alexcrichton

part of the infrastructure for public & private dependencies in the resolver

This is part of my work on public & private dependencies in the resolver from #6129. As discussed there the proptest fuzzers are happy to find exponential blow up with all the back jumping strategies I have tried. So this PR does not have a back jumping strategie nor does it have the proptest fuzzers generating public dependencies. These will both need to change for the feature to stabilize. In the meantime it gives the correct results on the cases it can handle.

With rust-lang/rust#57586 landed there is a lot of work to do on Cargos front end. Adding a UI for this, passing the relevant things to rustc, passing it to crates.io, passing it to cargo-metadata. This is good enough to allow that work to proceed.

Author: bors

src/cargo/core/dependency.rs

@@ -40,6 +40,7 @@ struct Inner {
     explicit_name_in_toml: Option<InternedString>,
 
     optional: bool,
+    public: bool,
     default_features: bool,
     features: Vec<InternedString>,
 
@@ -217,6 +218,7 @@ impl Dependency {
                 kind: Kind::Normal,
                 only_match_name: true,
                 optional: false,
+                public: false,
                 features: Vec::new(),
                 default_features: true,
                 specified_req: false,
@@ -293,6 +295,16 @@ impl Dependency {
         self.inner.kind
     }
 
+    pub fn is_public(&self) -> bool {
+        self.inner.public
+    }
+
+    /// Sets whether the dependency is public.
+    pub fn set_public(&mut self, public: bool) -> &mut Dependency {
+        Rc::make_mut(&mut self.inner).public = public;
+        self
+    }
+
     pub fn specified_req(&self) -> bool {
         self.inner.specified_req
     }

src/cargo/core/resolver/conflict_cache.rs

@@ -2,15 +2,15 @@ use std::collections::{BTreeMap, HashMap, HashSet};
 
 use log::trace;
 
-use super::types::ConflictReason;
+use super::types::{ConflictMap, ConflictReason};
 use crate::core::resolver::Context;
 use crate::core::{Dependency, PackageId};
 
 /// This is a trie for storing a large number of sets designed to
 /// efficiently see if any of the stored sets are a subset of a search set.
 enum ConflictStoreTrie {
     /// One of the stored sets.
-    Leaf(BTreeMap<PackageId, ConflictReason>),
+    Leaf(ConflictMap),
     /// A map from an element to a subtrie where
     /// all the sets in the subtrie contains that element.
     Node(BTreeMap<PackageId, ConflictStoreTrie>),
@@ -23,7 +23,7 @@ impl ConflictStoreTrie {
         &self,
         cx: &Context,
         must_contain: Option<PackageId>,
-    ) -> Option<&BTreeMap<PackageId, ConflictReason>> {
+    ) -> Option<&ConflictMap> {
         match self {
             ConflictStoreTrie::Leaf(c) => {
                 if must_contain.is_none() {
@@ -57,18 +57,14 @@ impl ConflictStoreTrie {
         }
     }
 
-    fn insert(
-        &mut self,
-        mut iter: impl Iterator<Item = PackageId>,
-        con: BTreeMap<PackageId, ConflictReason>,
-    ) {
+    fn insert(&mut self, mut iter: impl Iterator<Item = PackageId>, con: ConflictMap) {
         if let Some(pid) = iter.next() {
             if let ConflictStoreTrie::Node(p) = self {
                 p.entry(pid)
                     .or_insert_with(|| ConflictStoreTrie::Node(BTreeMap::new()))
                     .insert(iter, con);
             }
-            // Else, we already have a subset of this in the `ConflictStore`.
+        // Else, we already have a subset of this in the `ConflictStore`.
         } else {
             // We are at the end of the set we are adding, there are three cases for what to do
             // next:
@@ -147,7 +143,7 @@ impl ConflictCache {
         cx: &Context,
         dep: &Dependency,
         must_contain: Option<PackageId>,
-    ) -> Option<&BTreeMap<PackageId, ConflictReason>> {
+    ) -> Option<&ConflictMap> {
         let out = self
             .con_from_dep
             .get(dep)?
@@ -161,18 +157,19 @@ impl ConflictCache {
         }
         out
     }
-    pub fn conflicting(
-        &self,
-        cx: &Context,
-        dep: &Dependency,
-    ) -> Option<&BTreeMap<PackageId, ConflictReason>> {
+    pub fn conflicting(&self, cx: &Context, dep: &Dependency) -> Option<&ConflictMap> {
         self.find_conflicting(cx, dep, None)
     }
 
     /// Adds to the cache a conflict of the form:
     /// `dep` is known to be unresolvable if
     /// all the `PackageId` entries are activated.
-    pub fn insert(&mut self, dep: &Dependency, con: &BTreeMap<PackageId, ConflictReason>) {
+    pub fn insert(&mut self, dep: &Dependency, con: &ConflictMap) {
+        if con.values().any(|c| *c == ConflictReason::PublicDependency) {
+            // TODO: needs more info for back jumping
+            // for now refuse to cache it.
+            return;
+        }
         self.con_from_dep
             .entry(dep.clone())
             .or_insert_with(|| ConflictStoreTrie::Node(BTreeMap::new()))

src/cargo/core/resolver/context.rs

@@ -1,4 +1,4 @@
-use std::collections::{BTreeMap, HashMap, HashSet};
+use std::collections::{HashMap, HashSet};
 use std::rc::Rc;
 
 // "ensure" seems to require "bail" be in scope (macro hygiene issue?).
@@ -12,7 +12,9 @@ use crate::util::CargoResult;
 use crate::util::Graph;
 
 use super::errors::ActivateResult;
-use super::types::{ConflictReason, DepInfo, GraphNode, Method, RcList, RegistryQueryer};
+use super::types::{
+    ConflictMap, ConflictReason, DepInfo, GraphNode, Method, RcList, RegistryQueryer,
+};
 
 pub use super::encode::{EncodableDependency, EncodablePackageId, EncodableResolve};
 pub use super::encode::{Metadata, WorkspaceResolve};
@@ -25,8 +27,20 @@ pub use super::resolve::Resolve;
 #[derive(Clone)]
 pub struct Context {
     pub activations: Activations,
+    /// list the features that are activated for each package
     pub resolve_features: im_rc::HashMap<PackageId, Rc<HashSet<InternedString>>>,
+    /// get the package that will be linking to a native library by its links attribute
     pub links: im_rc::HashMap<InternedString, PackageId>,
+    /// for each package the list of names it can see,
+    /// then for each name the exact version that name represents and weather the name is public.
+    pub public_dependency:
+        Option<im_rc::HashMap<PackageId, im_rc::HashMap<InternedString, (PackageId, bool)>>>,
+
+    // This is somewhat redundant with the `resolve_graph` that stores the same data,
+    //   but for querying in the opposite order.
+    /// a way to look up for a package in activations what packages required it
+    /// and all of the exact deps that it fulfilled.
+    pub parents: Graph<PackageId, Rc<Vec<Dependency>>>,
 
     // These are two cheaply-cloneable lists (O(1) clone) which are effectively
     // hash maps but are built up as "construction lists". We'll iterate these
@@ -38,14 +52,21 @@ pub struct Context {
     pub warnings: RcList<String>,
 }
 
+/// list all the activated versions of a particular crate name from a source
 pub type Activations = im_rc::HashMap<(InternedString, SourceId), Rc<Vec<Summary>>>;
 
 impl Context {
-    pub fn new() -> Context {
+    pub fn new(check_public_visible_dependencies: bool) -> Context {
         Context {
             resolve_graph: RcList::new(),
             resolve_features: im_rc::HashMap::new(),
             links: im_rc::HashMap::new(),
+            public_dependency: if check_public_visible_dependencies {
+                Some(im_rc::HashMap::new())
+            } else {
+                None
+            },
+            parents: Graph::new(),
             resolve_replacements: RcList::new(),
             activations: im_rc::HashMap::new(),
             warnings: RcList::new(),
@@ -147,7 +168,7 @@ impl Context {
     pub fn is_conflicting(
         &self,
         parent: Option<PackageId>,
-        conflicting_activations: &BTreeMap<PackageId, ConflictReason>,
+        conflicting_activations: &ConflictMap,
     ) -> bool {
         conflicting_activations
             .keys()

src/cargo/core/resolver/errors.rs

@@ -1,4 +1,3 @@
-use std::collections::BTreeMap;
 use std::fmt;
 
 use crate::core::{Dependency, PackageId, Registry, Summary};
@@ -8,7 +7,7 @@ use failure::{Error, Fail};
 use semver;
 
 use super::context::Context;
-use super::types::{Candidate, ConflictReason};
+use super::types::{Candidate, ConflictMap, ConflictReason};
 
 /// Error during resolution providing a path of `PackageId`s.
 pub struct ResolveError {
@@ -73,16 +72,15 @@ pub(super) fn activation_error(
     registry: &mut dyn Registry,
     parent: &Summary,
     dep: &Dependency,
-    conflicting_activations: &BTreeMap<PackageId, ConflictReason>,
+    conflicting_activations: &ConflictMap,
     candidates: &[Candidate],
     config: Option<&Config>,
 ) -> ResolveError {
-    let graph = cx.graph();
     let to_resolve_err = |err| {
         ResolveError::new(
             err,
-            graph
-                .path_to_top(&parent.package_id())
+            cx.parents
+                .path_to_bottom(&parent.package_id())
                 .into_iter()
                 .cloned()
                 .collect(),
@@ -92,7 +90,9 @@ pub(super) fn activation_error(
     if !candidates.is_empty() {
         let mut msg = format!("failed to select a version for `{}`.", dep.package_name());
         msg.push_str("\n    ... required by ");
-        msg.push_str(&describe_path(&graph.path_to_top(&parent.package_id())));
+        msg.push_str(&describe_path(
+            &cx.parents.path_to_bottom(&parent.package_id()),
+        ));
 
         msg.push_str("\nversions that meet the requirements `");
         msg.push_str(&dep.version_req().to_string());
@@ -123,7 +123,7 @@ pub(super) fn activation_error(
                 msg.push_str(link);
                 msg.push_str("` as well:\n");
             }
-            msg.push_str(&describe_path(&graph.path_to_top(p)));
+            msg.push_str(&describe_path(&cx.parents.path_to_bottom(p)));
         }
 
         let (features_errors, other_errors): (Vec<_>, Vec<_>) = other_errors
@@ -154,7 +154,7 @@ pub(super) fn activation_error(
 
         for &(p, _) in other_errors.iter() {
             msg.push_str("\n\n  previously selected ");
-            msg.push_str(&describe_path(&graph.path_to_top(p)));
+            msg.push_str(&describe_path(&cx.parents.path_to_bottom(p)));
         }
 
         msg.push_str("\n\nfailed to select a version for `");
@@ -204,7 +204,9 @@ pub(super) fn activation_error(
             registry.describe_source(dep.source_id()),
         );
         msg.push_str("required by ");
-        msg.push_str(&describe_path(&graph.path_to_top(&parent.package_id())));
+        msg.push_str(&describe_path(
+            &cx.parents.path_to_bottom(&parent.package_id()),
+        ));
 
         // If we have a path dependency with a locked version, then this may
         // indicate that we updated a sub-package and forgot to run `cargo
@@ -265,7 +267,9 @@ pub(super) fn activation_error(
             msg.push_str("\n");
         }
         msg.push_str("required by ");
-        msg.push_str(&describe_path(&graph.path_to_top(&parent.package_id())));
+        msg.push_str(&describe_path(
+            &cx.parents.path_to_bottom(&parent.package_id()),
+        ));
 
         msg
     };