Toolchain/CMake: Improve dependency handling#7449
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR shifts RapidJSON and cereal integration to rely on proper CMake installs/packages (instead of manual header copying / FetchContent fallbacks) and updates CI/toolchain workflows to run consistently under bash and install required stage4 tools.
Changes:
- Install RapidJSON and cereal via CMake build+install in the toolchain, and stop exporting include paths via
CPATH. - Update project CMake to require
RapidJSONandcerealviafind_package(...), removing the customFindCereal.cmakeand RapidJSON FetchContent fallback. - CI workflows: standardize
defaults.run.shell: bash, ensurexztooling is present, and install stage4 external tools before building.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| toolchain/scripts/stage4/install_rapidjson.sh | Switch RapidJSON toolchain install to CMake configure/install; remove CPATH export. |
| toolchain/scripts/stage4/install_cereal.sh | Switch cereal toolchain install to CMake configure/install; remove CPATH export. |
| toolchain/root_requirements/install_requirements_ubuntu.sh | Add xz-utils dependency. |
| toolchain/root_requirements/install_requirements_fedora.sh | Add xz dependency. |
| toolchain/build_abacus_intel.sh | Remove manual CEREAL/RAPIDJSON paths passed to CMake. |
| toolchain/build_abacus_gnu.sh | Remove manual CEREAL/RAPIDJSON paths passed to CMake. |
| toolchain/build_abacus_gcc-mkl.sh | Remove manual CEREAL/RAPIDJSON paths passed to CMake. |
| toolchain/build_abacus_gcc-aocl.sh | Remove manual CEREAL/RAPIDJSON paths passed to CMake. |
| cmake/FindCereal.cmake | Remove custom cereal find-module (move to config package). |
| CMakeLists.txt | Require RapidJSON/cereal via find_package(...) and drop FetchContent/include-directories logic. |
| .github/workflows/version_check.yml | Set default bash shell for run: steps. |
| .github/workflows/toolchain_quick.yaml | Set default bash shell for run: steps. |
| .github/workflows/toolchain_full.yaml | Set default bash shell for run: steps. |
| .github/workflows/test.yml | Install stage4 external tools via toolchain script (broader than dftd4-only). |
| .github/workflows/pytest.yml | Set default bash shell for run: steps. |
| .github/workflows/performance.yml | Set default bash shell for run: steps. |
| .github/workflows/mirror_gitee.yml | Set default bash shell for run: steps. |
| .github/workflows/interface.yml | Set default bash shell for run: steps. |
| .github/workflows/dynamic.yml | Set default bash shell; add toolchain external tools install; source toolchain env before build. |
| .github/workflows/doxygen.yml | Set default bash shell for run: steps. |
| .github/workflows/devcontainer.yml | Set default bash shell for run: steps. |
| .github/workflows/cuda.yml | Install xz-utils; install stage4 external tools; source toolchain env before build. |
| .github/workflows/coverage.yml | Install xz-utils; install stage4 external tools; source toolchain env before build. |
| .github/workflows/build_test_makefile.yml | Set default bash shell for run: steps. |
| .github/workflows/build_test_cmake.yml | Add toolchain install step, source toolchain env; set default bash shell; matrix toolchain args. |
| .github/workflows/ase_plugin_test.yml | Install xz-utils; install stage4 external tools; source toolchain env before build. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
If possible, it's better that some other libraries such as libRI and libComm also be applied a simple CMake system along with properly-generated config files. |
|
I have switched to interface targets, which provide a more modern CMake approach for ensuring stable propagation of dependency usage requirements. |
Following this change there must be some more refactors, for example, |
Cereal and RapidJSON have their own CMake build systems, which should be used during toolchain installation rather than relying on direct file copies. Therefore, the custom logic for finding these two dependencies should no longer be needed, as it brings avoidable issues and maintenance work.
This PR switches the installation method of the two packages to CMake, and removes the related custom logic, including the
FindCereal.cmakefile. TheFetchContentfallback is also dropped, since installing these dependencies is the responsibility of the toolchain.On the ABACUS CMake side, cereal and RapidJSON are now consumed through interface targets, so their include directories and compile definitions are propagated through CMake usage requirements in a more explicit and stable way.
The
FindLibxc.cmakefile is also removed in favor of libxc's own CMake config files.