Re: 1.7.2 cycle will open soon

From: Jeff King <peff@peff.net>
Date: May 6, 2010, 06:54 UTC
Message-ID: <20100506065419.GA21009@coredump.intra.peff.net>
In-Reply-To: <20100506064428.GA29360@progeny.tock>

On Thu, May 06, 2010 at 01:44:28AM -0500, Jonathan Nieder wrote:

> > Am I missing something?
> 
> If cleanup fails, I want to catch it.  Would something like this do?

Ah, I see.

Show 12 quoted lines

>  test_run_ () {
> 	test_cleanup=:
> 	eval >&3 2>&4 "$1"
> 	eval_ret=$?
> 	eval >&3 2>&4 "$test_cleanup" && (exit "$eval_ret")
> 	eval_ret=$?
> 	return 0
>  }
> 
>  test_when_finished () {
> 	test_cleanup="$* && $test_cleanup"
>  }

Doesn't that violate your rule that the cleanup will _always_ be run? Here, if the first cleanup fails, we don't run subsequent ones.

Perhaps the simplest would be to just keep a cleanup_ret in the second eval (where you have eval_ret in the original patch), and then act appropriately after the eval finishes. That would be easier on the eyes, too, I think.

Show 8 quoted lines

> To permit a line break at the end of a cleanup command, one can do
> 
>  test_when_finished () {
> 	test_cleanup="{ $*
> 		} && $test_cleanup"
>  }
> 
> but that might not be worth the ugliness.

I doubt anyone will put a linebreak in, but it is probably better to be defensive, and the ugliness is at least contained only in that function.

-Peff