-
Notifications
You must be signed in to change notification settings - Fork 155
Update template, add features and image metadata info #21
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
Update template, add features and image metadata info #21
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tagging @AlexandraKemperMS for an additional pair of eyes when it comes to the C++ extension.
"extensions": [ | ||
"ms-vscode.cpptools" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This breaks the OOBE for this example.
Recommend ms-vscode.cpptools-extension-pack
instead, otherwise the user will get a toast notification for "C++ tools" that wants to install the extension pack.
See also comments on the related documentation update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It actually doesn't if you test out the example. The image now includes this extension as explained in the README updates. This part of the new image metadata capability for Dev Containers.
We could, however, flip to the extension pack in the image. Right now, its CMake and C++: https://github.com/devcontainers/images/blob/main/src/cpp/.devcontainer/devcontainer.json#L14-L15
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, if I understand this correctly then, the container image in the FROM
line of the Dockerfile
is constructed from a dev container spec at that link.
Do you have a link to the documentation for the new image metadata capability for Dev Containers? I'm not familiar with it.
When I tested the example, I got a VSCode notification asking to install C/C++ extensions. After I manually added back ms-vscode.cpptools
I expected that to go away, but it didn't, which is when I realized that that notification goes away with the extension pack instead. That might be worth improving on for the C++ extension (@AlexandraKemperMS).
I let an extra variable sneak into my test. :)
FWIW, I'm not crazy about hiding that implementation detail. If we expect people to learn something other than "I guess it does work." from these repositories, they may not immediately follow the leap towards the extensions being installed in the base image via a different devcontainer.json file. For me, who has some experience with using them, it really did feel like something critically important was just missing.
Is there some advantage to putting those in the base image that I'm not understanding?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure you are on the latest version of VS Code and the extension. This is a new capability. I can double check.
The link to the docs is in the README. https://devcontainers.github.io/implementors/spec/ has the details on processing and https://devcontainers.github.io/implementors/json_reference/ annotates which properties are supported.
The goal is to prevent config drift by keeping the settings needed for the image with the image. It also allows mass config updates across many repos at once which has been a common ask. Finally it simplifies reuse since you can just ref an image directly if you prefer.
}, | ||
"runArgs": [ "--cap-add=SYS_PTRACE", "--security-opt", "seccomp=unconfined"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had been curious about what this was meant to do when I saw it in the past. It looks like it's useful to enable some developer tools on a Docker container, but I agree it shouldn't be in this example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is required to enable ptrace-based debugging which, unless something has chainged, is needed to debug C++. This is now set on the image. https://github.com/devcontainers/images/blob/main/src/cpp/.devcontainer/devcontainer.json#L6-L7
Go and Rust also need this, but things like Python, C#, and Node do not. It's a common "gotcha".
@@ -0,0 +1,58 @@ | |||
#!/usr/bin/env bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is not necessary for this example and adds a lot of extra complexity. The C++ code does not use CMake and works just fine with the both the base C++ extension and the C++ extension pack (which includes the CMake extension... but not CMake...).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vcpkg's requirements in some cases require a more recent version of cmake than is in the distro itself. This was added to the template when Vcpkg was added to the image. I think Debian 11 has a recent enough cmake though, so we could remove it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example source code does not use vcpkg either though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The image is general purpose not specific to the sample. The README at the top of devcontainer.json points to the general purpose template. Happy to take a PR to illustrate its use if you would like.
1. **Install the GitHub CLI using a Dev Container Feature:** | ||
- Press <kbd>F1</kbd> and select the **Dev Containers: Configure Container Features...** or **Codespaces: Configure Container Features...** command. | ||
- Type "github" in the text box at the top. | ||
- Check check box next to "GitHub CLI" (published by devcontainers) | ||
- Click OK | ||
- Press <kbd>F1</kbd> and select the **Dev Containers: Rebuild Container** or **Codespaces: Rebuild Container** command so the modifications are picked up. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This addition prompts an important question for me.
What are the purposes of this repository?
- To prove to a new-to-dev containers C++ user that the system works for a simple example?
- To expose C++ to existing dev containers users (providing them an idea of what a functioning C++ project should look like)?
- To provide a playground to learn about dev container features for someone who just happens to prefer/select the C++ example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The purpose of this repository is to demonstrate the benefits of Dev Containers to C++ users. The sample focuses on things you can do with Dev Containers as a result. Key things to illustrate include:
- The extension works in this context just like it does locally
- Image metadata capabilities nowallow dev container metadata previously required in devcontainer.json to be directly tied to the image. This avoids "drift" where the image and devcontainer.json get out of sync, enables mass updates, and simplifies devcontainer.json contents at a project level.
- How easy it is to add additional capabilities using Dev Container Features
In some cases, extension authors have also opted to illustrate capabilities of the extension (see https://github.com/microsoft/vscode-remote-try-go). Feel free to submit a PR with steps like this if you would like.
Some things to try: | ||
|
||
1. **Edit:** | ||
- Open `main.cpp` | ||
- Try adding some code and check out the language features. | ||
- Notice that the C++ extension is already installed in the container since the `.devcontainer/devcontainer.json` lists `"ms-vscode.cpptools"` as an extension to install automatically when the container is created. | ||
- Make a spelling mistake and notice it is detected. The [Code Spell Checker](https://marketplace.visualstudio.com/items?itemName=streetsidesoftware.code-spell-checker) extension was automatically installed because it is referenced in `.devcontainer/devcontainer.json`. | ||
- Also notice that utilities like `Vcpkg` and the [C++](https://marketplace.visualstudio.com/items?itemName=ms-vscode.cpptools) extension are installed. Tools are installed in the `mcr.microsoft.com/devcontainers/cpp` image and Dev Container settings and metadata are automatically picked up from [image labels](https://containers.dev/implementors/reference/#labels). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See previous comments on what software should be installed. The current head of the branch removes the C++ extension.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To recap:
- The second note is highlighting the new image metadata capabilities that brings these extensions across without being in devcontainer.json. See https://containers.dev/implementors/reference/#labels
- Happy to have another extension, but this spell checker is present to illustrate that you can supplement image metadata with contents from devcontainer.json.
I still have questions about the "audience" for this repository and how these changes help them, but I suspect learning more about about the image metadata capabilities will help some with relevant context. Don't feel that those open questions should in any way block merging this PR. Thanks for all your responses @Chuxel. |
No description provided.