🐛 Update manifest entry when dependency source changes

CHANGELOG.md

@@ -48,6 +48,11 @@
 
   ([fruno](https://github.com/fruno-bulax/))
 
+- Fixed two bugs that made gleam not update the manifest correctly, causing
+  it to hit hex for version resolution on every operation and quickly reach
+  request limits in large projects.
+  ([fruno](https://github.com/fruno-bulax/))
+
 - The performance of `==` and `!=` has been improved for single-variant custom
   types when compiling to JavaScript. This was done by generating comparison
   code specific to the custom type rather than using the generic equality check

compiler-cli/src/cli.rs

@@ -1,7 +1,7 @@
 use gleam_core::{
     build::Telemetry,
     error::{Error, StandardIoAction},
-    manifest::{Changed, ChangedGit, Resolved},
+    manifest::{Changed, ChangedGit, PackageChanges},
 };
 use hexpm::version::Version;
 use itertools::Itertools as _;
@@ -57,8 +57,8 @@ impl Telemetry for Reporter {
         print_waiting_for_build_directory_lock()
     }
 
-    fn resolved_package_versions(&self, resolved: &Resolved) {
-        print_resolved(resolved)
+    fn resolved_package_versions(&self, changes: &PackageChanges) {
+        print_package_changes(changes)
     }
 }
 
@@ -154,22 +154,22 @@ pub(crate) fn print_running(text: &str) {
     print_colourful_prefix("Running", text)
 }
 
-pub(crate) fn print_resolved(resolved: &Resolved) {
-    for (name, version) in resolved.added.iter().sorted() {
+pub(crate) fn print_package_changes(changes: &PackageChanges) {
+    for (name, version) in changes.added.iter().sorted() {
         print_added(&format!("{name} v{version}"));
     }
-    for Changed { name, old, new } in resolved.changed.iter().sorted_by_key(|p| &p.name) {
+    for Changed { name, old, new } in changes.changed.iter().sorted_by_key(|p| &p.name) {
         print_changed(&format!("{name} v{old} -> v{new}"));
     }
     for ChangedGit {
         name,
         old_hash,
         new_hash,
-    } in resolved.changed_git.iter().sorted_by_key(|p| &p.name)
+    } in changes.changed_git.iter().sorted_by_key(|p| &p.name)
     {
         print_changed(&format!("{name} {old_hash} -> {new_hash}"));
     }
-    for name in resolved.removed.iter().sorted() {
+    for name in changes.removed.iter().sorted() {
         print_removed(name);
     }
 }

compiler-cli/src/dependencies.rs

@@ -20,7 +20,7 @@ use gleam_core::{
     error::{FileIoAction, FileKind, ShellCommandFailureReason, StandardIoAction},
     hex::{self, HEXPM_PUBLIC_KEY},
     io::{HttpClient as _, TarUnpacker, WrappedReader},
-    manifest::{Base16Checksum, Manifest, ManifestPackage, ManifestPackageSource, Resolved},
+    manifest::{Base16Checksum, Manifest, ManifestPackage, ManifestPackageSource, PackageChanges},
     paths::ProjectPaths,
     requirement::Requirement,
 };
@@ -291,10 +291,10 @@ pub fn cleanup<Telem: Telemetry>(paths: &ProjectPaths, telemetry: Telem) -> Resu
     write_manifest_to_disc(paths, &manifest)?;
     LocalPackages::from_manifest(&manifest).write_to_disc(paths)?;
 
-    let resolved = Resolved::with_updates(&old_manifest, manifest);
-    telemetry.resolved_package_versions(&resolved);
+    let changes = PackageChanges::between_manifests(&old_manifest, &manifest);
+    telemetry.resolved_package_versions(&changes);
 
-    Ok(resolved.manifest)
+    Ok(manifest)
 }
 
 /// Remove requirements and unneeded packages from manifest that are no longer present in config.

compiler-cli/src/dependencies/dependency_manager.rs

@@ -5,7 +5,7 @@ use gleam_core::{
     build::{Mode, Telemetry},
     config::PackageConfig,
     dependency,
-    manifest::{Manifest, ManifestPackageSource, Resolved},
+    manifest::{Manifest, ManifestPackageSource, PackageChanges, Resolved},
     paths::ProjectPaths,
     requirement::Requirement,
 };
@@ -87,24 +87,24 @@ where
         let existing_manifest = read_manifest_from_disc(paths)?;
 
         // If we have been asked not to use the manifest then
-        let manifest_for_resolver = match self.use_manifest {
-            UseManifest::No => None,
+        let (requirements_changed, manifest_for_resolver) = match self.use_manifest {
+            UseManifest::No => (true, None),
             UseManifest::Yes => {
+                let same_requirements = is_same_requirements(
+                    &existing_manifest.requirements,
+                    &config.all_direct_dependencies()?,
+                    paths.root(),
+                )?;
+
                 // If the manifest is to be used and the requirements have not changed then there's
                 // no point in performing resolution, it'll always result in the same versions
                 // already specified in the manifest.
-                if packages_to_update.is_empty()
-                    && is_same_requirements(
-                        &existing_manifest.requirements,
-                        &config.all_direct_dependencies()?,
-                        paths.root(),
-                    )?
-                {
+                if packages_to_update.is_empty() && same_requirements {
                     return Ok(Resolved::no_change(existing_manifest));
                 }
 
                 // Otherwise, use the manifest to inform resolution.
-                Some(&existing_manifest)
+                (!same_requirements, Some(&existing_manifest))
             }
         };
 
@@ -115,7 +115,12 @@ where
             manifest_for_resolver,
             packages_to_update,
         )?;
-        Ok(Resolved::with_updates(&existing_manifest, new_manifest))
+        let resolved = Resolved {
+            package_changes: PackageChanges::between_manifests(&existing_manifest, &new_manifest),
+            manifest: new_manifest,
+            requirements_changed,
+        };
+        Ok(resolved)
     }
 
     pub fn resolve_and_download_versions(
@@ -177,7 +182,8 @@ where
         LocalPackages::from_manifest(&resolved.manifest).write_to_disc(paths)?;
 
         // Display the changes in versions to the user.
-        self.telemetry.resolved_package_versions(&resolved);
+        self.telemetry
+            .resolved_package_versions(&resolved.package_changes);
 
         // If requested to do so, check if there are major upgrades that could be performed with
         // more relaxed version requirements, and inform the user if so.

compiler-core/src/build/telemetry.rs

@@ -3,13 +3,16 @@ use std::{
     time::{Duration, Instant},
 };
 
-use crate::{Warning, manifest::Resolved};
+use crate::{
+    Warning,
+    manifest::{PackageChanges, Resolved},
+};
 
 pub trait Telemetry: Debug {
     fn waiting_for_build_directory_lock(&self);
     fn running(&self, name: &str);
     fn resolving_package_versions(&self);
-    fn resolved_package_versions(&self, resolved: &Resolved);
+    fn resolved_package_versions(&self, changes: &PackageChanges);
     fn downloading_package(&self, name: &str);
     fn packages_downloaded(&self, start: Instant, count: usize);
     fn compiled_package(&self, duration: Duration);
@@ -31,5 +34,5 @@ impl Telemetry for NullTelemetry {
     fn checked_package(&self, _duration: Duration) {}
     fn checking_package(&self, _name: &str) {}
     fn packages_downloaded(&self, _start: Instant, _count: usize) {}
-    fn resolved_package_versions(&self, _resolved: &Resolved) {}
+    fn resolved_package_versions(&self, _changes: &PackageChanges) {}
 }