Skip to content

Update FindR to detect R.dll on windows.#2542

Merged
blowekamp merged 1 commit intoSimpleITK:mainfrom
blowekamp:findr_windows
Mar 26, 2026
Merged

Update FindR to detect R.dll on windows.#2542
blowekamp merged 1 commit intoSimpleITK:mainfrom
blowekamp:findr_windows

Conversation

@blowekamp
Copy link
Copy Markdown
Member

Improve finding R executables with registry keys. Fix finding and using the R.dll for linking with mingw on windows.

@blowekamp blowekamp force-pushed the findr_windows branch 2 times, most recently from 240cbcf to 9ca0d4b Compare March 26, 2026 16:59
@blowekamp blowekamp mentioned this pull request Mar 26, 2026
Improve finding R executables with registry keys. Fix finding and
using the R.dll for linking with mingw on windows.

Co-authored-by: Bradley Lowekamp <[email protected]>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the project’s FindR.cmake module to improve how R is discovered on Windows (via registry hints) and to better support MinGW/Rtools by allowing detection/linking against R.dll.

Changes:

  • Add Windows registry-based HINTS for locating R/Rscript.
  • Add MinGW-specific logic to allow find_library() to locate R.dll by temporarily extending CMAKE_FIND_LIBRARY_SUFFIXES.
  • Adjust which R variables are treated as required for find_package_handle_standard_args().
Comments suppressed due to low confidence (1)

CMake/FindR.cmake:154

  • LINUX is not a standard CMake platform variable and it’s not defined anywhere else in this repo. As written, if(NOT LINUX) will evaluate true on Linux too, making R_LIBRARIES/R_LIBRARY_BASE required even on Linux—contradicting the comment and likely breaking configurations where libR.so is absent. Use a reliable check like if(NOT CMAKE_SYSTEM_NAME STREQUAL "Linux") (or if(NOT (UNIX AND NOT APPLE))) instead.
if(NOT LINUX)
  # On Linux the libR.so is sometimes not available; on all other platforms
  # a link error results if the library is not found.
  list(
    APPEND
    _REQUIRED_R_VARIABLES
    R_LIBRARIES
    R_LIBRARY_BASE
  )
endif()

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread CMake/FindR.cmake
Comment thread CMake/FindR.cmake
DOC "R library (example libR.a, libR.dylib, R.dll, etc.)."
)

if(MINGW)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Possibly add a comment, just because this is a bit far in the code from the documentation above:
# Removing the .dll from list, see above.

@blowekamp blowekamp marked this pull request as ready for review March 26, 2026 20:34
@blowekamp blowekamp merged commit 8e1f9e3 into SimpleITK:main Mar 26, 2026
7 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants