Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

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

Closed
1 task done
vlovich opened this issue Feb 7, 2023 · 8 comments · Fixed by #4732
Closed
1 task done

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

vlovich opened this issue Feb 7, 2023 · 8 comments · Fixed by #4732
Labels
S-To triage Status: user report of a possible bug that needs to be triaged

Comments

@vlovich
Copy link

vlovich commented Feb 7, 2023

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

  • I agree to follow Rome's Code of Conduct
@vlovich vlovich added the S-To triage Status: user report of a possible bug that needs to be triaged label Feb 7, 2023
@realtimetodie
Copy link
Contributor

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.

let inserted = ctx.interner().intern_path(path.clone());
// Determine whether an equivalent path already exists
if !inserted {

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.

@vlovich
Copy link
Author

vlovich commented Mar 2, 2023

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
Copy link
Author

vlovich commented Mar 2, 2023

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.

@realtimetodie
Copy link
Contributor

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

@vlovich
Copy link
Author

vlovich commented Apr 8, 2023

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).

@ematipico
Copy link
Contributor

This should be fixed now?

@arendjr
Copy link
Contributor

arendjr commented Jul 24, 2023

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
Copy link
Contributor

arendjr commented Jul 26, 2023

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 :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
S-To triage Status: user report of a possible bug that needs to be triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants