Skip to content

Conversation

@dg0yt
Copy link
Contributor

@dg0yt dg0yt commented May 14, 2025

ITK_MSVC_STATIC_CRT was exported from installed ITKConfig.cmake, but
it wasn't available when building external modules (e.g. RTK) together
with ITK. This caused linking errors for tools. This PR is the minimal change to ensure consistent configuration.

ITK_MSVC_STATIC_CRT is used in CMake/ITKModuleExternal.cmake and in CMake/UseITK.cmake. The external RTK module includes the use file at
https://github.com/RTKConsortium/RTK/blob/d15c02af5d36098b5209e846c0585e70290fe6b7/applications/CMakeLists.txt#L32-L33
and started to use the wrong CRT from this point due to ITK_MSVC_STATIC_CRT being not set.

This issue was noticed in CI for the vcpkg port. The previous workaround was passing -DITK_MSVC_STATIC_CRT=ON to CMake. I don't expect this to be widely used.

PR Checklist

  • No API changes were made (or the changes have been approved)
  • No major design changes were made (or the changes have been approved)
  • Added test (or behavior not changed)
  • Updated API documentation (or API not changed)
  • Added license to new files (if any)
  • Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
  • Added ITK examples for all new major features (if any)

ITK_MSVC_STATIC_CRT was exported from installed ITKConfig.cmake, but
it wasn't available when building external modules (e.g. RTK) together
with ITK. This caused linking errors for tools.
@github-actions github-actions bot added type:Compiler Compiler support or related warnings type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots labels May 14, 2025
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thank you for contributing a pull request! 🙏

Welcome to the ITK community! 🤗👋☀️

We are glad you are here and appreciate your contribution. Please keep in mind our community participation guidelines. 📜
More support and guidance on the contribution process can be found in our contributing guide. 📖

This is an automatic message. Allow for time for the ITK community to be able to read the pull request and comment
on it.

Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

I would think that this change would have no effect.

@dzenanz dzenanz requested review from SimonRit, blowekamp and thewtex May 14, 2025 13:12
Copy link

@SimonRit SimonRit left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks!

Copy link
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

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

@dg0yt thank you!

@thewtex thewtex merged commit 8e9cb9d into InsightSoftwareConsortium:master May 14, 2025
16 checks passed
@dg0yt dg0yt deleted the msvc-crt branch May 14, 2025 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:Compiler Compiler support or related warnings type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants