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

Export informational variables to <Package>Config.cmake files for downstream CMake projects #516

Closed
bartlettroscoe opened this issue Aug 17, 2022 · 2 comments

Comments

@bartlettroscoe
Copy link
Member

Parent Issue

Description

It is often the case that downstream CMake projects need to know how an upstream CMake project was configured and what features it supports. For example:

  • Albany wants to know if the installed Anasazi package was configured with RBGen or not (i.e. the value of Anasazi_ENABLE_RBGen). (needed to resolve an issue for Address failures from merge of updated TriBITS (#10614, TriBITSPub/TriBITS#299) trilinos/Trilinos#10774).

  • Once Trilinos packages are made to be built and installed independently, then downstream Trilinos packages will need to know a lot about upstream Trilinos packages. For example, downstream packages like MueLu need to know a lot about how upstream package Tpetra was configured (e.g. what scalar types it supports and has explicit template instantiations for).

Proposed solution

The proposed solution is to provide an easy mechanism for a TriBITS packages to make sure that the values of select cache vars are set in the generated <Package>Config.cmake file. For example, TriBITS could provide the function:

tribits_pkg_export_cache_var(<cacheVarName>)

such that when called, the variable <cacheVarName> and its value would get written into the <Package>Config.cmake files for the package and its subpackages (if the package has subpackages). There would be code in tribits_pkg_export_cache_var() that asserted that the cache variable <cacheVarName> existing and it was indeed a cache var (so that internal downstream packages can already see the value).

Then many of the standard TriBITS variables like tribits_add_option_and_define(<userOptionName> <macroDefineName> ...) would call tribits_pkg_export_cache_var(<userOptionName>) and tribits_pkg_export_cache_var(<macroDefineName>) (when those functions are called with a package scope). Other examples of standard TriBITS option-setting functions that would call tribits_pkg_export_cache_var() are tribits_add_debug_option(), ???.

And to avoid duplication, we could provide TriBITS functions tribits_pkg_set_option_and_export(<optionName> <defaultVal>) and tribits_pkg_set_and_export(<varName> <defaultVal> CACHE <type> "<doc>") and packages could refactor that currently calls option(<optionName> <defaultVal>) and set(<varName> <defaultVal> CACHE <type> "<doc>"), respectively.

NOTE: The precedent of exporting <Package>_ENABLE_<something> variables into <Package>Config.cmake files has already been done for the direct upstream dependencies of a TriBITS package to tell downstream CMake projects what upstream dependencies are enabled for a given packages. An example of this is in TribitsExampleApp2:

if (Package2_ENABLE_Tpl3)
set(package2_tpl3_deps_str ", ${tpl3}")
else()
set(package2_tpl3_deps_str "")
endif()
set(package2 "Package2{${package1}${package2_tpl3_deps_str}}")
if (Package3_ENABLE_Package2)
set(package3_package2_deps_str "${package2}, ")
else()
set(package3_package2_deps_str "")
endif()
if (Package3_ENABLE_Tpl4)
set(package3_tpl4_deps_str "${tpl4}, ")
else()
set(package3_tpl4_deps_str "")
endif()
set(package3
"Package3{${package3_package2_deps_str}${package1}, ${package3_tpl4_deps_str}${tpl2a}, ${tpl2b}}")

@bartlettroscoe bartlettroscoe self-assigned this Aug 17, 2022
@bartlettroscoe bartlettroscoe added this to Selected in TriBITS Refactor Aug 17, 2022
@bartlettroscoe bartlettroscoe moved this from Selected to In Progress in TriBITS Refactor Aug 19, 2022
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Aug 23, 2022
…#516)

This started failing for some reason and I have no idea why. This refactored
command is better anyway.
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Aug 23, 2022
This updates some existing tests for the export of selected package cache
vars, including vars from parent package in subpackage <Package>Config.cmake
files.
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Aug 23, 2022
…iBITSPub#516)

This provides a mechanism for packages to provide information about how they
were configured to downstream CMake projects.

Any cache vars set with tribits_add_option_and_define() are exported
automatically but TriBITS packages can call
tribits_pkg_export_cache_var(<cacheVarName>) to export any cache vars.

See updated documentation for details.
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Aug 23, 2022
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Aug 23, 2022
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Aug 23, 2022
Added macro tribits_assert_cache_and_local_vars_same_value(<cacheVarName>)
(with strong unit tests) to ensure that if any exported cache vars also has
local vars of the same name that they also have the same value.

This was done in response to review of PR TriBITSPub#520 from @KyleFromKitware.
bartlettroscoe added a commit that referenced this issue Aug 23, 2022
Add support for exporting package info vars into <Package>Config.cmake files (#516)
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Aug 24, 2022
…g, refactor (TriBITSPub#516)

In PR TriBITSPub#520, I accidentally put a requirement that the macro var name had to be
set.  Turns out, there is special logic for it to not be set.  The
documentation nor any TriBITS test hinted to this.  But the file
muelu/CMakeLists.txt actually calls tribits_add_option_and_define() with an
empty macro var name.

To catch this use case, I added unit tests (which will fail if you undo this
patch).

In order for the unit tests to cover what is needed, I had to factor out the
function tribits_pkg_export_cache_var() into its own module
TribitsPkgExportCacheVars.cmake so it could be included in the unit test
driver.  Then, I decided it would be a good idea to factor out a function
tribits_pkg_append_set_commands_for_exported_vars() that is called in the
module TribitsWriteClientExportFiles.cmake.  Lastly, I moved the function
tribits_assert_cache_and_local_vars_same_value() into this module as well.
This encapsulates all of the logic to export of these package cache vars into
a single module (with just a few dependencies).
bartlettroscoe added a commit that referenced this issue Aug 24, 2022
tribits_add_option_and_define(): restore optional macro define var arg, refactor (#516)

Should also fix trilinos/Trilinos#10925
@bartlettroscoe
Copy link
Member Author

With the merge of PRs #520 and #521, I think enough of this scope is done. I will defer the implementation of the macros tribits_pkg_set_option_and_export(<optionName> <defaultVal>) and tribits_pkg_set_and_export(<varName> <defaultVal> CACHE <type> "<doc>") until later, if they are needed.

I will put in review for now.

@bartlettroscoe bartlettroscoe moved this from In Progress to In Review in TriBITS Refactor Aug 24, 2022
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this issue Aug 24, 2022
…os#10774, TriBITSPub/TriBITS#516)

A TriBITS update is exporting package cache vars to the <Package>Config.cmake
file and you can't have a local var with the same name as a cache var with
different values.

In this case, it was just lucky that no downstream package was reading this
var (through the cache var) because they would have gotten the wrong value.
It seems that only code in CMakeLists.txt files under packages/pliris/ were
reading this var.
jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Aug 26, 2022
…s:develop' (8906842).

* trilinos-develop: (128 commits)
  Intrepid2: update TensorData.setFirstComponentExtentInDimension0 to modify extents_[0] (trilinos#10929)
  Tpetra: Adding configure option to disable Kokkos integration test
  Automatic snapshot commit from tribits at 142e5362
  Disable Pliris tests in ATS2 GenConfig builds (trilinos#10931)
  Force disable Pliris in ATS2 builds (trilinos#10931)
  Automatic snapshot commit from tribits at ab419429
  Change cmake_minimum_required() from 3.17.1 to 3.0 (TriBITSPub/TriBITS#522)
  Pliris: Remove local var hiding cache var Pliris_ENABLE_DREAL (trilinos#10774, TriBITSPub/TriBITS#516)
  Remove printing of vars that are now empty (TriBITSPub/TriBITS#299)
  Panzer: move periodic helper typedefs into namespace
  Revert incorrect fix in previous commit
  Fix typos in some docs
  fix scratch typos
  STK: Snapshot 08-22-22 12:44
  Phalanx: remove cuda compiler warnings and add test for new use case for vov
  changed a double to a scalar_type to compile for complex arith
  MueLu: Fix signed vs unsigned comparison in Aggregates_kokkos.cpp
  Amesos2 : trying to fix MKL header including issues
  MueLu: Add Aggregates_kokkos.ComputeNodesInAggregate
  Testing on Geminga: Do not disable Kokkos in Epetra build
  ...
bartlettroscoe added a commit that referenced this issue Aug 26, 2022
This is a safer way to check for an empty var I have found in the past.
Something strange happens with macro arguments (at least in older versions of
CMake).

This is in response to feedback from @KyleFromKitware while reviewing PR #521.
@bartlettroscoe
Copy link
Member Author

Irina Tezaur confirmed that this works for the Anasazi_ENABLE_RBGen case described above.

Closing as complete.

TriBITS Refactor automation moved this from In Review to Done Aug 26, 2022
jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Aug 27, 2022
…s:develop' (8906842).

* trilinos-develop: (130 commits)
  Intrepid2: update TensorData.setFirstComponentExtentInDimension0 to modify extents_[0] (trilinos#10929)
  Tpetra: Adding configure option to disable Kokkos integration test
  MueLu: Allow to print Kokkos config when default node type is used
  Automatic snapshot commit from tribits at 142e5362
  Disable Pliris tests in ATS2 GenConfig builds (trilinos#10931)
  Force disable Pliris in ATS2 builds (trilinos#10931)
  Automatic snapshot commit from tribits at ab419429
  Change cmake_minimum_required() from 3.17.1 to 3.0 (TriBITSPub/TriBITS#522)
  Pliris: Remove local var hiding cache var Pliris_ENABLE_DREAL (trilinos#10774, TriBITSPub/TriBITS#516)
  Remove printing of vars that are now empty (TriBITSPub/TriBITS#299)
  Panzer: move periodic helper typedefs into namespace
  Revert incorrect fix in previous commit
  Fix typos in some docs
  fix scratch typos
  STK: Snapshot 08-22-22 12:44
  Phalanx: remove cuda compiler warnings and add test for new use case for vov
  changed a double to a scalar_type to compile for complex arith
  MueLu: Fix signed vs unsigned comparison in Aggregates_kokkos.cpp
  Amesos2 : trying to fix MKL header including issues
  MueLu: Add Aggregates_kokkos.ComputeNodesInAggregate
  ...
jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Aug 28, 2022
…s:develop' (8906842).

* trilinos-develop: (130 commits)
  Intrepid2: update TensorData.setFirstComponentExtentInDimension0 to modify extents_[0] (trilinos#10929)
  Tpetra: Adding configure option to disable Kokkos integration test
  MueLu: Allow to print Kokkos config when default node type is used
  Automatic snapshot commit from tribits at 142e5362
  Disable Pliris tests in ATS2 GenConfig builds (trilinos#10931)
  Force disable Pliris in ATS2 builds (trilinos#10931)
  Automatic snapshot commit from tribits at ab419429
  Change cmake_minimum_required() from 3.17.1 to 3.0 (TriBITSPub/TriBITS#522)
  Pliris: Remove local var hiding cache var Pliris_ENABLE_DREAL (trilinos#10774, TriBITSPub/TriBITS#516)
  Remove printing of vars that are now empty (TriBITSPub/TriBITS#299)
  Panzer: move periodic helper typedefs into namespace
  Revert incorrect fix in previous commit
  Fix typos in some docs
  fix scratch typos
  STK: Snapshot 08-22-22 12:44
  Phalanx: remove cuda compiler warnings and add test for new use case for vov
  changed a double to a scalar_type to compile for complex arith
  MueLu: Fix signed vs unsigned comparison in Aggregates_kokkos.cpp
  Amesos2 : trying to fix MKL header including issues
  MueLu: Add Aggregates_kokkos.ComputeNodesInAggregate
  ...
jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Aug 29, 2022
…s:develop' (8906842).

* trilinos-develop: (130 commits)
  Intrepid2: update TensorData.setFirstComponentExtentInDimension0 to modify extents_[0] (trilinos#10929)
  Tpetra: Adding configure option to disable Kokkos integration test
  MueLu: Allow to print Kokkos config when default node type is used
  Automatic snapshot commit from tribits at 142e5362
  Disable Pliris tests in ATS2 GenConfig builds (trilinos#10931)
  Force disable Pliris in ATS2 builds (trilinos#10931)
  Automatic snapshot commit from tribits at ab419429
  Change cmake_minimum_required() from 3.17.1 to 3.0 (TriBITSPub/TriBITS#522)
  Pliris: Remove local var hiding cache var Pliris_ENABLE_DREAL (trilinos#10774, TriBITSPub/TriBITS#516)
  Remove printing of vars that are now empty (TriBITSPub/TriBITS#299)
  Panzer: move periodic helper typedefs into namespace
  Revert incorrect fix in previous commit
  Fix typos in some docs
  fix scratch typos
  STK: Snapshot 08-22-22 12:44
  Phalanx: remove cuda compiler warnings and add test for new use case for vov
  changed a double to a scalar_type to compile for complex arith
  MueLu: Fix signed vs unsigned comparison in Aggregates_kokkos.cpp
  Amesos2 : trying to fix MKL header including issues
  MueLu: Add Aggregates_kokkos.ComputeNodesInAggregate
  ...
jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Aug 29, 2022
…s:develop' (8906842).

* trilinos-develop: (130 commits)
  Intrepid2: update TensorData.setFirstComponentExtentInDimension0 to modify extents_[0] (trilinos#10929)
  Tpetra: Adding configure option to disable Kokkos integration test
  MueLu: Allow to print Kokkos config when default node type is used
  Automatic snapshot commit from tribits at 142e5362
  Disable Pliris tests in ATS2 GenConfig builds (trilinos#10931)
  Force disable Pliris in ATS2 builds (trilinos#10931)
  Automatic snapshot commit from tribits at ab419429
  Change cmake_minimum_required() from 3.17.1 to 3.0 (TriBITSPub/TriBITS#522)
  Pliris: Remove local var hiding cache var Pliris_ENABLE_DREAL (trilinos#10774, TriBITSPub/TriBITS#516)
  Remove printing of vars that are now empty (TriBITSPub/TriBITS#299)
  Panzer: move periodic helper typedefs into namespace
  Revert incorrect fix in previous commit
  Fix typos in some docs
  fix scratch typos
  STK: Snapshot 08-22-22 12:44
  Phalanx: remove cuda compiler warnings and add test for new use case for vov
  changed a double to a scalar_type to compile for complex arith
  MueLu: Fix signed vs unsigned comparison in Aggregates_kokkos.cpp
  Amesos2 : trying to fix MKL header including issues
  MueLu: Add Aggregates_kokkos.ComputeNodesInAggregate
  ...
jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Aug 29, 2022
…s:develop' (8906842).

* trilinos-develop: (130 commits)
  Intrepid2: update TensorData.setFirstComponentExtentInDimension0 to modify extents_[0] (trilinos#10929)
  Tpetra: Adding configure option to disable Kokkos integration test
  MueLu: Allow to print Kokkos config when default node type is used
  Automatic snapshot commit from tribits at 142e5362
  Disable Pliris tests in ATS2 GenConfig builds (trilinos#10931)
  Force disable Pliris in ATS2 builds (trilinos#10931)
  Automatic snapshot commit from tribits at ab419429
  Change cmake_minimum_required() from 3.17.1 to 3.0 (TriBITSPub/TriBITS#522)
  Pliris: Remove local var hiding cache var Pliris_ENABLE_DREAL (trilinos#10774, TriBITSPub/TriBITS#516)
  Remove printing of vars that are now empty (TriBITSPub/TriBITS#299)
  Panzer: move periodic helper typedefs into namespace
  Revert incorrect fix in previous commit
  Fix typos in some docs
  fix scratch typos
  STK: Snapshot 08-22-22 12:44
  Phalanx: remove cuda compiler warnings and add test for new use case for vov
  changed a double to a scalar_type to compile for complex arith
  MueLu: Fix signed vs unsigned comparison in Aggregates_kokkos.cpp
  Amesos2 : trying to fix MKL header including issues
  MueLu: Add Aggregates_kokkos.ComputeNodesInAggregate
  ...
jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Aug 29, 2022
…s:develop' (8906842).

* trilinos-develop: (130 commits)
  Intrepid2: update TensorData.setFirstComponentExtentInDimension0 to modify extents_[0] (trilinos#10929)
  Tpetra: Adding configure option to disable Kokkos integration test
  MueLu: Allow to print Kokkos config when default node type is used
  Automatic snapshot commit from tribits at 142e5362
  Disable Pliris tests in ATS2 GenConfig builds (trilinos#10931)
  Force disable Pliris in ATS2 builds (trilinos#10931)
  Automatic snapshot commit from tribits at ab419429
  Change cmake_minimum_required() from 3.17.1 to 3.0 (TriBITSPub/TriBITS#522)
  Pliris: Remove local var hiding cache var Pliris_ENABLE_DREAL (trilinos#10774, TriBITSPub/TriBITS#516)
  Remove printing of vars that are now empty (TriBITSPub/TriBITS#299)
  Panzer: move periodic helper typedefs into namespace
  Revert incorrect fix in previous commit
  Fix typos in some docs
  fix scratch typos
  STK: Snapshot 08-22-22 12:44
  Phalanx: remove cuda compiler warnings and add test for new use case for vov
  changed a double to a scalar_type to compile for complex arith
  MueLu: Fix signed vs unsigned comparison in Aggregates_kokkos.cpp
  Amesos2 : trying to fix MKL header including issues
  MueLu: Add Aggregates_kokkos.ComputeNodesInAggregate
  ...
jmgate pushed a commit to tcad-charon/Trilinos that referenced this issue Aug 29, 2022
…s:develop' (8906842).

* trilinos-develop: (130 commits)
  Intrepid2: update TensorData.setFirstComponentExtentInDimension0 to modify extents_[0] (trilinos#10929)
  Tpetra: Adding configure option to disable Kokkos integration test
  MueLu: Allow to print Kokkos config when default node type is used
  Automatic snapshot commit from tribits at 142e5362
  Disable Pliris tests in ATS2 GenConfig builds (trilinos#10931)
  Force disable Pliris in ATS2 builds (trilinos#10931)
  Automatic snapshot commit from tribits at ab419429
  Change cmake_minimum_required() from 3.17.1 to 3.0 (TriBITSPub/TriBITS#522)
  Pliris: Remove local var hiding cache var Pliris_ENABLE_DREAL (trilinos#10774, TriBITSPub/TriBITS#516)
  Remove printing of vars that are now empty (TriBITSPub/TriBITS#299)
  Panzer: move periodic helper typedefs into namespace
  Revert incorrect fix in previous commit
  Fix typos in some docs
  fix scratch typos
  STK: Snapshot 08-22-22 12:44
  Phalanx: remove cuda compiler warnings and add test for new use case for vov
  changed a double to a scalar_type to compile for complex arith
  MueLu: Fix signed vs unsigned comparison in Aggregates_kokkos.cpp
  Amesos2 : trying to fix MKL header including issues
  MueLu: Add Aggregates_kokkos.ComputeNodesInAggregate
  ...
cgcgcg pushed a commit to cgcgcg/Trilinos that referenced this issue Sep 12, 2022
…os#10774, TriBITSPub/TriBITS#516)

A TriBITS update is exporting package cache vars to the <Package>Config.cmake
file and you can't have a local var with the same name as a cache var with
different values.

In this case, it was just lucky that no downstream package was reading this
var (through the cache var) because they would have gotten the wrong value.
It seems that only code in CMakeLists.txt files under packages/pliris/ were
reading this var.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

1 participant