Tribits snapshot

[gsjaardema]

No description provided.

[bartlettroscoe]

Hello @gsjaardema, is this PR pretty well validated for the TriBITS update?

[gsjaardema]

@bartlettroscoe Yes, it is ready to go at any time...

[bartlettroscoe]

@gsjaardema, regarding the GTest-related configure failure shown here showing:

Processing enabled TPL: GTest (enabled explicitly, disable with -DTPL_ENABLE_GTest=OFF)
-- Using FIND_PACKAGE(GTest ...) ...
-- Looking for pthread.h
-- Looking for pthread.h - found
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Failed
-- Looking for pthread_create in pthreads
-- Looking for pthread_create in pthreads - not found
-- Looking for pthread_create in pthread
-- Looking for pthread_create in pthread - found
-- Found Threads: TRUE  
-- GTest_LIBRARY_NAMES='gtest'
-- TPL_GTest_LIBRARIES='/home/runner/environments/gnu-V110-4.9.0-4.3.0/lib/libgtest.so.1.11.0'
-- TPL_GTest_INCLUDE_DIRS='/home/runner/environments/gnu-V110-4.9.0-4.3.0/include'
CMake Error at build/external_packages/GTest/GTestConfig.cmake:10 (add_library):
  add_library cannot create imported target "GTest::gtest" because another
  target with the same name already exists.
Call Stack (most recent call first):
  cmake/tribits/core/package_arch/TribitsTplFindIncludeDirsAndLibraries.cmake:700 (include)
  cmake/TPLs/FindTPLGTest.cmake:82 (tribits_tpl_find_include_dirs_and_libraries)
  cmake/tribits/core/package_arch/TribitsProcessEnabledTpl.cmake:99 (include)
  cmake/tribits/core/package_arch/TribitsGlobalMacros.cmake:1445 (tribits_process_enabled_tpl)

the problem is the the file cmake/TPLs/FindTPLGTest.cmake and the lines:

seacas/cmake/TPLs/FindTPLGTest.cmake

Lines 66 to 85 in 809cb29

	tribits_tpl_allow_pre_find_package(GTest GTest_ALLOW_PREFIND)
	if (GTest_ALLOW_PREFIND)
	message("-- Using FIND_PACKAGE(GTest ...) ...")
	find_package(GTest CONFIG)
	if (GTest_FOUND)
	get_target_property(GTest_INCLUDE_DIRS GTest::gtest INTERFACE_INCLUDE_DIRECTORIES)
	get_target_property(GTest_CONFIGURATION GTest::gtest IMPORTED_CONFIGURATIONS)
	get_target_property(GTest_LIBRARIES GTest::gtest IMPORTED_LOCATION_${GTest_CONFIGURATION})
	# Tell TriBITS that we found GTest and there no need to look any further!
	set(TPL_GTest_INCLUDE_DIRS ${GTest_INCLUDE_DIRS} CACHE PATH "...")
	set(TPL_GTest_LIBRARIES ${GTest_LIBRARIES} CACHE FILEPATH "...")
	set(TPL_GTest_LIBRARY_DIRS ${GTest_LIBRARY_DIRS} CACHE PATH "...")
	endif()
	endif()
	# Third, call `tribits_tpl_find_include_dirs_and_libraries()`
	tribits_tpl_find_include_dirs_and_libraries( GTest
	REQUIRED_HEADERS ${REQUIRED_HEADERS}
	REQUIRED_LIBS_NAMES ${REQUIRED_LIBS_NAMES}
	)

This will result in a new GTest::gtest imported target getting created in the tribits_tpl_find_include_dirs_and_libraries( GTest ... ) call.

I have exactly the same situation with the new example TriBITS TPLs Tpl1, Tpl2, Tpl3 and Tpl4 shown here:

• https://github.com/TriBITSPub/TriBITS/tree/master/tribits/examples/tpls

I have prototyped how to handle this in:

• https://github.com/TriBITSPub/TriBITS/blob/master/tribits/examples/TribitsExampleProject2/cmake/tpls/FindTPLTpl1.cmake

Let me refactor that code into a reusable TriBITS function tribits_external_package_create_imported_all_libs_target_and_package_config_wrapper_file() and updated instructions at:

• https://tribitspub.github.io/TriBITS/users_guide/index.html#how-to-use-find-package-for-a-tribits-tpl

Then can you review the updated instructions and example and provide feedback?

Another question, are you okay with always having your FindTPLGTest.cmake file call find_package(GTest) instead of allowing the user manually to specify the vars TPL_GTest_INCLUDE_DIRS and TPL _GTest_LIBRARY_DIRS? If so, I think we can avoid needing to call the functions tribits_tpl_allow_pre_find_package() and tribits_tpl_find_include_dirs_and_libraries() at all and can instead compress these types of FindTPL<tplName>.cmake files down to two lines. For example, for GTest, FindTPLGTest.cmake would be:

find_package(GTest REQUIRED)
tribits_external_package_create_imported_all_libs_target_and_package_config_wrapper_file(GTest
  GTest::gtest)

That might break backward compatibility for users that are currently setting TPL_GTest_INCLUDE_DIRS and TPL _GTest_LIBRARY_DIRS but it would make files like FindTPLGTest.cmake much simpler and easier to maintain.

[gsjaardema]

@bartlettroscoe For the GTest case, I am fine with not allowing users to specify TPL_GTest_INCLUDE_DIRS and TPL_GTest_LIBRARY_DIRS

[bartlettroscoe]

@gsjaardema, I know why the last TriBITS snapshot broke this TriBITS FindTPLGTest.cmake code. The issue is that the prior TriBITS implementation of tribits_tpl_find_include_dirs_and_libraries() was extracting the name gtest.so.1.11 to use for the IMPORTED target GTest::gtest.so.1.11 generated from the library file name libgtest.so.1.11.0. The recent TriBITS commit TriBITSPub/TriBITS@b019512 (rightly) changed this logic to instead extract the name gtest to use for the IMPORTED target GTest::gtest. But since the find_package(GTest) command is already creating the target GTest::gtest, hence the conflict.

I will provide a smooth way to fix this as per above.

[bartlettroscoe]

@gsjaardema, please see the PR #322 I just posted against this topic branch tribits-snapshot and the comment about what needs to be done before we can merge PR #322 to this branch.

[bartlettroscoe]

Hello @gsjaardema, now that I have merged TriBITSPub/TriBITS#493, can you please update the TriBITS snaphshot in this branch and then merge PR #322 to this topic branch? That should fix the GTest issue discussed above.

[gsjaardema]

@bartlettroscoe Is this PR then ready to merge, or should I still hold off.

[bartlettroscoe]

@bartlettroscoe Is this PR then ready to merge, or should I still hold off.

@gsjaardema, you should wait until after I merge trilinos/Trilinos#10614. Otherwise, your seacas 'master' branch will be incompatible with the Trilinos 'develop' branch.