Bring in the CMake/PyPI build overhaul - #63
Merged
Conversation
add v0 of new infra
…fork testing Fork-test-only commit: points at the ryanmrichard/.github fork's reusable workflows, and opts the merge-time PyPI deploy into Test PyPI rather than the real, production PyPI.
…oken with OIDC trusted publishing)
…test, publishes to Test PyPI)
The pybind11 module is a shared object; anything statically linked into it (including FetchContent'd deps like spdlog) needs -fPIC. Discovered via fork-testing ParallelZone: linking its Python module failed on Linux with "relocation R_X86_64_TPOFF32 ... can not be used when making a shared object" because spdlog was built without PIC.
…or fork testing PluginPlay's build_overhaul depends on ParallelZone and Utilities' refactored (get_dependencies-based) CMake structure, which only exists on their ryanmrichard/* forks' build_overhaul branches; NWChemEx/'s master still has the old, incompatible build system.
…m_required
libfort v0.4.2's CMakeLists.txt requests a CMake version older than
3.5, which recent CMake releases refuse to configure at all
("Compatibility with CMake < 3.5 has been removed"). Discovered via
fork-testing PluginPlay, which pulls libfort in through get_dependencies.
Lets the macro build test-only pybind11 helper modules (e.g. PluginPlay's
py_test_pluginplay/pluginplay_examples, TensorWrapper's
py_test_tensorwrapper) instead of every repo hand-rolling its own
pybind11_add_module block: NO_INSTALL skips shipping the module in the
wheel, DEPENDS links extra libraries beyond ${PROJECT_NAME}. Existing
2-positional-arg call sites are unaffected.
…ting Chemist depends on tensorwrapper via get_dependencies(); its fork-testing build needs the fork's build_overhaul branch, which carries the refactored CMake build system. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ork testing SimDE depends on both via get_dependencies(); its fork-testing build needs both forks' build_overhaul branches. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…st PROJECT_NAME Only the package's own library (e.g. libparallelzone.so) was ever co-installed alongside the pybind11 extension; a FetchContent'd shared dependency built shared (e.g. spdlog) was not, so wheel-repair tools (auditwheel/delocate) had nothing to bundle it from and failed with "library not found". install(TARGETS ... RUNTIME_DEPENDENCY_SET ...) walks the extension's actual dynamic-link dependencies at install time and stages every non-system shared library it finds, superseding the old single-target co-install. Verified end-to-end (docker ubuntu:24.04 + gcc-14, ParallelZone with BUILD_PYBIND11_BINDINGS=ON): both libparallelzone.so and libspdlog.so.1.16.0 land in the install dir now. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… the
main library to \$ORIGIN + \$ORIGIN/lib, restore explicit co-install
Root cause of the auditwheel/delocate "library not found" failure: it was
never actually a missing-install problem (an install(TARGETS ...
RUNTIME_DEPENDENCY_SET ...) attempt in the prior commit found that CMake
auto-excludes files that already have their own install() rule elsewhere,
e.g. spdlog's bundled install and this library's own install_library()
call -- so nothing new ever got installed by that route). The actual gap
was an rpath mismatch: the pybind11 extension (installed flat at the
platlib root) only searched $ORIGIN, but its shared dependencies land one
directory down at "lib/" via their own pre-existing install rules, and the
main library itself (e.g. libparallelzone.so) had no rpath at all, so even
once co-located with spdlog it still couldn't find it (RUNPATH is
non-transitive). Fixed by giving both the extension and the main library
"$ORIGIN;$ORIGIN/lib" ("@loader_path;@loader_path/lib" on macOS), which
covers both of a library's two install locations (its own "lib/" home, and
a flat co-install next to the extension) symmetrically.
Verified end-to-end (docker ubuntu:24.04 + gcc-14, ParallelZone): ldd
resolves libspdlog.so from both the extension and libparallelzone.so, and
`auditwheel repair` completes successfully.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ded convention
Root cause of the persistent auditwheel "library not found" failure:
spdlog (via its own bundled GNUInstallDirs-respecting install rules)
installs to a platform-specific lib dir -- "lib64" on the manylinux2014
(CentOS7-based) container specifically, since GNUInstallDirs defaults
there for 64-bit systems -- one level away from where install_target.cmake
always puts this project's own libraries ("lib", hardcoded, no
GNUInstallDirs). The rpath widening from the previous commit only ever
covered "lib", so it silently worked on Ubuntu (no lib64 split) but not
on the actual manylinux container CI publishes wheels from. Forcing
CMAKE_INSTALL_LIBDIR=lib before any FetchContent'd dependency configures
keeps everything installed to the one directory every rpath in this
ecosystem actually searches.
Verified against the exact CI container (docker
quay.io/pypa/manylinux2014_x86_64:2024.11.16-1): spdlog now lands in
lib/, not lib64/, and `auditwheel repair` succeeds.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…mode Config mode (CMP0167 NEW) requires Boost's own exported BoostConfig.cmake, only shipped since 1.70; every manylinux image's package manager tops out below that (manylinux2014's boost-devel is 1.53.0, manylinux_2_28's is 1.66.0), so any repo needing Boost failed to configure at all inside a cibuildwheel wheel build. Nothing in this ecosystem uses more than the header-only Boost::boost umbrella target, which CMake's own bundled (if deprecated) FindBoost module provides identically in Module mode, and that module works fine with much older Boost versions. Verified end-to-end (docker quay.io/pypa/manylinux2014_x86_64): Boost 1.53.0 resolves via find_package(Boost REQUIRED), producing a working Boost::boost target; PluginPlay configures and builds successfully. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fixes a leftover pointing at NWChemEx/SimDE@master that was missed when the other dependency files were retargeted for fork testing.
libxc's own CMakeLists.txt only defines the plain 'xc' target; 'Libxc::xc' is an imported-target alias created by its install(EXPORT) rule, which doesn't exist when consumed via FetchContent/add_subdirectory (same situation as gau2grid's plain 'gg' target). Fixes SCF's 'target was not found' CMake configure error.
…d deps GauXC transitively FetchContents its own libxc/ExchCXX/IntegratorXX, at least one of which has a cmake_minimum_required() below CMake's 3.5 floor -- newer CMake (4.x) refuses to configure that at all. Same fix already applied for libfort.cmake.
…s module
nwx_python_test's ENVIRONMENT test property baked in $ENV{PYTHONPATH}
at CMake *configure* time, so it only ever picked up whatever was on
PYTHONPATH before configure ran -- generally empty on CI, and useless
for a test needing a source-fetched dependency's own pybind11 module
(e.g. SCF/ChemCache's tests importing parallelzone, built as
parallelzone_python under build/_deps/, never installed anywhere).
get_dependencies now records the build-tree location of any fetched
dependency's own "<dep>_python" target (if it built one) in a GLOBAL
property, and nwx_python_test folds those into the test's PYTHONPATH
via target-file generator expressions (evaluated at test-run time, not
configure time) instead of relying solely on the inherited environment.
get_dependencies(nwchemex) is called by SCF's C++ integration tests but no dependencies/nwchemex.cmake ever existed to satisfy it. Add one that nests get_dependencies(integrals chemcache nux) and exposes the result as a single INTERFACE target named nwchemex. Also remove the old cmaize-era get_nwchemex.cmake, which is unused by the FetchContent-based get_dependencies() system and points at a NWChemEx CMakeLists.txt that no longer exists.
get_dependencies(nwchemex)'s nested get_dependencies(integrals chemcache nux) call needs each of these to have its own dependencies/*.cmake file, same pattern as simde.cmake/chemist.cmake. These never existed because none of these repos had previously been consumed as a CMake FetchContent dependency by another CMake project.
When another dependency in the same build has already declared a
FetchContent dependency named "eigen" (e.g. SCF's own gauxc/eigen
fetch, now reachable via the new nwchemex aggregator), libint2's
internal Eigen detection reuses that already-populated source dir but
wraps it in its own plain INTERFACE target ("libint2_Eigen") that it
unconditionally exports via its own install(EXPORT ...) -- which then
fails CMake's generate-time check because the wrapped include dir is a
build-tree path. We only need libint2's build-tree targets, never its
installed package config, so skip its install rules entirely while
it's being added.
BUILD_TESTING is a global CACHE flag, so without toggling it off around the nested get_dependencies(integrals chemcache nux) call, each of those repos also registers (and CTest then runs) its own unit-test suite as a side effect of being fetched as a library dependency here -- e.g. chemcache's py_utils_test_chemcache, which needs a test-only Python dependency (requests) this build never installs. We only need the library targets, same rationale as libint2.cmake's existing BUILD_TESTING toggle.
CMAKE_SKIP_INSTALL_RULES prevents libint2 from generating its own cmake_install.cmake, but the parent directory's generated cmake_install.cmake still contains an unconditional include() of that path, so `cmake --install` (and thus `pip install`) fails with "include could not find requested file". Write a no-op stand-in file so the forwarding include succeeds. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
AppleClang ships no OpenMP runtime, so GauXC's find_package(OpenMP REQUIRED) for C and CXX fails outright on macOS unless something points FindOpenMP.cmake at libomp. Set the standard Homebrew-recommended flags/lib hints when Homebrew's libomp is present and nothing has already hinted OpenMP. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Suppressing libint2's install rules (to dodge its broken install(EXPORT ...) when Eigen is already fetched elsewhere) also threw out the plain install(TARGETS ...) that puts its .dylib/.so next to consumers. Without it, anything linking libint2 builds and links fine but fails at runtime with "Library not loaded: libint2..." once installed. Add back just the binary install -- no EXPORT set, so it can't hit the wrapped-Eigen generate-time error this was all working around. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
pluginplay's doc_snippets example library already called nwx_library() with these keywords (to avoid globbing in Catch2 test files that live alongside the example implementation), but the macro silently ignored them, dumping the literal tokens into target_link_libraries() and falling back to a glob that swept up the Catch2-dependent sources without ever linking Catch2WithMain against them.
FetchContent_MakeAvailable runs a dependency's CMakeLists.txt as a subdirectory, sharing this project's BUILD_TESTING/INTEGRATION_TESTING cache variables verbatim -- so a developer build of e.g. pluginplay was also configuring and registering utilities' and parallelzone's own unit and integration tests. Force both off for the duration of the batched fetch, then restore, mirroring the per-dependency BUILD_TESTING backup/restore already used by libxc.cmake and friends.
libint2's bundled CMakeLists.txt registers its "libint2/..." tests (eritest, unit, hf, hf++) unconditionally -- it never checks BUILD_TESTING, so setting that off around FetchContent_MakeAvailable has no effect on them. Left alone, a consumer's ctest run tries to build them on demand via a nested cmake re-configure of the *consumer's* build tree, which fails outright since libint2's tests assume its own build layout. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
subdirectory's tests from the parent scope
The previous commit's get_property(DIRECTORY ...)/set_tests_properties
approach failed at configure time ("Can not find test to add
properties to") because CMake only lets a directory manage tests it
registered itself. Suppress add_test() for the duration of libint2's
FetchContent_MakeAvailable instead, restoring it immediately after so
integrals' (or any other consumer's) own tests still register normally.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The add_test()-override still needs to actually create each test: libint2's CMakeLists.txt follows several add_test() calls with its own set_tests_properties() (e.g. FIXTURES_SETUP), which errors out "Can not find test" if the test was never registered. Call the real _add_test, then mark the result DISABLED from within the same override -- that runs in libint2's own directory scope (the caller's scope at the point add_test() is invoked), so it can actually see the test it just made. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Marking them DISABLED still listed all 13 libint2/* tests in every ctest run (as "Not Run (Disabled)"), just cluttering the output for a dependency's internal tests nothing here wants. No-op set_tests_properties() too (in addition to add_test()) so libint2's own FIXTURES_SETUP/REQUIRED calls on the never-registered tests don't error out. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The build was rewritten around vanilla CMake modules (get_dependencies plus
a cmake/dependencies/*.cmake file per dependency), but the old CMaize files
were never removed. None of them is reachable from any repo CMakeLists.txt,
any CI workflow, or any other NWXCMake module, and none would even parse
without CMaize defining cmaize_find_or_build_dependency and the
CMaizeTarget/CMaizeProject API.
Removed:
- 12 get_<dep>.cmake CMaize shims, each superseded by the same-named file
under cmake/dependencies/ (or, for exachem/tamm, by a dependency that is
no longer part of the stack)
- get_cmaize.cmake and nwx_find_package.cmake, the CMaize bootstrap and glue
- nwx_pybind11.cmake, superseded by nwx_python_module.cmake and
nwx_python_test.cmake
Keeping them around meant a developer grepping for how a dependency gets
resolved found two competing answers.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Remove CMaize-era dead files
This repo's CI workflows called back into the shared workflows/actions in ryanmrichard/.github (the fork they were developed against); repoint them at NWChemEx/.github now that stage 1 of the migration has landed there. Also drops the TestPyPI repository-url override on the merge workflow's publish step, so a merge to master publishes to real PyPI -- this repo's package name is free there and a trusted publisher is being registered for it. The ecosystem's own C++ libraries fetched via cmake/dependencies/*.cmake (chemist, pluginplay, parallelzone, ...) get the same owner fix, but GIT_TAG deliberately stays build_overhaul for now: those repos' NWChemEx master branches don't have this content yet, so flipping the tag here too would break every downstream C++ build immediately. A follow-up PR flips GIT_TAG to master once the last of them has merged.
check_formatting was failing because this repo never had a .licenserc.yaml (every sibling repo has one). Without it, license-eye fell back to its own default ASF-style header and prepended that to every file instead of recognizing the existing NWChemEx-Project one. Add the same config the other repos use, then apply the actual pre-commit fixes it and the other hooks caught: a missing header on skbuild_python.cmake, missing trailing newlines, and trailing whitespace. test_pip_build was failing separately because test_nwx_pip_build.yaml runs pytest by default and this repo has no tests directory or pytest dev extra at all -- it's CMake modules and a thin Python package with no test suite of its own. Set run_python_tests: "false" alongside the existing run_cmake_tests: "false" (this repo has no CMakeLists.txt of its own either).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cmake/dependencies/*.cmake,get_dependencies,get_version_from_git,nwx_set_version,nwx_python_module, etc.) and the pip-packaging setup (pyproject.toml, scikit-build-core config) developed on theryanmrichardfork -- this repo is the shared CMake glue that every other NWChemEx C++ repo'sget_nwx_cmake.cmakefetches.merge.yaml,pull_request.yaml) fromryanmrichard/.githubtoNWChemEx/.github(landed in Land the build/CI overhaul's shared workflows and actions .github#187), and itscmake/dependencies/*.cmakefetches of the ecosystem's own C++ libraries (chemist, pluginplay, parallelzone, tensorwrapper, utilities, simde, chemcache, integrals, nux, nwchemex) fromryanmrichard/<Repo>toNWChemEx/<Repo>.GIT_TAGon those 10 fetches deliberately staysbuild_overhaulfor now -- their NWChemExmasterbranches don't have this content yet. A follow-up PR flips them tomasteronce the whole ecosystem has migrated.test.pypi.orgrepository-urloverride from the merge workflow's publish step, so merging tomasterpublishesnwchemex-nwxcmaketo real PyPI. This is the first package in the migration (nothing else has an NWXCMake floor to publish against yet), and a trusted publisher is being registered for it.Test plan
nwchemex-nwxcmakepublishes to PyPI on merge, and the version matches the newly minted git tag