Refresh bench tooling and Claude profiles#1
Conversation
|
@codex review |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR refreshes bench tooling by updating compiler versions in the README and adjusting devcontainer/Dockerfile configuration to align the bench environment with portable Claude profiles and the synchronized workBenches parent setup. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The compiler versions in the README have been updated; double-check that these match the versions installed in the Dockerfile and devcontainer configuration to avoid confusing discrepancies for users following the instructions.
- Consider parameterizing the compiler versions (e.g., via environment variables or CMake toolchain files) rather than hardcoding specific version numbers in the README so the bench can be updated more easily when toolchains change.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The compiler versions in the README have been updated; double-check that these match the versions installed in the Dockerfile and devcontainer configuration to avoid confusing discrepancies for users following the instructions.
- Consider parameterizing the compiler versions (e.g., via environment variables or CMake toolchain files) rather than hardcoding specific version numbers in the README so the bench can be updated more easily when toolchains change.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
Updates the cppBench development environment to newer compiler toolchains and refreshes devcontainer configuration to support additional Claude profile mounting.
Changes:
- Bump recommended compiler versions in docs to Clang 19 / GCC 14.
- Refresh Layer 2 bench image tooling to install and default to GCC 14 and LLVM/Clang 19.
- Add a devcontainer mount for portable Claude profiles (
~/.claude-profiles).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
README.md |
Updates example environment variables to use Clang 19 / GCC 14. |
Dockerfile.layer2 |
Upgrades installed compiler toolchain and updates layer version label. |
.devcontainer/devcontainer.json |
Adds bind mount for ~/.claude-profiles into the container. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| clang-19 \ | ||
| clang-tools-19 \ | ||
| clang-tidy-19 \ | ||
| clang-format-19 \ | ||
| llvm-19 \ |
| // Claude (Anthropic) | ||
| "source=${localEnv:HOME}/.claude,target=/home/${localEnv:USER}/.claude,type=bind,consistency=cached", | ||
| "source=${localEnv:HOME}/.claude.json,target=/home/${localEnv:USER}/.claude.json,type=bind,consistency=cached", | ||
| "source=${localEnv:HOME}/.claude-profiles,target=/home/${localEnv:USER}/.claude-profiles,type=bind,consistency=cached", |
| // Claude (Anthropic) | ||
| "source=${localEnv:HOME}/.claude,target=/home/${localEnv:USER}/.claude,type=bind,consistency=cached", | ||
| "source=${localEnv:HOME}/.claude.json,target=/home/${localEnv:USER}/.claude.json,type=bind,consistency=cached", | ||
| "source=${localEnv:HOME}/.claude-profiles,target=/home/${localEnv:USER}/.claude-profiles,type=bind,consistency=cached", |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 27f81921de
ℹ️ 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".
| gcc-14 \ | ||
| g++-14 \ |
There was a problem hiding this comment.
Keep devcontainer compiler overrides in sync
When users launch this image through the checked-in devcontainer, .devcontainer/docker-compose.yml still sets CC=/usr/bin/gcc-12 and CXX=/usr/bin/g++-12, and .devcontainer/devcontainer.json has the same overrides; those environment values override the Dockerfile ENV. Since this change replaces the layer's GCC install/export with GCC 14 only, a fresh container can end up with CC/CXX pointing at non-existent GCC 12 paths, causing CMake and builds that honor these variables to fail immediately.
Useful? React with 👍 / 👎.
| // Claude (Anthropic) | ||
| "source=${localEnv:HOME}/.claude,target=/home/${localEnv:USER}/.claude,type=bind,consistency=cached", | ||
| "source=${localEnv:HOME}/.claude.json,target=/home/${localEnv:USER}/.claude.json,type=bind,consistency=cached", | ||
| "source=${localEnv:HOME}/.claude-profiles,target=/home/${localEnv:USER}/.claude-profiles,type=bind,consistency=cached", |
There was a problem hiding this comment.
Guard optional Claude profiles mount
On hosts that have the normal Claude files but have not created ~/.claude-profiles, this new bind mount can prevent the devcontainer from starting because Dev Container mounts are passed through as Docker bind mounts and the source path must already exist. The existing initializeCommand creates a few optional host directories, but it does not create this new Claude profiles directory, so users without that optional profile setup hit the failure before the container can run.
Useful? React with 👍 / 👎.
| gcc-14 \ | ||
| g++-14 \ |
There was a problem hiding this comment.
Don't require Noble-only GCC packages on Jammy
When this layer is built on the still-documented Ubuntu 22.04/Jammy base, this apt-get install step cannot resolve gcc-14/g++-14 from the stock repositories; Ubuntu's own GCC availability table lists Jammy packages only through GCC 12. Without moving the base image to 24.04 or adding the toolchain repository before this RUN, scripts/build-layer2.sh fails before producing cpp-bench:latest.
Useful? React with 👍 / 👎.
Summary
Validation
git diff --checkbash -ndocker buildx build --checkSummary by Sourcery
Refresh development bench tooling and container setup while aligning documentation with updated compiler versions.
Enhancements:
Build:
Documentation: