-
Notifications
You must be signed in to change notification settings - Fork 774
[READY] Support multi-user installation for Java #1262
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
Conversation
90c0e69
to
8df7f46
Compare
Codecov Report
@@ Coverage Diff @@
## master #1262 +/- ##
==========================================
- Coverage 97.46% 97.39% -0.08%
==========================================
Files 95 95
Lines 7260 7294 +34
Branches 153 153
==========================================
+ Hits 7076 7104 +28
- Misses 143 149 +6
Partials 41 41 |
8df7f46
to
5ee86a6
Compare
Move the config directory inside the workspace, and allow the client to specify a workspace root directory. This means that it is possible to share the ycmd installation across multiple users by moving the mutable state directories elsewhere. By default we just move the config dir and use the old workspace directory. While we are at it, provide a way to wipe out the re-used workspace. When use_clean_workspace is False, the workspace can become corrupt, so we add a command to stop the server, delete the workspace (and optionally also the config directory), then restart the server.
5ee86a6
to
717a7b3
Compare
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.
Is java the only completer that required changes for shared ycmd?
Reviewed 6 of 6 files at r1.
Reviewable status: 0 of 2 LGTMs obtained (waiting on @puremourning)
ycmd/completers/language_server/language_server_completer.py, line 511 at r1 (raw file):
key, value = utils.ToUnicode( line ).split( ':', 1 ) headers[ key.strip() ] = value.strip() except Exception:
Just Exception
? Nothing more specific? When does this happen?
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.
Reviewable status: 0 of 2 LGTMs obtained (waiting on @bstaletic)
ycmd/completers/language_server/language_server_completer.py, line 511 at r1 (raw file):
Previously, bstaletic (Boris Staletic) wrote…
Just
Exception
? Nothing more specific? When does this happen?
you know I don't remember why this change is in this patch.
Anyway, yes Exception
because we want to print the data to the log when we fail here so that it can be debugged. We actually re-throw the exception so it's not like it's unsafely catching everything.
I do recall making this change when debugging something (you never get to see this data if we fail to decode/split it here). I could remove it as untestable if you like, but I'm certain that either the server was sending invalid garbage or I at least thought it was and this code was what was throwing, and I couldn't see from the log what the data that failed was.
at least that's how I recall it.
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.
Reviewable status: 1 of 2 LGTMs obtained
ycmd/completers/language_server/language_server_completer.py, line 511 at r1 (raw file):
I could remove it as untestable if you like
No need, I was just curious.
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.
Reviewed 1 of 1 files at r2.
Reviewable status: 1 of 2 LGTMs obtained
a298b39
to
95aa036
Compare
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.
Reviewed 6 of 6 files at r1, 1 of 1 files at r3.
Reviewable status:complete! 2 of 2 LGTMs obtained
Thanks for sending a PR! |
1 similar comment
Thanks for sending a PR! |
Thanks for sending a PR! |
ycm-core/ycmd#1275 Remap python GoTo* commands ycm-core/ycmd#1274 Migrate to Omnisharp-Roslyn ycm-core/ycmd#1239 Error ID in diagnostic messages ycm-core/ycmd#1267 Clangd extra conf support ycm-core/ycmd#1266 JDT update to 0.40.0 ycm-core/ycmd#1245 Generic (pluggable) LSP completer support ycm-core/ycmd#1263 Support JDT extensions ycm-core/ycmd#1261 Typescript update ycm-core/ycmd#1264 [vimspector](/puremourning/vimspector) configuration ycm-core/ycmd#1262 Multi-user installation ycm-core/ycmd#1243 Migrate to GOPLS ycm-core/ycmd#1249 Support MSVC 16 (VS2019) ycm-core/ycmd#1224 Migrate to RLS
ycm-core/ycmd#1275 Remap python GoTo* commands ycm-core/ycmd#1274 Migrate to Omnisharp-Roslyn ycm-core/ycmd#1239 Error ID in diagnostic messages ycm-core/ycmd#1267 Clangd extra conf support ycm-core/ycmd#1266 JDT update to 0.40.0 ycm-core/ycmd#1245 Generic (pluggable) LSP completer support ycm-core/ycmd#1263 Support JDT extensions ycm-core/ycmd#1261 Typescript update ycm-core/ycmd#1264 [vimspector](/puremourning/vimspector) configuration ycm-core/ycmd#1262 Multi-user installation ycm-core/ycmd#1243 Migrate to GOPLS ycm-core/ycmd#1249 Support MSVC 16 (VS2019) ycm-core/ycmd#1224 Migrate to RLS
ycm-core/ycmd#1275 Remap python GoTo* commands ycm-core/ycmd#1274 Migrate to Omnisharp-Roslyn ycm-core/ycmd#1239 Error ID in diagnostic messages ycm-core/ycmd#1267 Clangd extra conf support ycm-core/ycmd#1266 JDT update to 0.40.0 ycm-core/ycmd#1245 Generic (pluggable) LSP completer support ycm-core/ycmd#1263 Support JDT extensions ycm-core/ycmd#1261 Typescript update ycm-core/ycmd#1264 [vimspector](/puremourning/vimspector) configuration ycm-core/ycmd#1262 Multi-user installation ycm-core/ycmd#1243 Migrate to GOPLS ycm-core/ycmd#1249 Support MSVC 16 (VS2019) ycm-core/ycmd#1224 Migrate to RLS
ycm-core/ycmd#1275 Remap python GoTo* commands ycm-core/ycmd#1274 Migrate to Omnisharp-Roslyn ycm-core/ycmd#1239 Error ID in diagnostic messages ycm-core/ycmd#1267 Clangd extra conf support ycm-core/ycmd#1266 JDT update to 0.40.0 ycm-core/ycmd#1245 Generic (pluggable) LSP completer support ycm-core/ycmd#1263 Support JDT extensions ycm-core/ycmd#1261 Typescript update ycm-core/ycmd#1264 [vimspector](/puremourning/vimspector) configuration ycm-core/ycmd#1262 Multi-user installation ycm-core/ycmd#1243 Migrate to GOPLS ycm-core/ycmd#1249 Support MSVC 16 (VS2019) ycm-core/ycmd#1224 Migrate to RLS
ycm-core/ycmd#1275 Remap python GoTo* commands ycm-core/ycmd#1274 Migrate to Omnisharp-Roslyn ycm-core/ycmd#1239 Error ID in diagnostic messages ycm-core/ycmd#1267 Clangd extra conf support ycm-core/ycmd#1266 JDT update to 0.40.0 ycm-core/ycmd#1245 Generic (pluggable) LSP completer support ycm-core/ycmd#1263 Support JDT extensions ycm-core/ycmd#1261 Typescript update ycm-core/ycmd#1264 [vimspector](/puremourning/vimspector) configuration ycm-core/ycmd#1262 Multi-user installation ycm-core/ycmd#1243 Migrate to GOPLS ycm-core/ycmd#1249 Support MSVC 16 (VS2019) ycm-core/ycmd#1224 Migrate to RLS
jdt.ls workspace areas are mutable and written by the server. Putting them
underneath the ycmd installation, even in their own directory makes it
impossible to use a shared installation of ycmd. In order to allow that, we
expose another (hidden) option which moves the workspace root dirdectory
somewhere else, such as the user's home directory.
The 'config' directory is a bit of a misnomer. It is really a working area
for eclipse to store things that eclipse feels entitled to store,
that are not specific to a particular project or workspace.
Importantly, the server writes to this directory, which means that in order
to allow installations of ycmd on readonly filesystems (or shared
installations of ycmd), we have to make it somehow unique at least per user,
and possibly per ycmd instance.
To allow this, we let the client specify the workspace root and we always
put the (mutable) config directory under the workspace root path. The config
directory is simply a writable directory with the config.ini in it.
Note that we must re-copy the config when it changes. Otherwise, eclipse
just won't start. As the file is generated part of the jdt.ls build, we just
always copy and overwrite it.
Move the config directory inside the workspace, and allow the client to
specify a workspace root directory. This means that it is possible to
share the ycmd installation across multiple users by moving the mutable
state directories elsewhere. By default we just move the config dir and
use the old workspace directory.
While we are at it, provide a way to wipe out the re-used workspace.
When use_clean_workspace is False, the workspace can become corrupt, so
we add a command to stop the server, delete the workspace (and
optionally also the config directory), then restart the server.
I have been using this change for years at work and it is very stable. Would like to not maintain it in a fork any longer.
This change is