fix(config): make config auto-discovery skip the same errors on every platform#34558
Merged
Conversation
… platform `is_skippable_io_error` previously used `cfg!(windows)` / `cfg!(unix)` and raw errno comparisons (`ERROR_INVALID_NAME = 123`, `ENOTDIR = 20`) to decide which IO errors should be silently skipped while walking ancestors for a `deno.json`/`deno.jsonc`. The same logical condition (e.g. a non-directory parent component, an invalid path, a directory named `deno.json`) maps to different errors on the two platforms, so the function returned `true` for some failures on one OS and `false` for the same logical failure on the other. Discovery would abort on one platform and silently continue on the other, producing different config-detection results across CI runners. Match on the now-stable `ErrorKind` variants directly so both platforms agree — `NotADirectory`/`IsADirectory` (stable in 1.83) and `InvalidFilename` (stable in 1.87) cover the platform-specific raw errno cases. The Rust toolchain pinned in `rust-toolchain.toml` is 1.95.0. Closes denoland/orchid#304 Co-Authored-By: Divy Srivastava <[email protected]>
littledivy
approved these changes
May 30, 2026
littledivy
enabled auto-merge (squash)
May 30, 2026 03:50
littledivy
added a commit
to crowlKats/deno
that referenced
this pull request
Jun 10, 2026
… platform (denoland#34558) ## Summary `is_skippable_io_error` (the helper that decides which IO errors silently get skipped while walking ancestors for a `deno.json`/`deno.jsonc`) previously gated its checks on `cfg!(windows)` / `cfg!(unix)` and raw errno comparisons: ```rust if cfg!(windows) && e.raw_os_error() == Some(123) { return true; } // ERROR_INVALID_NAME match e.kind() { InvalidInput | PermissionDenied | NotFound => true, _ => cfg!(unix) && e.raw_os_error() == Some(20), // ENOTDIR } ``` The same logical condition — a non-directory parent component on the way to `deno.json`, an invalid filename, a directory that happens to be named `deno.json` — maps to different errors on Linux vs. Windows. The function returned `true` on one platform and `false` on the other for what was effectively the same situation, causing discovery to abort on one OS and silently continue on the other. That is the inconsistency reported in denoland#17428 — a test passing on Windows because the walk-up found a config file, while the same test on Ubuntu failed to find one (or vice versa). Match on the now-stable `ErrorKind` variants directly: ```rust matches!( e.kind(), InvalidInput | InvalidFilename | PermissionDenied | NotFound | NotADirectory | IsADirectory, ) ``` `NotADirectory`/`IsADirectory` are stable since Rust 1.83 and `InvalidFilename` since 1.87. The toolchain pinned in `rust-toolchain.toml` is 1.95.0, so all three are available. Behavior is now identical on every OS. ## Test plan - [x] `cargo test -p deno_config util::` — existing `is_skippable_io_error_win_invalid_filename` still asserts errno 123 is skippable, added a Unix twin for errno 20, and added a portable `is_skippable_io_error_kinds` that asserts the expanded `ErrorKind` set is skipped on every platform (and that unrelated kinds like `ConnectionRefused`/`UnexpectedEof` are not). - [x] `cargo check -p deno_config`, `cargo clippy -p deno_config --no-deps`, `cargo check --bin deno` — all clean. Closes denoland/orchid#304 Co-authored-by: divybot <[email protected]> Co-authored-by: Divy Srivastava <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #17428
Summary
is_skippable_io_error(the helper that decides which IO errors silently get skipped while walking ancestors for adeno.json/deno.jsonc) previously gated its checks oncfg!(windows)/cfg!(unix)and raw errno comparisons:The same logical condition — a non-directory parent component on the way to
deno.json, an invalid filename, a directory that happens to be nameddeno.json— maps to different errors on Linux vs. Windows. The function returnedtrueon one platform andfalseon the other for what was effectively the same situation, causing discovery to abort on one OS and silently continue on the other. That is the inconsistency reported in #17428 — a test passing on Windows because the walk-up found a config file, while the same test on Ubuntu failed to find one (or vice versa).Match on the now-stable
ErrorKindvariants directly:NotADirectory/IsADirectoryare stable since Rust 1.83 andInvalidFilenamesince 1.87. The toolchain pinned inrust-toolchain.tomlis 1.95.0, so all three are available. Behavior is now identical on every OS.Test plan
cargo test -p deno_config util::— existingis_skippable_io_error_win_invalid_filenamestill asserts errno 123 is skippable, added a Unix twin for errno 20, and added a portableis_skippable_io_error_kindsthat asserts the expandedErrorKindset is skipped on every platform (and that unrelated kinds likeConnectionRefused/UnexpectedEofare not).cargo check -p deno_config,cargo clippy -p deno_config --no-deps,cargo check --bin deno— all clean.Closes denoland/divybot#304