fix(cmake): Align CLP_BUILD_TESTING option requirements with unit test linkage; Drop SOURCE_FILES_clp_s_unitTest by moving clp-s tests under src/clp_s/tests.

[Bill-hbrhbr]

Description

This PR completes the missing CLP_BUILD_TESTING requirements and fixes minor build option inconsistencies, resulting in a more consistent and maintainable CMake setup across the clp-s codebase.

The unit test and clp-s executable build setups are updated to explicitly list the libraries built by clp-s, with a 1-to-1 mapping to the option requirements defined by CLP_BUILD_TESTING and CLP_BUILD_EXECUTABLES in options.cmake.

With the clp-s test files now moved under src/clp_s, this PR also removes the need to manually maintain clp-s source lists in the root CMakeLists.txt when building unit tests, reducing the scope of change for future clp-s PRs.

Checklist

• The PR satisfies the contribution guidelines.
• This is a breaking change and that has been indicated in the PR title, OR this isn't a
  breaking change.
• Necessary docs have been updated, OR no docs need to be updated.

Validation performed

• CI passes.

Summary by CodeRabbit

• Chores

  • Reorganized internal build infrastructure by introducing aggregated runtime and test libraries for simpler linking.
  • Enhanced build-option validation and dependency wiring for optional features, including new conditional checks for certain components.

• Refactor

  • Centralized component runtime linkage and grouped unit-test sources for clearer build structure.
  • Removed legacy search components from build lists to streamline compilation.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8d8b868f-e349-4c00-9714-0ededf86924c

📥 Commits

Reviewing files that changed from the base of the PR and between 21a2e4b and 0170912.

📒 Files selected for processing (3)

• components/core/CMakeLists.txt
• components/core/cmake/Options/options.cmake
• components/core/src/clp_s/CMakeLists.txt

---

Walkthrough

CMake build wiring restructured: testing and executable runtime linkages were encapsulated into new INTERFACE targets; test source grouping and unitTest linkage were changed; new CLP build options and validation for an FFI SFA feature were added and curl dependency setup moved; an FFI build gate was adjusted; several clp_s search sources were removed.

Changes

Cohort / File(s)	Summary
Core unit test runtime & unitTest wiring components/core/CMakeLists.txt	Added clp_unit_test_runtime (clp::unit_test_runtime) INTERFACE and linked it to unitTest; removed embedding many clp_s-specific test sources from unitTest; unitTest now includes local tests and links clp_s::unit_test_sources.
clp_s executable & test sources components/core/src/clp_s/CMakeLists.txt	Added clp_s_binary_runtime INTERFACE to aggregate runtime component link dependencies; introduced clp_s_unit_test_sources INTERFACE and public alias clp_s::unit_test_sources when CLP_BUILD_TESTING is enabled.
Build options & validation components/core/cmake/Options/options.cmake	Added CLP_BUILD_CLP_S_FFI_SFA and CLP_BUILD_CLP_S_ENABLE_CURL handling; introduced and wired validation/setup functions for FFI SFA; expanded CLP_BUILD_TESTING dependency checks to require additional clp_s components; moved curl/OpenSSL dependency setup.
FFI build gating components/core/src/clp_s/ffi/CMakeLists.txt	Changed conditional to if(CLP_BUILD_CLP_S_FFI_SFA) so clp_s_ffi_sfa library and alias clp_s::ffi::sfa are created only when the new FFI flag is enabled.
Search sources trimmed components/core/src/clp_s/search/CMakeLists.txt	Removed ../Defs.hpp, ../DictionaryEntry.cpp, ../DictionaryEntry.hpp, ../DictionaryWriter.cpp, and ../DictionaryWriter.hpp from CLP_S_SEARCH_SOURCES (reduced clp_s_search source list).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main changes: aligning CLP_BUILD_TESTING requirements with unit test linkage and moving clp-s tests to src/clp_s/tests, matching the refactored CMake configuration shown in the raw summary.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details

{}

[Bill-hbrhbr]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[gibber9809]

Extremely minor nit, but otherwise looks good.

[coderabbitai]

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 `@components/core/CMakeLists.txt`:
- Around line 676-691: Reorder the two archive libraries in the
target_link_libraries call for clp_unit_test_runtime so clp_s::archive_reader
appears before clp_s::archive_writer; locate the target_link_libraries block for
clp_unit_test_runtime and swap the positions of clp_s::archive_writer and
clp_s::archive_reader (leaving all other entries unchanged) to make the clp_s::*
list alphabetical.

In `@components/core/src/clp_s/CMakeLists.txt`:
- Around line 518-530: The test source file list in CMakeLists.txt is out of
ASCII-case-sensitive alphabetical order: move tests/test-FloatFormatEncoding.cpp
so it appears before any test-clp_s-* entries (e.g., before
filter/tests/test-clp_s-bloom_filter.cpp and tests/test-clp_s-*.cpp) to restore
correct ordering; update the sequence containing
filter/tests/test-clp_s-bloom_filter.cpp, filter/tests/test-clp_s-xxhash.cpp,
tests/clp_s_test_utils.cpp, tests/clp_s_test_utils.hpp,
tests/test-clp_s-delta-encode-log-order.cpp, tests/test-clp_s-end_to_end.cpp,
tests/test-clp_s-ffi_sfa_reader.cpp, tests/test-clp_s-range_index.cpp,
tests/test-clp_s-search.cpp, tests/test-FloatFormatEncoding.cpp,
tests/test-kql.cpp, tests/test-sql.cpp, tests/test_InputConfig.cpp so that
tests/test-FloatFormatEncoding.cpp is placed immediately before the test-clp_s
entries following ASCII ordering.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 140648dc-a7bb-47fd-a570-cc556e11f404

📥 Commits

Reviewing files that changed from the base of the PR and between f672412 and 8239b91.

📒 Files selected for processing (2)

• components/core/CMakeLists.txt
• components/core/src/clp_s/CMakeLists.txt

[gibber9809]

LGTM. PR title fine as well.

[junhaoliao]

deferring to @gibber9809 's review