Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TBBConfig.cmake should not require both the Debug and the Release libraries to be installed #113

Open
ldionne opened this Issue Dec 20, 2018 · 3 comments

Comments

Projects
None yet
3 participants
@ldionne
Copy link

ldionne commented Dec 20, 2018

Hi,

TBBConfig.cmake currently says:

foreach (_tbb_component ${TBB_FIND_COMPONENTS})
    set(_tbb_release_lib "${_tbb_lib_path}/lib${_tbb_component}.dylib")
    set(_tbb_debug_lib "${_tbb_lib_path}/lib${_tbb_component}_debug.dylib")

    if (EXISTS "${_tbb_release_lib}" AND EXISTS "${_tbb_debug_lib}")
        add_library(TBB::${_tbb_component} SHARED IMPORTED)
        ...
    elseif (TBB_FIND_REQUIRED AND TBB_FIND_REQUIRED_${_tbb_component})
        message(FATAL_ERROR "Missed required Intel TBB component: ${_tbb_component}")
    endif()
endforeach()

This is a problem when only the release library (or only the debug library) are found, as the configure script will error out. Instead, I believe the configure script should try to find as many configurations of the library as possible and simply populate those. It could error out if no configurations at all are found:

foreach (_tbb_component ${TBB_FIND_COMPONENTS})
    set(_tbb_release_lib "${_tbb_lib_path}/lib${_tbb_component}.dylib")
    set(_tbb_debug_lib "${_tbb_lib_path}/lib${_tbb_component}_debug.dylib")

    if (EXISTS "${_tbb_release_lib}")
        set(_imported_location_release IMPORTED_LOCATION_RELEASE "${_tbb_release_lib}")
        list(APPEND _imported_configurations "RELEASE")
    elseif(EXISTS "${_tbb_debug_lib}")
        set(_imported_location_debug IMPORTED_LOCATION_DEBUG "${_tbb_debug_lib}")
        list(APPEND _imported_configurations "DEBUG")
    elseif (TBB_FIND_REQUIRED AND TBB_FIND_REQUIRED_${_tbb_component})
        message(FATAL_ERROR "Missed required Intel TBB component: ${_tbb_component}")
    endif()

    add_library(TBB::${_tbb_component} SHARED IMPORTED)
    set_target_properties(TBB::${_tbb_component} PROPERTIES
                          IMPORTED_CONFIGURATIONS ${_imported_configurations}
                          ${_imported_location_release}
                          ${_imported_location_debug}
                          INTERFACE_INCLUDE_DIRECTORIES "${_tbb_root}/include")

    ...
endforeach()

This wouldn't require both the release and the debug versions of TBB to be installed on a system for the script to work. For example, right now, finding TBB fails after installing it with Homebrew because only the release library is installed.

@hjmjohnson

This comment has been minimized.

Copy link

hjmjohnson commented Jan 19, 2019

NOTE: I have a proposed a stop-gap fix to homebrew to install both release and debug.
Homebrew/homebrew-core#36175

I 100% agree that both configurations should NOT be required.

@tbbdev

This comment has been minimized.

Copy link
Contributor

tbbdev commented Jan 21, 2019

@ldionne thank you for the proposal!
We'll implement it in one of the next TBB releases.

@tbbdev

This comment has been minimized.

Copy link
Contributor

tbbdev commented Mar 1, 2019

Fixed in TBB 2019 Update 4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.