Bad test in compile_MET_all.sh?

[maddenp-cu]

Software Version

12.2.1

Text

Am I wrong in thinking that this test can never be false? Was maybe

if [[ -x ${MET_PYTHON}/bin/python3 ]]; then

intended?

[ChangkeunJ]

You're right, it can never be false. Inside [[ ]] a lone argument is a string test, equivalent to -n, and [[ ]] doesn't word split either, so the expansion is always exactly one word. Even with MET_PYTHON unset the word is /bin/python3, which is non-empty, so the branch always runs.

-x is the right test given the next line executes the thing. I'd quote it and guard the unset case as well, otherwise on a machine that has a system /bin/python3 you get a true when MET_PYTHON was never set at all, and then report the wrong interpreter's version:

if [[ -n "${MET_PYTHON}" && -x "${MET_PYTHON}/bin/python3" ]]; then

The blast radius on this particular one is small, since the block only echoes and runs --version, so today's failure mode is a stray "No such file or directory" in the build log rather than a broken build. Line 902 is the one I'd look at next for the same reason:

export MET_PYTHON_BIN_EXE=${MET_PYTHON_BIN_EXE:=${MET_PYTHON}/bin/python3}

Same unchecked path, but taken as a default that gets used much later, so if it's wrong you find out a long way from the cause.

[jprestop]

Thank you, @maddenp-cu and @ChangkeunJ, for your feedback and suggestions. I have created an issue for this work: Improve validation and handling of MET_PYTHON and Python executable paths in compile_MET_all.sh #3341. Please feel free to post any comments on that issue with any further discussion. Thank you for your interest in METplus and for helping to improve our compilation tools.