feat(devcontainer): improve performance and reliability of devcontainer launch #472
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Follow up to #119. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #472 +/- ##
==========================================
- Coverage 76.22% 76.19% -0.03%
==========================================
Files 20 20
Lines 3503 3503
==========================================
- Hits 2670 2669 -1
- Misses 833 834 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Improves devcontainer build/attach performance and reliability by removing unneeded features, standardizing the workspace path, and adding volume mounts and git-config handling for better cross-environment behavior.
Changes:
- Standardize devcontainer workspace to
/workspaceand add volumes for user config +node_modules. - Add post-create ownership workaround for mounted volumes and a post-attach git identity reconfiguration step.
- Add repository-level ignores/attributes to support the new devcontainer flow (gitconfig exports, dockerignore, line ending rules).
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
.gitignore |
Ignores generated gitconfig export files used by the devcontainer attach workaround. |
.gitattributes |
Adds explicit checkout line-ending rules for scripts and Dockerfiles. |
.dockerignore |
Excludes .git and node_modules from Docker build contexts for speed/smaller contexts. |
.devcontainer/scripts/post-create.sh |
Adds volume ownership fix step and installs Node dependencies during container setup. |
.devcontainer/scripts/post-attach.sh |
Applies exported git identity settings inside the container after attach. |
.devcontainer/devcontainer.json |
Normalizes workspace mount/folder, adds volumes, removes unused features, and wires initialize/postAttach commands. |
.devcontainer/README.md |
Updates documented toolset and troubleshooting guidance to match the new devcontainer behavior. |
| *.ps1 text eol=crlf | ||
| *.bat text eol=crlf | ||
| *.cmd text eol=crlf | ||
|
|
There was a problem hiding this comment.
Setting *.ps1 to eol=crlf will break direct execution on Linux/macOS when scripts rely on their #!/usr/bin/env pwsh shebang (the \r yields /usr/bin/env^M: bad interpreter). Repo docs show running these scripts as ./scripts/.../*.ps1. Consider removing the forced CRLF for *.ps1 (or switching to eol=lf) to keep cross-platform executability.
| *.ps1 text eol=crlf | |
| *.bat text eol=crlf | |
| *.cmd text eol=crlf | |
| *.bat text eol=crlf | |
| *.cmd text eol=crlf | |
| ## Cross-platform PowerShell scripts - use LF for shebang compatibility | |
| *.ps1 text eol=lf |
| "image": "mcr.microsoft.com/devcontainers/base:ubuntu", | ||
| // Rename the mount to /workspace for a predictable workspace paths in our scripts. | ||
| // The source path might also contain special characters, so it needs escaped double quotes. | ||
| "workspaceMount": "\"source=${localWorkspaceFolder}\",target=/workspace,type=bind", |
There was a problem hiding this comment.
workspaceMount includes literal quotes around the source= value. Docker’s --mount parser treats those quotes as part of the path, which can cause the bind mount to fail and prevent the devcontainer from starting. Consider removing the embedded quotes and relying on the mount string being passed as a single argument (spaces in paths are already handled).
| "workspaceMount": "\"source=${localWorkspaceFolder}\",target=/workspace,type=bind", | |
| "workspaceMount": "source=${localWorkspaceFolder},target=/workspace,type=bind", |
| } | ||
|
|
||
| npm_install() { | ||
| echo "Installing NPM dependencies..." |
There was a problem hiding this comment.
This script switches from npm ci to npm install. Since the repo includes a package-lock.json, npm ci is the reproducible and typically faster choice for container provisioning; npm install may modify the lockfile and produce non-deterministic installs. Consider using npm ci here.
| fi | ||
|
|
||
| echo "Setting volume ownership for $volume_path" |
There was a problem hiding this comment.
The volume ownership workaround uses sudo chown without -R and without non-interactive mode. If the volume already contains root-owned files, ownership may remain incorrect; and if sudo ever prompts, postCreateCommand can hang. Consider using recursive ownership and sudo -n (fail fast) for this step.
| #!/usr/bin/env bash | ||
| set -euo pipefail | ||
|
|
There was a problem hiding this comment.
This script doesn’t follow the structure used by the other devcontainer scripts in this repo (header comment + main() entrypoint). For consistency and easier maintenance, consider adding the standard header and wrapping execution in a main() function called at the end (see .devcontainer/scripts/on-create.sh).
| while IFS='=' read -r key value; do | ||
| local key value | ||
| case "$key" in | ||
| user.name | user.email | user.signingkey | commit.gpgsign) | ||
| echo "Set Git config ${key}=${value}" | ||
| git config --global "$key" "$value" |
There was a problem hiding this comment.
Inside the while read loop, local key value is declared after the variables are populated and is re-declared on every iteration. Consider declaring the locals once at the start of the function (before the loop) and then only assigning within the loop; this avoids confusing scoping and is more ShellCheck-friendly.
| } | ||
| }, | ||
| // This is to ensure support for config includes is properly handled, see microsoft/vscode-remote-release#2084 | ||
| "initializeCommand": { |
There was a problem hiding this comment.
initializeCommand is exporting every global config key, not just identity fields but you only need: user.name, user.email, user.signingkey, commit.gpgsign, right?
| { | ||
| "type": "volume", | ||
| "source": "${devcontainerId}-userconfig", | ||
| "target": "/home/vscode/.config" |
There was a problem hiding this comment.
should this just be the /home/vscode/.config/gh folder?
| @@ -0,0 +1,28 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
Can you set the SPDX and license header?
# Copyright (c) Microsoft Corporation.
# SPDX-License-Identifier: MIT
3 participants
Pull Request
Description
Captures learnings from recent customer projects to enhance the build speed and launch reliability of the dev container.
Related Issue(s)
Type of Change
Select all that apply:
Code & Documentation:
Infrastructure & Configuration:
AI Artifacts:
prompt-builderagent and addressed all feedback.github/instructions/*.instructions.md).github/prompts/*.prompt.md).github/agents/*.agent.md).github/skills/*/SKILL.md)Other:
.ps1,.sh,.py)Sample Prompts (for AI Artifact Contributions)
User Request:
Execution Flow:
Output Artifacts:
Success Indicators:
For detailed contribution requirements, see:
Testing
Checklist
Required Checks
AI Artifact Contributions
/prompt-analyzeto review contributionprompt-builderreviewRequired Automated Checks
The following validation commands must pass before merging:
npm run lint:mdnpm run spell-checknpm run lint:frontmatternpm run lint:md-linksnpm run lint:psSecurity Considerations
Additional Notes