GEOS-DEV / GEOS-DEV/GEOS

CMake: `GEOSX_USE_${DEP}` can be erroneuosly altered due to TPL discovery

Open
#1,800 0 comments 0 reactions 1 assignee View on GitHub

@wrtobin is already working on this.

Since Mar 7, 2022.

type: cleanup / refactor
Dominant language
C++
Stars
287
Forks
109
Avg merge
4d 41m
Merged PRs (30d)
5

Description

Describe the bug
GEOSX_USE_${DEP} is set in GeosxConfig.hpp which is a cmake configured file, and each term in the file is set to true using the following snippet from src/cmake/GeosxConfig.cmake:

    if( ${DEP}_FOUND OR ENABLE_${DEP} OR GEOSX_ENABLE_${DEP} )
        set( USE_${DEP} TRUE )
        set( GEOSX_USE_${DEP} TRUE )
    endif()

This occurs after we configure our TPLs (necessarily), so it is possible for a CACHE variable set or overwritten by one of our TPL discovery calls can erroneously set ${DEP}_FOUND or ENABLE_${DEP} to differ from either their default or the value they are set to in a host-config file.

Practically this will probably only occur in packages where we haven't written our own find_package discovery scripts and are using instead the packages own packageConfig.cmake config.

In particular, on crusher we're currently building without openmp until HIP-offload is fully functional for simplicity, but depending on a TPL which sets OPENMP_FOUND erroneously causes GEOSX to attempt to configure with openmp active despite the host-config set(ENABLE_OPENMP FALSE BOOL CACHE "" FORCE ).

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.