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 components, adding new conditional checks.

• 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: eaa8e6e3-c338-4b46-809e-96a6ba20fdbb

📥 Commits

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

📒 Files selected for processing (2)

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

---

Walkthrough

CMake build rules restructured: new INTERFACE libraries aggregate runtime and test dependencies; test source grouping added; option validation reordered and extended for a new FFI flag; an FFI conditional gate changed; several clp_s search sources removed.

Changes

Cohort / File(s)	Summary
Core interface targets & test linkage components/core/CMakeLists.txt, components/core/src/clp_s/CMakeLists.txt	Added clp_unit_test_runtime (clp::unit_test_runtime) INTERFACE; created clp_s_binary_runtime INTERFACE for executable linkage; added clp_s_unit_test_sources (clp_s::unit_test_sources) INTERFACE with test sources; updated unitTest to link against clp::unit_test_runtime and clp_s::unit_test_sources and include .../tests.
Dependency options & validation components/core/cmake/Options/options.cmake	Reordered CLP_BUILD_CLP_S_ENABLE_CURL setup; introduced validate_clp_s_ffi_sfa_dependencies and set_clp_s_ffi_sfa_dependencies and wired them into overall validation; expanded validate_clp_tests_dependencies to require additional clp_s components; relocated set_clp_s_enable_curl_dependencies.
FFI build gating components/core/src/clp_s/ffi/CMakeLists.txt	Changed conditional from if(CLP_BUILD_CLP_S_ARCHIVEREADER) to if(CLP_BUILD_CLP_S_FFI_SFA), so clp_s_ffi_sfa and alias clp_s::ffi::sfa are created only when CLP_BUILD_CLP_S_FFI_SFA 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 source list for clp_s_search).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and specifically describes the main changes: aligning CLP_BUILD_TESTING requirements with unit test linkage and removing SOURCE_FILES_clp_s_unitTest by restructuring test sources.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ 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.