Skip to content
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.

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

Changes to pre-downloaded external dependencies #2980

Open
Lestropie opened this issue Aug 30, 2024 · 1 comment
Open

Changes to pre-downloaded external dependencies #2980

Lestropie opened this issue Aug 30, 2024 · 1 comment
Assignees

Comments

@Lestropie
Copy link
Member

Lestropie commented Aug 30, 2024

From Reviewing #2979, but listing separately as it's not critical there.

  • For Eigen, ideally do a search across different zip / tarball extensions, so that it works regardless of which may have been downloaded; or alternatively require that the tarball have been pre-extracted in that directory (maybe more sensible given points below)

  • For JSON for Modern C++, just expect the .hpp file to be in that directory; their GitHub offers a direct download to that one file

  • Increment half dependency to 2.2.0, and as with JSON just check for the presence of that one file

  • For the NIfTI headers, I wouldn't demand that they be placed in their own "nifti/ sub-directory; just expect both files to be present in the specified directory.
    Edit: As per response, may be better if, rather than the current state where only the NIfTI headers get their own sub-directory, all third-party dependencies have their own individualised directory within that pre-downloaded & pre-extracted dependency directory, even if some may contain one file only.

@daljit46
Copy link
Member

I think the cleanest way to go about this would be to have separate directories for each dependency. This provides better encapsulation and allows for CMake targets to only include headers that they link against (e.g. if we place all headers in the same directory, then a target linking against half::half will have access to all the other headers too). Not really a big issue, but I believe it's less idiomatic than having separate directories per dependency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants