Skip to content

[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

Merged
merged 3 commits into from
Jun 16, 2019

Conversation

puremourning
Copy link
Member

@puremourning puremourning commented Jun 15, 2019

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 Reviewable

@codecov
Copy link

codecov bot commented Jun 15, 2019

Codecov Report

Merging #1262 into master will decrease coverage by 0.07%.
The diff coverage is 86.36%.

@@            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

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.
Copy link
Collaborator

@bstaletic bstaletic left a 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?

Copy link
Member Author

@puremourning puremourning left a 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.

Copy link
Collaborator

@bstaletic bstaletic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

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.

Copy link
Collaborator

@bstaletic bstaletic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm_strong:

Reviewed 1 of 1 files at r2.
Reviewable status: 1 of 2 LGTMs obtained

Copy link
Member Author

@puremourning puremourning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 6 of 6 files at r1, 1 of 1 files at r3.
Reviewable status: :shipit: complete! 2 of 2 LGTMs obtained

@mergify
Copy link
Contributor

mergify bot commented Jun 15, 2019

Thanks for sending a PR!

1 similar comment
@mergify
Copy link
Contributor

mergify bot commented Jun 15, 2019

Thanks for sending a PR!

@mergify mergify bot merged commit 840453c into ycm-core:master Jun 16, 2019
@mergify
Copy link
Contributor

mergify bot commented Jun 16, 2019

Thanks for sending a PR!

bstaletic added a commit to bstaletic/YouCompleteMe that referenced this pull request Jul 3, 2019
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
bstaletic added a commit to bstaletic/YouCompleteMe that referenced this pull request Jul 4, 2019
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
bstaletic added a commit to bstaletic/YouCompleteMe that referenced this pull request Jul 4, 2019
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
bstaletic added a commit to bstaletic/YouCompleteMe that referenced this pull request Jul 4, 2019
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
bstaletic added a commit to bstaletic/YouCompleteMe that referenced this pull request Jul 5, 2019
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants