From a5ef75a9c2cfa7191c328fa6fccfed897db89f49 Mon Sep 17 00:00:00 2001 From: Neven Villani Date: Wed, 11 Jun 2025 22:00:20 +0200 Subject: [PATCH 1/7] I want to get all of this working --- crates/typst-kit/src/package.rs | 2 +- crates/typst-library/src/diag.rs | 9 +++++++-- pending/import1.typ | 1 + pending/import2.typ | 1 + pending/import3.typ | 1 + pending/import4.typ | 1 + pending/import5.typ | 1 + pending/runall.sh | 5 +++++ tests/suite/scripting/import.typ | 13 +++++++++++++ 9 files changed, 31 insertions(+), 3 deletions(-) create mode 100644 pending/import1.typ create mode 100644 pending/import2.typ create mode 100644 pending/import3.typ create mode 100644 pending/import4.typ create mode 100644 pending/import5.typ create mode 100755 pending/runall.sh diff --git a/crates/typst-kit/src/package.rs b/crates/typst-kit/src/package.rs index 584ec83c0..f08eb897c 100644 --- a/crates/typst-kit/src/package.rs +++ b/crates/typst-kit/src/package.rs @@ -185,7 +185,7 @@ impl PackageStorage { if let Ok(version) = self.determine_latest_version(&spec.versionless()) { return Err(PackageError::VersionNotFound(spec.clone(), version)); } else { - return Err(PackageError::NotFound(spec.clone())); + return Err(PackageError::NotFound(spec.clone(), Some(eco_format!("attempted to download")))); } } Err(err) => { diff --git a/crates/typst-library/src/diag.rs b/crates/typst-library/src/diag.rs index 41b92ed65..6a55a21da 100644 --- a/crates/typst-library/src/diag.rs +++ b/crates/typst-library/src/diag.rs @@ -523,7 +523,9 @@ pub type PackageResult = Result; #[derive(Debug, Clone, Eq, PartialEq, Hash)] pub enum PackageError { /// The specified package does not exist. - NotFound(PackageSpec), + /// Optionally provides information on where we tried to find the package, + /// defaults to simply "searched for" if absent. + NotFound(PackageSpec, Option), /// The specified package found, but the version does not exist. VersionNotFound(PackageSpec, PackageVersion), /// Failed to retrieve the package through the network. @@ -539,9 +541,12 @@ impl std::error::Error for PackageError {} impl Display for PackageError { fn fmt(&self, f: &mut Formatter) -> fmt::Result { match self { - Self::NotFound(spec) => { + Self::NotFound(spec, None) => { write!(f, "package not found (searched for {spec})",) } + Self::NotFound(spec, Some(attempted)) => { + write!(f, "package not found ({attempted} {spec})",) + } Self::VersionNotFound(spec, latest) => { write!( f, diff --git a/pending/import1.typ b/pending/import1.typ new file mode 100644 index 000000000..36894382d --- /dev/null +++ b/pending/import1.typ @@ -0,0 +1 @@ +#import "@local/mypkg:0.0.1" diff --git a/pending/import2.typ b/pending/import2.typ new file mode 100644 index 000000000..7d1515e1a --- /dev/null +++ b/pending/import2.typ @@ -0,0 +1 @@ +#import "@nope/mypkg:0.0.1" diff --git a/pending/import3.typ b/pending/import3.typ new file mode 100644 index 000000000..5010e4daa --- /dev/null +++ b/pending/import3.typ @@ -0,0 +1 @@ +#import "@local/penpo:5.0.0" diff --git a/pending/import4.typ b/pending/import4.typ new file mode 100644 index 000000000..cdd6c755a --- /dev/null +++ b/pending/import4.typ @@ -0,0 +1 @@ +#import "@local/noperm:0.0.1" diff --git a/pending/import5.typ b/pending/import5.typ new file mode 100644 index 000000000..f354b64e3 --- /dev/null +++ b/pending/import5.typ @@ -0,0 +1 @@ +#import "@preview/nope:0.0.1" diff --git a/pending/runall.sh b/pending/runall.sh new file mode 100755 index 000000000..bfe75c63d --- /dev/null +++ b/pending/runall.sh @@ -0,0 +1,5 @@ +#! /usr/bin/env bash + +for file in $(ls pending/*.typ); do + cargo run -- compile $file +done diff --git a/tests/suite/scripting/import.typ b/tests/suite/scripting/import.typ index 382e444cc..1866af3d6 100644 --- a/tests/suite/scripting/import.typ +++ b/tests/suite/scripting/import.typ @@ -483,3 +483,16 @@ This is never reached. --- import-from-file-package-lookalike --- // Error: 9-28 file not found (searched at tests/suite/scripting/#test/mypkg:1.0.0) #import "#test/mypkg:1.0.0": * + +--- import-from-package-namespace-not-found --- +// Error: todo +#import "@missingdir/test:0.0.0": * + +--- import-from-package-not-found --- +// Error: todo +#import "@test/missingpkg:0.0.0": * + +--- import-from-package-version-not-found --- +// Error: todo +#import "@test/mypkg:5.0.0": * + From 91ec946283a10c7e2e6ec563cc62e991b973a91d Mon Sep 17 00:00:00 2001 From: Neven Villani Date: Wed, 11 Jun 2025 22:32:10 +0200 Subject: [PATCH 2/7] first attempt --- crates/typst-kit/src/package.rs | 51 +++++++++++++++++++++++---------- 1 file changed, 36 insertions(+), 15 deletions(-) diff --git a/crates/typst-kit/src/package.rs b/crates/typst-kit/src/package.rs index f08eb897c..d0fb0ffef 100644 --- a/crates/typst-kit/src/package.rs +++ b/crates/typst-kit/src/package.rs @@ -87,13 +87,9 @@ impl PackageStorage { ) -> PackageResult { let subdir = format!("{}/{}/{}", spec.namespace, spec.name, spec.version); - if let Some(packages_dir) = &self.package_path { - let dir = packages_dir.join(&subdir); - if dir.exists() { - return Ok(dir); - } - } - + // 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? if let Some(cache_dir) = &self.package_cache_path { let dir = cache_dir.join(&subdir); if dir.exists() { @@ -102,14 +98,39 @@ impl PackageStorage { // Download from network if it doesn't exist yet. if spec.namespace == DEFAULT_NAMESPACE { - self.download_package(spec, cache_dir, progress)?; - if dir.exists() { - return Ok(dir); - } + return self.download_package(spec, cache_dir, progress); } } - Err(PackageError::NotFound(spec.clone())) + // In all paths from now, we either succeed or fail with a NotFound. + 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); + }; + // 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()))); + } + 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()))); + } + 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!() } /// Tries to determine the latest version of a package. @@ -171,7 +192,7 @@ impl PackageStorage { spec: &PackageSpec, cache_dir: &Path, progress: &mut dyn Progress, - ) -> PackageResult<()> { + ) -> PackageResult { assert_eq!(spec.namespace, DEFAULT_NAMESPACE); let url = format!( @@ -235,8 +256,8 @@ impl PackageStorage { // broken packages still occur even with the rename safeguard, we might // consider more complex solutions like file locking or checksums. match fs::rename(&tempdir, &package_dir) { - Ok(()) => Ok(()), - Err(err) if err.kind() == io::ErrorKind::DirectoryNotEmpty => Ok(()), + Ok(()) => Ok(package_dir), + Err(err) if err.kind() == io::ErrorKind::DirectoryNotEmpty => Ok(package_dir), Err(err) => Err(error("failed to move downloaded package directory", err)), } } From c354369b057012350357fc918dc5e8ac96607eca Mon Sep 17 00:00:00 2001 From: Neven Villani Date: Wed, 11 Jun 2025 22:43:14 +0200 Subject: [PATCH 3/7] What's with the namespacing on the unit tests ? --- tests/suite/scripting/import.typ | 6 +++--- tests/testit | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) create mode 100644 tests/testit diff --git a/tests/suite/scripting/import.typ b/tests/suite/scripting/import.typ index 1866af3d6..9fbd889fa 100644 --- a/tests/suite/scripting/import.typ +++ b/tests/suite/scripting/import.typ @@ -485,14 +485,14 @@ This is never reached. #import "#test/mypkg:1.0.0": * --- import-from-package-namespace-not-found --- -// Error: todo +// Error: 9-33 package not found #import "@missingdir/test:0.0.0": * --- import-from-package-not-found --- -// Error: todo +// Error: 9-33 package not found #import "@test/missingpkg:0.0.0": * --- import-from-package-version-not-found --- -// Error: todo +// Error: 9-28 package not found #import "@test/mypkg:5.0.0": * diff --git a/tests/testit b/tests/testit new file mode 100644 index 000000000..2be10fee3 --- /dev/null +++ b/tests/testit @@ -0,0 +1,2 @@ +alias testit="cargo test --workspace --test tests --" + From 85e03abe4588a91aba95c934a56f6e5af76fa612 Mon Sep 17 00:00:00 2001 From: Neven Villani Date: Thu, 12 Jun 2025 13:36:13 +0200 Subject: [PATCH 4/7] Improve messages - restore ability to override a package locally - cut down on some repeated logic - more detailed errors --- crates/typst-kit/src/package.rs | 70 ++++++++++++------- crates/typst-library/src/diag.rs | 30 ++++---- pending/import1.typ | 1 - pending/import4.typ | 1 - pending/manifest-unreadable.typ | 1 + ...import2.typ => namespace-doesnt-exist.typ} | 0 pending/no-versions.typ | 1 + pending/package-doesnt-exist.typ | 1 + ...5.typ => preview-package-doesnt-exist.typ} | 0 pending/preview-version-doesnt-exist.typ | 1 + pending/read-any-file.typ | 1 + .../{import3.typ => version-doesnt-exist.typ} | 0 12 files changed, 63 insertions(+), 44 deletions(-) delete mode 100644 pending/import1.typ delete mode 100644 pending/import4.typ create mode 100644 pending/manifest-unreadable.typ rename pending/{import2.typ => namespace-doesnt-exist.typ} (100%) create mode 100644 pending/no-versions.typ create mode 100644 pending/package-doesnt-exist.typ rename pending/{import5.typ => preview-package-doesnt-exist.typ} (100%) create mode 100644 pending/preview-version-doesnt-exist.typ create mode 100644 pending/read-any-file.typ rename pending/{import3.typ => version-doesnt-exist.typ} (100%) 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/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/manifest-unreadable.typ b/pending/manifest-unreadable.typ new file mode 100644 index 000000000..9da916486 --- /dev/null +++ b/pending/manifest-unreadable.typ @@ -0,0 +1 @@ +#import "@local/mypkg:0.0.0" diff --git a/pending/import2.typ b/pending/namespace-doesnt-exist.typ similarity index 100% rename from pending/import2.typ rename to pending/namespace-doesnt-exist.typ diff --git a/pending/no-versions.typ b/pending/no-versions.typ new file mode 100644 index 000000000..d279c96ff --- /dev/null +++ b/pending/no-versions.typ @@ -0,0 +1 @@ +#import "@local/nover:0.0.1" diff --git a/pending/package-doesnt-exist.typ b/pending/package-doesnt-exist.typ new file mode 100644 index 000000000..d035abd9c --- /dev/null +++ b/pending/package-doesnt-exist.typ @@ -0,0 +1 @@ +#import "@local/nopkg:0.0.1" diff --git a/pending/import5.typ b/pending/preview-package-doesnt-exist.typ similarity index 100% rename from pending/import5.typ rename to pending/preview-package-doesnt-exist.typ diff --git a/pending/preview-version-doesnt-exist.typ b/pending/preview-version-doesnt-exist.typ new file mode 100644 index 000000000..0914b42da --- /dev/null +++ b/pending/preview-version-doesnt-exist.typ @@ -0,0 +1 @@ +#import "@preview/cetz:5.0.0" diff --git a/pending/read-any-file.typ b/pending/read-any-file.typ new file mode 100644 index 000000000..3e19ec3ae --- /dev/null +++ b/pending/read-any-file.typ @@ -0,0 +1 @@ +#let _ = read("/some" + "/path") diff --git a/pending/import3.typ b/pending/version-doesnt-exist.typ similarity index 100% rename from pending/import3.typ rename to pending/version-doesnt-exist.typ From f3d5cdc6a5cc10fe38522ebbc1ae88e8cc63d54d Mon Sep 17 00:00:00 2001 From: Neven Villani Date: Thu, 12 Jun 2025 15:22:51 +0200 Subject: [PATCH 5/7] Report path on a FileError --- crates/typst-cli/src/world.rs | 6 ++-- crates/typst-kit/src/package.rs | 1 - crates/typst-library/src/diag.rs | 39 ++++++++++++++++-------- crates/typst/src/lib.rs | 2 +- pending/manifest-unreadable.typ | 1 - pending/namespace-doesnt-exist.typ | 1 - pending/no-versions.typ | 1 - pending/package-doesnt-exist.typ | 1 - pending/preview-package-doesnt-exist.typ | 1 - pending/preview-version-doesnt-exist.typ | 1 - pending/read-any-file.typ | 1 - pending/runall.sh | 5 --- pending/version-doesnt-exist.typ | 1 - tests/testit | 2 -- 14 files changed, 31 insertions(+), 32 deletions(-) delete mode 100644 pending/manifest-unreadable.typ delete mode 100644 pending/namespace-doesnt-exist.typ delete mode 100644 pending/no-versions.typ delete mode 100644 pending/package-doesnt-exist.typ delete mode 100644 pending/preview-package-doesnt-exist.typ delete mode 100644 pending/preview-version-doesnt-exist.typ delete mode 100644 pending/read-any-file.typ delete mode 100755 pending/runall.sh delete mode 100644 pending/version-doesnt-exist.typ delete mode 100644 tests/testit diff --git a/crates/typst-cli/src/world.rs b/crates/typst-cli/src/world.rs index 95bee235c..3dd0a6337 100644 --- a/crates/typst-cli/src/world.rs +++ b/crates/typst-cli/src/world.rs @@ -404,7 +404,9 @@ fn system_path( // Join the path to the root. If it tries to escape, deny // access. Note: It can still escape via symlinks. - id.vpath().resolve(root).ok_or(FileError::AccessDenied) + id.vpath() + .resolve(root) + .ok_or_else(|| FileError::AccessDenied(id.vpath().as_rootless_path().into())) } /// Reads a file from a `FileId`. @@ -427,7 +429,7 @@ fn read( fn read_from_disk(path: &Path) -> FileResult> { let f = |e| FileError::from_io(e, path); if fs::metadata(path).map_err(f)?.is_dir() { - Err(FileError::IsDirectory) + Err(FileError::IsDirectory(path.into())) } else { fs::read(path).map_err(f) } diff --git a/crates/typst-kit/src/package.rs b/crates/typst-kit/src/package.rs index 3bfb46c3e..b51c0a7c8 100644 --- a/crates/typst-kit/src/package.rs +++ b/crates/typst-kit/src/package.rs @@ -143,7 +143,6 @@ impl PackageStorage { } /// Tries to determine the latest version of a package. - /// TODO improve here too pub fn determine_latest_version( &self, spec: &VersionlessPackageSpec, diff --git a/crates/typst-library/src/diag.rs b/crates/typst-library/src/diag.rs index 4c7424875..2214b63f4 100644 --- a/crates/typst-library/src/diag.rs +++ b/crates/typst-library/src/diag.rs @@ -440,13 +440,13 @@ pub enum FileError { /// A file was not found at this path. NotFound(PathBuf), /// A file could not be accessed. - AccessDenied, + AccessDenied(PathBuf), /// A directory was found, but a file was expected. - IsDirectory, + IsDirectory(PathBuf), /// The file is not a Typst source file, but should have been. - NotSource, + NotSource(PathBuf), /// The file was not valid UTF-8, but should have been. - InvalidUtf8, + InvalidUtf8(Option), /// The package the file is part of could not be loaded. Package(PackageError), /// Another error. @@ -460,11 +460,11 @@ impl FileError { pub fn from_io(err: io::Error, path: &Path) -> Self { match err.kind() { io::ErrorKind::NotFound => Self::NotFound(path.into()), - io::ErrorKind::PermissionDenied => Self::AccessDenied, + io::ErrorKind::PermissionDenied => Self::AccessDenied(path.into()), io::ErrorKind::InvalidData if err.to_string().contains("stream did not contain valid UTF-8") => { - Self::InvalidUtf8 + Self::InvalidUtf8(Some(path.into())) } _ => Self::Other(Some(eco_format!("{err}"))), } @@ -479,10 +479,19 @@ impl Display for FileError { Self::NotFound(path) => { write!(f, "file not found (searched at {})", path.display()) } - Self::AccessDenied => f.pad("failed to load file (access denied)"), - Self::IsDirectory => f.pad("failed to load file (is a directory)"), - Self::NotSource => f.pad("not a typst source file"), - Self::InvalidUtf8 => f.pad("file is not valid utf-8"), + Self::AccessDenied(path) => { + write!(f, "failed to load file {} (access denied)", path.display()) + } + Self::IsDirectory(path) => { + write!(f, "failed to load file {} (is a directory)", path.display()) + } + Self::NotSource(path) => { + write!(f, "{} is not a typst source file", path.display()) + } + Self::InvalidUtf8(Some(path)) => { + write!(f, "file {} is not valid utf-8", path.display()) + } + Self::InvalidUtf8(None) => f.pad("file is not valid utf-8"), Self::Package(error) => error.fmt(f), Self::Other(Some(err)) => write!(f, "failed to load file ({err})"), Self::Other(None) => f.pad("failed to load file"), @@ -492,13 +501,13 @@ impl Display for FileError { impl From for FileError { fn from(_: Utf8Error) -> Self { - Self::InvalidUtf8 + Self::InvalidUtf8(None) } } impl From for FileError { fn from(_: FromUtf8Error) -> Self { - Self::InvalidUtf8 + Self::InvalidUtf8(None) } } @@ -545,7 +554,11 @@ impl Display for PackageError { write!(f, "package not found: {detail} (searching for {spec})",) } Self::VersionNotFound(spec, latest, registry) => { - write!(f, "package found, but version {} does not exist", spec.version,)?; + write!( + f, + "package '{}' found, but version {} does not exist", + spec.name, spec.version + )?; if let Some(version) = latest { write!(f, " (latest version provided by {registry} is {version})") } else { diff --git a/crates/typst/src/lib.rs b/crates/typst/src/lib.rs index a6bb4fe38..9d388f3bb 100644 --- a/crates/typst/src/lib.rs +++ b/crates/typst/src/lib.rs @@ -190,7 +190,7 @@ fn hint_invalid_main_file( file_error: FileError, input: FileId, ) -> EcoVec { - let is_utf8_error = matches!(file_error, FileError::InvalidUtf8); + let is_utf8_error = matches!(file_error, FileError::InvalidUtf8(_)); let mut diagnostic = SourceDiagnostic::error(Span::detached(), EcoString::from(file_error)); diff --git a/pending/manifest-unreadable.typ b/pending/manifest-unreadable.typ deleted file mode 100644 index 9da916486..000000000 --- a/pending/manifest-unreadable.typ +++ /dev/null @@ -1 +0,0 @@ -#import "@local/mypkg:0.0.0" diff --git a/pending/namespace-doesnt-exist.typ b/pending/namespace-doesnt-exist.typ deleted file mode 100644 index 7d1515e1a..000000000 --- a/pending/namespace-doesnt-exist.typ +++ /dev/null @@ -1 +0,0 @@ -#import "@nope/mypkg:0.0.1" diff --git a/pending/no-versions.typ b/pending/no-versions.typ deleted file mode 100644 index d279c96ff..000000000 --- a/pending/no-versions.typ +++ /dev/null @@ -1 +0,0 @@ -#import "@local/nover:0.0.1" diff --git a/pending/package-doesnt-exist.typ b/pending/package-doesnt-exist.typ deleted file mode 100644 index d035abd9c..000000000 --- a/pending/package-doesnt-exist.typ +++ /dev/null @@ -1 +0,0 @@ -#import "@local/nopkg:0.0.1" diff --git a/pending/preview-package-doesnt-exist.typ b/pending/preview-package-doesnt-exist.typ deleted file mode 100644 index f354b64e3..000000000 --- a/pending/preview-package-doesnt-exist.typ +++ /dev/null @@ -1 +0,0 @@ -#import "@preview/nope:0.0.1" diff --git a/pending/preview-version-doesnt-exist.typ b/pending/preview-version-doesnt-exist.typ deleted file mode 100644 index 0914b42da..000000000 --- a/pending/preview-version-doesnt-exist.typ +++ /dev/null @@ -1 +0,0 @@ -#import "@preview/cetz:5.0.0" diff --git a/pending/read-any-file.typ b/pending/read-any-file.typ deleted file mode 100644 index 3e19ec3ae..000000000 --- a/pending/read-any-file.typ +++ /dev/null @@ -1 +0,0 @@ -#let _ = read("/some" + "/path") 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 diff --git a/pending/version-doesnt-exist.typ b/pending/version-doesnt-exist.typ deleted file mode 100644 index 5010e4daa..000000000 --- a/pending/version-doesnt-exist.typ +++ /dev/null @@ -1 +0,0 @@ -#import "@local/penpo:5.0.0" diff --git a/tests/testit b/tests/testit deleted file mode 100644 index 2be10fee3..000000000 --- a/tests/testit +++ /dev/null @@ -1,2 +0,0 @@ -alias testit="cargo test --workspace --test tests --" - From 3b5ee3c488a0807c7ddc6ddd75666284bc65773e Mon Sep 17 00:00:00 2001 From: Neven Villani Date: Thu, 12 Jun 2025 15:53:36 +0200 Subject: [PATCH 6/7] Update in tests/ too the PathBuf in FileError --- tests/src/world.rs | 6 ++++-- tests/suite/scripting/import.typ | 16 ++-------------- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/tests/src/world.rs b/tests/src/world.rs index 6fa1cf60f..03bbb20b1 100644 --- a/tests/src/world.rs +++ b/tests/src/world.rs @@ -172,7 +172,9 @@ pub(crate) fn system_path(id: FileId) -> FileResult { None => PathBuf::new(), }; - id.vpath().resolve(&root).ok_or(FileError::AccessDenied) + id.vpath() + .resolve(&root) + .ok_or_else(|| FileError::AccessDenied(id.vpath().as_rootless_path().into())) } /// Read a file. @@ -186,7 +188,7 @@ pub(crate) fn read(path: &Path) -> FileResult> { let f = |e| FileError::from_io(e, path); if fs::metadata(path).map_err(f)?.is_dir() { - Err(FileError::IsDirectory) + Err(FileError::IsDirectory(path.into())) } else { fs::read(path).map(Cow::Owned).map_err(f) } diff --git a/tests/suite/scripting/import.typ b/tests/suite/scripting/import.typ index 9fbd889fa..f094042bc 100644 --- a/tests/suite/scripting/import.typ +++ b/tests/suite/scripting/import.typ @@ -302,11 +302,11 @@ #import 5 as x --- import-from-string-invalid --- -// Error: 9-11 failed to load file (is a directory) +// Error: 9-11 failed to load file tests/suite/scripting (is a directory) #import "": name --- import-from-string-renamed-invalid --- -// Error: 9-11 failed to load file (is a directory) +// Error: 9-11 failed to load file tests/suite/scripting (is a directory) #import "" as x --- import-file-not-found-invalid --- @@ -484,15 +484,3 @@ This is never reached. // Error: 9-28 file not found (searched at tests/suite/scripting/#test/mypkg:1.0.0) #import "#test/mypkg:1.0.0": * ---- import-from-package-namespace-not-found --- -// Error: 9-33 package not found -#import "@missingdir/test:0.0.0": * - ---- import-from-package-not-found --- -// Error: 9-33 package not found -#import "@test/missingpkg:0.0.0": * - ---- import-from-package-version-not-found --- -// Error: 9-28 package not found -#import "@test/mypkg:5.0.0": * - From 143732d2a82c057f9efe48895fc798e422894fec Mon Sep 17 00:00:00 2001 From: Neven Villani Date: Tue, 24 Jun 2025 14:42:23 +0200 Subject: [PATCH 7/7] One comment out of date --- crates/typst-kit/src/package.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/typst-kit/src/package.rs b/crates/typst-kit/src/package.rs index b51c0a7c8..5b5c23c42 100644 --- a/crates/typst-kit/src/package.rs +++ b/crates/typst-kit/src/package.rs @@ -111,13 +111,13 @@ impl PackageStorage { // 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. + // We try `namespace/` then `namespace/name/` then `namespace/name/version/` + // and see where the first error occurs. let not_found = |msg| Err(PackageError::NotFound(spec.clone(), msg)); let Some(packages_dir) = &self.package_path else { return not_found(eco_format!("cannot access local package storage")); }; - // 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(eco_format!(