Move r and library_paths into the cache again

Author: DavisVaughan

crates/oak_library/src/library_definitions.rs

@@ -10,12 +10,6 @@ use crate::package::Package;
 
 #[derive(Clone, Debug)]
 pub struct LibraryDefinitions {
-    /// Path to an R binary
-    r: PathBuf,
-
-    /// Paths to library directories, i.e. what `base::libPaths()` returns.
-    library_paths: Arc<Vec<PathBuf>>,
-
     cache: Arc<PackageCache>,
 
     definitions: Arc<RwLock<HashMap<String, Option<Arc<PackageDefinitions>>>>>,
@@ -24,9 +18,7 @@ pub struct LibraryDefinitions {
 impl LibraryDefinitions {
     pub fn new(r: PathBuf, library_paths: Vec<PathBuf>) -> anyhow::Result<Self> {
         Ok(Self {
-            r,
-            library_paths: Arc::new(library_paths),
-            cache: Arc::new(PackageCache::new()?),
+            cache: Arc::new(PackageCache::new(r, library_paths)?),
             definitions: Arc::new(RwLock::new(HashMap::new())),
         })
     }
@@ -61,10 +53,7 @@ impl LibraryDefinitions {
     pub fn load_package(&self, package: &Package) -> anyhow::Result<Option<PackageDefinitions>> {
         // Try loading sources from the cache, this may take a moment if sources have to
         // be populated first!
-        let Some(directory) =
-            self.cache
-                .get(&package.description().name, &self.r, &self.library_paths)
-        else {
+        let Some(directory) = self.cache.get(&package.description().name) else {
             // No package sources
             return Ok(None);
         };

crates/oak_sources/src/lib.rs

@@ -112,6 +112,12 @@ const ONE_WEEK: TimeDelta = TimeDelta::weeks(1);
 /// [`clean`]: PackageCache::clean
 #[derive(Debug)]
 pub struct PackageCache {
+    /// Path to an R executable
+    r: PathBuf,
+
+    /// Library paths to consider
+    library_paths: Vec<PathBuf>,
+
     /// On disk cache directory root
     cache_root: file_lock::Filesystem,
 
@@ -144,7 +150,7 @@ struct Metadata {
 }
 
 impl PackageCache {
-    pub fn new() -> anyhow::Result<Self> {
+    pub fn new(r: PathBuf, library_paths: Vec<PathBuf>) -> anyhow::Result<Self> {
         let cache_root = file_lock::Filesystem::new(crate::fs::sources_dir()?);
         cache_root.create_dir()?;
 
@@ -160,6 +166,8 @@ impl PackageCache {
         let cache_root_lock = cache_root.open_ro_shared_create(LOCK_FILENAME)?;
 
         Ok(Self {
+            r,
+            library_paths,
             cache_root,
             cache_root_lock,
             source_unavailable: RwLock::new(HashSet::new()),
@@ -170,13 +178,8 @@ impl PackageCache {
     ///
     /// May spawn an R subprocess or download from CRAN (in a blocking manner) to
     /// generate the sources, so keep that in mind when calling this.
-    pub fn get<P: AsRef<Path>, Q: AsRef<Path>>(
-        &self,
-        package: &str,
-        r: P,
-        library_paths: &[Q],
-    ) -> Option<PathBuf> {
-        match self.get_result(package, r, library_paths) {
+    pub fn get(&self, package: &str) -> Option<PathBuf> {
+        match self.get_result(package) {
             Ok(Some(sources)) => Some(sources),
             Ok(None) => None,
             Err(err) => {
@@ -186,13 +189,8 @@ impl PackageCache {
         }
     }
 
-    fn get_result<P: AsRef<Path>, Q: AsRef<Path>>(
-        &self,
-        package: &str,
-        r: P,
-        library_paths: &[Q],
-    ) -> anyhow::Result<Option<PathBuf>> {
-        let Some(package) = InstalledPackage::find(package, library_paths)? else {
+    fn get_result(&self, package: &str) -> anyhow::Result<Option<PathBuf>> {
+        let Some(package) = InstalledPackage::find(package, &self.library_paths)? else {
             // Not even installed
             return Ok(None);
         };
@@ -216,9 +214,9 @@ impl PackageCache {
         // Write path
         let result = if matches!(package.description().priority, Some(Priority::Base)) {
             // R version to download is the same as the base package version
-            self.try_populate_base(&package.description().version, library_paths)
+            self.try_populate_base(&package.description().version, &self.library_paths)
         } else {
-            self.try_populate(&package, r, library_paths)
+            self.try_populate(&package, &self.r, &self.library_paths)
         };
 
         match result {
@@ -539,22 +537,3 @@ impl PackageCache {
         Ok(())
     }
 }
-
-// // For local testing
-// #[cfg(test)]
-// mod tests {
-//     use std::path::PathBuf;
-//
-//     use crate::PackageCache;
-//
-//     #[test]
-//     fn testit() {
-//         let r_script_path = PathBuf::from("/usr/local/bin/Rscript");
-//         let r_libpaths = vec![
-//             PathBuf::from("/Users/davis/Library/R/arm64/4.5/library"),
-//             PathBuf::from("/Library/Frameworks/R.framework/Versions/4.5-arm64/Resources/library"),
-//         ];
-//         let cache = PackageCache::new(r_script_path, r_libpaths).unwrap();
-//         cache.get("utils");
-//     }
-// }