ExternalLibraries/zlib gives bad error message if "patch" is not available

Create issue
Issue #832 closed
Ian Hinder created an issue

The PATCH variable is used in the zlib configuration script but there is no check that it has been set. It is also not declared in the configuration.ccl as being used. Should all environment variables used in the script be declared? PATCH is usually set by autoconf, unless it is unavailable, in which case it is not set.

Replacing $PATCH with ${PATCH?} would be enough to give a sensible error message. I don't know if there are versions of bash that would not understand this.

Keyword:

Comments (10)

  1. Roland Haas
    • removed comment

    The same ${PATCH} vs ${PATCH?} issue exists in eg. LORENE as well as for ${TAR} (most likely in all ExternalLibraries). Cactus' configure tests for PATCH and TAR. Would the build system abort with a useful error message if the zlib had declared PATCH (and TAR) in its OPTIONS (that would be rather neat)?

  2. Ian Hinder reporter
    • removed comment

    Why don't we just use

    set -u

    in all the configuration scripts? This gives a fatal runtime error if a variable is used that has not been assigned a value. It is mostly equivalent to using ${var?} syntax for all variables.

  3. Erik Schnetter
    • removed comment

    "set -u" is probably too rigorous, because many people will leave e.g. HDF5_LIB_DIRS undefined if they want it to be empty.

    We could set PATCH, TAR, etc. to "false" in configure. This way they will always be defined, and will return an error when they are used.

  4. Ian Hinder reporter
    • removed comment

    If it is valid for a variable to be undefined, then we can test explicitly for that in the configure script and reset it to empty if it is not defined. I think it is better to have the configure scripts robust.

  5. Erik Schnetter
    • removed comment

    I expect that most configuration variables are optional, and thus explicitly setting them to empty if they are unset would be inconvenient.

  6. Log in to comment