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

Merged
merged 10 commits into from
May 23, 2024
Merged
13 changes: 13 additions & 0 deletions .devcontainer/Readme.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
## Using VSCode devcontainers

Official tutorial: https://code.visualstudio.com/docs/devcontainers/tutorial

### Recommended Settings for macOS

Some of these are defaults:
- Recommended settings for macOS (some of these are defaults):
- General:
- "Choose file sharing implementation for your containers": VirtioFS (better IO performance)
- Resources:
- CPUs: Allow docker to use most or all of your CPUs
- Memory: Allow docker to use most or all of your memory
10 changes: 10 additions & 0 deletions .devcontainer/default/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
FROM swiftlang/swift:nightly-main-jammy
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be 6.0? @ahoppen what are your thoughts?

Suggested change
FROM swiftlang/swift:nightly-main-jammy
FROM swiftlang/swift:nightly-6.0-jammy

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have strong thoughts, but when I put this together I believe the guidance was that I should be using a "recent nightly". This should follow whatever the guidance is currently.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ahoppen can make the call. I guess if it is for development maybe it should stay at main

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use the latest main because the main branch of SourceKit-LSP might be using features from sourcekitd, that are only available in main. When running SourceKit-LSP tests with an older version of sourcekitd, those tests would be skipped.


RUN \
# Disable apt interactive prompts for this RUN command
export DEBIAN_FRONTEND="noninteractive" && \
# Update apt package list
apt-get update && \
# Install sourcekit-lsp dependencies
apt-get install -y libsqlite3-dev libncurses5-dev python3

32 changes: 32 additions & 0 deletions .devcontainer/default/devcontainer.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Reference: https://containers.dev/implementors/json_reference/
{
"name": "SourceKit-LSP",
"dockerFile": "Dockerfile",

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worthwhile using the common-utils feature to setup a vscode user so we aren't running as root eg

    "features": {
        "ghcr.io/devcontainers/features/common-utils:2": {
            "installZsh": "false",
            "username": "vscode",
            "userUid": "1000",
            "userGid": "1000",
            "upgradePackages": "false"
        },
        "ghcr.io/devcontainers/features/git:1": {
            "version": "os-provided",
            "ppa": "false"
        },
    },
    "remoteUser": "vscode"

I also include the git feature although I'm not certain that is necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to add this but just curious what the downsides are of running as root? I'd also want to make sure that this is indeed something that is relevant to the project, and not just something the user can set in their own dev.containers.defaultFeatures setting. I also don't know how the project setting interacts with a user setting (i.e. if I have installZsh = true in my defaults and it is false here what happens?)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it's a security issue. All of Microsoft's language images have a vscode user. And they use the common-utils feature to set it up. So there shouldn't be any issues. I guess in theory you shouldn't need the git feature as git is already in the swift image.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to keep the .devcontainer.json file minimal. You will be running everything as root if you’re using the swift:nightly-main-jammy docker image, so I don’t think there’s any inherent harm in also running as root inside the devcontainer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not an expert, but AFAICT the "not running as root" stuff is more to do with software in the container making bad assumptions and not running properly as root than anything to do with security.

// Allow the processes in the container to attach a debugger
"capAdd": [ "SYS_PTRACE" ],
"securityOpt": [ "seccomp=unconfined" ],

"mounts": [
// Use a named volume for the build products for optimal performance (https://code.visualstudio.com/remote/advancedcontainers/improve-performance#_use-a-targeted-named-volume)
"source=${localWorkspaceFolderBasename}-build,target=${containerWorkspaceFolder}/.build,type=volume",
// Do the same for experimental background indexing
"source=${localWorkspaceFolderBasename}-index-build,target=${containerWorkspaceFolder}/.index-build,type=volume"
],
"customizations": {
"vscode": {
"extensions": [
"sswg.swift-lang"
],
"settings": {
"lldb.library": "/usr/lib/liblldb.so",
"swift.buildArguments": [
"-Xcxx",
"-I/usr/lib/swift",
"-Xcxx",
"-I/usr/lib/swift/Block"
]
}
}
}
}