[Proposal] link libeigen with CPM

[eduardz1]

I would like to propose using CPM to link the libeigen library instead of searching for it on the system with find_package. This makes Spectra more portable, including the library in another project is more transparent, and the build more reproducible.

The CPM snipped derives from the one suggested on the CPM wiki

[yixuan]

Hi @eduardz1, thanks for the PR and it looks a good direction. But can you make it an option rather than modifying the current method? After all, it needs to download the CPM script, so it may be better to keep the scripts minimal for basic use, and use CPM with an additional flag in CMake.

[eduardz1]

Hi @eduardz1, thanks for the PR and it looks a good direction. But can you make it an option rather than modifying the current method? After all, it needs to download the CPM script, so it may be better to keep the scripts minimal for basic use, and use CPM with an additional flag in CMake.

Thanks for the feedback! You're right, I included the CPM utils directly to avoid unnecessary downloads and restored the previous logic with the difference that instead of find_package I am now using CPMFindPackage which calls find_package first and only falls back to FetchContent if that fails. The behavior can be overridden with the CPM_DOWNLOAD_ALL variabile, to force downloads, or CPM_LOCAL_PACKAGES_ONLY to force the opposite behavior. This should probably be documented in the README. A small nitpick I have about my proposed change is that, when download, Eigen3 does not appear in the feature_summary, this issue is tracked at cpm-cmake/CPM.cmake#344

[yixuan]

This looks good to me, thanks! And as you have mentioned, it would be nice if those variables are documented in the README. Could you add a paragraph in the Installation section?

[eduardz1]

This looks good to me, thanks! And as you have mentioned, it would be nice if those variables are documented in the README. Could you add a paragraph in the Installation section?

Tell me if you think the way I documented it is sufficient

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.1%. Comparing base (0724d99) to head (8772b4e).
Report is 8 commits behind head on master.

Additional details and impacted files

@@          Coverage Diff           @@
##           master    #178   +/-   ##
======================================
  Coverage    94.1%   94.1%           
======================================
  Files          48      48           
  Lines        2555    2555           
  Branches      294     294           
======================================
  Hits         2406    2406           
  Misses        149     149           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

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

[yixuan]

This looks good to me. Thanks!