Fix import* and read* skipping symlinked files

[nkakouros]

Problem

import*() and read*() glob expansion silently omits symlinked files from results. Regular files matching the same pattern in the same directory are returned, so the partial result is easy to miss. There's also no warning that the glob returned only some matches.

Reproduction

mkdir -p proj/sources
echo 'name = "real"'   > proj/sources/real.pkl
echo 'name = "target"' > proj/sources/target.pkl
ln -s target.pkl proj/sources/linked.pkl
echo 'result = import*("sources/*.pkl")' > proj/main.pkl
pkl eval proj/main.pkl
# Before: result contains only sources/real.pkl and sources/target.pkl
# After:  result additionally contains sources/linked.pkl                                       

Cause

FileResolver.listElements() skipped every directory entry that was a symbolic link, with the comment "skip symlinks to prevent cyclical globs". The intent, preventing cycles caused by directory symlinks, was good, but the check was too broad and also excluded symlinks to regular files that don't cause cyclical globs.

Fix

Narrow the skip to symlinks whose target is a directory. Symlinks to regular files are now surfaced as ordinary PathElement entries. Broken symlinks are also surfaced; their failure now shows up as a clear load-time error rather than a silent omission.

read*() shares the same FileResolver.listElements() code path as import*(), so the same change fixes both.

Tests

• New unit test FileResolverTest covering: symlink to a regular file (the bug), symlink to a directory (still
  skipped, invariant preserved), and broken symlink (now surfaced).

• New language snippet test importGlobSymlink.pkl exercising the fix end-to-end through import*(). Fixture lives in a new input-helper/globtest-symlink/ directory so existing snippet tests are untouched.

Future work

Following directory symlinks (with real-path cycle tracking) is intentionally out of scope for this PR. This is the minimal change to stop the silent-drop bug; full shell-glob-style symlink following can be a follow-up.

[bioball]

This is more of a behavior change than a fix.

In the past, we've chatted about just allowing symlinks, and bubbling up cycles as errors. Either way, I think just allowing symlinking files but not directories is surprising behavior. Either, it should follow symlinks, or not.

[nkakouros]

This is more of a behavior change than a fix.

In the past, we've chatted about just allowing symlinks, and bubbling up cycles as errors. Either way, I think just allowing symlinking files but not directories is surprising behavior. Either, it should follow symlinks, or not.

Fair. To support directory symlinks, is there a preferred approach? I am thinking of using a Set of traversed paths through the recursion in GlobResolver.doExpandHierarchicalGlobPart, and raising an error when a directory's real path is already in the set before visiting it.

[bioball]

Tested what happens when you have circular symlinks and try to resolve the real path (I added this as a junit test):

  @Test
  fun `test circular link`(@TempDir tempDir: Path) {
    val linkOne = tempDir.resolve("a.lnk")
    val linkTwo = tempDir.resolve("b.lnk")
    Files.createSymbolicLink(linkOne, linkTwo)
    Files.createSymbolicLink(linkTwo, linkOne)
    linkOne.toRealPath()
  }

I got this thrown error:

java.nio.file.FileSystemException: /var/folders/vz/_qq5dbl5099fg0gz8b304f9h0000gn/T/junit-1170409976093690362/a.lnk: Too many levels of symbolic links or unable to access attributes of symbolic link

I think we should be able to handle the circular symlink stuff by calling toRealPath and letting that resulting IOException bubble up (or perhaps catching it and re-throwing it as a VmException).

[nkakouros]

Superseded by #1562 .