Toolchain: Remove redundant exporting variables by Growl1234 · Pull Request #7230 · deepmodeling/abacus-develop

Growl1234 · 2026-04-10T04:25:46Z

Remove redundant exporting variables
For CMake 4.x compatibility: Add -DCMAKE_POLICY_VERSION_MINIMUM=3.5 flag to libXC installing script, and update ScaLAPACK to version 2.2.3.

Resolves #7226.

Copilot

Pull request overview

This PR streamlines the ABACUS toolchain install scripts by reducing redundant environment-variable exports in generated setup_* files (relying more consistently on prepend_path), while also updating some package configuration details (e.g., ScaLAPACK versions and a LibXC CMake option).

Changes:

Remove redundant export ...="$dir:$VAR" patterns in multiple install scripts and standardize variable expansion/quoting.
Adjust how toolchain setup files populate CPATH, CMAKE_PREFIX_PATH, PKG_CONFIG_PATH, and library paths via prepend_path.
Update ScaLAPACK main/alt versions (and checksums) in centralized package version management.

Reviewed changes

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

Show a summary per file

File	Description
toolchain/scripts/stage4/install_rapidjson.sh	Simplifies setup generation (use `prepend_path` + consolidated exports) and standardizes quoting.
toolchain/scripts/stage4/install_nep.sh	Removes redundant exports in generated setup file; keeps root/flags exports.
toolchain/scripts/stage4/install_libtorch.sh	Removes redundant exports in setup generation (but currently impacts CPU include path setup).
toolchain/scripts/stage4/install_libri.sh	Simplifies CPATH setup and standardizes quoting.
toolchain/scripts/stage4/install_libnpy.sh	Simplifies CPATH setup and standardizes quoting.
toolchain/scripts/stage4/install_libcomm.sh	Simplifies CPATH setup and standardizes quoting.
toolchain/scripts/stage4/install_cereal.sh	Simplifies setup generation and adjusts CMake prefix handling.
toolchain/scripts/stage3/install_scalapack.sh	Removes redundant exports from path-setup block; keeps root/flags exports.
toolchain/scripts/stage3/install_libxc.sh	Simplifies setup generation and adds a CMake policy minimum option.
toolchain/scripts/stage3/install_fftw.sh	Removes redundant exports in setup generation.
toolchain/scripts/stage3/install_elpa.sh	Removes redundant exports in setup generation and consolidates `ELPA_ROOT` export.
toolchain/scripts/stage2/install_openblas.sh	Removes redundant exports in one block; still sets/uses pkg-config/cmake prefix paths in setup.
toolchain/scripts/stage2/install_aocl.sh	Drops previously generated path-prepend setup block; keeps AOCL flags/root exports.
toolchain/scripts/stage1/install_openmpi.sh	Removes redundant exports and attempts to switch to `prepend_path` for env setup.
toolchain/scripts/stage1/install_mpich.sh	Removes redundant exports and attempts to switch to `prepend_path` for env setup.
toolchain/scripts/stage1/install_intelmpi.sh	Removes non-system PATH/lib/include prepend block; keeps wrapper/flags exports.
toolchain/scripts/stage0/install_intel.sh	Removes redundant path-prepend block; keeps compiler/flags exports.
toolchain/scripts/stage0/install_gcc.sh	Removes redundant exports in generated setup file; keeps prepend-based env setup.
toolchain/scripts/stage0/install_cmake.sh	Removes redundant PATH export and relies on `prepend_path`.
toolchain/scripts/stage0/install_amd.sh	Removes redundant path-prepend block; keeps compiler/flags exports.
toolchain/scripts/package_versions.sh	Bumps ScaLAPACK versions/checksums in centralized version list.

Comments suppressed due to low confidence (1)

toolchain/scripts/stage3/install_scalapack.sh:130

SCALAPACK_ROOT is exported twice in the generated setup file. This is redundant and makes future edits error-prone; keep a single export (preferably once in the common export block).

    cat << EOF >> "${BUILDDIR}/setup_scalapack"
export SCALAPACK_ROOT="${pkg_install_dir}"
export SCALAPACK_LDFLAGS="${SCALAPACK_LDFLAGS}"
export SCALAPACK_LIBS="${SCALAPACK_LIBS}"
export SCALAPACK_ROOT="${pkg_install_dir}"
export CP_DFLAGS="\${CP_DFLAGS} IF_MPI(-D__SCALAPACK|)"

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

toolchain/scripts/stage1/install_openmpi.sh

toolchain/scripts/stage1/install_mpich.sh

toolchain/scripts/stage4/install_libtorch.sh

toolchain/scripts/stage4/install_libri.sh

toolchain/scripts/stage4/install_libcomm.sh

toolchain/scripts/stage2/install_openblas.sh

QuantumMisaka · 2026-04-10T13:18:19Z

Thanks for your PR! I'll have some quick tests

toolchain/scripts/package_versions.sh

QuantumMisaka · 2026-04-10T14:28:08Z

There is a problem during my quick test for gnu toolchain in my WSL environment

🖥️  System Information:
   ├─ OS: Linux x86_64
   ├─ Kernel: 6.6.87.2-microsoft-standard-WSL2
   ├─ glibc: 2.35
   ├─ CPU Cores: 32
   ├─ CPU Model: AMD Ryzen 9 3950X 16-Core Processor
   ├─ Memory: 31GB
   └─ GPU: NVIDIA GeForce RTX 2070 SUPER

⚙️  Build Configuration:
   ├─ MPI Mode:       openmpi
   ├─ Math Mode:      openblas
   ├─ Target CPU:     native
   ├─ GPU Version:    no
   └─ Parallel Jobs:  32

...


==================== Finding GCC from system paths ====================
path to gcc is /usr/bin/gcc
path to g++ is /usr/bin/g++
path to gfortran is /usr/bin/gfortran
Found include directory /usr/include
Step gcc took 0.00 seconds.
Step intel took 0.00 seconds.
Step amd took 0.00 seconds.
==================== Getting proc arch info using OpenBLAS tools ====================
OpenBLAS-0.3.30.tar.gz is found
OpenBLAS detected LIBCORE = zen
OpenBLAS detected ARCH    = x86_64
==================== Installing CMake ====================
cmake-3.31.7 is already installed, skipping it.
Step cmake took 0.00 seconds.

────────────────────────────────────────────────────────────
[*] Stage 1: MPI Implementations
────────────────────────────────────────────────────────────

[^] Installing packages for stage 1...
==================== Installing OpenMPI ====================
openmpi-5.0.8 is already installed, skipping it.
Found directory /home/james/toys/abacus/abacus-develop/toolchain/install/openmpi-5.0.8/bin
Found directory /home/james/toys/abacus/abacus-develop/toolchain/install/openmpi-5.0.8/lib
Found directory /home/james/toys/abacus/abacus-develop/toolchain/install/openmpi-5.0.8/include
mpiexec is installed as /home/james/toys/abacus/abacus-develop/toolchain/install/openmpi-5.0.8/bin/mpiexec
mpicc is installed as /home/james/toys/abacus/abacus-develop/toolchain/install/openmpi-5.0.8/bin/mpicc
mpicxx is installed as /home/james/toys/abacus/abacus-develop/toolchain/install/openmpi-5.0.8/bin/mpicxx
mpifort is installed as /home/james/toys/abacus/abacus-develop/toolchain/install/openmpi-5.0.8/bin/mpifort
sed: -e expression #1, char 81: unknown option to `s'

Do you encounter the same problem ?

Growl1234 · 2026-04-10T14:42:34Z

I'll have a look, but I haven't encountered this problem before. Update: This should now be resolved. It's my fault forgetting to remove the trailing `:${PATH}`, but it also seems the sed command in `remove_path` function is not that robust.

QuantumMisaka · 2026-04-12T03:16:46Z

@Growl1234 Please link your issue in your PR introduction.

QuantumMisaka

LGTM

Growl1234 added 3 commits April 10, 2026 12:13

Remove redundant exporting variables

415a49f

Make pretty

e076667

Compatibility with CMake 4.x

3a476b9

Copilot AI review requested due to automatic review settings April 10, 2026 04:25

Growl1234 mentioned this pull request Apr 10, 2026

Coexistence of prepend_path and explicit export PATH statements #7226

Open

Copilot started reviewing on behalf of Growl1234 April 10, 2026 04:26 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

Growl1234 force-pushed the toolchain branch 5 times, most recently from 46f60c8 to a180b61 Compare April 10, 2026 04:37

QuantumMisaka self-requested a review April 10, 2026 13:16

QuantumMisaka reviewed Apr 10, 2026

View reviewed changes

toolchain/scripts/package_versions.sh Show resolved Hide resolved

Fix

ca4944f

Growl1234 force-pushed the toolchain branch 4 times, most recently from fca9a25 to 867bcaf Compare April 10, 2026 16:09

Make remove_path more robust

bf35a25

Growl1234 force-pushed the toolchain branch from 867bcaf to bf35a25 Compare April 10, 2026 16:10

QuantumMisaka approved these changes Apr 12, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Toolchain: Remove redundant exporting variables#7230

Toolchain: Remove redundant exporting variables#7230
Growl1234 wants to merge 5 commits intodeepmodeling:developfrom
Growl1234:toolchain

Growl1234 commented Apr 10, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

QuantumMisaka commented Apr 10, 2026

Uh oh!

Uh oh!

QuantumMisaka commented Apr 10, 2026

Uh oh!

Growl1234 commented Apr 10, 2026 via email •

edited

Loading

Uh oh!

QuantumMisaka commented Apr 12, 2026

Uh oh!

QuantumMisaka left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Growl1234 commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

QuantumMisaka commented Apr 10, 2026

Uh oh!

Uh oh!

QuantumMisaka commented Apr 10, 2026

Uh oh!

Growl1234 commented Apr 10, 2026 via email • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

QuantumMisaka commented Apr 12, 2026

Uh oh!

QuantumMisaka left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Growl1234 commented Apr 10, 2026 •

edited

Loading

Growl1234 commented Apr 10, 2026 via email •

edited

Loading