ManiVaultStudio / core

A Flexible and Extensible Visual Analytics Framework for High-Dimensional Data
https://www.manivault.studio/
GNU Lesser General Public License v3.0
9 stars 1 forks source link

The install directory should be exposed in CMAKE #144

Closed thoellt closed 1 year ago

thoellt commented 3 years ago

Relying on the environment variable for the HDPS_INSTALL_DIR is prone to issues (e.g. setting it in the shell does not expose it to GUI applications (like the CMAKE GUI or Xcode) on macOS).

I would propose to generally get rid of the global $ENV variable, but at least, if it is not set CMAKE should allow to manually set the folder. (My understanding is that the $ENV variable is piped to a CMAKE path anyways and only evaluated once)

thoellt commented 3 years ago

I would propose to change

# Normalize the incoming install path
file(TO_CMAKE_PATH $ENV{HDPS_INSTALL_DIR} INSTALL_DIR)

to

# Get the env path and allow the user to change it
set(INSTALL_DIR_PATH $ENV{HDPS_INSTALL_DIR} CACHE PATH "The HDPS Install Directoy")
# Normalize the incoming install path
file(TO_CMAKE_PATH ${INSTALL_DIR_PATH} INSTALL_DIR)

and consistently only use ${INSTALL_DIR} instead of $ENV{HDPS_INSTALL_DIR} after that line. This will make the path editable by the user but has no impact on users who have the HDPS_INSTALL_DIR in their environment.

alxvth commented 1 year ago

Fixed with #329