On 31 January 2015 at 00:45, Cristian Ionescu-Idbohrn <[email protected]> wrote: > On Fri, 30 Jan 2015, Lubomir I. Ivanov wrote: > > I'll just nitpick on this one: > >> diff --git a/scripts/write-version b/scripts/write-version >> new file mode 100644 >> index 0000000..42505ae >> --- /dev/null >> +++ b/scripts/write-version >> @@ -0,0 +1,61 @@ >> +#!/bin/sh > > Try this same thing with adding: > > set -eu > #set -x #xtrace > > Any of these may fail in absence of > > [ $# -eq 3 ] || { > echo "ERROR: missing argument(s)" # alternative, usage message > exit 1 > } >
i know of this, but it was problematic because $# seems buggy under MSYS, invoking from GNU-Make $# == 2, invoking from the cmd-line $# == 3. thus, the above error checking is non-portable and it simply fails. >> +TARGET=$1 >> +TARGET_TEMP=$TARGET.tmp >> +VERSION=$2 >> +OS=$3 >> + >> +# check if git is installed >> +GIT_ERROR=1 > > This is, AFAIK, not portable: > >> +HAS_GIT=`git --version 2> NUL` > ^^^ > I'd write this: > >> +if [ "$HAS_GIT" ]; then > > as: > > if HAS_GIT=$(git --version 2>/dev/null) && [ "$HAS_GIT" ]; then > > Backticks are considered deprecated. not according to POSIX of course, but i agree they are a deprecated in a way. for these and everything else you can send improvements with your patches on top of mine. ;-) (and i will let you know if such are going to be problematic with MSYS) and unless more people think that the patch and 'write-version' should be improved. lubomir -- _______________________________________________ subsurface mailing list [email protected] http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
