Check for source in resolution as well

Author: Keats

src/http.rs

@@ -7,7 +7,7 @@ use crate::fs::untar_archive;
 use ureq::http::{HeaderName, HeaderValue};
 use ureq::Agent;
 
-use ureq::tls::{TlsConfig, RootCerts};
+use ureq::tls::{RootCerts, TlsConfig};
 
 /// Downloads a remote content to the given writer.
 /// Returns the number of bytes written to the writer, 0 for a 404 or an empty 200
@@ -16,18 +16,17 @@ pub fn download<W: Write>(
     writer: &mut W,
     headers: Vec<(&str, String)>,
 ) -> Result<u64, HttpError> {
-
     let agent = Agent::config_builder()
-    .tls_config(
-        TlsConfig::builder()
-            .root_certs(RootCerts::PlatformVerifier)
-            .build()
-    )
-    .timeout_global(Some(Duration::from_secs(200)))
-    .build()
-    .new_agent();
-
-    let mut request_builder = agent.get(url); 
+        .tls_config(
+            TlsConfig::builder()
+                .root_certs(RootCerts::PlatformVerifier)
+                .build(),
+        )
+        .timeout_global(Some(Duration::from_secs(200)))
+        .build()
+        .new_agent();
+
+    let mut request_builder = agent.get(url);
 
     {
         let req_headers = request_builder.headers_mut().unwrap();

src/lockfile.rs

@@ -157,7 +157,14 @@ impl Source {
             (
                 Source::Repository { repository: r1 },
                 ConfigDependency::Detailed { repository: r2, .. },
-            ) => Some(r1) == r2.as_ref(),
+            ) => {
+                // We are only comparing the repositories here
+                if let Some(r) = r2 {
+                    r == r1
+                } else {
+                    true
+                }
+            },
             (Source::Repository { .. }, ConfigDependency::Simple(..)) => true,
             _ => false,
         }
@@ -302,6 +309,23 @@ impl LockedPackage {
     pub fn install_suggests(&self) -> bool {
         !self.suggests.is_empty()
     }
+
+    pub fn is_matching(&self, dep: &ConfigDependency, repo_urls: &HashSet<&str>) -> bool {
+        if dep.install_suggestions() && !self.install_suggests() {
+            return false;
+        }
+
+        match self.source {
+            Source::Repository { ref repository } => {
+                if !repo_urls.contains(repository.as_str()) {
+                    return false;
+                }
+            }
+            _ => (),
+        }
+
+        true
+    }
 }
 
 #[derive(Debug, Deserialize, Clone, PartialEq)]
@@ -416,7 +440,7 @@ impl Lockfile {
     ) -> Option<&LockedPackage> {
         if let Some(p) = self.packages.iter().find(|p| p.name == name) {
             if let Some(d) = dep {
-                if p.source.is_matching(d) && p.install_suggests() == d.install_suggestions() {
+                if p.source.is_matching(d) {
                     return Some(p);
                 }
             } else {
@@ -449,17 +473,9 @@ impl Lockfile {
         let repo_urls = repos.iter().map(|x| x.url()).collect::<HashSet<_>>();
         for d in deps {
             if let Some(pkg) = self.get_package(d.name(), Some(d)) {
-                if d.install_suggestions() && !pkg.install_suggests() {
+                if !pkg.is_matching(d, &repo_urls) {
                     return false;
                 }
-                match pkg.source {
-                    Source::Repository { ref repository } => {
-                        if !repo_urls.contains(repository.as_str()) {
-                            return false;
-                        }
-                    }
-                    _ => ()
-                }
             } else {
                 return false;
             }

src/resolver/mod.rs

@@ -377,6 +377,11 @@ impl<'d> Resolver<'d> {
     ) -> Resolution<'d> {
         let mut result = Resolution::default();
         let mut processed = HashSet::with_capacity(dependencies.len() * 10);
+        let repo_urls = self
+            .repositories
+            .iter()
+            .map(|(x, _)| x.url.as_str())
+            .collect::<HashSet<_>>();
         let mut queue: VecDeque<_> = dependencies
             .iter()
             .map(|d| QueueItem {
@@ -388,9 +393,10 @@ impl<'d> Resolver<'d> {
                 parent: None,
                 remote: None,
                 local_path: d.local_path(),
-                matching_in_lockfile: self
-                    .lockfile
-                    .map(|l| l.get_package(d.name(), Some(d)).is_some()),
+                matching_in_lockfile: self.lockfile.and_then(|l| {
+                    l.get_package(d.name(), Some(d))
+                        .map(|p| p.is_matching(d, &repo_urls))
+                }),
             })
             .collect();
 
@@ -657,7 +663,7 @@ mod tests {
             }
             res
         } else {
-            let mut repo = RepositoryDatabase::new("");
+            let mut repo = RepositoryDatabase::new("http://cran");
             repo.parse_source(parts[1]);
             vec![(repo, false)]
         };

src/resolver/snapshots/rv__resolver__tests__different_repo_from_lockfile.txt.snap

@@ -0,0 +1,5 @@
+---
+source: src/resolver/mod.rs
+expression: out
+---
+R6=2.5.1 (repository(url: http://test), type=binary, path='', from_lockfile=false, from_remote=false)

src/resolver/snapshots/rv__resolver__tests__simple_no_lockfile.txt.snap

@@ -2,4 +2,4 @@
 source: src/resolver/mod.rs
 expression: out
 ---
-R6=2.5.1 (repository(url: ), type=source, path='', from_lockfile=false, from_remote=false)
+R6=2.5.1 (repository(url: http://cran), type=source, path='', from_lockfile=false, from_remote=false)

src/resolver/snapshots/rv__resolver__tests__unmet_version_req.txt.snap

@@ -2,6 +2,6 @@
 source: src/resolver/mod.rs
 expression: out
 ---
-unmet-version-req=0.0.1 (repository(url: ), type=source, path='', from_lockfile=false, from_remote=false)
+unmet-version-req=0.0.1 (repository(url: http://cran), type=source, path='', from_lockfile=false, from_remote=false)
 --- unresolved --- 
 zzlite (>= 1.0)  [required by: unmet-version-req]

src/tests/resolution/different_repo_from_lockfile.txt

@@ -0,0 +1,22 @@
+# even with a higher version in the package db it should prefer the lockfile
+[project]
+name = "test"
+r_version = "4.4"
+repositories = []
+# This will fetch it from the 'test' repository even though it's in the lockfile with a different source
+# since the cran source is not in the config
+dependencies = [
+    "R6",
+]
+---
+repos = [{name = "test", binary = "cran-binary", source = "posit-src", force_source = false}]
+---
+version = 1
+r_version = "4.4"
+
+[[packages]]
+name = "R6"
+version = "2.5.1"
+source = { repository = "http://cran" }
+force_source = false
+dependencies = []

src/tests/resolution/suggestions.txt

@@ -3,7 +3,7 @@ name = "test"
 r_version = "4.4"
 repositories = []
 # The lockfile is generated without install_suggestions = true
-# so a new call to the resolution should
+# so a new call to the resolution should invalidate the lockfile
 dependencies = [
     { name = "A3", install_suggestions = true }
 ]