search

MRtrix3 / mrtrix3

MRtrix3 provides a set of tools to perform various advanced diffusion MRI analyses, including constrained spherical deconvolution (CSD), probabilistic tractography, track-density imaging, and apparent fibre density
http://www.mrtrix.org
Mozilla Public License 2.0
281 stars 176 forks source link

New option to build non-core code as a static lib #2907

Closed daljit46 closed 2 weeks ago

daljit46 commented 1 month ago

As mentioned in the meeting today, it'd make sense for production builds to have the C++ code in src built as a static library. This PR adds a new MRTRIX_BUILD_STATIC option to allow for this (@MRtrix3/mrtrix3-devs if anyone has a better name please let me know). The option is disabled by default and is only intended to be enabled in production and external projects. For every day builds, making shared libraries provides a faster incremental build workflow.

github-actions[bot] commented 1 month ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 1 month ago

clang-tidy review says "All clean, LGTM! :+1:"

daljit46 commented 1 month ago

Regarding the name, it probably needs to be clear that it's not the entire build that's static, only that outside of core, especially given earlier builds have had full static build capability (though from memory it's been a little patchy at times).

Unfortunately, I don't have good ideas for the name (neither does ChatGPT). The best I could come up with is MRTRIX_BUILD_NON_CORE_STATIC.

Lestropie commented 1 month ago

If ChatGPT can't even come up with variable names, is it even fit for purpose? I mean, that's 98% of the work to be done. 🤡 No clear superior alternative to MRTRIX_BUILD_NON_CORE_STATIC from me. Will take the slight clumsiness for the sake of accuracy.

github-actions[bot] commented 1 month ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 1 month ago

clang-tidy review says "All clean, LGTM! :+1:"

daljit46 commented 2 weeks ago

@Lestropie can we merge this?