Introduce workspace-vscode-base and workspace-vscode-cpp images #11096
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #11096 +/- ##
==========================================
+ Coverage 72.01% 72.05% +0.04%
==========================================
Files 587 595 +8
Lines 95104 95717 +613
Branches 7680 7717 +37
==========================================
+ Hits 68490 68971 +481
- Misses 26126 26248 +122
- Partials 488 498 +10 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Thanks for doing this! This seems like broadly the right direction to move in.
You'll need to update our CI process to build and publish these images. There are two relevant workflows:
- https://github.com/PrairieLearn/PrairieLearn/blob/master/.github/workflows/images.yml: this is responsible for building/publishing after pushing to
master - https://github.com/PrairieLearn/PrairieLearn/blob/master/.github/workflows/main.yml: this is responsible for building on PR branches and pushing the resulting images with a tag based on the Git SHA.
Take care to build the images in the correct order. That is, you'll need to build ...-base before building ...-python and ...-cpp.
workspaces/vscode-python/README.md
Outdated
| instead of using Microsoft's official VS Code marketplace. Many extensions | ||
| are available from Open VSX by the same names, but some extensions might not | ||
| be compatible. | ||
| This image is a copy of `workspace-vscode-basic` with Python installed. |
There was a problem hiding this comment.
| This image is a copy of `workspace-vscode-basic` with Python installed. | |
| This image is based on `prairielearn/workspace-vscode-base`. It includes Python, an opinionated set of packages, and some Python extensions for VS Code. |
I'd also encourage you to link to the ...-base documentation so that users can see details about home directories and such (the problem that you yourself ran into recently on Slack!).
There was a problem hiding this comment.
I'd be inclined to explicitly say something more like "see [link] for recommended settings when using this workspace in this question", or something like that (please improve as you see fit). We really want people to click that link!
Please make similar changes to the cpp workspace image's readme.
workspaces/vscode-python/Dockerfile
Outdated
| @@ -66,39 +28,15 @@ RUN export arch=`uname -m` \ | |||
| && mamba clean --all --yes --quiet \ | |||
| && pip cache purge | |||
|
|
|||
| # Install VS Code extensions and clear the extension cache to reduce image size. | |||
| # Install VS Code extensions. | |||
There was a problem hiding this comment.
Why did you remove the "...and clear the extension cache" part of the comment and script?
There was a problem hiding this comment.
The removal was done in vscode-base and I didn't think that /home/coder/.local/share/code-server/CachedExtensionVSIXs needed to be removed again. but my understanding of extensions and the cache is very limited!
There was a problem hiding this comment.
We want this to be done after each time an extension is installed, as the cache will be re-populated with stuff whenever a new extension is installed.
# Conflicts: # workspaces/vscode-python/Dockerfile
|
Because of the licensing issues, I don't think we can add |
Is there another reason you were worried about the licensing of the extension? |
This is the key part of https://github.com/microsoft/vscode-cpptools/blob/main/LICENSE.md See also https://coder.com/docs/code-server/FAQ#why-cant-code-server-use-microsofts-extension-marketplace - non-VSCode editors can't use extensions published to Microsoft's extension marketplace. |
|
@jgfoster for the images to build correctly, you'll need to add something like this (note that the build is currently failing because PrairieLearn/.github/workflows/main.yml Lines 256 to 262 in 522794e This should be added after the cc @reteps this will have implications for #11102. I'd like you to propose a plan to handle image dependencies like this before we merge that PR. |
There was a problem hiding this comment.
Two more notes:
- Please add this image to the list of workspace images in the workspace docs:
PrairieLearn/docs/workspaces/index.md
Lines 231 to 239 in 49955d8
- Please add an example question in
exampleCourse. Theprairielearn/workspace-vscode-pythonhas an example question here: https://github.com/PrairieLearn/PrairieLearn/tree/master/exampleCourse/questions/demo/workspace/vscode-python. You can hopefully pattern match off that.
After that, this should be more or less ready to go! Just pending input from Jonatan on appropriate extensions to install.
exampleCourse/questions/demo/workspace/vscode-cpp/question.html
Outdated
Show resolved
Hide resolved
exampleCourse/questions/demo/workspace/vscode-cpp/workspace/source.cpp
Outdated
Show resolved
Hide resolved
| # Apt installs: Enable manpages with "unminimize", and common utilities. | ||
| USER root | ||
| ARG DEBIAN_FRONTEND=noninteractive | ||
| RUN apt-get update && \ | ||
| apt-get install -y --no-install-recommends \ | ||
| man-db unminimize \ | ||
| gosu curl git htop less nano unzip vim wget zip && \ | ||
| yes | unminimize && \ | ||
| apt-get clean && \ | ||
| rm -rf /var/lib/apt/lists/* && \ | ||
| find /tmp -not -path /tmp -delete |
There was a problem hiding this comment.
The original image had these as two separate steps with a cache clean in between:
# Apt installs: Enable manpages with "unminimize", and then install common utilities.
USER root
ARG DEBIAN_FRONTEND=noninteractive
RUN apt-get update && \
apt-get install -y man-db unminimize && \
yes | unminimize && \
apt-get clean && \
rm -rf /var/lib/apt/lists/* && \
find /tmp -not -path /tmp -delete
RUN apt-get update && \
apt-get install -y --no-install-recommends \
gosu curl git htop less nano unzip vim wget zip && \
apt-get clean && \
rm -rf /var/lib/apt/lists/* && \
find /tmp -not -path /tmp -delete
I opted to combine them, as it's a little faster if we can avoid removing all the cached data in between.
There was a problem hiding this comment.
I think my logic in installing man first was that if you install it and unminimize after other packages are installed, then some redundant work seems to be done adding their man pages belatedly. So, I thought it would be more efficient to make sure that man and unminimize are done before installing additional stuff.
There was a problem hiding this comment.
...Also, I had it as a separate RUN step out in front so that if more packages needed to be added later (in the subsequent RUN step), it would be slightly faster to rebuild. I didn't benchmark any of those hunches, though.
|
I found a set of extensions that gives a better user experience: This does not include support for Makefile, so it's not perfect, but it has some configurable options for flags and binary name, so it should be good as an MVP. |
|
|
Would this extension be any good instead? https://open-vsx.org/extension/eclipse-cdt/cdt-gdb-vscode |
|
The main problem with many of these extensions (including the ones from Eclipse and franneck94) is that they to some extent depend on functionality provided by the VSCode default extensions, which are not available on code-server. I checked the Eclipse extension and the vscode-lldb extension, and they both only provide the debugging functionality, but not the ability to compile the code or even actually launch the debugger itself. I'm playing around with other combinations of extensions to see if I can make these work. |
|
I managed to get a decent user flow while limiting some of the weird extensions involved, and adding support for Makefile. In essence, the extensions are: Then the workspace code includes a .vscode/tasks.json file: {
"tasks": [
{
"type": "shell",
"label": "Make",
"command": "make",
}
],
"version": "2.0.0"
}And a .vscode/launch.json file: {
"version": "0.2.0",
"configurations": [
{
"name": "(gdb) Launch",
"type": "cppdbg",
"request": "launch",
"program": "${workspaceFolder}/main",
"args": [],
"cwd": "${fileDirname}",
"environment": [],
"externalConsole": false,
"MIMode": "gdb",
"preLaunchTask": "Make"
}
]
}I also included a Makefile that compiles the code as needed. This causes the make step to be executed as a task, and debugging automatically runs that task before launching. I assume an instructor should be able to use this as a starting point and configured as needed if it's documented (possibly in README.md and/or as a sample in the example course). |
These two are no longer needed if the launch.json and tasks.json files above are included.
It seems the repository (https://gitee.com/quanzhuo/cpp-debug) has, as its last commit, the deletion of its license, which I presume to mean it's not a permissive license. I managed to get away with it, by using the lldb provider for debugging, which provides a pretty good debugging experience as well. My launch.json is now: {
"version": "0.2.0",
"configurations": [
{
"type": "lldb",
"request": "launch",
"name": "Launch",
"program": "${workspaceFolder}/main",
"args": [],
"cwd": "${workspaceFolder}",
"preLaunchTask": "Make"
}
]
}tasks.json remains the same as the one above. This no longer requires kylin, so extensions are: In essence:
Here's a capture: If you're ok with this I can push these changes and a suggested update to README.md with steps instructors are encouraged to use. |
|
I'm fine with whatever the maintainers want as far as extensions. If we don't settle quickly on a consensus, however, my inclination would be to start with little or nothing just to get the primary feature in, and then deal with extensions as a separate PR. I'd hate for the workspace to get bogged down on extension debates. |
|
@nwalters512 as discussed in the meeting I have added the extensions and documented the files for tasks and launch info. I also added these files to the example course question that @jgfoster has created, and it works fine locally. I decided to skip the Assembly extension, as it's not directly related to C/C++ and instructors have the option of adding it if they wish in a custom image. |
There was a problem hiding this comment.
I tested a local build with amd64 and it's building and working as expected. I couldn't test arm64 but I assume @nwalters512 did. In any case, I'd be fine with either merging as is or with the suggested change in retrieving the architecture.
| RUN arch=$(uname -m) && \ | ||
| if [ "$arch" = "x86_64" ]; then arch="x64"; \ | ||
| elif [ "$arch" = "aarch64" ]; then arch="arm64"; \ | ||
| else echo "Unsupported architecture: $arch"; exit 1; fi && \ |
There was a problem hiding this comment.
To double-check what the extension itself uses to identify the platform I checked this:
https://github.com/vadimcn/codelldb/blob/d1ae2ebef7e44ea13d3aa8282f0b0b85d4838b6f/extension/install.ts#L96-L103
https://github.com/vadimcn/codelldb/blob/d1ae2ebef7e44ea13d3aa8282f0b0b85d4838b6f/package.json#L1124-L1135
Since we're working with a guaranteed Linux environment and we're assuming that the build only happens in either amd64 or arm64 I think this approach makes sense, however an alternative option may be to use:
| RUN arch=$(uname -m) && \ | |
| if [ "$arch" = "x86_64" ]; then arch="x64"; \ | |
| elif [ "$arch" = "aarch64" ]; then arch="arm64"; \ | |
| else echo "Unsupported architecture: $arch"; exit 1; fi && \ | |
| RUN arch=$(/usr/lib/code-server/lib/node -p 'os.arch()') && \ |
Won't block on this, though.
There was a problem hiding this comment.
I would prefer to stick with uname -m for consistency reasons in the rest of our Dockerfiles / shell scripts.
In Slack, @nwalters512 suggested that "We'd definitely like to have a first party VSCode C++ image, and in fact we'd like to get a workspace-vscode-base image or the like that we can build all language-specific images on top of. PRs are welcome if you'd like to contribute either/both of those!"
Now that Python is building again (miniconda issues), I've been able to work on this. This PR represents a (somewhat simplistic) separation of
vscode-pythonintovscode-base,vscode-python, andvscode-cpp. Is this in the general direction you were suggesting? I welcome advice/direction!