BLT subproject changes behavior of non-BLT parent project

[selamberson]

I have a large pre-existing CMake-based code base. We added a subproject to our code that uses BLT (pulled in as a subdirectory using git submodules), and many things in the parent project broke because BLT changes some global CMake variables. Specifically, SetupBLT.cmake sets LIBRARY_OUTPUT_PATH, CMAKE_RUNTIME_OUTPUT_DIRECTORY, and CMAKE_Fortran_MODULE_DIRECTORY, which changes the default behavior of many cmake routines for the entire project.

As a workaround, we had to go to every cmake routine affected by these global variables and explicitly set local variables to restore the old behavior.

I would expect and prefer that adding a CMake subproject that uses BLT to a CMake main project that does not use BLT would not result in any changes to the CMake behavior of the main project. This would be accomplished by having SetupBLT.cmake not change LIBRARY_OUTPUT_PATH, CMAKE_RUNTIME_OUTPUT_DIRECTORY, and CMAKE_Fortran_MODULE_DIRECTORY, and instead change the BLT macros to set local variables to replicate the desired behavior.

[gzagaris]

Thanks for reporting this issue @selamberson!

My initial thought is that BLT should probably define and use its own internal variables instead of modifying global CMake variables that can leak out to other projects, e.g.,

• BLT_LIBRARY_OUTPUT_PATH,
• BLT_RUNTIME_OUTPUT_DIRECTORY,
• BLT_FORTRAN_MODULE_DIRECTORY
• etc.

@white238 , @kennyweiss , @cyrush, @rhornung67, do you have any thoughts or suggestions on this?

[gzagaris]

I think this also relates to the issue reported in #711

[white238]

This is a large breaking change, which I was planning to do soon but got derailed by other priorities.

Until then would it be helpful if I created those variables listed by @gzagaris to override the default behavior if they are defined?

For example:

    # Set the path where all the libraries will be stored
    if (NOT DEFINED BLT_LIBRARY_OUTPUT_PATH)
        set(BLT_LIBRARY_OUTPUT_PATH ${PROJECT_BINARY_DIR}/lib)
    endif()

    set(LIBRARY_OUTPUT_PATH
        ${BLT_LIBRARY_OUTPUT_PATH}
        CACHE PATH
        "Directory where compiled libraries will go in the build tree")

[selamberson]

We already have implemented the workaround, so you don't need anything immediately to unlock us. It's just that if this were fixed it would have saved us quite a few headaches.

I don't think the change you showed would have been useful in our case. It still results in setting LIBRARY_OUTPUT_PATH, which will change the default location for every library that cmake creates that isn't overridden with a per-target instruction to put it somewhere else. If you leave LIBRARY_OUTPUT_PATH unset, then cmake will build libraries in CMAKE_CURRENT_BINARY_DIR, which is a context-sensitive variable. There is no value you can set LIBRARY_OUTPUT_PATH to that will result in the default CMake behavior. So, my recommendation is that you come up with a solution where BLT does not set things like LIBRARY_OUTPUT_PATH at all.

For the record, the real problem is that if a BLT project is included as a sub-project in a larger project that already has multiple non-BLT sub-projects, then the introduction of BLT will potentially break a lot of things unless a) the project was already setting the global variables, or b) none of the sub-projects happen to have any libraries or executables with the same name. In our project, many of our sub-projects had libraries and executables with the same name (mostly in the tests for these sub-projects), so when we added BLT and all of these things started getting put in the same location we were inadvertently linking to the wrong thing and/or running the wrong thing in our tests. The default cmake behavior builds these things in different locations, so it only becomes a problem if you need to install them.

[cyrush]

These type of nightmares are a general issue with fusing CMake build systems, not just BLT.

Projects have to be very careful to avoid side effects in a global system.

This is one of the reasons we largely use package managers to manage tpls instead of building via a fused build system with source from git submodules.

We have suffered several head scratching events from git submodule attempts.

That's not to say you should change your development model -- BLT can be improved.

If there is a project you need to include in this way, in addition to BLT improvements -- it would be good to make sure that project knows your use case as well - so they can weigh testing building their library this way to avoid other surprises with CMake conflicts. But that is also hard to test b/c it depends on the composed projects.

[white238]

This setting of default install structure was a very early decision in BLT, possibly even before BLT existed as a separate project.

Ideally we would have a BLT specific variable that we use inside of the blt_add_executable and blt_add_library functions and if it is not set just default to our current defaults and honor the CMake global variables.

But again this is a breaking change to lots of our projects and needs to be handled correctly. @selamberson , I am glad to hear you have a work around. Sorry to hear we caused you a headache.

We have a list of breaking changes I want to make here: #692

[gzagaris]

This setting of default install structure was a very early decision in BLT, possibly even before BLT existed as a separate project.

Yes -- I think it came from Axom. If I could time-travel, I would probably give myself a firm slap for that! 😆

Ideally we would have a BLT specific variable that we use inside of the blt_add_executable and blt_add_library functions and if it is not set just default to our current defaults and honor the CMake global variables.

Yes -- I was thinking along those lines and I was hoping that doing that would allow maintaining the current behavior without breaking anything while avoid modifying global variables, which could leak out to parent non-BLT projects. But, perhaps I am missing something