search

LLNL / serac

Serac is a high order nonlinear thermomechanical simulation code
BSD 3-Clause "New" or "Revised" License
178 stars 33 forks source link

Resolve duplication issue when configuring with Ninja and codevelop #1218

Closed chapman39 closed 2 weeks ago

chapman39 commented 3 weeks ago

This is a workaround to an error that occurs when configuring with codevelop and Ninja. See this BLT issue for more details https://github.com/LLNL/blt/issues/695

samuelpmish commented 2 weeks ago

did we ever make an mfem issue to address the underlying cause of this problem?

edit: maybe it's possible to avoid the issue in Sundials too by setting this: https://github.com/LLNL/sundials/blob/c28eaa3764a03705d61decb6025b409360e9d53f/CMakeLists.txt#L208-L212

chapman39 commented 2 weeks ago

did we ever make an mfem issue to address the underlying cause of this problem?

edit: maybe it's possible to avoid the issue in Sundials too by setting this: https://github.com/LLNL/sundials/blob/c28eaa3764a03705d61decb6025b409360e9d53f/CMakeLists.txt#L208-L212

Originally, I created an MFEM issue about it https://github.com/mfem/mfem/issues/4437. They mentioned it was to maintain consistency between gnu and CMake builds. I am not able to reproduce the issue in MFEM, since it's related to section of BLT: https://github.com/LLNL/blt/blob/develop/SetupBLT.cmake#L158-L162

edit: to clarify, this is not BLT or MFEM's fault but rather that the two build systems are conflicting, since MFEM doesn't normally set and use CMAKE_RUNTIME_OUTPUT_DIRECTORY. This variable sets all the executables to be placed in one location, which is causing the duplication error.

samuelpmish commented 2 weeks ago

I think we're talking about separate problems here, so let me put it another way: there's a lot of CMake projects out there that have a target named "ex1":

https://github.com/search?q=%22add_executable%28ex1+%22&type=code

Right now, the way mfem's CMake project is set up, if those projects tried to bring mfem into their CMake project with add_subdirectory (which is a common pattern), CMake will emit an error about duplicate target name. Even worse, the way mfem handles the flag MFEM_BUILD_EXAMPLES provides mfem's users no way to disable those targets from being registered, so there is no recourse to address the duplicate target error.

Of course the fix is simple: just change 2 lines in mfem's CMakeLists.txt and solve the problem described above (as well as #1218).

If no change is made to mfem, won't every other project that sets CMAKE_RUNTIME_OUTPUT_DIRECTORY (which is also common) hit this very same issue?