Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid duplicate state entries (by avoiding symlink evaluation) #699

Merged
merged 1 commit into from
Nov 18, 2021

Conversation

radeksimko
Copy link
Member

@radeksimko radeksimko commented Nov 8, 2021

We use module paths in many different places and this ensures we will compare the same paths consistently and avoid duplicate entries in memdb.

There is unfortunately no easy way to test this as the potential problem would arise as part of a module operation which is executed asynchronously, and/or when publishing diagnostics, which is in itself hard to test currently.

This bug is more visible in #714 and #700 when you open a folder that has symlink somewhere in the path.


We will likely re-evaluate this decision more holistically (in the context of the whole codebase) in #241

@radeksimko radeksimko added the bug Something isn't working label Nov 8, 2021
@radeksimko

This comment has been minimized.

@radeksimko radeksimko closed this Nov 8, 2021
@radeksimko radeksimko deleted the b-fix-symlinks branch November 8, 2021 16:32
@radeksimko radeksimko restored the b-fix-symlinks branch November 8, 2021 18:56
@radeksimko

This comment has been minimized.

@radeksimko radeksimko reopened this Nov 8, 2021
@radeksimko radeksimko marked this pull request as ready for review November 17, 2021 12:36
@radeksimko radeksimko changed the title avoid evaluating symlinks for now Avoid duplicate state entries (by avoiding symlink evaluation) Nov 17, 2021
@radeksimko radeksimko requested a review from a team November 17, 2021 12:43
We use module paths in many different places and we this ensure
we will compare the same paths consistently and avoid duplicate
entries in memdb.
@radeksimko radeksimko merged commit 7d46fc6 into main Nov 18, 2021
@radeksimko radeksimko deleted the b-fix-symlinks branch November 18, 2021 12:53
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants