Hi Paul, On Oct 27, 2013, at 5:17 PM, Paul Eggert <egg...@cs.ucla.edu> wrote:
> I don't have time to review the new function library entirely, Oh, I wasn’t expecting a review at all, which is why I started by submitting funclib.sh, a standalone module. Otherwise we’d just be back to square one with the submissions being too large for review, and nothing being merged. > but I see some problems with the version-number comparison. > It assumes that, for example, "test 3n -lt 4n" will succeed, > but POSIX doesn't guarantee this, and I expect some 'test' > implementations will reject that usage. ...but, your feedback is no less welcome, of course :) > Instead of using 'cut' and 'test', how about just using sort? > That should be faster anyway. Something like this: > > printf '%s\n%s\n' "$1" "$2" | > sort -t. -k1n -k1 -k2n -k2 -k3n -k3 -k4n -k4 -k5n -k5 -k6n -k6 -k7n -k7 -k8n > -k8 -k9n -k9 > > I figure 9 ought to be enough, but of course one could write > the code to work no matter how many periods are in the version > numbers, without invoking any more processes than these two. That’s a great idea, thanks. I’ve already applied it upstream. > Also, this: > > newer_ver=$(func_sort_ver $prev_ver $ver |cut -d' ' -f2) > test "$newer_ver" != "$ver" && \ > die "invalid version: $ver (<= $prev_ver)" > > looks awkward. Instead, how about having > a function func_lt_ver that succeeds if its first > argument is less than the second? That way, one can > simply write: > > func_lt_ver "$prev_ver" "$ver" || > die "invalid version: $ver (<= $prev_ver)" > > which is simpler and is more robust in the presence of > oddities like spaces in version numbers. Agreed. Also applied. On balance, I picked a poor time to submit useful bits of libtool to gnulib, since I’m starting to get feedback on the prerelease which is producing some churn in the funclib.sh and others. I’ll wait for the testing to finish, and then resubmit what I have then. Cheers, — Gary V. Vaughan (gary AT gnu DOT org)
signature.asc
Description: Message signed with OpenPGP using GPGMail
