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

Address FindTPL<tplName>.cmake inner find_package() calls finding TriBITS-installed <tplName>Config.camke files and add beginning of new examples '2' (#299, #427) #431

Merged

Conversation

bartlettroscoe
Copy link
Member

@bartlettroscoe bartlettroscoe commented Nov 12, 2021

This PR first adds a failing tests that basically reproduces the problems highlighted in bug report #427 in commit fb79c7f. The (what should be) super robust solution for this given in commits b4a6628 and b4a6628 and is is explained at the bottom of the TriBITS Users Guide section "How to use find_package() for a TriBITS TPL":

which is:

Therefore, to avoid having an installed TriBITS-generated HDF5Config.cmake
file, for example, being found by the inner call to find_package(HDF5 ...)
in the file FindTPLHDF5.cmake (which would be disastrous), TriBITS employs
two safeguards. First, TriBITS-generated <tplName>Config.cmake files are
placed into the build directory <buildDir>/external_packages/ and
installed into the directory <installDir>/lib/external_packages/ so they
will not be found by default when <buildDir>/cmake_packages and/or
<installDir>, respectively, are added to CMake_PREFIX_PATH. Also,
even if <installDir>/lib/external_packages or
<buildDir>/external_packages do get added to the search path somehow
(e.g. through CMAKE_INSTALL_PREFIX), the TriBITS framework will set the
variable TRIBITS_FINDING_RAW_<tplName>_PACKAGE_FIRST=TRUE before including
FindTPL<tplName>.cmake and there is special logic in the TriBITS-generated
<tplName>ConfigVersion.cmake file that will set
PACKAGE_VERSION_COMPATIBLE=OFF and result in find_package(<tplName>)
not selecting <tplName>Config.cmake. (It turns out that CMake's
find_package(<Package>) command always includes the file
<Package>ConfigVersion.cmake, even if no version information is passed to
the command find_package(<Package>). This allows special logic to be
placed in the file <Package>ConfigVersion.cmake to determine if
find_package(<Package>) will select a given <Package>Config.cmake file
that is in the search path based on a number of different criteria such as in
this case.)

Also in this PR:

  • 6ff0323: Provide a first example of a FindTPL<tplName>.cmake file that writes a custom <tplName>Config.cmake file that uses find_dependency() with <tplName>_DIR set to find upstream external non-TriBITS package.
  • 6f0e888: Added error message for find_pacakge(<Project> REQUIRED COMPONENTS <c0> <c1>) describing what packages were not present when finding a project's <Project>Config.cmake file with components and a test to ensure the right error message gets printed.
  • Created the beginnings of TribitsExampleProject2 and TribitsExampleApp2 that will allow deeper testing with dependent TPLs so that I can finish implementing the move to modern CMake targets and stripping the old complex TriBITS logic (Move to modern CMake target-based propagation of build information #299).

…Config.cmake (TriBITSPub#299, TriBITSPub#427)

This is one solution to the problem of finding <tplName>Config.cmake files in
the CMAKE_INSTALL_PREFIX directory in the middile of a FindTpl<tplName>.cmake
module.  If you have to write a FindTpl<tplName>.cmake module, you are not
expecting/wanting to find a TriBTIS-written <tplName>Config.cmake file.  See
the comment in TribitsProcessEnabledTpl.cmake for more details.

This should fix the bug reported in TriBITSPub#427 where the find_package(HDF5 ...) call
in FindTPLHDF5.cmake was finding HDF5Config.cmake in the install tree written
by TriBITS in a previous installation.

I am not sure this is the best solution to this problem but it has the
advantage that we keep the <Package>Config.cmake files for external package
and TriBITS package side-by-side.  I suspect that I will implement another
solution but at least this is a start to writing <tplName>ConfigVersion.cmake
files.
…riBITSPub#299, TriBITSPub#427)

This has two benefits:

* It makes a distinction between <Package>Config.cmake files that are unique
  and created by the CMake project (and are put in <buildDir>/cmake_packages/
  on installed under <installDir>/lib/cmake/) and those <tplName>Config.cmake
  files that are created by TriBITS to create standard targets needed to be
  used by downstream TriBITS projects.

* It avoids find_package() from accidentally finding these TriBITS-generated
  <tplName>Config.cmake files in all contexts when <buildDir>/cmake_packages
  and <installDir> are added to CMAKE_PREFIX_PATH.  This way, for example,
  downstream customer raw CMake projects will not accidentally find the
  TriBITS-generated HDF5Config.cmake when the Trilinos install dir is added to
  CMAKE_PREFIX_PATH.

NOTE: This change is not needed to make the test
TribitsExampleProject2_install_config_again pass due to commit:

  Write out <tplName>ConfigVersion.cmake and disallow finding <tplName>Config.cmake (TriBITSPub#299, TriBITSPub#427)

but this makes it even more robust to not find these <tplName>Config.cmake
files by accident.
This ensures that we keep the option to extract the include dirs and libraries
to show that this can be an option as well and to test the function
tribits_get_imported_location_property().
…ency() (TriBITSPub#299)

As part of this, I had to move the logic to write the default version file
<tplName>ConfigVersion.cmake to the calling function
tribits_process_enabled_tpl() and moved the addition of the install targets
there as well.  Therefore, those commands will get called reguardless if
tribits_tpl_find_include_dirs_and_libraries() gets called in the
FindTPL<tplName>.cmake module or not.

I also added a test for this case as well of course.

But note that there are not tests that actually use the
TplConfig[Version].cmake files yet.  That will come next.
This was from a copy and paste that was left over and should have been taken
out.
…BITSPub#299)

These should be super simple given the purpose of this project.
…ITSPub#299)

I also added error output for component find failure for <Project>Config.cmake
files and a test to make sure it gets printed.
…TriBITSPub#299, TriBITSPub#427)

This is the start for adding documentaiton for new <tplName>Config.cmake and
<tplName>ConfigVersion.cmake files.  It mentions a few issues that I felt were
important enough to mention to TriBITS Project Developers and TriBITS Project
Users.
@bartlettroscoe bartlettroscoe force-pushed the 299-427-find-package-install-dir branch from f67960a to 8fe2a49 Compare November 12, 2021 12:21
@bartlettroscoe bartlettroscoe changed the title WIP: Address TribitsTPL<tplName>.cmake inner find_package() calls finding TriBITS-installed <tplName>Config.camke files and add beginning of new examples '2' (#299, #427) WIP: Address FindTPL<tplName>.cmake inner find_package() calls finding TriBITS-installed <tplName>Config.camke files and add beginning of new examples '2' (#299, #427) Nov 12, 2021
@bartlettroscoe bartlettroscoe self-assigned this Nov 12, 2021
@bartlettroscoe
Copy link
Member Author

I will go ahead and do the merge and any reviews can take place post-merge. I need to address a known problem with MS Windows and then get this PR added to the TriBITS snapshot for trilinos/Trilinos#9894.

@bartlettroscoe bartlettroscoe changed the title WIP: Address FindTPL<tplName>.cmake inner find_package() calls finding TriBITS-installed <tplName>Config.camke files and add beginning of new examples '2' (#299, #427) Address FindTPL<tplName>.cmake inner find_package() calls finding TriBITS-installed <tplName>Config.camke files and add beginning of new examples '2' (#299, #427) Nov 12, 2021
@bartlettroscoe bartlettroscoe merged commit 3eebd91 into TriBITSPub:master Nov 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

1 participant