Problem 2221 - With `set -u` set, the launch script fails
Summary: With `set -u` set, the launch script fails
Status: CLOSED FIXED
Alias: None
Product: Geant4
Classification: Unclassified
Component: cmake (show other problems)
Version: 10.6
Hardware: All Linux
: P4 minor
Assignee: Ben Morgan
URL:
Depends on:
Blocks:
 
Reported: 2020-01-29 16:37 CET by Michael Reilly
Modified: 2020-03-09 18:31 CET (History)
0 users

See Also:


Attachments
Patch to fix the described issue. (1.07 KB, patch)
2020-01-29 16:37 CET, Michael Reilly
Details | Diff
Corrected patch to fix (1.07 KB, patch)
2020-02-02 17:46 CET, Michael Reilly
Details | Diff

Note You need to log in before you can comment on or make changes to this problem.
Description Michael Reilly 2020-01-29 16:37:42 CET
Created attachment 603 [details]
Patch to fix the described issue.

Some administration policies and distributions ensure that `set -u` is set by default.  Unfortunately, this causes the following shell script code (and similar constructs elsewhere in the file) to fail:

`if test "x$LD_LIBRARY_PATH" = "x"; then`

if the variable in question isn't defined, (which is precisely what this command is testing for).

The attached patch modifies these lines to be:

`if [ -z "${LD_LIBRARY_PATH+}" ]; then`

Which is (as far as I can determine) the proper way of testing if a variable is either undefined or empty.  This variable expansion should even be available under the Bourne Shell, if my research is correct, but I have not tested specifically this, as on my distribution (and most others) /bin/sh is just a symlink to bash.

The quotes are unnecessary in this case, but I thought it made the intent a little more clear.
Comment 1 Michael Reilly 2020-02-02 17:44:51 CET
This patch is erroneous.  The constructs ${FOO+} should be ${FOO-}, please forgive me for being bad at shell scripting.
Comment 2 Michael Reilly 2020-02-02 17:46:09 CET
Created attachment 606 [details]
Corrected patch to fix
Comment 3 Ben Morgan 2020-02-04 11:58:00 CET
Thanks for the patch (and correction)! I've tested and applied these on Gitlab, so after testing they should go into the upcoming 10.6.1 patch release. I'll mark as RESOLVED FIXED once that's complete.
Comment 4 Michael Reilly 2020-02-17 17:03:42 CET
Thanks for the merge.  I've tested 10.6.1, and it's working great!
Comment 5 Michael Reilly 2020-03-09 18:13:55 CET
Just a friendly reminder, this bug is probably good to close.  I can't mark it as RESOLVE FIXED.  I can only do RESOLVED, or else I would have done it myself.
Comment 6 Ben Morgan 2020-03-09 18:28:26 CET
Thanks! Markin as such.