diff --git a/crates/typst-kit/src/package.rs b/crates/typst-kit/src/package.rs index d0fb0ffef..3bfb46c3e 100644 --- a/crates/typst-kit/src/package.rs +++ b/crates/typst-kit/src/package.rs @@ -87,9 +87,15 @@ impl PackageStorage { ) -> PackageResult { 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 { 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) => { diff --git a/crates/typst-library/src/diag.rs b/crates/typst-library/src/diag.rs index 6a55a21da..4c7424875 100644 --- a/crates/typst-library/src/diag.rs +++ b/crates/typst-library/src/diag.rs @@ -519,15 +519,15 @@ pub type PackageResult = Result; /// 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), + /// 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, EcoString), /// Failed to retrieve the package through the network. NetworkFailed(Option), /// 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})") diff --git a/pending/import1.typ b/pending/import1.typ deleted file mode 100644 index 36894382d..000000000 --- a/pending/import1.typ +++ /dev/null @@ -1 +0,0 @@ -#import "@local/mypkg:0.0.1" diff --git a/pending/import2.typ b/pending/import2.typ deleted file mode 100644 index 7d1515e1a..000000000 --- a/pending/import2.typ +++ /dev/null @@ -1 +0,0 @@ -#import "@nope/mypkg:0.0.1" diff --git a/pending/import3.typ b/pending/import3.typ deleted file mode 100644 index 5010e4daa..000000000 --- a/pending/import3.typ +++ /dev/null @@ -1 +0,0 @@ -#import "@local/penpo:5.0.0" diff --git a/pending/import4.typ b/pending/import4.typ deleted file mode 100644 index cdd6c755a..000000000 --- a/pending/import4.typ +++ /dev/null @@ -1 +0,0 @@ -#import "@local/noperm:0.0.1" diff --git a/pending/import5.typ b/pending/import5.typ deleted file mode 100644 index f354b64e3..000000000 --- a/pending/import5.typ +++ /dev/null @@ -1 +0,0 @@ -#import "@preview/nope:0.0.1" diff --git a/pending/runall.sh b/pending/runall.sh deleted file mode 100755 index bfe75c63d..000000000 --- a/pending/runall.sh +++ /dev/null @@ -1,5 +0,0 @@ -#! /usr/bin/env bash - -for file in $(ls pending/*.typ); do - cargo run -- compile $file -done