DOI-USGS / ISIS3

Integrated Software for Imagers and Spectrometers v3. ISIS3 is a digital image processing software package to manipulate imagery collected by current and past NASA and International planetary missions.
https://isis.astrogeology.usgs.gov
Other
195 stars 166 forks source link

isisStartup scripts fail outside of USGS systems #5478

Open rvwagner opened 4 months ago

rvwagner commented 4 months ago

ISIS version(s) affected: Every version since $ISISROOT/../data became ISISROOT/../isis_data (4.4.0, looks like)

Description

All of the $ISISROOT/isis/scripts/isis*Startup.* scripts check for the existence of the old data directory within $ISISROOT, correctly find that the old path does not exist, and fall back on a path that only exists on USGS internal systems. All subsequent attempts to do anything with the isis_data directory (e.g. spiceinit) will then fail if you don't work at the USGS.

The bad code (from the bash version, others are all similar):

if [ -d $ISISROOT/../data ]; then
    ISISDATA=$ISISROOT/../isis_data

These may be deprecated, I'm not sure, but they're convenient for initializing ISIS in a script. Which I do all the time, but it's only recently that public ISIS has gotten all of the LROC-specific stuff needed for me to try to move away from our custom build of 3.10.2, and thus find this bug.

How to reproduce

Start a terminal session with no ISIS variables set.

export ISISROOT=/some/path/to/an/ISIS/install/that/is/not/usgs/cpkgs/isis3/isis
source "$ISISROOT/scripts/isisStartup.sh"
echo "Should be '$ISISROOT/../isis_data'"
echo "Is actually '$ISISDATA'"

Solution

In all the isis*Startup.* scripts, change the path in the conditional to look for /isis_data instead of /data.

acpaquette commented 4 months ago

Hi @rvwagner most of us internally don't use the scripts much either since most of the astro dev team is remote. We have also largely switched to using isisVarInit.py for setting environment variables.

These scripts can be updated but will likely require some input as to what they should be doing. I think a fairly basic update would be doing as you suggest and change the data/testData folders to what they have been updated to. Outside of that, if you install ISIS through conda, I would strongly recommend using the isisVarInit.py script

rvwagner commented 4 months ago

Yeah, doesn't look like what that script produces will work well if you aren't activating ISIS via conda, which I almost never do on any system (even my personal systems- I like being able to use other conda environments in conjunction with ISIS), and can't do at all on the work systems I usually use the isis*Startup.* scripts on. The basic update I suggested of making the paths match between the test condition and current filesystem layout would at least bring these scripts back to the same level of functionality they had before 4.4.0.

And, if not, it's not that hard to put

export ISISDATA="${ISISROOT}/../isis_data"
export PATH="${PATH}:${ISISROOT}/bin"

in a script instead of . $ISISROOT/scripts/isisStartup.sh, but maybe then remove the broken isis*Startup.* scripts outright, since right now they're useless to apparently everyone?