BIC-MNI / minc-toolkit-v2

Version 2 of the minc-toolkit, uses tools based on ITK version 4.x
GNU General Public License v3.0
49 stars 21 forks source link

minc-toolkit-config.sh accidentally adds current directory to empty profile variables #102

Closed ghost closed 4 years ago

ghost commented 4 years ago

There is a weird slowness when using the minc tools on a sshfs mounted directory.

$ cd tmp; time firefox
real    0m0,587s
user    0m0,097s
sys 0m0,016s

$ cd /mnt/sshfs/some_directory; time firefox
real    0m18,212s
user    0m0,111s
sys 0m0,020s

It turns out the issue is in the configurations settings, more specifically:

export LD_LIBRARY_PATH=${MINC_TOOLKIT}/lib:${MINC_TOOLKIT}/lib/InsightToolkit:${LD_LIBRARY_PATH}
echo ${LD_LIBRARY_PATH}
/opt/minc/1.9.16/lib:/opt/minc/1.9.16/lib/InsightToolkit:

The semi column at the end add the current working directory to the lookup paths.

To fix it:

export LD_LIBRARY_PATH=${MINC_TOOLKIT}/lib:${MINC_TOOLKIT}/lib/InsightToolkit${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}

The same issue for PATH, PERL5LIB and MANPATH.

gdevenyi commented 4 years ago

Quoting from the bash manual the source of the problem:

PATH The search path for commands. It is a colon-separated list of directories in which the shell looks for commands (see COMMAND EXECUTION below). A zero-length (null) directory name in the value of PATH indicates the current directory. A null directory name may appear as two adjacent colons, or as an initial or trailing colon. The default path is sys‐tem-dependent, and is set by the administrator who installs bash. A common value is ``/usr/gnu/bin:/usr/local/bin:/usr/ucb:/bin:/usr/bin''.

Need to fix https://github.com/BIC-MNI/minc-toolkit-v2/blob/master/minc-toolkit-config.unix.sh.cmake

Thanks for the report.

vfonov commented 4 years ago

technically, we don't need to setup LD_LIBRARY_PATH at all, since on MacOS it is very restrictive, all the binaries now should have properly set RPATHs to find the shared libraries.

gdevenyi commented 4 years ago

@vfonov indeed, I was thinking about that recently when I redid the ANTs build/install. Will test commenting out the unneeded variables and see if RPATH is properly working on Linux.

gdevenyi commented 4 years ago

Fixed.