Skip to content

COMP: Fix ExternalData when ITK is built with FetchContent #5678

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.

Sign up for GitHub

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

Jump to bottom
Merged
blowekamp merged 1 commit into from
Dec 10, 2025
Merged

COMP: Fix ExternalData when ITK is built with FetchContent #5678

blowekamp merged 1 commit into from
Dec 10, 2025
+3 −2

Conversation

@blowekamp
Copy link
Member

@blowekamp blowekamp commented Dec 9, 2025

Addresses two issue:

  • When ITK is built from FetchContent, the fetched source is outside of the main source. This is an error with external data paths, so use the ITK PROJECT source path as the default for ITK's External data.
  • Use PROJECT_BINARY_DIR for output of download data, to prevent target conflics with the main project.

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)

Refer to the ITK Software Guide for
further development details if necessary.

@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 Dec 9, 2025
dzenanz
dzenanz reviewed Dec 9, 2025
View reviewed changes
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.

Is the problem motivating this change encountered in SimpleITK superbuild, or someplace else?

@blowekamp
Copy link
Member Author

blowekamp commented Dec 9, 2025

Is the problem motivating this change encountered in SimpleITK superbuild, or someplace else?

I am experimenting with using CMake FetchData instead of an ExternalProject with SimpleITK.

@blowekamp
COMP: Fix ExternalData when ITK is built with FetchContent
10d63b1 
Addresses two issue:
- When ITK is built from FetchContent, the fetched source is outside of
the main source. This is an error with external data paths, so use the
ITK PROJECT source path explicitly as the External data.
- Use PROJECT_BINARY_DIR for output of download data, to prevent
target conflics with the main project.
@blowekamp blowekamp force-pushed the externaldata_project branch from 4196df9 to 10d63b1 Compare December 9, 2025 22:08
@blowekamp blowekamp marked this pull request as ready for review December 10, 2025 13:42
dzenanz
dzenanz approved these changes Dec 10, 2025
View reviewed changes
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.

Looks good on a glance.

thewtex
thewtex approved these changes Dec 10, 2025
View reviewed changes
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.

👍

@blowekamp blowekamp merged commit 92e815d into InsightSoftwareConsortium:main Dec 10, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thewtex thewtex thewtex approved these changes

@dzenanz dzenanz dzenanz approved these changes

@hjmjohnson hjmjohnson Awaiting requested review from hjmjohnson

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@blowekamp @thewtex @dzenanz