-
Notifications
You must be signed in to change notification settings - Fork 201
Add docker compose as a version manager #2919
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
base: main
Are you sure you want to change the base?
Conversation
I have signed the CLA! |
272316f
to
2136991
Compare
Well, perhaps the proposed changes are too significant. I came to this conclusion after noticing this pull request ;) |
👋 Hi @d-lebed, I'm not a Docker user, but my understanding is that Ruby LSP is already usable with it via the approach described here: https://github.com/Shopify/ruby-lsp/tree/main/vscode#developing-on-containers Have you tried that? |
Hello, @andyw8. Sure, we've tried this. But unfortunately limitations VSCode adds when using dev containers approach make this unusable for us (and many other developers I believe). For example some other VSCode extensions does not work in dev containers modes, it requires complex configuration for services and networks if you need more then just MySQL and Redis, and so on. |
@andyw8, @vinistock, I can actually propose an alternative solution that eliminates the need for path mapping in the VSCode extension: adding path mapping support directly to |
b5b2464
to
f6276dd
Compare
@d-lebed We're currently using |
Hi @phil-janeapp |
This pull request is being marked as stale because there was no activity in the last 2 months |
Thanks for the PR, but I do have some concerns. First, is that this PR would make launching the LSP work, but it is not a complete solution in terms of the entire editor functionality. VS Code's preferred way of connecting to docker containers is through their own configuration so that everything in the editor works thanks to the file system API. Every part of the editor and every extension that needs the file system (terminal, debugging, LSPs and so on), will simply work once it is connected to the container. If we were to take on all of this complexity, it wouldn't be a matter of just shipping it. Every single time we have to manipulate files in the extension, we will need to remember to take this use case into account - since the file system API would not automatically handle this for us. If there are shortcomings with the VS Code experience for containers and custom workflows, have you tried providing this feedback to them? Our experience has been that they are quite receptive to feedback and maybe this is something they would like to improve. Also, would adding support for using sockets in the LSP work for your use case? If the language server is booted from the container on a port and the extension connects to it, will that do the trick? If so, then I would prefer we implemented that instead. Using sockets is a part of LSP specification and it will be a much less complex change. Finally, I would like to understand how other language servers deal with this issue. It might be a solved problem and getting a better understanding of how other implementations do it will help (like Rust Analyzer). |
Motivation
Many developers working on complex projects rely on Docker Compose to set up their development environments. However, it is not always feasible to use VS Code Dev Containers due to their limited functionality and lack of compatibility with custom workflows. To address this limitation, I have implemented a Docker Compose version manager for the Ruby LSP VS Code extension. This change allows developers to use the LSP server seamlessly within their existing Docker Compose setups, including setups enhanced with tools like Mutagen or even "dip". This change aims to improve compatibility and flexibility for developers using diverse development environments.
This change should also solve #2223
Implementation
I had to make several significant changes:
Process Execution: To retrieve Ruby information, I replaced the use of
exec
withspawn
, passing a script for execution. This change was made to avoid issues with escaping and to address various problems with container entrypoint implementations.Enhanced Manager Functionality: Since commands like
bundle install
, Rails generators, and others need to be executed inside Docker Compose, themanager
object now has expanded functionality and is available within theRuby
class.URI Converters: I implemented
uriConverters
methods (code2Protocol
andprotocol2Code
) to map paths between the local file system and the container. I also applied the converter logic in relevant parts of the code that I was able to identify.Docker Compose Configuration Parsing: I added automatic parsing of the Docker Compose configuration. Initially, path mapping was intended to be configured manually, but I found it straightforward to extract this information from the configuration itself.
I did not check the Sorbet extension or others that rely on
process.env
. They will not work, obviously.Automated Tests
I updated all the existing tests and added a couple of new ones. If the proposed changes are approved, I will add sufficient new tests to cover the new functionality.
Manual Tests
I tested the extension locally on macOS using both Docker Compose and Mutagen Compose. Additionally, I verified compatibility with
rvm
andshadowenv
.To test the functionality, you can use a similar
docker-compose.yml
configuration: