nemequ/configure-cmake

Don't use bash arrays

nemequ opened this issue · 8 comments

FreeBSD apparently doesn't even have bash as an option by default (though it is available in ports), so it looks like I should figure out how to make csh/tcsh work.

Oops, didn't notice this before opening #5. You don't really have to support csh for FreeBSD - I don't think it's possible with a single script as csh has weird and completely incompatible syntax. Instead, you should just use #!/bin/sh and stick to POSIX shell features (e.g. no arrays, foo() {} instead function foo() {} etc.)

Arrays are the big one. As I noted in #5, I did actually try to get it working without arrays but the quoting was never right, and eventually I gave up because bash fulfills all my use cases. I'm happy to accept patches, but I don't plan on trying to get plain variables working again myself.

Some of the stuff that needs to work correctly:

./configure --prefix=/usr
./configure --prefix="/usr"
./configure --prefix="/usr" --libdir="/usr/lib64"
./configure --prefix="/usr" --libdir="/usr/lib64" CFLAGS="-Wall -Wextra -fsanitize=undefined"
./configure --prefix="/usr" --libdir="/usr/lib64" CFLAGS="-Wall -Wextra -fsanitize=undefined" CXXFLAGS="-Wall -Wextra -Wadd -DFOO=BAR"

Dropping the "function" keyword is trivial (TBH I didn't actually know that wasn't portable; AFAIK even dash supports it). I'll change the title to "Don't use bash arrays", since that's really the issue. If csh works that's great, but you're right, the real issue is POSIX-compliant shells.

Some of the stuff that needs to work correctly:

I don't see any problem with this. You get it as a single argument, you handle it as a single argument, (just don't forget to quote it everywhere).

I don't see any problem with this. You get it as a single argument, you handle it as a single argument, (just don't forget to quote it everywhere).

No you don't. You get them as multiple arguments, and have to concatenate them into a string which the shell will then treat as multiple arguments when you pass them to cmake. You don't want to pass values which may override per-project defaults unless they were actually provided to the configure script.

In theory it should be possible. I don't remember what the issues were (it's been a while), but no matter what I tried there was too much quoting or too little. I didn't need it to work with anything except for bash, so I just decided to use an array (which I find much more elegant anyways).

Oh, now I get it. It seems like you'll have to quote. Something like this (see also "Working with arrays" here):

quote() {
    echo "$1" | sed -e "s|'|'\\\\''|g; 1s/^/'/; \$s/\$/'/"
}

while [ $# != 0 ]; do
    case "$1" in
    --prefix=*)
        ARGS="$ARGS -DCMAKE_INSTALL_PREFIX=$(quote "${1#--prefix=}")"
        ;;
    --prefix)
        ARGS="$ARGS -DCMAKE_INSTALL_PREFIX=$(quote "$2")"
        ;;
    esac
    shift
done

eval "cmake $ARGS"

Also note a simpler way of cutting off parameter name without calling cut and prefix_to_offset. I've been told that this feature is POSIX compliant, but I'm not 100% sure. Though it's at least well supported by /bin/sh on FreeBSD. It is compliant, see here, §2.6.2.

Nice, thanks! I've changed the script to use that code (as well as removing a few other trivial bashisms, such as "source" instead of "."). I've only tested with dash, but AFAIK it should work everywhere…

It works for me, congrats! Couple more suggestions on code:

  • Everything passed to eval should be quoted. Eval interprets its arguments as a single string, so you actually have
eval "cmake ${TOP_SRCDIR} -DCMAKE_BUILD_TYPE=${BUILD_TYPE} -DCMAKE_INSTALL_PREFIX=${PREFIX} -DCMAKE_INSTALL_LIBDIR=${LIBDIR} ${CMAKE_ARGS}"

(and better to make it like that in code to make it apparent). You need to quote paths as well.

  • I've just realized that prefix cutting may be simplified even more: ${1#--prefix=} may be changed to `${1#*=}``.

Everything passed to eval should be quoted. Eval interprets its arguments as a single string, so you actually have

Done, thanks.

I've just realized that prefix cutting may be simplified even more: ${1#--prefix=} may be changed to ${1#*=}`.

Ah, nice. I already liked ${1#--foo=} a lot more than the old function, that is even better. Huge improvemets, thanks!

Closing this now, thanks for all your help. I really appreciate it.