bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] tests/init.sh: new file to be used via most *.sh tests


From: Ralf Wildenhues
Subject: Re: [PATCH] tests/init.sh: new file to be used via most *.sh tests
Date: Fri, 27 Nov 2009 21:32:14 +0100
User-agent: Mutt/1.5.20 (2009-08-09)

Hi Jim,

* 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,
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

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'.

          $ trap 'false

          echo $?' 0
          $ exit
          0

     Fortunately, this bug only affects `trap'.


> +  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'.

> +  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'?

Cheers,
Ralf




reply via email to

[Prev in Thread] Current Thread [Next in Thread]