Improve messages

- restore ability to override a package locally
- cut down on some repeated logic
- more detailed errors

crates/typst-kit/src/package.rs

@@ -87,9 +87,15 @@ impl PackageStorage {
     ) -> PackageResult<PathBuf> {
         let subdir = format!("{}/{}/{}", spec.namespace, spec.name, spec.version);
 
-        // QUESTION: is it sometimes the case that both package_path
-        // and package_cache_path are Some? Is it a problem if we change
-        // this order?
+        // By default, search for the package locally
+        if let Some(packages_dir) = &self.package_path {
+            let dir = packages_dir.join(&subdir);
+            if dir.exists() {
+                return Ok(dir);
+            }
+        }
+
+        // As a fallback, look into the cache and possibly download from network.
         if let Some(cache_dir) = &self.package_cache_path {
             let dir = cache_dir.join(&subdir);
             if dir.exists() {
@@ -102,38 +108,42 @@ impl PackageStorage {
             }
         }
 
-        // In all paths from now, we either succeed or fail with a NotFound.
+        // None of the strategies above found the package, so all code paths
+        // from now on fail. The rest of the function is only to determine the
+        // cause of the failure.
         let not_found = |msg| Err(PackageError::NotFound(spec.clone(), msg));
 
         let Some(packages_dir) = &self.package_path else {
-            // QUESTION: when is this code path hit?
-            return not_found(None);
+            return not_found(eco_format!("cannot access local package storage"));
         };
-        // We try the fast path
-        let dir = packages_dir.join(&subdir);
-        if dir.exists() {
-            return Ok(dir);
-        }
         // Fast path failed. Instead we try to diagnose the issue by going
         // down the subdirectories of the expected path one at a time.
         let namespace_dir = packages_dir.join(format!("{}", spec.namespace));
         if !namespace_dir.exists() {
-            return not_found(Some(eco_format!("did not find namespace '{}' while searching at {} for", spec.namespace, namespace_dir.display())));
+            return not_found(eco_format!(
+                "namespace @{} should be located at {}",
+                spec.namespace,
+                namespace_dir.display()
+            ));
         }
         let package_dir = namespace_dir.join(format!("{}", spec.name));
         if !package_dir.exists() {
-            return not_found(Some(eco_format!("did not find package '{}' while searching at {} for", spec.name, package_dir.display())));
+            return not_found(eco_format!(
+                "{} does not have package '{}'",
+                namespace_dir.display(),
+                spec.name
+            ));
         }
-        let version_dir = package_dir.join(format!("{}", spec.version));
-        if !version_dir.exists() {
-            return not_found(Some(eco_format!("did not find version {} while searching at {} for", spec.version, version_dir.display())));
-        }
-        // We'll never hit this path as long as subdir is indeed namespace/name/version.
-        // What's Typst's policy on `unreachable`?
-        unreachable!()
+        let latest = self.determine_latest_version(&spec.versionless()).ok();
+        Err(PackageError::VersionNotFound(
+            spec.clone(),
+            latest,
+            eco_format!("{}", namespace_dir.display()),
+        ))
     }
 
     /// Tries to determine the latest version of a package.
+    /// TODO improve here too
     pub fn determine_latest_version(
         &self,
         spec: &VersionlessPackageSpec,
@@ -195,18 +205,26 @@ impl PackageStorage {
     ) -> PackageResult<PathBuf> {
         assert_eq!(spec.namespace, DEFAULT_NAMESPACE);
 
-        let url = format!(
-            "{DEFAULT_REGISTRY}/{DEFAULT_NAMESPACE}/{}-{}.tar.gz",
-            spec.name, spec.version
-        );
+        let namespace_url = format!("{DEFAULT_REGISTRY}/{DEFAULT_NAMESPACE}");
+        let url = format!("{namespace_url}/{}-{}.tar.gz", spec.name, spec.version);
 
         let data = match self.downloader.download_with_progress(&url, progress) {
             Ok(data) => data,
             Err(ureq::Error::Status(404, _)) => {
                 if let Ok(version) = self.determine_latest_version(&spec.versionless()) {
-                    return Err(PackageError::VersionNotFound(spec.clone(), version));
+                    return Err(PackageError::VersionNotFound(
+                        spec.clone(),
+                        Some(version),
+                        eco_format!("{namespace_url}"),
+                    ));
                 } else {
-                    return Err(PackageError::NotFound(spec.clone(), Some(eco_format!("attempted to download"))));
+                    return Err(PackageError::NotFound(
+                        spec.clone(),
+                        eco_format!(
+                            "{namespace_url} does not have package '{}'",
+                            spec.name
+                        ),
+                    ));
                 }
             }
             Err(err) => {

crates/typst-library/src/diag.rs

@@ -519,15 +519,15 @@ pub type PackageResult<T> = Result<T, PackageError>;
 
 /// An error that occurred while trying to load a package.
 ///
-/// Some variants have an optional string can give more details, if available.
+/// Some variants have an optional string that can give more details, if available.
 #[derive(Debug, Clone, Eq, PartialEq, Hash)]
 pub enum PackageError {
     /// The specified package does not exist.
-    /// Optionally provides information on where we tried to find the package,
-    /// defaults to simply "searched for" if absent.
-    NotFound(PackageSpec, Option<EcoString>),
+    /// Additionally provides information on where we tried to find the package.
+    NotFound(PackageSpec, EcoString),
     /// The specified package found, but the version does not exist.
-    VersionNotFound(PackageSpec, PackageVersion),
+    /// TODO: make the registry part of the error better typed
+    VersionNotFound(PackageSpec, Option<PackageVersion>, EcoString),
     /// Failed to retrieve the package through the network.
     NetworkFailed(Option<EcoString>),
     /// The package archive was malformed.
@@ -541,18 +541,16 @@ impl std::error::Error for PackageError {}
 impl Display for PackageError {
     fn fmt(&self, f: &mut Formatter) -> fmt::Result {
         match self {
-            Self::NotFound(spec, None) => {
-                write!(f, "package not found (searched for {spec})",)
+            Self::NotFound(spec, detail) => {
+                write!(f, "package not found: {detail} (searching for {spec})",)
             }
-            Self::NotFound(spec, Some(attempted)) => {
-                write!(f, "package not found ({attempted} {spec})",)
-            }
-            Self::VersionNotFound(spec, latest) => {
-                write!(
-                    f,
-                    "package found, but version {} does not exist (latest is {})",
-                    spec.version, latest,
-                )
+            Self::VersionNotFound(spec, latest, registry) => {
+                write!(f, "package found, but version {} does not exist", spec.version,)?;
+                if let Some(version) = latest {
+                    write!(f, " (latest version provided by {registry} is {version})")
+                } else {
+                    write!(f, " ({registry} contains no versions for this package)")
+                }
             }
             Self::NetworkFailed(Some(err)) => {
                 write!(f, "failed to download package ({err})")

pending/import1.typ

@@ -1 +0,0 @@
-#import "@local/mypkg:0.0.1"

pending/import4.typ

@@ -1 +0,0 @@
-#import "@local/noperm:0.0.1"

pending/manifest-unreadable.typ

@@ -0,0 +1 @@
+#import "@local/mypkg:0.0.0"

pending/no-versions.typ

@@ -0,0 +1 @@
+#import "@local/nover:0.0.1"

pending/package-doesnt-exist.typ

@@ -0,0 +1 @@
+#import "@local/nopkg:0.0.1"

pending/preview-version-doesnt-exist.typ

@@ -0,0 +1 @@
+#import "@preview/cetz:5.0.0"

pending/read-any-file.typ

@@ -0,0 +1 @@
+#let _ = read("/some" + "/path")