Migrate library and installation configuration to use Cetmodules by greenc-FNAL · Pull Request #165 · Framework-R-D/phlex

greenc-FNAL · 2025-12-05T18:55:52Z

Description

This PR modernizes the build system by fully adopting cetmodules for library generation, export sets, and installation configuration. It also transitions the majority of the test suite to use cet_test.

Key Changes

Library Configuration:
- Fully converted all libraries in phlex/ (core, graph, metaprogramming, model, utilities, app) to use cet_make_library.
- Established the phlex export set via cet_register_export_set.
Installation Fixes:
- Added INSTALL_RPATH to the phlex executable to ensure it can locate librun_phlex.so relative to the binary.
- Workaround: Applied a workaround for cetmodules issue Remove for_each(...) registration API #34 (regarding FILE_SET in install(TARGETS)) by replacing FILE_SET HEADERS with explicit install(FILES ...) for phlex/app headers.
Test Suite Modernization:
- Transitioned the entire test suite (unit tests, benchmarks, integration tests) to use cet_test (except test/form, to follow as a separate PR).
- Updated integration tests to use ${PROJECT_BINARY_DIR} for PHLEX_PLUGIN_PATH, ensuring plugins are found regardless of the build directory structure.

Notes

This PR ensures that the core library and application installation structure is correct and relocatable.

Copilot

Pull request overview

This PR modernizes the build system by migrating from manual CMake library management to cetmodules, a specialized CMake framework for C++ projects. The changes include converting all library definitions to use cet_make_library, establishing a unified export set for the phlex namespace, and transitioning the test suite to use cet_test.

Key changes:

Converted 6 library modules (core, graph, metaprogramming, model, utilities, app) to use cet_make_library with proper export names
Migrated 30+ unit tests and integration tests from custom helper functions to cet_test
Updated integration tests to use ${PROJECT_BINARY_DIR} for plugin paths, making the build directory structure more flexible

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
CMakeLists.txt	Added `cet_cmake_env()` and `cet_cmake_config()` calls to initialize cetmodules infrastructure
phlex/CMakeLists.txt	Registered the `phlex` export set and reorganized subdirectory inclusion order; created interface libraries for `module` and `configuration`
phlex/app/CMakeLists.txt	Converted `run_phlex` library to use `cet_make_library`, added `INSTALL_RPATH` to phlex executable, and switched to explicit header installation
phlex/core/CMakeLists.txt	Migrated core library to `cet_make_library` with explicit file-based header installation and interface library setup
phlex/graph/CMakeLists.txt	Converted graph library to cetmodules with manual header installation
phlex/metaprogramming/CMakeLists.txt	Transitioned metaprogramming interface library to `cet_make_library`
phlex/model/CMakeLists.txt	Migrated model library to cetmodules with explicit header file installation
phlex/utilities/CMakeLists.txt	Converted utilities library to `cet_make_library` with manual header installation
test/CMakeLists.txt	Replaced custom `add_unit_test` and `add_catch_test` functions with direct `cet_test` calls for all 25 unit tests
test/benchmarks/CMakeLists.txt	Converted benchmark tests to use `cet_test` with `HANDBUILT` and updated plugin path to use `${PROJECT_BINARY_DIR}`
test/max-parallelism/CMakeLists.txt	Migrated parallelism check tests to `cet_test` framework
test/memory-checks/CMakeLists.txt	Converted memory check test to `cet_test`
test/mock-workflow/CMakeLists.txt	Updated mock workflow integration test to use `cet_test`
test/plugins/CMakeLists.txt	Converted plugin test to `cet_test` framework
test/utilities/CMakeLists.txt	Migrated utility tests (sized_tuple, sleep_for, thread_counter) to `cet_test`

phlex/app/CMakeLists.txt

test/plugins/CMakeLists.txt

test/max-parallelism/CMakeLists.txt

phlex/core/CMakeLists.txt

phlex/app/CMakeLists.txt

phlex/metaprogramming/CMakeLists.txt

knoepfel

@greenc-FNAL, modulo some awkward cmake-format choices, this PR looks fine. Do you anticipate any other changes?

greenc-FNAL · 2025-12-05T20:05:19Z

@greenc-FNAL, modulo some awkward cmake-format choices, this PR looks fine. Do you anticipate any other changes?

I'm just about to take a look what Copilot has to say in its review, but otherwise it's self-contained. As for the cmake-format issues, if you tell me exactly what you don't like I can look into changing it.

- Adopt `cetmodules` for library generation and export sets: - Convert `phlex::module` and `phlex::configuration` to use `cet_make_library`. - Establish `phlex` export set via `cet_register_export_set`. - Update `phlex::core` and other libraries to use `cet_make_library`. - Re-enable `phlex/app` build (previously disabled) and update `run_phlex`. - Fix installation configuration: - Add `INSTALL_RPATH` to `phlex` executable. - Workaround for `cetmodules` issue with `FILE_SET` in `install(TARGETS)`: - Replaced `FILE_SET HEADERS` with explicit `install(FILES ...)` for `phlex/app` headers. - This addresses `cetmodules` issue Framework-R-D#34 (FNALssi/cetmodules#34). - Update integration tests to use `${PROJECT_BINARY_DIR}` for plugin paths.

- Replace custom `add_unit_test` and `add_catch_test` CMake functions with standard `cet_test()` from `cetmodules`. - Update `test/CMakeLists.txt` to include `CetTest` module. - Refactor all unit and Catch2 tests in `test/CMakeLists.txt`, `test/memory-checks/CMakeLists.txt`, and `test/utilities/CMakeLists.txt` to use `cet_test`. - Utilize `USE_CATCH2_MAIN` option for Catch2 tests to leverage built-in main function generation. - Explicitly specify source files using `SOURCE` keyword for better clarity. - This change standardizes the testing infrastructure, reduces boilerplate code, and ensures better integration with the `cetmodules` ecosystem. Convert integration tests to use cet_test() - Replace manual add_test() calls with cet_test(HANDBUILT ...). - Ensure PHLEX_PLUGIN_PATH is correctly set to PROJECT_BINARY_DIR. Run `cmake-format` (non-FORM)

knoepfel · 2025-12-05T20:33:01Z

@greenc-FNAL, modulo some awkward cmake-format choices, this PR looks fine. Do you anticipate any other changes?

I'm just about to take a look what Copilot has to say in its review, but otherwise it's self-contained. As for the cmake-format issues, if you tell me exactly what you don't like I can look into changing it.

I find long lists like:

cet_make_library(
  LIBRARY_NAME
  phlex_utilities_int
  EXPORT_NAME
  utilities
  INTERFACE
  NO_SOURCE
  LIBRARIES
  phlex_utilities
  TBB::tbb
  spdlog::spdlog
  )

difficult to parse. However, now is probably not the time to revisit the CMake formatting. We can do that another different time.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

codecov · 2025-12-05T20:46:13Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@            Coverage Diff             @@
##             main     #165      +/-   ##
==========================================
+ Coverage   80.90%   80.95%   +0.04%     
==========================================
  Files         115      115              
  Lines        2042     2042              
  Branches      330      330              
==========================================
+ Hits         1652     1653       +1     
  Misses        256      256              
+ Partials      134      133       -1

Flag	Coverage Δ
unittests	`80.95% <ø> (+0.04%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 1 file with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 062be31...31013aa. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

greenc-FNAL requested review from Copilot, knoepfel and marcpaterno December 5, 2025 18:55

Copilot started reviewing on behalf of greenc-FNAL December 5, 2025 18:56 View session

Copilot AI reviewed Dec 5, 2025

View reviewed changes

knoepfel previously approved these changes Dec 5, 2025

View reviewed changes

greenc-FNAL added 3 commits December 5, 2025 14:10

Convert remaining add_library() calls to cet_make_library()

4ac6626

greenc-FNAL dismissed knoepfel’s stale review via 4ac6626 December 5, 2025 20:29

greenc-FNAL force-pushed the maintenance/installation-verification branch from 8f8acdc to 4ac6626 Compare December 5, 2025 20:29

greenc-FNAL and others added 2 commits December 5, 2025 14:36

Combine ENVIRONMENT arguments

93fa622

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Specify PRIVATE linkage explicitly

0ede0d3

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

greenc-FNAL added 2 commits December 5, 2025 14:58

Tighten up dependencies, per Framework-R-D#165 (comment)

97abc36

Use `cet_test_env() to handle duplicated environment

31013aa

knoepfel approved these changes Dec 5, 2025

View reviewed changes

knoepfel merged commit 58501e5 into Framework-R-D:main Dec 5, 2025
36 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate library and installation configuration to use Cetmodules#165

Migrate library and installation configuration to use Cetmodules#165
knoepfel merged 7 commits intoFramework-R-D:mainfrom
greenc-FNAL:maintenance/installation-verification

greenc-FNAL commented Dec 5, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

knoepfel left a comment

Uh oh!

greenc-FNAL commented Dec 5, 2025

Uh oh!

knoepfel commented Dec 5, 2025

Uh oh!

codecov bot commented Dec 5, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

greenc-FNAL commented Dec 5, 2025

Description

Key Changes

Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

knoepfel left a comment

Choose a reason for hiding this comment

Uh oh!

greenc-FNAL commented Dec 5, 2025

Uh oh!

knoepfel commented Dec 5, 2025

Uh oh!

codecov bot commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Dec 5, 2025 •

edited

Loading