perf: Docker ビルドを高速化（Rust/similarity-ts/不要パッケージ削除） - #637
Conversation
arm64 QEMU下での cargo install similarity-ts が20分かかっていた問題を解消。 - similarity-ts をビルドから削除（オンデマンドインストールに変更） - Rust ツールチェイン（rustup, cargo, rustfmt, clippy）を削除 - 不要パッケージ（alsa-utils, sox, libasound2-dev, pkg-config, libssl-dev）を削除 - similarity-analysis コマンドに未インストール時のフォールバック手順を追加 期待されるビルド時間短縮: 約20分（40分 → 20分） Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis pull request removes the Rust toolchain from the DevContainer base image, updates PATH handling, moves build-time Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,200,255,0.5)
participant User as Developer Shell
participant Cmd as similarity-analysis cmd
participant Env as PATH / Shell
participant Installer as cargo-binstall / cargo
end
User->>Cmd: run similarity-analysis
Cmd->>Env: check for `similarity-ts` on PATH
alt found
Env-->>Cmd: `similarity-ts` available
Cmd->>User: run `similarity-ts ${path:-.} --threshold ...`
else not found
Cmd->>Installer: attempt `cargo-binstall -y similarity-ts`
alt cargo-binstall success
Installer-->>Cmd: installed binary
Cmd->>User: run `similarity-ts ${path:-.} --threshold ...`
else cargo-binstall fail
Cmd->>Installer: attempt `cargo install similarity-ts`
alt cargo install success
Installer-->>Cmd: installed binary
Cmd->>User: run `similarity-ts ${path:-.} --threshold ...`
else both missing
Cmd->>User: error -> hint to install Rust toolchain
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 974ac587a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| cargo binstall similarity-ts -y | ||
| elif command -v cargo &>/dev/null; then | ||
| cargo install similarity-ts |
There was a problem hiding this comment.
This new fallback branch invokes cargo binstall / cargo install, but this command’s frontmatter still only allows Bash(similarity-ts:*) (plus git/gh). In a fresh image where similarity-ts is no longer preinstalled, /similarity-analysis now depends on this path and will fail under the tool allowlist before installation can happen. Please add the necessary Bash allowlist entries (at least cargo and the check command) so the fallback can actually execute.
Useful? React with 👍 / 👎.
コードレビュー全体的に、ビルド時間を大幅に短縮するための合理的なアプローチです。 良い点
懸念点・改善提案1. pkg-config / libssl-dev の削除（リスク: 中）
ADR では「 2. similarity-analysis.md のフォールバック（リスク: 低） Rust が未インストールの場合のエラーメッセージにインストールコマンドが案内されていますが、ユーザーが次のアクションを取りやすいよう選択肢を提示するか、ドキュメントへのリンクを追加するとより UX が向上します。現状の「案内のみ」でも十分ではあります。 3. alsa-utils / sox / libasound2-dev の削除 これらは音声関連のパッケージです。ADR または PR 説明に「これらのパッケージは何のために追加されていたか・なぜ今回削除して問題ないか」の説明があると将来の参照に役立ちます。ADR の Context セクションに「alsa/sox パッケージは過去に追加されたが現在使用されていない」旨を追記することを提案します。 マイナーな指摘
総評設計判断は適切で、ADR による記録も CLAUDE.md の「非自明なアーキテクチャ変更には ADR を作成する」方針に沿っています。 マージ前に確認推奨:
CI の Trivy Container Scan / Generate SBOM が pending のため、それらが pass してからマージを推奨します。 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/commands/similarity-analysis.md:
- Around line 25-41: The fallback install script for similarity-ts conflicts
with the command allowlist and the devcontainer image: update the allowlist
entries (where Bash(git:*), Bash(gh:*), Bash(similarity-ts:*) are defined) to
permit cargo and cargo-binstall commands, AND add Rust/Cargo to the base image
in the .devcontainer/Dockerfile (or alternatively remove the automatic fallback
and document Rust/Cargo and cargo-binstall as manual prerequisites in
similarity-analysis.md); locate the installation check block (the similarity-ts
install conditional) and either enable the installer commands in the allowlist
and add the toolchain to the Dockerfile, or replace the on-demand install with a
clear manual preflight note in the docs.
In @.devcontainer/Dockerfile:
- Around line 120-121: Replace the Dockerfile base image from
mcr.microsoft.com/devcontainers/base:2.0-ubuntu-24.04 to
ghcr.io/keito4/config-base:latest, and harden the Cursor installer RUN step by
enabling shell pipefail for the pipeline (e.g., use set -o pipefail before or
within the RUN that executes the curl | bash) so download failures propagate and
the build fails instead of being masked; update the RUN that invokes the Cursor
installer and the PATH export line accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 044d71a3-05a3-4640-8cf3-2f0124686254
📒 Files selected for processing (3)
.claude/commands/similarity-analysis.md.devcontainer/Dockerfiledocs/adr/0003-remove-rust-from-base-image.md
| 1. **similarity-ts のインストール確認** | ||
|
|
||
| similarity-ts が未インストールの場合、オンデマンドでインストールします: | ||
|
|
||
| ```bash | ||
| if ! command -v similarity-ts &>/dev/null; then | ||
| echo "similarity-ts が未インストールです。インストールを開始します..." | ||
| if command -v cargo-binstall &>/dev/null; then | ||
| cargo binstall similarity-ts -y | ||
| elif command -v cargo &>/dev/null; then | ||
| cargo install similarity-ts | ||
| else | ||
| echo "Rust ツールチェインが必要です: curl -sSf https://sh.rustup.rs | sh" | ||
| exit 1 | ||
| fi | ||
| fi | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the file exists and read the full context
git ls-files | grep -E "similarity-analysis|Dockerfile|README" | head -20Repository: keito4/config
Length of output: 562
🏁 Script executed:
# Read the similarity-analysis.md file to check front matter and code
cat -n .claude/commands/similarity-analysis.md | head -50Repository: keito4/config
Length of output: 1751
🏁 Script executed:
# Check the Dockerfile for Rust/Cargo presence
cat -n .devcontainer/DockerfileRepository: keito4/config
Length of output: 10922
The new fallback cannot execute in the default devcontainer.
Line 3 restricts Claude Code to Bash(git:*), Bash(gh:*), and Bash(similarity-ts:*), but this fallback invokes cargo-binstall and cargo, which violate the allowlist. Additionally, the base image in .devcontainer/Dockerfile does not include Rust or Cargo, so the fallback lands in the error branch rather than installing on-demand. Either widen the allowlist to permit the installer commands and add Rust/Cargo to the base image, or document this as a manual prerequisite instead of an automatic fallback.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/commands/similarity-analysis.md around lines 25 - 41, The fallback
install script for similarity-ts conflicts with the command allowlist and the
devcontainer image: update the allowlist entries (where Bash(git:*), Bash(gh:*),
Bash(similarity-ts:*) are defined) to permit cargo and cargo-binstall commands,
AND add Rust/Cargo to the base image in the .devcontainer/Dockerfile (or
alternatively remove the automatic fallback and document Rust/Cargo and
cargo-binstall as manual prerequisites in similarity-analysis.md); locate the
installation check block (the similarity-ts install conditional) and either
enable the installer commands in the allowlist and add the toolchain to the
Dockerfile, or replace the on-demand install with a clear manual preflight note
in the docs.
| RUN curl https://cursor.com/install -fsS | bash \ | ||
| && echo 'export PATH="$HOME/.local/bin:$PATH"' >> /home/vscode/.bashrc |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
grep -n "^FROM\|^SHELL" .devcontainer/Dockerfile | head -20Repository: keito4/config
Length of output: 117
🏁 Script executed:
sed -n '75,90p' .devcontainer/Dockerfile | cat -nRepository: keito4/config
Length of output: 689
🏁 Script executed:
sed -n '115,125p' .devcontainer/Dockerfile | cat -nRepository: keito4/config
Length of output: 519
🏁 Script executed:
grep -n "^SHELL\|^ENV SHELL" .devcontainer/DockerfileRepository: keito4/config
Length of output: 79
Fix base image and add pipefail to the Cursor installer pipeline.
Lines 120-121 run the Cursor installer via pipeline without pipefail, so transient download failures can be masked. Additionally, the Dockerfile violates the .devcontainer/** guideline, which requires base image ghcr.io/keito4/config-base:latest instead of mcr.microsoft.com/devcontainers/base:2.0-ubuntu-24.04.
🔧 Proposed fixes
Update the base image:
-FROM mcr.microsoft.com/devcontainers/base:2.0-ubuntu-24.04
+FROM ghcr.io/keito4/config-base:latestAdd pipefail to the Cursor installer:
-RUN curl https://cursor.com/install -fsS | bash \
+RUN bash -o pipefail -c 'curl -fsS https://cursor.com/install | bash' \
&& echo 'export PATH="$HOME/.local/bin:$PATH"' >> /home/vscode/.bashrc🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.devcontainer/Dockerfile around lines 120 - 121, Replace the Dockerfile base
image from mcr.microsoft.com/devcontainers/base:2.0-ubuntu-24.04 to
ghcr.io/keito4/config-base:latest, and harden the Cursor installer RUN step by
enabling shell pipefail for the pipeline (e.g., use set -o pipefail before or
within the RUN that executes the curl | bash) so download failures propagate and
the build fails instead of being masked; update the RUN that invokes the Cursor
installer and the PATH export line accordingly.
native installer は ~/.claude/local/bin ではなく ~/.local/bin に バイナリを配置するため、プラグインインストール時に claude が 見つからなかった。両方のパスを PATH に追加。 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
🎉 This PR is included in version 1.106.5 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Summary
Why
arm64 QEMU下での
cargo install similarity-tsが20分、Rust toolchain インストールが1-2分かかっていた。similarity-ts は1コマンドでしか使われず、|| trueで失敗許容だった。What
Dockerfile
alsa-utils,sox,libasound2-dev,pkg-config,libssl-devを apt-get から削除rustup,cargo,rustfmt,clippyのインストールを削除RUSTUP_HOME,CARGO_HOMEENV を削除cargo install similarity-ts || trueを削除similarity-analysis.md
similarity-ts未インストールならcargo binstall/cargo installでフォールバックADR
docs/adr/0003-remove-rust-from-base-image.mdを追加Expected Build Time
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Chores
Documentation