-
Notifications
You must be signed in to change notification settings - Fork 8
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
Trying to run standardrb inside our existing Docker Compose setup with difficulty #13
Comments
Hey Bo! Always a pleasure to hear from you. Two thoughts without trying to replicate anything myself. This is just fact-finding and context. Not sure what the ideal solution is yet or what prior art / best practices for extensions like this is. Customizing the command pathYou can customize the command path with this property here. This may partly fix the issue but it looks like there are references to getCwd()All of your suspicions are probably correct. Reviewing now, the only way we grab the current working directory is this getCwd() function here that looks for the topmost workspace folder path—clearly not always the right answer. I am a docker noob, but if there is an addressable file path for invoking the docker version of standard seems like a dupe of #9, to customize that CWD value through a configuration. |
Hey Justin! Yeah the command path thing seems solvable. At first I tried it and it seemed to only be taking the first component of the string, but after having read through the code, that doesn't seem right, so that might have been PEBCAK. Either way, that's not the root problem as even using a shim would be a fair enough solution there. The root issue seems to be that the extension passes an absolute path over stdio to that process, but that process is running in the context of a virtualised filesystem. That is, linting/formatting/diagnosing I think the heard of it is that the extension is invoking vscode-standard-ruby/src/extension.ts Line 395 in 841e780
I am not familiar with VSCode's API, but had a squiz through the docs and there didn't seem an obvious alternative to get a workspace-relative path from the So there seems to be two alternatives as I see it:
Neither solution seems delightful to me though. WDYT? |
Hey Bo, heads up that I'm not likely to have time to fix this for a while. I agree your options aren't without downsides, but one thing I'm sorta stumped by the error message you opened with. I was sure that the LSP being passed a file URL for a non-existent path actually would actually be OK when in STDIN mode (which is the way the server invokes rubocop without reinstantiating it for each request). The file path and full text document are both passed to the server over STDIO by the LSP client and it doesn't read the source listings from the file system directly (see this and this). I also recall reading in the RuboCop source that it doesn't care what the path is in STDIN mode and it only requires it b/c some cops work differently based on the path/file name (Gemfile stuff, maybe?) To test this, I just ran this:
This exited nonzero, but fixing the file exited cleanly/0. As a result, I don't know what the nature of the error or issue is.
This was running rubocop@1.53.0, which seems to be newer than 15.2.1 from your report. Without diving into the RuboCop source and maybe throwing a |
Ack.
Huh. OK 🤔. I'll run this again and post the full stacktrace. That way we might learn what the actual codepath which is trying to read the file from disk is. |
Actually, there's no need, because it is directly been raised in that first line before I truncated the stack trace. For posterity, this is the code (from the version of Rubocop in my stacktrace) : processed_source = if @options[:stdin]
ProcessedSource.new(@options[:stdin], ruby_version, file)
else
begin
ProcessedSource.from_file(file, ruby_version)
rescue Errno::ENOENT
raise RuboCop::Error, "No such file or directory: #{file}"
end
end So, for whatever reason, when it hits this part I'm going to upgrade to the latest Rubocop and see if the issue persists. |
Yeah, OK, setting
Ah I can't because of Standard's requirement on the minor version.
So in putting a few debug statements through some of these lines, I realised something new: I am actually getting lint output in the Problems tab and I only see the By adding some Further, it appears that
|
Since
I looked in the I'm not sure where to go from here. I don't really understand what "diagnostics" means here or why it's important. |
I may have a workaround that works in my case but mightn't for others, but at least reduces any urgency on a resolution. I am still testing, but I think I can get away with mounting my app into the Docker container using the same path as the host FS: diff --git i/docker-compose.yml w/docker-compose.yml
index b406ee5e3..e242e93d8 100644
--- i/docker-compose.yml
+++ w/docker-compose.yml
@@ -13,8 +13,9 @@ services:
+ working_dir: "${PWD}"
volumes:
- - ./:/app
+ - "${PWD}:${PWD}"
- bundle-data:/usr/local/bundle
ports:
- 127.0.0.1:3000:3000
@@ -44,6 +45,7 @@ services:
# High start period to account for cold setup, where dependencies are being installed
start_period: 300s
environment:
+ - HOME=${PWD}
- DEVELOPMENT_DATABASE_URL=postgres://postgres@db/db
- TEST_DATABASE_URL=postgres://postgres@db/test-db
- SELENIUM_URL=http://selenium:4444/wd/hub |
Hi there! We came across some issues with the above workaround, and it also was quite a wide-reaching solution for a problem that only affects VSCode and the Standard Ruby extension. After a bunch of trial and error, we arrived on the following wrapper script, which solves the problem: #!/bin/bash
exec docker compose \
run \
--rm \
--no-deps \
--no-TTY \
--volume "$PWD:$PWD" \
--name "${COMPOSE_PROJECT_NAME:-$(basename $PWD)}-standardrb-$$" \
app \
standardrb "$@" It does so by mounting a volume at |
Do you all use any other extensions that execute commands? This seems like something that would come up for almost every similar extension when run under Docker, right? It'd be helpful to find some prior art |
I don't believe we do. @jonleighton actually uses Vim and just jumped in to save my arse haha |
Sorry, I have so little experience trying to integrate dev tools with Docker that I don't think I'll be able to implement a fix for this on my own without clearer direction on what changes to the code would actually solve the problem. Hopefully someone finds this and can point to another extension that works in a way that Docker agrees with |
@jonleighton Thanks, I made it work using your example. I just also needed to switch the mode to Run only globally, never via Bundler. BTW it may be handy to have this in the README.md. I might create a PR. |
I was running into a similar issue and before I found this thread (I thought the issue was directly related to VS Code LSP's implementation) I found people pointing to the Apparently this let's you transform the paths that are coming in and out of the LSP? I'm not really sure. Just thought I'd leave this here for the next person to come across it. I found out that standard was actually linting after even with the errors as those above have stated. |
Hello!
Because stock Ruby on macOS is old (
2.6.10p210
on latest Ventura) and not everybody on our team who works on our app is a Ruby dev who can be expected to maintain a local matching Ruby version, and because ourGemfile
fails to install on the host by VSCode out of the box, we're trying to getstandard
to run inside our existing docker setup.In Vim, this was possible with:
However, with this VSCode extension, things have been a bit rockier.
First, I had to create a shim
.vscode/standardrb
to setstandardRuby.commandPath
to (I had initially tried to set this todocker compose run --rm app standardrb
, but it expects the path to be a command on the path and just tried to invokedocker
, afaict). So my shim essentially isdocker compose run --rm app standardrb $*
.However, it appears that the extension is passing absolute path names to the LSP, as evidenced by this error output:
Would it be possible to have these files passed in with root-relative paths (even if gated behind a configuration option)?
The text was updated successfully, but these errors were encountered: