build: establish src/core layout for support modules - #617
build: establish src/core layout for support modules#617somethingwithproof wants to merge 5 commits into
Conversation
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
There was a problem hiding this comment.
🟡 Changes recommended
The make cppcheck Docker target is likely to fail to resolve locks.h/keywords.h after the move because it does not pass updated include paths.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR begins establishing a src/core/ domain layout by relocating two low-coupling support modules (locks and keywords) while preserving existing header include names and keeping the Autotools build as the primary build system.
Changes:
- Move
locks.c/.handkeywords.c/.hintosrc/core/(rename-only) - Update Autotools source/header lists and global include paths to build against the new layout
- Update fuzz build source paths and include paths to reach the relocated modules
File summaries
| File | Description |
|---|---|
Makefile.am |
Updates Autotools spine_SOURCES, linked-test sources, distributed headers list, and adds src/core to include paths. |
tests/fuzz/Makefile |
Points fuzz build at src/core/{locks,keywords}.c and adds -I.../src/core include path. |
src/core/locks.c |
Relocated implementation (rename-only per PR description). |
src/core/locks.h |
Relocated header (rename-only per PR description). |
src/core/keywords.c |
Relocated implementation (rename-only per PR description). |
src/core/keywords.h |
Relocated header (rename-only per PR description). |
Review details
- Files reviewed: 2/6 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
scripts/verify.sh now depends on Bash-only features but is invoked via /bin/sh in at least one existing path, making it fragile unless it self-reexecs under Bash.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 4/8 changed files
- Comments generated: 1
- Review effort level: Lite
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
7db62ee to
03861df
Compare
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
There was a problem hiding this comment.
🟡 Changes recommended
configure.ac enables subdir-objects, which contradicts the PR’s stated intent to defer it and changes object placement beyond a rename-only move.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 6/11 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🟢 Approval recommended
The moved modules resolve correctly via updated include paths and all referenced build/verification entry points shown in the diff consistently reference the new src/core locations.
Review details
- Files reviewed: 6/11 changed files
- Comments generated: 0 new
- Review effort level: Lite
What
keywords.c/.handlocks.c/.hintosrc/core/subdir-objectsfor collision-safe nested object placementThe four production source/header files are 100% renames with no content changes. The remaining changes are build and verification plumbing required by the new paths.
Why
Spine production sources currently live in the repository root. This starts the domain layout in a small, reviewable batch instead of repeating #523, where the source move was coupled to an 11k-line CMake/platform rewrite.
keywordsandlockswere selected because active PRs #597 and #614 do not modify their implementation files; only shared build manifests overlap #597.subdir-objectsis enabled in this first production move so future modules with the same basename cannot write the same top-level object under parallel builds.Closes #616.
Verification
./bootstrapwith no subdirectory-source warnings./configure --with-mysql=$(brew --prefix mariadb-connector-c) --with-snmp=$(brew --prefix net-snmp) CPPFLAGS=-I$(brew --prefix openssl@3)/include LDFLAGS=-L$(brew --prefix openssl@3)/libmake -j$(sysctl -n hw.ncpu); project sources compile, with Homebrew Net-SNMP header warnings onlymake check(cmocka unavailable to Autoconf locally; 0 local tests)make distcheck; archive verified to contain all foursrc/corefilestests/unit/Makefilecompiles through header resolution with-I../../src/core(the local macOS link retains unrelated existing client-library/toolchain issues)distcheck, gcc/clang, SNMPv3, and Rocky LinuxThe local macOS fuzz build reaches both relocated files, then fails identically on untouched
util.c(INT_MAX) and Net-SNMP API declarations on both this branch anddevelop. Linux fuzz CI is the authoritative fuzz-build result.