tools icon indicating copy to clipboard operation
tools copied to clipboard

🐛 Infinite symlink expansion error when there's no infinite symlink AFAICT

Open vlovich opened this issue 2 years ago • 6 comments

Environment information

CLI:
  Version:              11.0.0
  Color support:        true

Platform:
  CPU Architecture:     x86_64
  OS:                   linux

Environment:
  ROME_LOG_DIR:         unset
  NO_COLOR:             unset
  TERM:                 "xterm-256color"

Rome Configuration:
  Status:               loaded
  Formatter disabled:   false
  Linter disabled:      false

Workspace:
  Open Documents:       0

Discovering running Rome servers...

What happened?

  1. running npx rome check .
  2. It complains about a bunch of file symlinks I have

For example:

../tools/node/npm-runner.js internalError/fs ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ⚠ Infinite symlink expansion
  
  ✖ Rome encountered a file system entry that leads to an infinite symbolic link expansion, causing an infinite cycle: ../tools/node/npm-runner.js

Problems:

  1. I have no idea what working directory it's in because I run rome from . and the npm-runner.js script is located at workflow/tools/node/npm-runner.js
  2. I have no idea what the source of the symlink is that it's having problems with
  3. npm-runner.js is not a symlink:
$ ls -l workflow/tools/node/npm-runner.js
-rwxr-xr-x 1 vlovich vlovich 7707 Jan 24 18:20 workflow/tools/node/npm-runner.js

Relevant information: An example of a symlink looks like:

ls -l scripts/my-tool
lrwxrwxrwx 1 vlovich vlovich 36 Jan 12 20:22 scripts/my-tool -> ../workflow/tools/node/npm-runner.js

workflow is a submodule to another project. I get the similar errors when running rome from within the submodule (or even as a standalone checkout) so most of the errors about the symlinks that exist within workflow (the npm-runner.js script is reused within workflow and within projects that import it).

Expected result

Simple symlinks to JS files shouldn't cause infinite symlink errors.

Code of Conduct

  • [X] I agree to follow Rome's Code of Conduct

vlovich avatar Feb 07 '23 17:02 vlovich

The symbolic link detection is optimized for speed and does not cache the entire resolution tree. For this reason, the displayed error message is not very detailed. The warning message is shown, when an already traversed file path is found more than once. The interner is a cache of all traversed file paths.

https://github.com/rome/tools/blob/99ee725674d3bbb7641a20f06c3e99c0788f14c5/crates/rome_fs/src/fs/os.rs#L240-L243

This warning message can only be caused by using symbolic links. Please verify, that no symbolic link points to a parent directory where the file is located.

realtimetodie avatar Feb 21 '23 15:02 realtimetodie

There are no symbolic links to directories. All symbolic links in this setup always resolve to a specific file. So it may go through a parent directory (e.g. ln -s ../sibling-folder/file ./my-symlink), but I don't see why symlinks to files would ever trigger an infinite loop check. I would think that you just either readlink and cap how many indirect links you can go through before a non symlink must be hit or use realpath and let the kernel let you know via ELOOP error and then only trigger infinite symlink detection when the symlink resolves to a directory.

vlovich avatar Mar 02 '23 23:03 vlovich

My reading of the code is that any symlinks in multiple directories will trigger this because the infinite symlink expansion warning gets triggered if the same file appears in 2 separate directories (even ln -s ./file1 ./file2 I think triggers). I believe this is tautologically true for any file symlink.

vlovich avatar Mar 02 '23 23:03 vlovich

Yes, exactly. The provided example is too generic, but I assume there might be some symbolic link inside of a node_modules dependency.

realtimetodie avatar Mar 06 '23 12:03 realtimetodie

I explicitly create a symlink in my project. That's tripping it up. What I'm suggesting is that if ln -s file1 file2 or ln -s dir1 dir2 triggers the warning, the warning isn't actually useful as it's not catching any issue. It needs to be a bit more careful (eg encountering a symlink directory while following a symlink directory).

vlovich avatar Apr 08 '23 22:04 vlovich

This should be fixed now?

ematipico avatar May 08 '23 14:05 ematipico

I think #4395 is a duplicate of this issue, but the issue is not fixed in Rome in 12.1.3 yet (which included the fix from #4166).

arendjr avatar Jul 24 '23 15:07 arendjr

I've created a branch that extends the test cases so the issue is reproduced: https://github.com/rome/tools/compare/main...arendjr:rome-tools:infinite-links?expand=1#diff-6a7c89207a1d417336bc86aeec0899754eb6e72e50c235828d7fd7ca80ea0391R964

Next step will be trying to come up with a fix :)

arendjr avatar Jul 26 '23 08:07 arendjr