Skip to content

Toolchain/CMake: Improve dependency handling#7449

Open
Growl1234 wants to merge 3 commits into
deepmodeling:developfrom
Growl1234:toolchain
Open

Toolchain/CMake: Improve dependency handling#7449
Growl1234 wants to merge 3 commits into
deepmodeling:developfrom
Growl1234:toolchain

Conversation

@Growl1234
Copy link
Copy Markdown

@Growl1234 Growl1234 commented Jun 7, 2026

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.cmake file. The FetchContent fallback 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.cmake file is also removed in favor of libxc's own CMake config files.

Copilot AI review requested due to automatic review settings June 7, 2026 16:19
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 RapidJSON and cereal via find_package(...), removing the custom FindCereal.cmake and RapidJSON FetchContent fallback.
  • CI workflows: standardize defaults.run.shell: bash, ensure xz tooling 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.

Comment thread toolchain/scripts/stage4/install_rapidjson.sh
Comment thread toolchain/scripts/stage4/install_cereal.sh
Comment thread toolchain/scripts/stage4/install_cereal.sh
Comment thread CMakeLists.txt Outdated
Comment thread CMakeLists.txt Outdated
Comment thread .github/workflows/dynamic.yml
Comment thread .github/workflows/coverage.yml
Comment thread .github/workflows/build_test_cmake.yml
Comment thread .github/workflows/ase_plugin_test.yml
@Growl1234
Copy link
Copy Markdown
Author

Growl1234 commented Jun 7, 2026

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.

@Growl1234 Growl1234 changed the title Toolchain: Rely on installed CMake config file for cereal and RapidJSON Toolchain: Rely on installed CMake config file for libxc, cereal, and RapidJSON Jun 7, 2026
@Growl1234 Growl1234 marked this pull request as draft June 7, 2026 18:33
@Growl1234
Copy link
Copy Markdown
Author

I have switched to interface targets, which provide a more modern CMake approach for ensuring stable propagation of dependency usage requirements.

@Growl1234 Growl1234 marked this pull request as ready for review June 7, 2026 19:37
@Growl1234 Growl1234 changed the title Toolchain: Rely on installed CMake config file for libxc, cereal, and RapidJSON CMake: Improve dependency handling for libxc, cereal, and RapidJSON Jun 7, 2026
@Growl1234 Growl1234 changed the title CMake: Improve dependency handling for libxc, cereal, and RapidJSON Toolchain/CMake: Improve dependency handling for libxc, cereal, and RapidJSON Jun 7, 2026
@Growl1234
Copy link
Copy Markdown
Author

Growl1234 commented Jun 7, 2026

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, remove_definitions(...) is no longer appropriate.

@Growl1234 Growl1234 changed the title Toolchain/CMake: Improve dependency handling for libxc, cereal, and RapidJSON Toolchain/CMake: Improve dependency handling Jun 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants