Skip to content

remove default separate_build_dir = True from openkim-models#23317

Merged
Micket merged 1 commit intoeasybuilders:developfrom
branfosj:20250704102849_new_pr_openkim-models20210811
Jul 9, 2025
Merged

remove default separate_build_dir = True from openkim-models#23317
Micket merged 1 commit intoeasybuilders:developfrom
branfosj:20250704102849_new_pr_openkim-models20210811

Conversation

@branfosj
Copy link
Copy Markdown
Member

@branfosj branfosj commented Jul 4, 2025

(created using eb --new-pr)

@branfosj
Copy link
Copy Markdown
Member Author

branfosj commented Jul 4, 2025

This fails for me - with or without the change in this PR

CMake Error at model-drivers/DUNN__MD_292677547454_000/CMakeLists.txt:40 (project):
  The CMAKE_CXX_COMPILER:

    /dev/shm/branfosj/tmp-up-EL8/eb-3f7_hj8x/tmpxgx_teh5/rpath_wrappers/gxx_wrapper/g++

  is not a full path to an existing compiler tool.

  Tell CMake where to find the compiler by setting either the environment
  variable "CXX" or the CMake cache entry CMAKE_CXX_COMPILER to the full path
  to the compiler, or to the compiler name if it is in the PATH.

Repeated for the gcc and gfortran.

@boegel boegel added this to the release after 5.1.1 milestone Jul 4, 2025
@Micket
Copy link
Copy Markdown
Contributor

Micket commented Jul 5, 2025

Maybe due to rpath being enabled now? (and it wasn't when we we merged this the first time)

@Micket
Copy link
Copy Markdown
Contributor

Micket commented Jul 9, 2025

I dug a bit. The problems are not with this change, it's due to this build using the old wrapper paths from the kim-api installations (that of course no longer exist)

When building openkim-* i added some tracing, and found the culprit.

/apps/Test/software/kim-api/2.4.1-GCC-13.3.0/share/cmake/kim-api-items/kim-api-items-macros.cmake(167):  set(CMAKE_CXX_COMPILER /dev/shm/eb-m99nh4sf/tmpux6bv1g0/rpath_wrappers/gxx_wrapper/g++ )

I tried to find a way to be able to enforce this from the command line, making it impossible for scripts to fuck it up like this, and the only way i think it can be done is via a toolchain file combined with https://stackoverflow.com/questions/61499646/cmake-set-variable-readonly-protect-from-override this technique to rewrite the variables back to the correct value whenever they are modified.

# eb-shell> cat enforce-compilers.cmake 
macro(set_readonly VAR)
  # Set the variable itself
  set("${VAR}" "${ARGN}")
  # Store the variable's value for restore it upon modifications.
  set("_${VAR}_readonly_val" "${ARGN}")
  # Register a watcher for a variable
  variable_watch("${VAR}" readonly_guard)
endmacro()

# Watcher for a variable which emulates readonly property.
macro(readonly_guard VAR access value current_list_file stack)
  if ("${access}" STREQUAL "MODIFIED_ACCESS")
    message(WARNING "Attempt to change readonly variable '${VAR}'!")
    # Restore a value of the variable to the initial one.
    set(${VAR} "${_${VAR}_readonly_val}")
  endif()
endmacro()

set_readonly(CMAKE_C_COMPILER "/dev/shm/eb-qjo2wf97/tmpuovb52j4/rpath_wrappers/gcc_wrapper/gcc")
set_readonly(CMAKE_CXX_COMPILER "/dev/shm/eb-qjo2wf97/tmpuovb52j4/rpath_wrappers/gxx_wrapper/g++")
set_readonly(CMAKE_Fortran_COMPILER "/dev/shm/eb-qjo2wf97/tmpuovb52j4/rpath_wrappers/gfortran_wrapper/gfortran")

combined with

cmake -DCMAKE_TOOLCHAIN_FILE=enforce-compilers.cmake ...

does the trick. We could consider always doing this. Generating enforce-compilers.cmake is easy enough.

Copy link
Copy Markdown
Contributor

@Micket Micket left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm (I've tested locally with a newer version to confirm that this change is unrelated to the compiler problem, which stems from kim-api and is a pretty deep issue with how cmakemake easyblock works, so I can't even fix it in this PR)

@Micket Micket merged commit 996cb6f into easybuilders:develop Jul 9, 2025
8 checks passed
@branfosj branfosj deleted the 20250704102849_new_pr_openkim-models20210811 branch July 18, 2025 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants