Skip to content

Follow symbolic links during glob expansion#1562

Open
nkakouros wants to merge 1 commit intoapple:mainfrom
nkakouros:worktree-bioball-approach
Open

Follow symbolic links during glob expansion#1562
nkakouros wants to merge 1 commit intoapple:mainfrom
nkakouros:worktree-bioball-approach

Conversation

@nkakouros
Copy link
Copy Markdown

@nkakouros nkakouros commented Apr 27, 2026

FileResolver.listElements() unconditionally skipped all symlinks to prevent cyclical globs from directory symlinks. As a side effect, import*() and read*() silently dropped symlinks to regular files from their results, returning partial output with no warning, and skipped symlinks to directories even when no cycle existed.

Stop filtering symlinks. Switch the directory check from Files.isDirectory(), which silently returns false on errors, to Files.readAttributes(), which lets I/O errors propagate. This way the existing OS-level symlink-resolution limit surfaces a clear "Too many levels of symbolic links" error on cycles instead of a silent truncation. Broken symlinks are still surfaced as non-directory entries via a NoSuchFileException catch.

Test on [windows] and [native]

Copy link
Copy Markdown
Contributor

@HT154 HT154 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Comment on lines +9 to +10
* Follow symbolic links during `import*` and `read*` glob expansion; symlink cycles now surface as a clear I/O error
(pr:https://github.com/apple/pkl/pull/XXXX[]).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No action needed here, but just a note for us when prepping the release: this is a breaking change and we'll need to ensure it's marked accordingly.

@HT154 HT154 force-pushed the worktree-bioball-approach branch from cd6e0ec to 6e76106 Compare April 27, 2026 22:13
FileResolver.listElements() unconditionally skipped all symlinks to
prevent cyclical globs from directory symlinks. As a side effect,
import*() and read*() silently dropped symlinks to regular files from
their results, returning partial output with no warning, and skipped
symlinks to directories even when no cycle existed.

Stop filtering symlinks. Switch the directory check from
Files.isDirectory(), which silently returns false on errors, to
Files.readAttributes(), which lets I/O errors propagate. This way the
existing OS-level symlink-resolution limit surfaces a clear "Too many
levels of symbolic links" error on cycles instead of a silent
truncation. Broken symlinks are still surfaced as non-directory entries
via a NoSuchFileException catch.
@HT154 HT154 force-pushed the worktree-bioball-approach branch from 6e76106 to 1940a07 Compare April 27, 2026 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants