feat: optimize devcontainer build performance - #24
Conversation
- Add build cacheFrom configuration for better layer caching - Enable postCreateCommand instead of postStartCommand for setup - Optimize npm install in common feature with caching and offline preference - Improve error handling and path resolution in install script - Add cache configuration file for future BuildKit optimizations Co-authored-by: keito4 <keito4@users.noreply.github.com>
|
""" WalkthroughThe pull request adds a Docker BuildKit cache configuration file, updates the devcontainer to activate BuildKit and shift Homebrew setup to container creation, and enhances the npm installation script with improved npm detection, cache handling, global.json file search, and safer dependency installation. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant DevContainer
participant Docker
participant ShellScript
Developer->>DevContainer: Build container
DevContainer->>Docker: Enable BuildKit and use cache mounts
Docker-->>DevContainer: Build with cached layers
DevContainer->>ShellScript: Run install.sh (postCreateCommand)
ShellScript->>ShellScript: Check npm, set cache, find global.json
ShellScript->>ShellScript: Extract dependencies and install npm packages
ShellScript-->>DevContainer: Output status and completion message
Assessment against linked issues
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.devcontainer/.devcontainer-cache (1)
8-8: Consider making cache paths dynamic or configurable
Hardcoding/tmp/.npm-cache:/home/linuxbrew/.linuxbrew/var/cachemay break if Brew’s cache directory moves. You could compute Homebrew’s cache at runtime (e.g.,brew --cache) or allow overriding via an environment variable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.devcontainer/.devcontainer-cache(1 hunks).devcontainer/devcontainer.json(2 hunks).devcontainer/features/common/install.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
.devcontainer/features/common/install.sh
[warning] 36-36: Quote this to prevent word splitting.
(SC2046)
🔇 Additional comments (5)
.devcontainer/devcontainer.json (1)
31-31: Approved: Switching topostCreateCommandfor Homebrew initialization
Moving Homebrew setup topostCreateCommandensures it only runs once at creation time, reducing overhead on every container start. The quoting looks correct and will source your updated~/.zshrc..devcontainer/features/common/install.sh (4)
13-17: Correctly skip npm steps if npm is missing
The check and early exit for missingnpmis clear and prevents errors.
19-23: Configure npm to use a dedicated cache directory
Creating and globally settingNPM_CACHE_DIRis an effective way to leverage layer caching for npm.
57-67: Robust handling of JSON dependencies
Thejqchecks and conditional installation logic are solid, ensuring you only runnpm install -gwhen packages exist.
25-30:Details
✅ Verification successful
Verify all expected
global.jsonpaths are accurate
You’ve added/workspace/npm/global.jsonandnpm/global.json; please ensure these align with your workspace layout (e.g., correct top-level dir names).You can run:
to confirm where your configuration files actually live.
🏁 Script executed:
rg --ignore-case "global.json" -n .Length of output: 280
🏁 Script executed:
#!/bin/bash # Locate where REPO_PATH is defined in install.sh for context on workspace root rg -n "REPO_PATH" .devcontainer/features/common/install.shLength of output: 58
🏁 Script executed:
#!/bin/bash # Find where REPO_PATH is set or exported in the repository to verify workspace root rg -n "REPO_PATH=" .Length of output: 102
✅ GLOBAL_JSON paths are correct
I’ve confirmed viarg -n global.jsonthat the onlyglobal.jsonfile lives atnpm/global.jsonin the repo root. The added patterns
/workspaces/*/npm/global.json(Codespaces default mount)/workspace/npm/global.json(Remote-Containers default mount)$HOME/npm/global.json(home-directory fallback)npm/global.json(relative-path fallback)cover all expected layouts. No further changes needed.
| # This file is used to improve build performance by caching layers | ||
|
|
||
| # Enable Docker BuildKit for better caching | ||
| DOCKER_BUILDKIT=1 |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Ensure BuildKit is actually enabled during devcontainer builds
Setting DOCKER_BUILDKIT=1 is correct, but verify that this file is consumed by your build invocation (e.g., via --env-file or equivalent in devcontainer.json). Without loading it, BuildKit won’t be enabled and layer caching won’t apply.
Run this script to confirm references to .devcontainer-cache exist in your config:
🏁 Script executed:
#!/bin/bash
# Verify that .devcontainer/.devcontainer-cache is referenced in devcontainer.json
rg --fixed-strings --line-number ".devcontainer-cache" -nLength of output: 57
Add the cache env-file to your devcontainer configuration
It appears .devcontainer/.devcontainer-cache is never referenced in your devcontainer.json, so BuildKit won’t actually be enabled. You need to load that file via the build.envFile (or runArgs: ["--env-file", ...]) property.
Locations to update:
- devcontainer.json (at project root)
Suggested change:
--- devcontainer.json
+++ devcontainer.json
@@
"build": {
"dockerfile": "Dockerfile",
- // …other build options
+ "envFile": [
+ ".devcontainer/.devcontainer-cache"
+ ]
},
// …other configCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .devcontainer/.devcontainer-cache at line 5, the environment variable
DOCKER_BUILDKIT=1 is set but this file is not referenced in devcontainer.json,
so BuildKit is not enabled during builds. To fix this, update devcontainer.json
at the project root to include the .devcontainer/.devcontainer-cache file in the
build.envFile property or add it to runArgs with --env-file. This ensures the
environment variable is loaded and BuildKit is activated for caching.
| "build": { | ||
| "cacheFrom": "mcr.microsoft.com/devcontainers/base:ubuntu" | ||
| }, |
There was a problem hiding this comment.
Incomplete build configuration prevents Docker layer caching
The cacheFrom field alone isn’t enough—Dev Containers require a dockerfile (and optionally context) to build your image. Without them, your container will be pulled instead of built, and caching won’t apply.
Please update to something like:
"build": {
- "cacheFrom": "mcr.microsoft.com/devcontainers/base:ubuntu"
+ "dockerfile": "Dockerfile",
+ "context": ".",
+ "cacheFrom": "mcr.microsoft.com/devcontainers/base:ubuntu"
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "build": { | |
| "cacheFrom": "mcr.microsoft.com/devcontainers/base:ubuntu" | |
| }, | |
| "build": { | |
| "dockerfile": "Dockerfile", | |
| "context": ".", | |
| "cacheFrom": "mcr.microsoft.com/devcontainers/base:ubuntu" | |
| }, |
🤖 Prompt for AI Agents
In .devcontainer/devcontainer.json around lines 18 to 20, the build
configuration is incomplete because it only includes the cacheFrom field. To
enable Docker layer caching properly, add the dockerfile field specifying the
path to your Dockerfile, and optionally include the context field if your build
context differs from the default. This ensures the container image is built
locally with caching instead of being pulled.
| for path in "${GLOBAL_JSON_PATHS[@]}"; do | ||
| # Use find to avoid shell expansion issues with wildcards | ||
| if [[ "$path" == *"*"* ]]; then | ||
| found_file=$(find $(dirname "$path") -name "global.json" -type f 2>/dev/null | head -1) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prevent word-splitting in find invocation
Per ShellCheck SC2046, wrap the directory in quotes to avoid splitting on spaces:
-found_file=$(find $(dirname "$path") -name "global.json" -type f 2>/dev/null | head -1)
+dir="$(dirname "$path")"
+found_file=$(find "$dir" -maxdepth 1 -type f -name "global.json" 2>/dev/null | head -1)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| found_file=$(find $(dirname "$path") -name "global.json" -type f 2>/dev/null | head -1) | |
| dir="$(dirname "$path")" | |
| found_file=$(find "$dir" -maxdepth 1 -type f -name "global.json" 2>/dev/null | head -1) |
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 36-36: Quote this to prevent word splitting.
(SC2046)
🤖 Prompt for AI Agents
In .devcontainer/features/common/install.sh at line 36, the directory argument
to the find command is not quoted, which can cause word splitting if the path
contains spaces. To fix this, wrap $(dirname "$path") in double quotes so the
find command receives the directory as a single argument and avoids word
splitting issues.
1 participant
devcontainerのビルド時間を最適化しました。
変更内容
Fixes #23
Generated with Claude Code
Summary by CodeRabbit