Hello!
On Tue, Sep 06, 2022 at 05:52:51PM +0400, Sergey Kandaurov wrote:
> # HG changeset patch
> # User Sergey Kandaurov <pluk...@nginx.com>
> # Date 1662472340 -14400
> # Tue Sep 06 17:52:20 2022 +0400
> # Node ID 7eb87eaf06f4b9161cfe298f195947dbddedd01d
> # Parent 78fe648d54a79822a72f3a5f8cc79651b3b1f5a7
> Tests: fixed try_run() to remove temporary directory on failure.
>
> Keeping the file handle pointing to the "stderr" file prevented removal
> of the temporary directory on win32 if execution aborted in run().
> The fix is to move safe file operations surrounding run() out of the
> eval block such that the state of file handles is consistent.
> Fixes a1874249496d.
I believe it might need some additional emphasis that this is
about Windows only. For example:
Tests: fixed try_run() to remove temporary directory on win32.
>
> diff -r 78fe648d54a7 -r 7eb87eaf06f4 lib/Test/Nginx.pm
> --- a/lib/Test/Nginx.pm Tue Aug 30 17:24:16 2022 +0400
> +++ b/lib/Test/Nginx.pm Tue Sep 06 17:52:20 2022 +0400
> @@ -290,8 +290,9 @@ sub has_daemon($) {
> sub try_run($$) {
> my ($self, $message) = @_;
>
> + open OLDERR, ">&", \*STDERR;
> +
> eval {
> - open OLDERR, ">&", \*STDERR;
> open NEWERR, ">", $self->{_testdir} . '/stderr'
> or die "Can't open stderr: $!";
> close STDERR;
> @@ -299,10 +300,10 @@ sub try_run($$) {
> close NEWERR;
>
> $self->run();
> + };
>
> - close STDERR;
> - open STDERR, ">&", \*OLDERR;
> - };
> + close STDERR;
> + open STDERR, ">&", \*OLDERR;
>
> return $self unless $@;
Wouldn't it be better to move all stderr handling out of the eval?
diff --git a/lib/Test/Nginx.pm b/lib/Test/Nginx.pm
--- a/lib/Test/Nginx.pm
+++ b/lib/Test/Nginx.pm
@@ -291,14 +291,13 @@ sub try_run($$) {
my ($self, $message) = @_;
open OLDERR, ">&", \*STDERR;
+ open NEWERR, ">", $self->{_testdir} . '/stderr'
+ or die "Can't open stderr: $!";
+ close STDERR;
+ open STDERR, ">&", \*NEWERR;
+ close NEWERR;
eval {
- open NEWERR, ">", $self->{_testdir} . '/stderr'
- or die "Can't open stderr: $!";
- close STDERR;
- open STDERR, ">&", \*NEWERR;
- close NEWERR;
-
$self->run();
};
Alternatively, it might be the right time to backout a1874249496d
completely and rely on "-e ..." instead, given that it was
introduced in 1.19.5 and already present in all supported
versions.
--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-le...@nginx.org
