refactor(dependencies): use the same PackageFetcher during dependency resolution and major version resolution to reduce network calls to hexpm ♻️

Author: andho

compiler-cli/src/dependencies.rs

@@ -81,9 +81,12 @@ pub fn tree(paths: &ProjectPaths, options: TreeOptions) -> Result<()> {
 fn get_manifest_details(paths: &ProjectPaths) -> Result<(PackageConfig, Manifest)> {
     let runtime = tokio::runtime::Runtime::new().expect("Unable to start Tokio async runtime");
     let config = crate::config::root_config(paths)?;
+    let package_fetcher: Box<dyn dependency::PackageFetcher> =
+        PackageFetcher::boxed(runtime.handle().clone());
     let (_, manifest) = get_manifest(
         paths,
         runtime.handle().clone(),
+        &*package_fetcher,
         Mode::Dev,
         &config,
         &cli::Reporter::new(),
@@ -385,11 +388,14 @@ pub fn download<Telem: Telemetry>(
 
     // Start event loop so we can run async functions to call the Hex API
     let runtime = tokio::runtime::Runtime::new().expect("Unable to start Tokio async runtime");
+    let package_fetcher: Box<dyn dependency::PackageFetcher> =
+        PackageFetcher::boxed(runtime.handle().clone());
 
     // Determine what versions we need
     let (manifest_updated, manifest) = get_manifest(
         paths,
         runtime.handle().clone(),
+        &*package_fetcher,
         mode,
         &config,
         &telemetry,
@@ -419,9 +425,8 @@ pub fn download<Telem: Telemetry>(
     }
     LocalPackages::from_manifest(&manifest).write_to_disc(paths)?;
 
-    let package_fetcher = PackageFetcher::boxed(runtime.handle().clone());
     let major_versions_available =
-        dependency::check_for_major_version_updates(&manifest, package_fetcher);
+        dependency::check_for_major_version_updates(&manifest, &*package_fetcher);
     if !major_versions_available.is_empty() {
         eprintln!(
             "{}",
@@ -665,9 +670,10 @@ impl LocalPackages {
     }
 }
 
-fn get_manifest<Telem: Telemetry>(
+fn get_manifest<'a, Telem: Telemetry>(
     paths: &ProjectPaths,
     runtime: tokio::runtime::Handle,
+    package_fetcher: &'a dyn dependency::PackageFetcher,
     mode: Mode,
     config: &PackageConfig,
     telemetry: &Telem,
@@ -689,7 +695,16 @@ fn get_manifest<Telem: Telemetry>(
     };
 
     if should_resolve {
-        let manifest = resolve_versions(runtime, mode, paths, config, None, telemetry, Vec::new())?;
+        let manifest = resolve_versions(
+            runtime,
+            package_fetcher,
+            mode,
+            paths,
+            config,
+            None,
+            telemetry,
+            Vec::new(),
+        )?;
         return Ok((true, manifest));
     }
 
@@ -710,6 +725,7 @@ fn get_manifest<Telem: Telemetry>(
         tracing::debug!("manifest_outdated");
         let manifest = resolve_versions(
             runtime,
+            package_fetcher,
             mode,
             paths,
             config,
@@ -865,6 +881,7 @@ impl PartialEq for ProvidedPackageSource {
 
 fn resolve_versions<Telem: Telemetry>(
     runtime: tokio::runtime::Handle,
+    package_fetcher: &dyn dependency::PackageFetcher,
     mode: Mode,
     project_paths: &ProjectPaths,
     config: &PackageConfig,
@@ -916,7 +933,7 @@ fn resolve_versions<Telem: Telemetry>(
         .collect();
 
     let resolved = dependency::resolve_versions(
-        PackageFetcher::boxed(runtime.clone()),
+        package_fetcher,
         provided_hex_packages,
         config.name.clone(),
         root_requirements.into_iter(),

compiler-core/src/dependency.rs

@@ -7,7 +7,6 @@ use hexpm::{
     Dependency, Release,
     version::{Range, ResolutionError, Version},
 };
-use itertools::Itertools;
 use pubgrub::{
     solver::{Dependencies, choose_package_with_fewest_versions},
     type_aliases::Map,
@@ -18,7 +17,7 @@ pub type PackageVersions = HashMap<String, Version>;
 type PubgrubRange = pubgrub::range::Range<Version>;
 
 pub fn resolve_versions<Requirements>(
-    package_fetcher: Box<dyn PackageFetcher>,
+    package_fetcher: &dyn PackageFetcher,
     provided_packages: HashMap<EcoString, hexpm::Package>,
     root_name: EcoString,
     dependencies: Requirements,
@@ -70,7 +69,7 @@ where
 pub type PackageVersionDiffs = HashMap<String, (Version, Version)>;
 
 fn resolve_major_versions(
-    package_fetcher: Box<dyn PackageFetcher>,
+    package_fetcher: &dyn PackageFetcher,
     versions: PackageVersions,
 ) -> PackageVersionDiffs {
     versions
@@ -100,20 +99,20 @@ fn resolve_major_versions(
 /// constraints.
 pub fn check_for_major_version_updates(
     manifest: &manifest::Manifest,
-    package_fetcher: Box<dyn PackageFetcher>,
+    package_fetcher: &dyn PackageFetcher,
 ) -> HashMap<String, (Version, Version)> {
     // get the resolved versions of the direct dependencies to check for major
     // version updates.
     let versions = manifest
         .packages
         .iter()
-        .map(|manifest_pkg| (manifest_pkg.name.to_string(), manifest_pkg.version.clone()))
-        .filter(|(name, _)| {
+        .filter(|manifest_package| {
             manifest
                 .requirements
                 .iter()
-                .any(|(required_pkg, _)| name == required_pkg)
+                .any(|(required_pkg, _)| manifest_package.name == *required_pkg)
         })
+        .map(|manifest_pkg| (manifest_pkg.name.to_string(), manifest_pkg.version.clone()))
         .collect();
 
     resolve_major_versions(package_fetcher, versions)
@@ -203,7 +202,7 @@ pub trait PackageFetcher {
 
 struct DependencyProvider<'a> {
     packages: RefCell<HashMap<EcoString, hexpm::Package>>,
-    remote: Box<dyn PackageFetcher>,
+    remote: &'a dyn PackageFetcher,
     locked: &'a HashMap<EcoString, Version>,
     // Map of packages where an exact version was requested
     // We need this because by default pubgrub checks exact version by checking if a version is between the exact
@@ -214,7 +213,7 @@ struct DependencyProvider<'a> {
 
 impl<'a> DependencyProvider<'a> {
     fn new(
-        remote: Box<dyn PackageFetcher>,
+        remote: &'a dyn PackageFetcher,
         mut packages: HashMap<EcoString, hexpm::Package>,
         root: hexpm::Package,
         locked: &'a HashMap<EcoString, Version>,
@@ -612,7 +611,7 @@ mod tests {
     fn resolution_with_locked() {
         let locked_stdlib = ("gleam_stdlib".into(), Version::parse("0.1.0").unwrap());
         let result = resolve_versions(
-            make_remote(),
+            &*make_remote(),
             HashMap::new(),
             "app".into(),
             vec![("gleam_stdlib".into(), Range::new("~> 0.1".into()))].into_iter(),
@@ -630,7 +629,7 @@ mod tests {
     #[test]
     fn resolution_without_deps() {
         let result = resolve_versions(
-            make_remote(),
+            &*make_remote(),
             HashMap::new(),
             "app".into(),
             vec![].into_iter(),
@@ -643,7 +642,7 @@ mod tests {
     #[test]
     fn resolution_1_dep() {
         let result = resolve_versions(
-            make_remote(),
+            &*make_remote(),
             HashMap::new(),
             "app".into(),
             vec![("gleam_stdlib".into(), Range::new("~> 0.1".into()))].into_iter(),
@@ -661,7 +660,7 @@ mod tests {
     #[test]
     fn resolution_with_nested_deps() {
         let result = resolve_versions(
-            make_remote(),
+            &*make_remote(),
             HashMap::new(),
             "app".into(),
             vec![("gleam_otp".into(), Range::new("~> 0.1".into()))].into_iter(),
@@ -682,7 +681,7 @@ mod tests {
     #[test]
     fn resolution_with_optional_deps() {
         let result = resolve_versions(
-            make_remote(),
+            &*make_remote(),
             HashMap::new(),
             "app".into(),
             vec![("package_with_optional".into(), Range::new("~> 0.1".into()))].into_iter(),
@@ -703,7 +702,7 @@ mod tests {
     #[test]
     fn resolution_with_optional_deps_explicitly_provided() {
         let result = resolve_versions(
-            make_remote(),
+            &*make_remote(),
             HashMap::new(),
             "app".into(),
             vec![
@@ -731,7 +730,7 @@ mod tests {
     #[test]
     fn resolution_with_optional_deps_incompatible() {
         let result = resolve_versions(
-            make_remote(),
+            &*make_remote(),
             HashMap::new(),
             "app".into(),
             vec![
@@ -747,7 +746,7 @@ mod tests {
     #[test]
     fn resolution_with_optional_deps_required_by_nested_deps() {
         let result = resolve_versions(
-            make_remote(),
+            &*make_remote(),
             HashMap::new(),
             "app".into(),
             vec![
@@ -779,7 +778,7 @@ mod tests {
     #[test]
     fn resolution_locked_to_older_version() {
         let result = resolve_versions(
-            make_remote(),
+            &*make_remote(),
             HashMap::new(),
             "app".into(),
             vec![("gleam_otp".into(), Range::new("~> 0.1.0".into()))].into_iter(),
@@ -800,7 +799,7 @@ mod tests {
     #[test]
     fn resolution_retired_versions_not_used_by_default() {
         let result = resolve_versions(
-            make_remote(),
+            &*make_remote(),
             HashMap::new(),
             "app".into(),
             vec![("package_with_retired".into(), Range::new("> 0.0.0".into()))].into_iter(),
@@ -822,7 +821,7 @@ mod tests {
     #[test]
     fn resolution_retired_versions_can_be_used_if_locked() {
         let result = resolve_versions(
-            make_remote(),
+            &*make_remote(),
             HashMap::new(),
             "app".into(),
             vec![("package_with_retired".into(), Range::new("> 0.0.0".into()))].into_iter(),
@@ -846,7 +845,7 @@ mod tests {
     #[test]
     fn resolution_prerelease_can_be_selected() {
         let result = resolve_versions(
-            make_remote(),
+            &*make_remote(),
             HashMap::new(),
             "app".into(),
             vec![("gleam_otp".into(), Range::new("~> 0.3.0-rc1".into()))].into_iter(),
@@ -867,7 +866,7 @@ mod tests {
     #[test]
     fn resolution_exact_prerelease_can_be_selected() {
         let result = resolve_versions(
-            make_remote(),
+            &*make_remote(),
             HashMap::new(),
             "app".into(),
             vec![("gleam_otp".into(), Range::new("0.3.0-rc1".into()))].into_iter(),
@@ -888,7 +887,7 @@ mod tests {
     #[test]
     fn resolution_not_found_dep() {
         let _ = resolve_versions(
-            make_remote(),
+            &*make_remote(),
             HashMap::new(),
             "app".into(),
             vec![("unknown".into(), Range::new("~> 0.1".into()))].into_iter(),
@@ -900,7 +899,7 @@ mod tests {
     #[test]
     fn resolution_no_matching_version() {
         let _ = resolve_versions(
-            make_remote(),
+            &*make_remote(),
             HashMap::new(),
             "app".into(),
             vec![("gleam_stdlib".into(), Range::new("~> 99.0".into()))].into_iter(),
@@ -912,7 +911,7 @@ mod tests {
     #[test]
     fn resolution_locked_version_doesnt_satisfy_requirements() {
         let err = resolve_versions(
-            make_remote(),
+            &*make_remote(),
             HashMap::new(),
             "app".into(),
             vec![("gleam_stdlib".into(), Range::new("~> 0.1.0".into()))].into_iter(),
@@ -934,7 +933,7 @@ mod tests {
     #[test]
     fn resolution_with_exact_dep() {
         let result = resolve_versions(
-            make_remote(),
+            &*make_remote(),
             HashMap::new(),
             "app".into(),
             vec![("gleam_stdlib".into(), Range::new("0.1.0".into()))].into_iter(),
@@ -1035,7 +1034,7 @@ mod tests {
                 },
             ],
         };
-        let result = check_for_major_version_updates(&manifest, make_remote());
+        let result = check_for_major_version_updates(&manifest, &*make_remote());
 
         // indirect package with major version will not be in the result even though a major
         // version of it is available