devcontainers / templates

Repository for Dev Container Templates that are managed by Dev Container spec maintainers. See https://github.com/devcontainers/template-starter to create your own!
https://containers.dev/templates
MIT License
1.02k stars 262 forks source link

Use vscode user to install gems. #202

Closed ThomasOwens closed 1 year ago

ThomasOwens commented 1 year ago

Updated the line to install the Rails gem to use the vscode user. This brings it in line with the optional line to install node packages. Also updated the example line for additional gems.

ThomasOwens commented 1 year ago

This pull request appears, initially, to address the immediate concern that I raised in #188. The directory where the gems are installed appear to be owned by the vscode user and it looks like the ruby-lsp gem is able to be installed when the VS Code extension is installed. This is all the desired behavior.

I'm not sure what, if any, tests are necessary to cover this. I will continue exploring this change manually, but could use some guidance if I should be update the tests and how.

However, there are still fundamental issues that remain unaddressed:

ThomasOwens commented 1 year ago

@microsoft-github-policy-service agree

ThomasOwens commented 1 year ago

@samruddhikhandale This is ready for your review. I've created a basic Rails application and used this devcontainer to run an existing Rails application without error. Do let me know if you want the tests to be updated - I suspect the test would be to check who owns the gems directory for the default rvm version and ensure it's "vscode" and not "root", but I'm not sure if that is the right test. If you'd like to see a specific test, let me know and I can look at see what it would take to implement.

ThomasOwens commented 1 year ago

@samruddhikhandale I've added the tests.

I left out the webdriver test. Honestly, I'm not sure why webdriver is being installed where it is. I know it's common, but it's not a dependency of Rails as far as I can see. If a project is using it, I'd expect it in the gemfile or however the project is managing dependencies, I don't want to remove it from the container setup because I feel that could be a breaking change for someone out there and isn't related. So I ended up adding two tests to verify that (1) Rails is installed and (2) the user can install gems.

I did run these tests on the devcontainer without the other changes. The Rails test passes and the check for the user's ability to install gems fails.

Let me know if there's anything else you'd like to see.

ThomasOwens commented 1 year ago

@samruddhikhandale

My solution here is starting to feel a bit hacky.

I fixed the issue with the failing tests in GitHub Actions. I think I was running into a similar problem that the fixTestProjectFolderPrivs function is designed to work around. In my local builds, the vscode user was owning the directories. However, in the GitHub Actions builds, it was owned by 1000. I didn't want to force the ownership of the directories, so I started to look for another solution.

I noticed that the rvm group didn't have write permission to the gems directories, even though it should. Since the vscode user is indeed in the rvm group, making sure the group had the right permissions made sense. So even though it wasn't necessary for local builds to work, I also wanted a solution that would pass the tests.

We could get away with just running the rvm fix-permissions command and not running the gem install as the vscode user, but it seems like we shouldn't be installing gems as the root user anyway.

So, at this point, the end result of the devcontainer build process is a devcontainer where a user can install gems. I still have lots of questions about the utility and viability of this devcontainer and if installing rvm (and, apparently, rbenv) is appropriate. But those are longer term discussions and decisions that go beyond me. The important thing is that anyone who picks up this devcontainer shouldn't have broken VS Code extensions and should be able to run a Rails project out-of-the-box.

Let me know if there are any other questions or concerns and I'll do my best to address them.