Skip to content
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

Is the dependency to the Rust devcontainer feature needed? #32

Closed
andreimaxim opened this issue May 26, 2024 · 5 comments
Closed

Is the dependency to the Rust devcontainer feature needed? #32

andreimaxim opened this issue May 26, 2024 · 5 comments

Comments

@andreimaxim
Copy link

While playing around with the devcontainers in edge Rails, I've noticed that the following extensions were added to my VSCode:

  • CodeLLDB
  • crates
  • Even Better TOML
  • rust-analyzer

After a bit of an investigation, I discovered that these extensions are installed by the Rust feature, which has been recently marked as a dependency for the Ruby feature. I could not find any arguments in favor of this approach (the PR mentions an incompatibility with a newer version of Rust), so I figured I could start a conversation on this topic.

I'd argue in favor of replacing the dependency to the Rust feature with installing the rustc package from the operating system. There are four reasons:

  • It adds unnecessary packages, like cargo, and third-party VS Code extensions that cannot be disabled
  • It increases the build time as Rust needs to be compiled
  • Following a specific version of Rust will eventually require rebuilding the container quite often, since Rust seems to be releasing a new version every month or so
  • It adds little to no benefit for the regular Rails developer and can be confusing for newcomers (to Rails or to devcontainers)

If my arguments are strong enough, I'd be more than happy to create a PR :-)

@ryanjafari
Copy link

ryanjafari commented May 28, 2024

Thanks for this, I also noticed it

My guess is that it was probably done this way through adding the Rust feature to devcontainer.json because it was easier than adding it to the Dockerfile, yet it included a bunch of stuff that probably wasn't intended

This isn't a solution to your issue, but just FYI you have the ability to remove unwanted extensions with devcontainer.json

...
  "customizations": {
    "vscode": {
      "extensions": [
        "-vadimcn.vscode-lldb",
        "-serayuzgur.crates",
        "-tamasfe.even-better-toml",
        "-rust-lang.rust-analyzer",
        "shopify.ruby-lsp"
      ]
    }
  },
  ...

By prepending the minus before the extension identifier

Just for anyone who is annoyed by those extensions being there in your Rails project like I was

@andreimaxim
Copy link
Author

andreimaxim commented May 28, 2024

@ryanjafari I don't believe there's a need to change the Dockerfile, as Ruby is provided via a feature which already pulls the operating system packages required by ruby-build. I'd argue it's simpler to just add rustc there instead of adding Rust as a feature.

Also, thank you for the code snippet, I was unable to figure out how to disable an unwanted extension and your code snippet helps a lot!

@ryanjafari
Copy link

Ah I see, yes that makes sense to me.

Happy to help.

@andrewn617
Copy link
Collaborator

Hi @andreimaxim thank you for your report.

I think it's not a bad idea to use rustc instead of compiling rust outselves (with the feature or otherwise). Originally I did not do it because it would not allow us to use the latest version of rust, but the debian package looks like it is on 1.63 so I think it's ok.

I will work on that. If I do run into any issues, alternatively I will just disable the extensions from the image's dev container.

One thing I want to clarify:

Following a specific version of Rust will eventually require rebuilding the container quite often, since Rust seems to be releasing a new version every month or so

It is not our intention to lock to a specific Rust version for the long term. There is an open issue with Rust 1.78 that causes ruby to not build on arm machines, so we locked to 1.77 for now until that issue is fixed. But I will remove that once the fix is released (or just by using rustc) and new versions of rust will not be a reason to release new versions of the image - unless of course they fix some breaking issue with ruby / yjit, in which case we may have to revisit using debian rustc.

@andreimaxim
Copy link
Author

@andrewn617 thank you very much for the change!

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

No branches or pull requests

4 participants