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

Handle multiple users with /tmp/vscode-typescript #75547

Merged
merged 1 commit into from Jun 24, 2019
Merged

Handle multiple users with /tmp/vscode-typescript #75547

merged 1 commit into from Jun 24, 2019

Conversation

asztal
Copy link
Contributor

@asztal asztal commented Jun 14, 2019

This fixes an issue where the typescript language server fails to load if multiple users launch VS Code on the same Linux machine.

Steps to reproduce:

  • Log in as user1
  • Launch VS Code
  • Log out
  • Log in as user2
  • Launch VS Code
  • It tries to write to files in /tmp/vscode-typescript, but that directory is not writeable because it is owned by user1
  • You cannot use TypeScript intellisense

This fix namespaces the directory with the current uid so that each user will get their own.

On Windows, this shouldn't be an issue anyway since each user gets their own temp directory.

This fixes an issue where the typescript language server fails to load if multiple users launch VS Code on the same Linux machine.

Steps to reproduce:
- Log in as user1
- Launch VS Code
- Log out
- Log in as user2
- Launch VS Code
- It tries to write to files in /tmp/vscode-typescript, but that directory is not writeable because it is owned by user1
- You cannot use TypeScript intellisense

This fix namespaces the directory with the current uid so that each user will get their own. 

On Windows, this shouldn't be an issue anyway since each user gets their own temp directory.
@mjbvz mjbvz added this to the June 2019 milestone Jun 20, 2019
@mjbvz
Copy link
Contributor

mjbvz commented Jun 20, 2019

Thanks for the PR. Just two quick things:

  • Can you please create an issue for this is. Include the steps to reproduce the problem. This helps us track and test what is delivered each milestone.

  • Can we exclude windows from the new logic? We've seen a few users complain about a large number of vscode directories under temp, which apparently isn't automatically cleaned up like on most unix systems. Having a single folder avoids polluting the top level of temp directory on windows even if we don't properly clean up

@asztal
Copy link
Contributor Author

asztal commented Jun 20, 2019

Sure, I'll create an issue when I can. I'm not sure if it's as simple as I thought, or I probably would have come across this sooner.

process.getuid doesn't exist on windows, as far as I know, so it should work as before - I'll check and add a comment to that effect.

@mjbvz
Copy link
Contributor

mjbvz commented Jun 21, 2019

Sounds good. If you can, please just try to create the issue over this weekend so we can merge this PR in before we lock down for the 1.36 release

@asztal
Copy link
Contributor Author

asztal commented Jun 21, 2019

I haven't been able to verify that process.getuid really is undefined on Windows, I don't have it installed unfortunately - but the node source code does confirm it.
https://github.com/nodejs/node/blob/master/lib/internal/bootstrap/node.js#L120

In any case, I assume on Windows each user has their own temp directory so windows users would have already been getting separate vscode-typescript caches per user?

@mjbvz
Copy link
Contributor

mjbvz commented Jun 22, 2019

Thanks for creating the issue.

Yes, I think that's that case on windows. I just don't want to create separate directories per process for windows specifically. Can you please make just make this explicit by adding a check such as process.platform !== 'win32'

@mjbvz mjbvz merged commit e642a0a into microsoft:master Jun 24, 2019
@mjbvz
Copy link
Contributor

mjbvz commented Jun 24, 2019

Thanks. Will add the explicit platform check

@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 2020
@asztal asztal deleted the patch-1 branch April 19, 2022 15:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants