svalinn/DAGMC

Tests build with installed version of DAGMC if present

pshriwise opened this issue · 8 comments

Describe the Bug

With the merge of #876 I noticed after pulling those changes that I couldn't rebuild DagMC successfully due to missing symbols at the linking stage of building our test programs. The error was a missing symbol to the DagMC constructor with signature

DagMC::DagMC(std::shared_ptr<Interface>, double, double)

We of course now have an extra int argument at the end of our constructors. The problem turned out to be that the test program is using the installed DagMC.hpp header instead of the one in the build directory. The installed header came from before I pulled the recent changes in that PR and contains the old constructor signature. When that test program tries to link to the DAGMC library compiled in the build directory, it then throws an error for a missing symbol.

To Reproduce

For an existing build of DAGMC (w/ PATH env var set to include the current DAGMC installation), pull the changes from #876 and attempt to rebuild DAGMC.

gonuke commented

To make sure I understand: the solution would be to force all tests to use the headers from the build directory, right?

To make sure I understand: the solution would be to force all tests to use the headers from the build directory, right?

That would be my proposal, yes. I haven't looked at the CMake files, but I don't expect it would be a big change.

One way to do this is by adding the build directory to the include path before the installation directory.

include_directories(${CMAKE_BINARY_DIR}/src/dagmc ${CMAKE_INSTALL_INCLUDEDIR})

Or, we can modify the tests themselves.

#include "${CMAKE_BINARY_DIR}/src/dagmc/DagMC.hpp"
gonuke commented

What if we add SET(CMAKE_INCLUDE_DIRECTORES_BEFORE ON)?

Seems like a big hammer, and surprised we haven't encountered it before, but should work?

What if we add SET(CMAKE_INCLUDE_DIRECTORES_BEFORE ON)?

Seems like a big hammer, and surprised we haven't encountered it before, but should work?

According to my knowledge, DAGMC depends on third-party libraries installed in the system include directories. So, if these libraries have conflicting versions or headers with DAGMC, this could lead to unexpected behavior or compilation errors. However, we can resolve that by testing DAGMC in different build environments (Geant4, OpenMC, PyNE etc) to ensure that it works as expected.

gonuke commented

But for the purpose of testing we probably always want the most local headers, right?

But for the purpose of testing we probably always want the most local headers, right?

Yes, it is usually best to use the most local headers to ensure that the tests are using the correct version of the code.

Setting CMAKE_INCLUDE_DIRECTORIES_BEFORE to ON can be one way to achieve this, but as I mentioned earlier, it can have unintended consequences if not used carefully.

I have tried all the methods above, but nothing has worked out. I think we need to reconfigure the dagmc_install_exe or dagmc_get_link_libs macros?