SyneRBI / SIRF

Main repository for the CCP SynerBI software
http://www.ccpsynerbi.ac.uk
Other
59 stars 29 forks source link

remove need for SIRF_PATH and SIRF_INSTALL_PATH env variables #1235

Open KrisThielemans opened 7 months ago

KrisThielemans commented 7 months ago

We still use the SIRF_PATH env variable explicitly in 2 places in the C++ tests where we should use examples_data_path() instead https://github.com/SyneRBI/SIRF/blob/5ec1fac1a7f8f0d6e836b797fb078b66fc493b9a/src/xSTIR/cSTIR/tests/test3.cpp#L51-L52 https://github.com/SyneRBI/SIRF/blob/5ec1fac1a7f8f0d6e836b797fb078b66fc493b9a/src/xSTIR/cSTIR/tests/test5.cpp#L50-L54 (It is also used in the obsolete matlab code, so let's ignore that).

In fact, we should not have to rely on the STIR_INSTALL_PATH env variable either. This should be resolved by making a .h.in file setting something like

const char * const SIRF_INSTALL_PATH = "@CMAKE_INSTALL_PREFIX@";
const char * const SIRF_INSTALLED_EXAMPLE_DATA_PATH = "@SIRF_EXAMPLE_DATA_PATH";

where the latter is set in CMake, rewriting the following slightly: https://github.com/SyneRBI/SIRF/blob/6fc60a4b81e67f38f8a4343563cc509b0b8ec19e/CMakeLists.txt#L205 https://github.com/SyneRBI/SIRF/blob/6fc60a4b81e67f38f8a4343563cc509b0b8ec19e/CMakeLists.txt#L228-L230

After that, examples_data_path() should be rewritten to only use the optional SIRF_DATA_PATH env variable and the new SIRF_INSTALLED_EXAMPLE_DATA_PATH.

Note that the current corresponding doc is not quite accurate. This paragraph should be removed I believe https://github.com/SyneRBI/SIRF/blob/5ec1fac1a7f8f0d6e836b797fb078b66fc493b9a/examples/README.md?plain=1#L41-L43