Ralf Wildenhues wrote: > * Jim Meyering wrote on Wed, Nov 25, 2009 at 02:49:59PM CET: >> I've pushed tests/init.sh, as yet unused. > > I haven't looked at this in detail, due to time constraints,
Hi Ralf, Thanks for the feedback. FYI, the mkdtemp-related code has been in gnulib for some time, Even if I've been the only user of it via parted's tests, I know it has gotten some decent exposure. > so just a couple of notes: Automake has a similar file tests/defs.in > that is not as elaborate; still, you might be able to profit from it. > For example, turning on VERBOSE if srcdir is not set but derived from > $0 is very handy: it typically causes manual > $ ../../source/tests/foo.test In tests/defs.in the only use of VERBOSE is to unset it. I'll look further tomorrow. > invocations to produce verbose output. > > Then, there are a couple of patches pending for that file, that we still > need to rework to be portable enough, but a proposed run_command > function to catch output looks quite helpful. > >> +# Run the user-overridable cleanup_ function, remove the temporary >> +# directory and exit with the incoming value of $?. >> +remove_tmp_() >> +{ >> + __st=$? > > Have you checked whether using this function in a trap 0 correctly > catches $? with FreeBSD sh? I'm thinking of (autoconf.info): > > The shell in FreeBSD 4.0 has the following bug: `$?' is reset to 0 > by empty lines if the code is inside `trap'. Thanks, but that code has been in coreutils' test-lib.sh for a long time without reports of trouble, so I'm not worried. And besides, since FreeBSD 8.0 was just released, I won't lose sleep over portability problems in 4.0. >> + cleanup_ >> + # cd out of the directory we're about to remove >> + cd "$initial_cwd_" || cd / || cd /tmp > > That 'cd /' looks dangerous to me. Why not simply abort if you can't go > where you expect to? I always get nervous with something like this > before a 'rm -rf'. Yes, this might be worth adjusting. The combination of a test failing to return to $initial_cwd_ and a mistake that set $test_dir_ ... not likely at all, but not impossible, either. But then you can argue that a misbehaving test could simply set $initial_cwd_=/. Not sure, after all... >> + chmod -R u+rwx "$test_dir_" >> + # If removal fails and exit status was to be 0, then change it to 1. >> + rm -rf "$test_dir_" || { test $__st = 0 && __st=1; } >> + exit $__st >> +} > >> +# Create a temporary directory, much like mktemp -d does. >> +# Written by Jim Meyering. > > Why not reuse the code snippet from `info Autoconf --index mktemp'? Partly because that relies on $RANDOM, which is not portable. Also because I wanted an XXXX... template of user-selectable length. I required that this function work safely also when creating a directory in /tmp. With just a PID, it's too subject to DoS abuse.