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

Add a VSCode devcontainer definition #916

Closed
wants to merge 3 commits into from

Conversation

GeorgeLyon
Copy link
Contributor

I'm not sure if this is interesting, but when putting together this PR I did my development in a devcontainer (instructions provided in a Readme). As I'm often managing many different environments and tools (and VSCode extensions), I really like the devcontainer workflow and on M-series Macs the performance is quite good.

@GeorgeLyon
Copy link
Contributor Author

One thing that I wasn't able to figure out is if we can add the -Xcxx -I/usr/lib/swift -Xcxx -I/usr/lib/swift/Block arguments somewhere global in the devcontainer, which I believe would enable using VSCode's test explorer with this configuration which would be very nice.

@adam-fowler
Copy link
Contributor

@GeorgeLyon Do you know about the Add Dev Container Configuration Files... command?
If you

  • select From a predefined container configuration ...
  • select Show All Definitions...
  • Type swift into the filter box
  • Select the Swift devcontainer
    You get the recommended swift container. You can find the template here https://github.com/swift-server/swift-devcontainer-template
  • There are also a number of features you can add, including both swift formats, sqlite, foundation networking and jemalloc

When you talk about VSCode's test explorer which test explorer are you talking about? There are multiple test explorers.

@GeorgeLyon
Copy link
Contributor Author

GeorgeLyon commented Oct 21, 2023

Thanks for the info! This was adapted from a slightly more complicated devcontainer I had on another project, but knowing there is a default Swift template is nice. In my other project I also used the devcontainers/ci action to run CI on the devcontainer which I really like.

I haven't used features much, but I opted to use the Dockerfile to install most things to keep things simple and more standard (I'm pretty sure Features are a devcontainer thing not a Docker thing). That said... I think they provide some interesting functionality and would be worth looking into.

One not on the Swift devcontainer template... I also really like using zsh in my devcontainers, but I do it in a slightly-bad way using a dotfiles repo and the "dotfiles.repository": VSCode setting. This way I can separate "things actually related to the project" and "things George likes" (the "default devcontainer extensions" option helps with this too)

@GeorgeLyon
Copy link
Contributor Author

GeorgeLyon commented Oct 21, 2023

When you talk about VSCode's test explorer which test explorer are you talking about? There are multiple test explorers.

I'm talking about the default "beaker" one in the UI. I gather sourcekit-lsp opts in to some "I detect tests" API which populates it. Unfortunately, because you need to specify extra commandline flags for the sourcekit-lsp project (-Xcxx -I/usr/lib/swift..., which I could find no way to specify for running from the test explorer), the tests fail to build correctly.
image

Perhaps the right thing to do here would be to make a package-config file named something like "swiftToolchain" that adds these flags. Might give it a shot later today.

@adam-fowler
Copy link
Contributor

That Test Explorer is generated by the swift extension. Go into the settings and you can add additional build arguments in the Swift: Build Arguments section. Make sure you only do it for the current workspace.

As an alternative you can add a tasks.json entry with command line parameters. Press Cmd+Shift+B and click on cog symbol at right side of menu entry for the build you want to edit.

Both of these options create files in the .vscode folder a settings.json or a tasks.json respectively. Perhaps one of these should be committed to the source kit-lsp repo

@GeorgeLyon
Copy link
Contributor Author

Go into the settings and you can add additional build arguments in the Swift: Build Arguments section
Oh man, I missed this because I only checked the sections nested under "Swift" in the extensions UI. I can confirm this works, and I added this to the devcontainer "settings" section, which writes it to the "Remote Settings" section in the devcontainer. Now the Test Explorer UI works out-of-the-box in the devcontainer, which is pretty cool.

Not sure this is the best approach as it is workspace-specific. The approach I've been taking in my own projects is to bind a different directory at .vscode in the devcontainer. This works pretty well if you commit to not having per-project VSCode settings, tasks or launch jsons (though you can still achieve this with some symlinking). I like this approach because I almost always need to tweak some setting in different devcontainer environments.

@GeorgeLyon
Copy link
Contributor Author

Also all that being said, I think the pkg-config approach is actually the "correct" way to handle these sorts of arguments. Setting this for the whole project won't work as the path is different depending on what system you are on and what toolchain you are using. Conceptually, you are depending on a library that is installed on the system, much the same way you would depend on something like sqlite3 if that was installed on the system. In a devcontainer this would work well since you likely would never need more than one Swift toolchain installed, and could just create a pkg-config for "the Swift toolchain installed on my system".
A more robust solution would be for the toolchain itself to provide a "this toolchain" .systemLibrary (or even a new enum, .toolchainLibrary or something like that) which resolves correctly even in an environment where you have multiple toolchains installed. Not something I have time to chase down at the moment, so just leaving my thoughts here.

@ahoppen
Copy link
Collaborator

ahoppen commented Oct 31, 2023

Sorry for taking a while to look at this. I think having a devcontainer is an exciting idea and it should simplify getting started with sourcekit-lsp development, especially on Linux where the setup is non-trivial.

I am not sure if I followed the entire conversation but are there any open questions left? With regards to the Dockerfile, I would prefer if it follows the default template because that way we have one standardized Dockerfile setting for new packages adding a container and sourcekit-lsp.

@GeorgeLyon
Copy link
Contributor Author

With regards to the Dockerfile, I would prefer if it follows the default template

Not sure what you mean by "follows the default template". The swift template @adam-fowler was referring to is a template for adding these files (devcontainer.json, etc) to a Swift project which doesn't have them. It directly references a Docker image by name, which is insufficient for developing sourcekit-lsp. My Dockerfile uses an official image as the base of the devcontainer (the FROM command) and then installs the dependencies of sourcekit-lsp on top of that (the RUN command).

One way to think about it is the template is a way to get a reasonable starting point when adding a new devcontainer to a project that doesn't have one, and this PR is a fleshed-out devcontainer that actually works for sourcekit-lsp.

@ahoppen
Copy link
Collaborator

ahoppen commented Oct 31, 2023

Oh, sorry. I misunderstood you there. From the discussion I thought that Add Dev Container Configuration Files… added a Dockerfile. Ignore my comment then about the Dockerfile then.

Are there any open questions/tasks on this PR or is this ready for review as far as you are concerned @GeorgeLyon? Just trying to make sure I’m not commenting on something that’s not fully ready yet.

@GeorgeLyon
Copy link
Contributor Author

Not from my end, I just have really liked the devcontainer experience recently and this is just the setup I used to dabble in this repo, so if it can be helpful to someone else, great!

@ahoppen
Copy link
Collaborator

ahoppen commented Oct 31, 2023

Definitely! I agree, having a standardized installation setup is great. I’d like to test the devcontainer myself but it might take me a while until I get to it. I’ll come back to to this PR once I tried it.

@GeorgeLyon GeorgeLyon closed this May 13, 2024
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.

None yet

3 participants