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

Fix logic for checking value of exported enable variable (#516, #63) #551

Open
bartlettroscoe opened this issue Dec 12, 2022 · 2 comments
Open

Comments

@bartlettroscoe
Copy link
Member

Description

When I implemented the logic to export package enable vars in <Package>Config.cmake files in #516, I put in the function tribits_assert_cache_and_local_vars_same_value(<varName>) which checks that the cache var is the same value as the local var. But that is not the correct logic because it is fine to hide the cache enable var with a project-level local var which the enable/disable logic does in TriBITS.

As a result, we are getting configure errors with Trilinos like:

CMake Error at TriBITS/tribits/core/utils/MessageWrapper.cmake:77 (message):
  ERROR: The cache variable TpetraTSQR_ENABLE_CUBLAS with the cache var value
  '' is not the same value as the local variable TpetraTSQR_ENABLE_CUBLAS
  with value 'OFF'!
Call Stack (most recent call first):
  TriBITS/tribits/core/package_arch/TribitsPkgExportCacheVars.cmake:92 (message_wrapper)
  TriBITS/tribits/core/package_arch/TribitsPkgExportCacheVars.cmake:125 (tribits_assert_cache_and_local_vars_same_value)
  TriBITS/tribits/core/package_arch/TribitsWriteClientExportFiles.cmake:469 (tribits_pkg_append_set_commands_for_exported_vars)
  TriBITS/tribits/core/package_arch/TribitsWriteClientExportFiles.cmake:263 (tribits_append_dependent_package_config_file_includes_and_enables)
  TriBITS/tribits/core/package_arch/TribitsWriteClientExportFiles.cmake:201 (tribits_generate_package_config_file_for_build_tree)
  TriBITS/tribits/core/package_arch/TribitsWriteClientExportFiles.cmake:599 (tribits_write_flexible_package_client_export_files)
  TriBITS/tribits/core/package_arch/TribitsPackageMacros.cmake:656 (tribits_write_package_client_export_files)
  TriBITS/tribits/core/package_arch/TribitsSubPackageMacros.cmake:162 (tribits_package_postprocess_common)
  packages/tpetra/tsqr/CMakeLists.txt:49 (TRIBITS_SUBPACKAGE_POSTPROCESS)

Proposed solution

The correct solution is to check that the local var value is the same as the project-level local var value. But the problem is that it is not clear how to do that from another scope (like multiple add_directory() and function() calls deep).

How to check this?

@bartlettroscoe bartlettroscoe created this issue from a note in TriBITS Refactor (In Progress) Dec 12, 2022
@bartlettroscoe bartlettroscoe changed the title Fix logic for checking value of exported enable variable Fix logic for checking value of exported enable variable (#516, #63) Dec 12, 2022
@bartlettroscoe
Copy link
Member Author

bartlettroscoe commented Dec 12, 2022

One way to address this is to change tribits_assert_cache_and_local_vars_same_value() to tribits_assert_dual_scope_local_vars_same_value(). That will catch if the current scope has the same value as the parent scope. But is that possible? You can set the value of a variable in the direct parent scope, but can you read the value of a variable in a direct parent scope?

For now, I will just comment out that check and see if that fixes the problem for now.

Longer term, I will need find a way to assert this. I think that can be done by setting up a data-structure that remembers what enable vars are exported and then checks them once we get back to the base-level project scope.

bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Dec 13, 2022
Turns out this is the wrong assert to do.  For now just commenting out
tribits_assert_cache_and_local_vars_same_value() and will decide how to fix
this for real later.
@bartlettroscoe
Copy link
Member Author

I am going to put this back into the backlog for now. Adding this check will be important for catching mistakes in the future when we start to build and install more TriBITS packages independently.

@bartlettroscoe bartlettroscoe moved this from In Progress to ToDo in TriBITS Refactor Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ToDo
Development

No branches or pull requests

1 participant