-
-
Notifications
You must be signed in to change notification settings - Fork 504
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
feat: use user local data dir for lsp daemon files #1238
feat: use user local data dir for lsp daemon files #1238
Conversation
✅ Deploy Preview for biomejs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Signed-off-by: bestmike007 <i@bestmike007.com>
dd665a1
to
ce673be
Compare
Signed-off-by: bestmike007 <i@bestmike007.com>
Because the socket is a temporary data, I think that |
Signed-off-by: bestmike007 <i@bestmike007.com>
Changed. Is it also appropriate to store daemon logs? |
I think so (We could change it in the future if we think it is not). |
Signed-off-by: bestmike007 <i@bestmike007.com>
Signed-off-by: bestmike007 <i@bestmike007.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
The test is failing, what should I do to fix it? @ematipico |
@bestmike007 It seems that your lock file updates a bunch of the internal dependencies. One of them is Can you make sure to revert the lock file and add only the new crate without updating the rest of the dependencies? |
The lock file was rebuilt when I tried to resolve the conflicts. Let me see what I can do to revert it. |
Signed-off-by: bestmike007 <i@bestmike007.com>
@ematipico Reverted. Can you help me re-trigger the GA? |
All good now. |
Any chance to include this in the next release? |
We would love to, but this change requires some changes in the documentation. Would like to update it? We document where people can check their logs, now that this PR changes it, we need to document this change. This is very important: |
Sure, let me work on that. |
Signed-off-by: bestmike007 <i@bestmike007.com>
Documents updated, and I also added a |
Summary
biome lsp server does not work on linux with multiple users. Below is a demonstration using a Dockerfile.
This is because biome lsp server is using the shared temp_dir (/tmp) to create linux socket file along with the logs.
This PR is to change it to using user local data directory, and fallback to
env::temp_dir()
if it's not available.Test Plan
cargo build && docker run --rm -v $PWD/target/debug/biome:/usr/local/bin/biome node:slim sh -c "useradd -m alice && useradd -m bob && su - alice -c 'whoami && biome start && biome __print_socket' && su - bob -c 'whoami && biome start && biome __print_socket'"