[HTTP] CC-1594: Move away from alpine based buildpacks to debian based ones for C #113
Conversation
This commit updates the C starter template to: - Switch from gcc to CMake for build system - Update to C23 language standard - Restructure project layout from `app/` to `src/` - Add vcpkg configuration for potential future dependencies - Update compilation and run scripts to use CMake
WalkthroughThis pull request transitions the build process of multiple C projects from direct gcc commands to a CMake‐based approach. Compilation and execution scripts have been updated to configure and build the project with CMake and to execute the built binary via a relative path. New CMake projects, vcpkg JSON configurations, and comprehensive .gitignore rules have been added across compiled starters, solution examples, and starter templates. Additionally, documentation and configuration settings (e.g. language pack and entry point paths) have been updated, and a Dockerfile for the new C/C++ environment is introduced. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant CS as Compile Script
participant CMake as CMake
participant Build as Build Directory
participant RS as Run Script
participant Server as http-server
Dev->>CS: Run compile.sh / your_program.sh
CS->>CMake: Configure project (cmake -B build -S .)
CMake->>Build: Generate build files
CS->>CMake: Build project (cmake --build ./build)
Build-->>CS: Binary created
Dev->>RS: Run run.sh
RS->>Server: Execute http-server with arguments
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 0
🧹 Nitpick comments (6)
compiled_starters/c/.codecrafters/run.sh (1)
11-11: Quote variable substitutions in the exec command to prevent word splitting.
The static analysis (Shellcheck SC2046) warns about unquoted command substitutions. This can lead to issues if$(dirname $0)contains spaces. Consider updating the command as follows:- exec $(dirname $0)/build/http-server "$@" + exec "$(dirname "$0")/build/http-server" "$@"This ensures that the path is treated as a single argument.
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Quote this to prevent word splitting.
(SC2046)
solutions/c/01-at4/code/.codecrafters/run.sh (1)
11-11: Quote Command Substitution to Prevent Word Splitting
The current exec invocation may lead to unintended word splitting if the directory name contains spaces. Consider quoting the command substitution to ensure robust behavior.-exec $(dirname $0)/build/http-server "$@" +exec "$(dirname "$0")/build/http-server" "$@"🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Quote this to prevent word splitting.
(SC2046)
starter_templates/c/code/.codecrafters/run.sh (1)
11-11: Quote Command Substitution to Prevent Word Splitting
Similar to the changes in the analogous run script, quoting the command substitution avoids potential issues with spaces in directory names.-exec $(dirname $0)/build/http-server "$@" +exec "$(dirname "$0")/build/http-server" "$@"🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Quote this to prevent word splitting.
(SC2046)
compiled_starters/c/.codecrafters/compile.sh (1)
11-12: Validate Build Environment Variable
The new CMake-based build commands rely on theVCPKG_ROOTenvironment variable. Ensure thatVCPKG_ROOTis correctly set in the build environment, as its absence may lead to build failures.solutions/c/01-at4/code/your_program.sh (1)
25-25: Quote command substitution in exec call to prevent word splitting
To avoid potential issues with word splitting in shell, please update the exec call by quoting the command substitution. For example, change:- exec $(dirname $0)/build/http-server "$@" + exec "$(dirname "$0")"/build/http-server "$@"This ensures that paths with spaces are handled correctly.
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 25-25: Quote this to prevent word splitting.
(SC2046)
compiled_starters/c/your_program.sh (1)
25-25: Quote command substitution in exec call to prevent word splitting
As with the similar changes in other scripts, please update the exec call by quoting the directory name to prevent unexpected word splitting. Suggested change:- exec $(dirname $0)/build/http-server "$@" + exec "$(dirname "$0")"/build/http-server "$@"🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 25-25: Quote this to prevent word splitting.
(SC2046)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
compiled_starters/c/.codecrafters/compile.sh(1 hunks)compiled_starters/c/.codecrafters/run.sh(1 hunks)compiled_starters/c/.gitignore(1 hunks)compiled_starters/c/CMakeLists.txt(1 hunks)compiled_starters/c/README.md(2 hunks)compiled_starters/c/codecrafters.yml(1 hunks)compiled_starters/c/vcpkg-configuration.json(1 hunks)compiled_starters/c/vcpkg.json(1 hunks)compiled_starters/c/your_program.sh(1 hunks)dockerfiles/c-23.Dockerfile(1 hunks)solutions/c/01-at4/code/.codecrafters/compile.sh(1 hunks)solutions/c/01-at4/code/.codecrafters/run.sh(1 hunks)solutions/c/01-at4/code/.gitignore(1 hunks)solutions/c/01-at4/code/CMakeLists.txt(1 hunks)solutions/c/01-at4/code/README.md(2 hunks)solutions/c/01-at4/code/codecrafters.yml(1 hunks)solutions/c/01-at4/code/vcpkg-configuration.json(1 hunks)solutions/c/01-at4/code/vcpkg.json(1 hunks)solutions/c/01-at4/code/your_program.sh(1 hunks)solutions/c/01-at4/explanation.md(1 hunks)starter_templates/c/code/.codecrafters/compile.sh(1 hunks)starter_templates/c/code/.codecrafters/run.sh(1 hunks)starter_templates/c/code/.gitignore(1 hunks)starter_templates/c/code/CMakeLists.txt(1 hunks)starter_templates/c/code/vcpkg-configuration.json(1 hunks)starter_templates/c/code/vcpkg.json(1 hunks)starter_templates/c/config.yml(1 hunks)
✅ Files skipped from review due to trivial changes (12)
- compiled_starters/c/vcpkg.json
- solutions/c/01-at4/code/vcpkg.json
- solutions/c/01-at4/explanation.md
- solutions/c/01-at4/code/.gitignore
- solutions/c/01-at4/code/codecrafters.yml
- starter_templates/c/code/vcpkg.json
- compiled_starters/c/codecrafters.yml
- solutions/c/01-at4/code/vcpkg-configuration.json
- starter_templates/c/code/.gitignore
- compiled_starters/c/.gitignore
- compiled_starters/c/vcpkg-configuration.json
- starter_templates/c/code/vcpkg-configuration.json
🧰 Additional context used
🪛 Shellcheck (0.10.0)
compiled_starters/c/.codecrafters/run.sh
[warning] 11-11: Quote this to prevent word splitting.
(SC2046)
starter_templates/c/code/.codecrafters/run.sh
[warning] 11-11: Quote this to prevent word splitting.
(SC2046)
solutions/c/01-at4/code/.codecrafters/run.sh
[warning] 11-11: Quote this to prevent word splitting.
(SC2046)
compiled_starters/c/your_program.sh
[warning] 25-25: Quote this to prevent word splitting.
(SC2046)
solutions/c/01-at4/code/your_program.sh
[warning] 25-25: Quote this to prevent word splitting.
(SC2046)
🪛 Hadolint (2.12.0)
dockerfiles/c-23.Dockerfile
[error] 31-31: invalid flag: --exclude
(DL1000)
🔇 Additional comments (12)
starter_templates/c/config.yml (1)
2-3: Update configuration attributes to reflect the new build system and project structure.
The attributes have been updated correctly:
required_executableis now set tocmake, which aligns with the migration from gcc to CMake.user_editable_filehas been updated tosrc/main.c, matching the new project layout.
Ensure that all dependent documentation and scripts (e.g., README files and build scripts) are updated accordingly.compiled_starters/c/CMakeLists.txt (1)
1-9: New CMake configuration for the compiled starter.
This CMakeLists.txt file is well structured and sets up the build process for the project:
- It specifies a minimum CMake version (3.13) and sets the C standard to C23.
- The use of
file(GLOB_RECURSE ...)correctly gathers all source and header files from thesrcdirectory.- The executable target
http-serveris defined appropriately using all source files.Consideration: For large projects,
GLOB_RECURSEmay miss new files added during incremental builds, so explicit file listings might be more robust as the project scales.solutions/c/01-at4/code/CMakeLists.txt (1)
1-10: CMake configuration for the solution example is properly set up.
The file mirrors the configuration of the compiled starter, ensuring consistency:
- It sets a minimum required CMake version and specifies the project.
- The C23 standard is enforced, and source files are collected recursively.
- The executable
http-serveris built as expected.This consistent structure across different parts of the repository reduces complexity in maintaining multiple build configurations.
starter_templates/c/code/CMakeLists.txt (1)
1-10: Consistent CMake configuration for the starter template.
This file successfully applies the CMake-based build approach:
- It correctly defines the minimum required version, project name, and the C23 standard.
- The use of
file(GLOB_RECURSE ...)ensures that all.cand.hfiles from thesrcdirectory are included in the build.- The executable target is set up consistently with the other CMakeLists.txt files in the project.
Keep the configuration in sync with the documentation regarding the new project structure.
solutions/c/01-at4/code/.codecrafters/compile.sh (1)
11-12: Transition to CMake Build Process
The updated compile script successfully replaces directgcccompilation with CMake commands. Please ensure that theVCPKG_ROOTenvironment variable is available to provide the necessary toolchain file path; otherwise, the CMake configuration may fail.starter_templates/c/code/.codecrafters/compile.sh (1)
11-12: Adopt Structured Build with CMake
The shift from a directgcccompilation to using CMake improves maintainability and dependency management. As with the other compile scripts, verify that theVCPKG_ROOTenvironment variable is properly set. This ensures the toolchain file is located correctly during the configuration step.solutions/c/01-at4/code/your_program.sh (1)
17-18: Use of CMake commands is correctly implemented
The updated commands correctly configure (cmake -B build -S . -DCMAKE_TOOLCHAIN_FILE=${VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake) and build (cmake --build ./build) the project using CMake. This structured approach improves maintainability and aligns with the new Debian-based environment.compiled_starters/c/README.md (2)
19-20: Entry point update accurately reflects new project structure
The updated text clearly states that the HTTP server’s entry point is now insrc/main.c, which aligns with the overall restructuring.
33-35: Installation instructions now require CMake
The instructions correctly specify that users must havecmakeinstalled, replacing the previousgccrequirement. This consistency helps prevent build confusion.solutions/c/01-at4/code/README.md (2)
19-20: Updated entry point and toolchain requirements are clear
The documentation now properly indicates that the server implementation entry point is insrc/main.cand thatcmakeis required for local builds. This improves clarity for users following the setup instructions.
33-35: Instructions for running the program are precise
The steps to run./your_program.shand the subsequent guidance match the new build and directory structure, ensuring users can quickly validate their setup.dockerfiles/c-23.Dockerfile (1)
31-31: 🛠️ Refactor suggestion
⚠️ Potential issueInvalid flag in COPY command for Dockerfile
The usage of the--excludeflag in the COPY command is not supported by Docker. Remove these flags and instead list.gitandREADME.mdin a.dockerignorefile to avoid including them in the build context. For example, change:- COPY --exclude=.git --exclude=README.md . /app + COPY . /app # Ensure .git and README.md are excluded via .dockerignoreThis will resolve the error indicated by Hadolint.
⛔ Skipped due to learnings
Learnt from: SughiY PR: codecrafters-io/build-your-own-http-server#112 File: dockerfiles/clojure-1.12.Dockerfile:10-10 Timestamp: 2025-02-17T19:29:27.993Z Learning: The `--exclude` flag in COPY commands is supported in Dockerfile syntax 1.7-labs and should be retained for optimizing layer caching, despite static analysis tools like Hadolint flagging it as an error.🧰 Tools
🪛 Hadolint (2.12.0)
[error] 31-31: invalid flag: --exclude
(DL1000)
This commit updates the C starter template to:
app/tosrc/Summary by CodeRabbit
Refactor
Documentation
Chore