feat: improve lockfile v4 to store normalized version constraints and be more terse (#25247)

Stores normalized version constraints in the lockfile, which will
improve reproducibility and will fix a bug with duplicate specifiers
ending up in the lockfile. Also, gets rid of some duplicate data in the
specifiers area of the lockfile.

1 files changed, 74 insertions, 21 deletions

diff --git a/cli/graph_util.rs b/cli/graph_util.rs
index 6f0e6acd9..c8432b67e 100644
--- a/cli/graph_util.rs
+++ b/cli/graph_util.rs
@@ -44,10 +44,13 @@ use deno_graph::SpecifierError;
 use deno_runtime::deno_fs::FileSystem;
 use deno_runtime::deno_node;
 use deno_runtime::deno_permissions::PermissionsContainer;
+use deno_semver::jsr::JsrDepPackageReq;
 use deno_semver::package::PackageNv;
-use deno_semver::package::PackageReq;
+use deno_semver::Version;
 use import_map::ImportMapError;
 use std::collections::HashSet;
+use std::error::Error;
+use std::ops::Deref;
 use std::path::PathBuf;
 use std::sync::Arc;
 
@@ -110,7 +113,7 @@ pub fn graph_valid(
         ModuleGraphError::ModuleError(error) => {
           enhanced_lockfile_error_message(error)
             .or_else(|| enhanced_sloppy_imports_error_message(fs, error))
-            .unwrap_or_else(|| format!("{}", error))
+            .unwrap_or_else(|| format_deno_graph_error(error))
         }
       };
 
@@ -164,7 +167,10 @@ pub fn graph_valid(
   } else {
     // finally surface the npm resolution result
     if let Err(err) = &graph.npm_dep_graph_result {
-      return Err(custom_error(get_error_class_name(err), format!("{}", err)));
+      return Err(custom_error(
+        get_error_class_name(err),
+        format_deno_graph_error(err.as_ref().deref()),
+      ));
     }
     Ok(())
   }
@@ -463,7 +469,7 @@ impl ModuleGraphBuilder {
           .content
           .packages
           .jsr
-          .get(&package_nv.to_string())
+          .get(package_nv)
           .map(|s| LoaderChecksum::new(s.integrity.clone()))
       }
 
@@ -477,7 +483,7 @@ impl ModuleGraphBuilder {
         self
           .0
           .lock()
-          .insert_package(package_nv.to_string(), checksum.into_string());
+          .insert_package(package_nv.clone(), checksum.into_string());
       }
     }
 
@@ -556,16 +562,21 @@ impl ModuleGraphBuilder {
             }
           }
         }
-        for (key, value) in &lockfile.content.packages.specifiers {
-          if let Some(key) = key
-            .strip_prefix("jsr:")
-            .and_then(|key| PackageReq::from_str(key).ok())
-          {
-            if let Some(value) = value
-              .strip_prefix("jsr:")
-              .and_then(|value| PackageNv::from_str(value).ok())
-            {
-              graph.packages.add_nv(key, value);
+        for (req_dep, value) in &lockfile.content.packages.specifiers {
+          match req_dep.kind {
+            deno_semver::package::PackageKind::Jsr => {
+              if let Ok(version) = Version::parse_standard(value) {
+                graph.packages.add_nv(
+                  req_dep.req.clone(),
+                  PackageNv {
+                    name: req_dep.req.name.clone(),
+                    version,
+                  },
+                );
+              }
+            }
+            deno_semver::package::PackageKind::Npm => {
+              // ignore
             }
           }
         }
@@ -603,16 +614,15 @@ impl ModuleGraphBuilder {
         if has_jsr_package_mappings_changed {
           for (from, to) in graph.packages.mappings() {
             lockfile.insert_package_specifier(
-              format!("jsr:{}", from),
-              format!("jsr:{}", to),
+              JsrDepPackageReq::jsr(from.clone()),
+              to.version.to_string(),
             );
           }
         }
         // jsr packages
         if has_jsr_package_deps_changed {
-          for (name, deps) in graph.packages.packages_with_deps() {
-            lockfile
-              .add_package_deps(&name.to_string(), deps.map(|s| s.to_string()));
+          for (nv, deps) in graph.packages.packages_with_deps() {
+            lockfile.add_package_deps(nv, deps.cloned());
           }
         }
       }
@@ -726,7 +736,7 @@ pub fn error_for_any_npm_specifier(
 
 /// Adds more explanatory information to a resolution error.
 pub fn enhanced_resolution_error_message(error: &ResolutionError) -> String {
-  let mut message = format!("{error}");
+  let mut message = format_deno_graph_error(error);
 
   if let Some(specifier) = get_resolution_error_bare_node_specifier(error) {
     if !*DENO_DISABLE_PEDANTIC_NODE_WARNINGS {
@@ -1022,6 +1032,49 @@ impl deno_graph::source::JsrUrlProvider for CliJsrUrlProvider {
   }
 }
 
+// todo(dsherret): We should change ModuleError to use thiserror so that
+// we don't need to do this.
+fn format_deno_graph_error(err: &dyn Error) -> String {
+  use std::fmt::Write;
+
+  let mut message = format!("{}", err);
+  let mut maybe_source = err.source();
+
+  if maybe_source.is_some() {
+    let mut past_message = message.clone();
+    let mut count = 0;
+    let mut display_count = 0;
+    while let Some(source) = maybe_source {
+      let current_message = format!("{}", source);
+      maybe_source = source.source();
+
+      // sometimes an error might be repeated due to
+      // being boxed multiple times in another AnyError
+      if current_message != past_message {
+        write!(message, "\n    {}: ", display_count,).unwrap();
+        for (i, line) in current_message.lines().enumerate() {
+          if i > 0 {
+            write!(message, "\n       {}", line).unwrap();
+          } else {
+            write!(message, "{}", line).unwrap();
+          }
+        }
+        display_count += 1;
+      }
+
+      if count > 8 {
+        write!(message, "\n    {}: ...", count).unwrap();
+        break;
+      }
+
+      past_message = current_message;
+      count += 1;
+    }
+  }
+
+  message
+}
+
 #[cfg(test)]
 mod test {
   use std::sync::Arc;