codecheck/codeformat CMake targets skip C++ sources, so CI never checks clang-format
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 72/100
- Issue type
- Bug
- Clarity
- Clearly specified
- Activity status
- Active
- Tech stack
- bash, cmake, cpp, devtools
- Domain
- build-system
Research direction
The check is defined by the codecheck and codeformat targets in CMakeLists.txt, around lines 280-290, where SDIR is set to ${PROJECT_SOURCE_DIR}/src and the find-pipe-xargs clang-format command runs. Reproduce by running make codecheck after adding a badly formatted line to src/model/BloodVessel.cpp; it should fail. Fix the glob so it recurses into subdirectories (for example with find's -name patterns), then run clang-format over the 11 listed files. Done when codecheck fails on bad formatting and passes on the reformatted tree.
Written by the indexing model from the issue text.
Description
Description
make codecheck (and therefore the code-format job in the Codechecks workflow) does not check almost any C++ source files, so CI passes regardless of how src/ is formatted. Only the Prettier check on *.json is actually enforced.
The targets in CMakeLists.txt run:
https://github.com/SimVascular/svZeroDSolver/blob/62aecfec/CMakeLists.txt#L280-L290
set(SDIR ${PROJECT_SOURCE_DIR}/src)
...
COMMAND find ${SDIR}/*.h ${SDIR}/*.cpp | xargs clang-format ... --dry-run --Werror
All sources except src/Assertions.h live in subdirectories (algebra/, interface/, model/, optimize/, solve/), so the glob matches only Assertions.h, and find errors on the literal src/*.cpp:
find: .../src/*.cpp: No such file or directory
Built target codecheck
Because find is on the left of a pipe, its non-zero exit status is discarded and the target succeeds.
SDIR was originally ${PROJECT_SOURCE_DIR}/src/**/ (894694e8, #56), which matched one level of subdirectories. It was changed to ${PROJECT_SOURCE_DIR}/src in ce68a7c3 (#234).
Reproduction
- On
master(62aecfec), configure and run the check:
It passes.mkdir Release && cd Release && cmake .. && make codecheck - Append a badly formatted line to any source file, e.g.
src/model/BloodVessel.cpp:int badly_formatted_var=1 ; - Run
make codecheckagain. It still passes.
Running clang-format over all of src/ recursively (clang-format 23.1.0, the version the CI job currently installs) shows 11 files on master that don't conform to .clang-format:
src/algebra/Integrator.{cpp,h}src/model/ActivationFunction.{cpp,h}src/model/Block.hsrc/model/ChamberSphere.{cpp,h}src/model/SphereMaterial.{cpp,h}src/solve/SimulationParameters.cppsrc/solve/Solver.cpp
Expected behavior
make codecheck should run clang-format on every .h/.cpp file under src/ (at any depth) and fail when any of them is not formatted. make codeformat should format the same set of files.
Additional context
Once the check is real, a related issue appears: the CI job installs whatever clang-format version brew install clang-format currently provides (23.1.0 in recent runs; Homebrew is already at 23.1.3). Different clang-format versions can format the same code differently, so a new release could fail CI on unchanged code, and contributors with another local version can't reproduce CI results. Pinning the clang-format version used by CI (and documenting it in the developer guide) would avoid this.
Open PRs that touch src/ will likely need make codeformat after the fix lands.
Code of Conduct
- I agree to follow this project's Code of Conduct and Contributing Guidelines
- Dominant language
- C++
- Stars
- 22
- Forks
- 45
- Avg merge
- 2d 16h
- Merged PRs (30d)
- 4
Getting set up
- No Dockerfile or Docker Compose file
- No pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from SimVascular/svZeroDSolver
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
SimVascular/svZeroDSolver#238 · 5 comments ·
-
Add AGENTS.mdOpenenhancement
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
SimVascular/svZeroDSolver#232 · 1 comment ·
-
CI fails fetching Eigen from GitLab under loadPossibly taken @zasexton claimed this today. Openbug
Difficulty 3/5 Half a day Newbie friendliness 30/100
SimVascular/svZeroDSolver#255 · 6 comments ·
-
enhancement
Difficulty 5/5 Over a week Newbie friendliness 8/100
SimVascular/svZeroDSolver#252 · 5 comments ·
-
Difficulty 4/5 3-5 days Newbie friendliness 45/100
SimVascular/svZeroDSolver#246 ·
All issues in SimVascular/svZeroDSolver
Similar issues
-
P2: A timed rotation maneuver does nothing on a ship that had an earlier timed lateral maneuverOpen
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
scp-fs2open/fs2open.github.com#7914 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
Maintainers usually reply within 2 days
-
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
eclipse-score/inc_someip_gateway#356 ·
Maintainers usually reply within 2 days
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
HarperFast/rocksdb-js#945 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
Maintainers usually reply within 1 day