[Einstein Toolkit] #832: ExternalLibraries/zlib gives bad error message if "patch" is not available
#832: ExternalLibraries/zlib gives bad error message if "patch" is not available -----------------------------------+---------------------------------------- Reporter: hinder | Owner: Type: defect | Status: new Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Keywords: | -----------------------------------+---------------------------------------- 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.
#832: ExternalLibraries/zlib gives bad error message if "patch" is not available ------------------------------------+--------------------------------------- Reporter: hinder | Owner: Type: defect | Status: new Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------
Comment (by eschnett):
I suggest using ${PATCH?}.
#832: ExternalLibraries/zlib gives bad error message if "patch" is not available ------------------------------------+--------------------------------------- Reporter: hinder | Owner: Type: defect | Status: review Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+--------------------------------------- Changes (by eschnett):
* status: new => review
#832: ExternalLibraries/zlib gives bad error message if "patch" is not available ------------------------------------+--------------------------------------- Reporter: hinder | Owner: Type: defect | Status: reviewed_ok Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+--------------------------------------- Changes (by eschnett):
* status: review => reviewed_ok
#832: ExternalLibraries/zlib gives bad error message if "patch" is not available ------------------------------------+--------------------------------------- Reporter: hinder | Owner: Type: defect | Status: reviewed_ok Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------
Comment (by rhaas):
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)?
#832: ExternalLibraries/zlib gives bad error message if "patch" is not available ------------------------------------+--------------------------------------- Reporter: hinder | Owner: Type: defect | Status: reviewed_ok Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------
Comment (by hinder):
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.
#832: ExternalLibraries/zlib gives bad error message if "patch" is not available ------------------------------------+--------------------------------------- Reporter: hinder | Owner: Type: defect | Status: reviewed_ok Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------
Comment (by eschnett):
"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.
#832: ExternalLibraries/zlib gives bad error message if "patch" is not available ------------------------------------+--------------------------------------- Reporter: hinder | Owner: Type: defect | Status: reviewed_ok Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------
Comment (by hinder):
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.
#832: ExternalLibraries/zlib gives bad error message if "patch" is not available ------------------------------------+--------------------------------------- Reporter: hinder | Owner: Type: defect | Status: reviewed_ok Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------
Comment (by eschnett):
I expect that most configuration variables are optional, and thus explicitly setting them to empty if they are unset would be inconvenient.
#832: ExternalLibraries/zlib gives bad error message if "patch" is not available ------------------------------------+--------------------------------------- Reporter: hinder | Owner: Type: defect | Status: closed Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: fixed | Keywords: ------------------------------------+--------------------------------------- Changes (by eschnett):
* status: reviewed_ok => closed * resolution: => fixed
Comment:
Using ${PATCH?} and ${TAR?} in all configuration scripts.
trac@lists.einsteintoolkit.org
-
Einstein Toolkit