avogadrolibs icon indicating copy to clipboard operation
avogadrolibs copied to clipboard

standalone tests file due to removal of Eigen configuration

Open drew-parsons opened this issue 1 year ago • 7 comments

Avogadro version: (please complete the following information from the About box):

  • Avogadrolibs: 1.98.0
  • Qt: 5.15.10+dfsg-4

Desktop version: (please complete the following information):

  • OS: Debian Linux
  • Version unstable
  • Compiler gcc 13.2.0-6

Describe the bug

PR #1267 removed Eigen configuration from CMakeLists.txt files throughout avogadrolibs. This makes a problem when running the test standalone against an existing avogadrolibs installation (debian does that for CI testing)

Running cmake against tests/CMakeLists.txt only and then building tests gets an error "Eigen/Dense: No such file or directory" from /usr/include/avogadro/core/vector.h:10. This is because (on debian systems), Eigen is installed under /usr/include/eigen3. So EIGEN3_INCLUDE_DIR is needed.

To Reproduce Steps to reproduce the behavior:

  1. create test dir under tests
mkdir tests/run_test 
cd tests/run_test
  1. Configure tests, referring to installed avogadrolib headers
cmake -DAvogadroLibs_BINARY_DIR=/usr/include/ -DUSE_QT=ON -DUSE_OPENGL=ON -Wno-dev ..
  1. make AvogadroTests
  2. See error

Expected behavior

Should be possible to build and run tests standalone for the purpose of CI testing of an existing avogadrolibs installation.

Screenshots Error is

[  9%] Building CXX object core/CMakeFiles/AvogadroTests.dir/atomtest.o
cd /tmp/autopkgtest.Uz9Zw5/tree/tests/run_test/core && /usr/bin/c++  -I/usr/include/avogadro/core -DGTEST_HAS_PTHREAD=1 -MD -MT core/CMakeFiles/AvogadroTests.dir/atomtest.o -MF CMakeFiles/AvogadroTests.dir/atomtest.o.d -o CMakeFiles/AvogadroTests.dir/atomtest.o -c /tmp/autopkgtest.Uz9Zw5/tree/tests/core/atomtest.cpp
In file included from /usr/include/avogadro/core/atom.h:10,
                 from /usr/include/avogadro/core/bond.h:11,
                 from /usr/include/avogadro/core/molecule.h:14,
                 from /tmp/autopkgtest.Uz9Zw5/tree/tests/core/atomtest.cpp:19:
/usr/include/avogadro/core/vector.h:10:10: fatal error: Eigen/Dense: No such file or directory
   10 | #include <Eigen/Dense>
      |          ^~~~~~~~~~~~~
compilation terminated.
make[3]: *** [core/CMakeFiles/AvogadroTests.dir/build.make:90: core/CMakeFiles/AvogadroTests.dir/atomtest.o] Error 1

Additional context

Can be resolve by reinstating

find_package(Eigen3 REQUIRED)
include_directories(SYSTEM
  ${GTEST_INCLUDE_DIRS}
  ${EIGEN3_INCLUDE_DIR})

in tests/CMakeLists.txt

drew-parsons avatar Oct 30 '23 12:10 drew-parsons

I'd be happy to merge a PR on tests/CMakeLists.txt (I'd do it myself, but today has a lot of meetings and few gaps for code.)

ghutchis avatar Oct 30 '23 14:10 ghutchis

heh no specific rush :) I can patch around it in the meantime.

drew-parsons avatar Oct 30 '23 14:10 drew-parsons

When I said "can be resolved", that's only in regard to finding Eigen/Dense. There are a couple of other issues before I can run the tests standalone. The steps I gave above, after patching for eigen, give

[100%] Linking CXX executable AvogadroTests
cd /tmp/autopkgtest.Nb2RRo/tree/tests/run_test/core && /usr/bin/cmake -E cmake_link_script CMakeFiles/AvogadroTests.dir/link.txt --verbose=1
/usr/bin/c++ -rdynamic CMakeFiles/AvogadroTests.dir/arraytest.o CMakeFiles/AvogadroTests.dir/atomtest.o CMakeFiles/AvogadroTests.dir/atomtypertest.o CMakeFiles/AvogadroTests.dir/basissettest.o CMakeFiles/AvogadroTests.dir/bondtest.o CMakeFiles/AvogadroTests.dir/coordinateblockgeneratortest.o CMakeFiles/AvogadroTests.dir/coordinatesettest.o CMakeFiles/AvogadroTests.dir/cubetest.o CMakeFiles/AvogadroTests.dir/eigentest.o CMakeFiles/AvogadroTests.dir/elementtest.o CMakeFiles/AvogadroTests.dir/graphtest.o CMakeFiles/AvogadroTests.dir/meshtest.o CMakeFiles/AvogadroTests.dir/moleculetest.o CMakeFiles/AvogadroTests.dir/mutextest.o CMakeFiles/AvogadroTests.dir/neighborperceivertest.o CMakeFiles/AvogadroTests.dir/ringperceivertest.o CMakeFiles/AvogadroTests.dir/spacegrouptest.o CMakeFiles/AvogadroTests.dir/utilitiestest.o CMakeFiles/AvogadroTests.dir/unitcelltest.o CMakeFiles/AvogadroTests.dir/varianttest.o CMakeFiles/AvogadroTests.dir/variantmaptest.o -o AvogadroTests  -lAvogadro::Core /usr/lib/x86_64-linux-gnu/libgtest.a /usr/lib/x86_64-linux-gnu/libgtest_main.a /usr/lib/x86_64-linux-gnu/libgtest.a 
/usr/bin/ld: cannot find -lAvogadro::Core: No such file or directory

So some other patching seems to be needed to get from cmake's Avogadro::Core to -lAvogadroCore. If I add find_package(AvogadroLibs REQUIRED), it resolves Avogadro::Core but gets

[  4%] Linking CXX executable AvogadroTests
cd /projects/avogadrolibs/tests/run_test/core && /usr/bin/cmake -E cmake_link_script CMakeFiles/AvogadroTests.dir/link.txt --verbose=1
/usr/bin/c++ -rdynamic CMakeFiles/AvogadroTests.dir/arraytest.o CMakeFiles/AvogadroTests.dir/atomtest.o CMakeFiles/AvogadroTests.dir/atomtypertest.o CMakeFiles/AvogadroTests.dir/basissettest.o CMakeFiles/AvogadroTests.dir/bondtest.o CMakeFiles/AvogadroTests.dir/coordinateblockgeneratortest.o CMakeFiles/AvogadroTests.dir/coordinatesettest.o CMakeFiles/AvogadroTests.dir/cubetest.o CMakeFiles/AvogadroTests.dir/eigentest.o CMakeFiles/AvogadroTests.dir/elementtest.o CMakeFiles/AvogadroTests.dir/graphtest.o CMakeFiles/AvogadroTests.dir/meshtest.o CMakeFiles/AvogadroTests.dir/moleculetest.o CMakeFiles/AvogadroTests.dir/mutextest.o CMakeFiles/AvogadroTests.dir/neighborperceivertest.o CMakeFiles/AvogadroTests.dir/ringperceivertest.o CMakeFiles/AvogadroTests.dir/spacegrouptest.o CMakeFiles/AvogadroTests.dir/utilitiestest.o CMakeFiles/AvogadroTests.dir/unitcelltest.o CMakeFiles/AvogadroTests.dir/varianttest.o CMakeFiles/AvogadroTests.dir/variantmaptest.o -o AvogadroTests  /usr/lib/x86_64-linux-gnu/libAvogadroCore.so.1.98.0 /usr/lib/x86_64-linux-gnu/libgtest.a /usr/lib/x86_64-linux-gnu/libgtest_main.a -lEigen3::Eigen3 /usr/lib/x86_64-linux-gnu/libgtest.a 
/usr/bin/ld: cannot find -lEigen3::Eigen3: No such file or directory

drew-parsons avatar Oct 30 '23 14:10 drew-parsons

I'm not expert enough in CMake to know how to handle that.

@cryos - do you have a moment to understand why find_package(AvogadroLibs REQUIRED) would add -lEigen3::Eigen3 to linking the tests?

ghutchis avatar Oct 31 '23 03:10 ghutchis

The tests were never designed to be run standalone, I am shocked it works as well as you say. I don't really understand the desire to build the tests against an installed Avogadro, they are designed to be built in the same build tree. The testing subdirectory would really need a fair bit of work to find AvigadroLibs in order to reliably compile.

cryos avatar Oct 31 '23 03:10 cryos

CI testing. It's important to ensure the whole Linux distribution continues to keep functioning. For instance, when Qt libraries get upgraded.

drew-parsons avatar Oct 31 '23 07:10 drew-parsons

You are not wrong, but the tests were never designed with this use case in mind and I don't think we should be obligated to change them to accommodate this use case. We could look at making the tests themselves installable, but I really think modernizing the CMake is more important than fixing something that only really worked through luck rather than design.

cryos avatar Nov 09 '23 22:11 cryos