Re: t/SMOKE on win32

2003-10-08 Thread Stas Bekman
Steve Hay wrote:
Randy Kobes wrote:

On Fri, 3 Oct 2003, Stas Bekman wrote:
 

If it is happening on the server side, may be it happens
due to a previous request not restoring the overriden STD
stream at the end of its run? We have the error checking,
but it may fail nevertheless?
  


It sounds like that might be happening, especially
since it's on a predictable subset of tests (eg,
apache/scanhdrs, but apache/constants is OK). I'll
try to put it through the debugger to see if that
sheds any light.
 

I'm not sure that it's related to the actions of previous requests 
because it happens with a single test which only makes one request.

I've been trying to debug what goes on when I run this:

   perl t/SMOKE modperl\post_utf8

This single test is one of those that fails with the "Failed to dup 
STDOUT" error.  I've put a break point on the line that fails and a 
pause near the start of the test script itself so that I can attach the 
debugger to Apache.exe when the test script starts up.  The one and only 
time that the breakpoint is visited is the time that it fails.  At this 
point the call stack is:

=
modperl_io_perlio_override_stdout(interpreter * 0x011fa0f4, request_rec 
* 0x03386160) line 137
modperl_response_handler_cgi(request_rec * 0x03386160) line 918 + 13 bytes
ap_run_handler(request_rec * 0x03386160) line 195 + 78 bytes
ap_invoke_handler(request_rec * 0x03386160) line 401 + 9 bytes
ap_process_request(request_rec * 0x03386160) line 288 + 9 bytes
ap_process_http_connection(conn_rec * 0x0084c650) line 293 + 9 bytes
ap_run_process_connection(conn_rec * 0x0084c650) line 85 + 78 bytes
ap_process_connection(conn_rec * 0x0084c650, void * 0x0084c588) line 213
worker_main(long 0) line 731
_threadstartex(void * 0x0028d2c8) line 212 + 13 bytes
KERNEL32! 77e7d33b()
=

I stepped into the Perl_do_open() call, and into Perl_do_openn().  
Around line 106 of Perl (5.8.1)'s doio.c we have:

   /* If currently open - close before we re-open */
   if (IoIFP(io)) {
   ...
   }
It may or may not be relevant to note that the above if-block was 
skipped over.  Does that imply that STDOUT is actually closed at this 
point after all?
No, 'io' is the handle that STDOUT is going to be dupped to, and as you tested 
with my last patch it doesn't make any difference if we explicitly close it.

Continuing walking through Perl_do_openn() we visit line 389 (with dodup 
set to 2):

   fp = PerlIO_fdupopen(aTHX_ that_fp, NULL, dodup);
looks good

[which returns via line 536 of perlio.c] and later on line 555:

   fd = PerlIO_fileno(fp);

which sets fd to 35 (!),
sounds quite possible

.  We then go to line 561:

   if (PerlLIO_fstat(fd,&PL_statbuf) < 0) {

and then it all goes wrong because sadly that PerlLIO_fstat() fails and 
we return FALSE via lines 563, 564, 697, etc.  The PerlLIO_fstat() call 
actually winds up being _fstati64() on line 500 of win32/win32sck.c, and 
returned -1.
I'd step through
  PerlIO_fdupopen(aTHX_ that_fp, NULL, dodup);
and see that the dup goes correctly. If stat fails that means that the above 
has failed. May be it needs a check?

Does any of this help anyone divine what is going wrong?

I also tried running the script with I/O tracing on.  Here's what I get 
in the error_log:

   modperl_io.c:99: start
   modperl_io_apache.c:57: done
   modperl_io_apache.c:32: mode r
   modperl_io_apache.c:71: did nothing
   modperl_io.c:118: end
   modperl_io.c:131: start
   Failed to dup STDOUT: Permission denied.
The first start ... end section is modperl_io_perlio_override_stdin(); 
then we start modperl_io_perlio_override_stdout() and croak() straight 
away.  Doesn't really tell me much either :-(

What else can I try?
Beside the suggestion above, I'd try to step though through a good do_openn 
called from modperl_io_perlio_override_stdout and compare with the failing one.



__
Stas BekmanJAm_pH --> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


[PATCH] Re: Problem building mp1 on Win32

2003-10-08 Thread Steve Hay
Randy Kobes wrote:

On Mon, 6 Oct 2003, Steve Hay wrote:

 

Hi,

During the course of testing mp1.29-rc1 I ran into a strange problem,
probably Windows-specific, which I've seen before but never got to the
bottom of.
I have Apache 1.3.28 installed in C:\apache and Perl 5.8.1 in C:\perl5.
The latter has all the paths in it's Config.pm pointing to C:\perl5
subdirs, and I have C:\perl5\bin on the PATH (and no other Perl
directories on the PATH).
I then extract the mp1 source archive into C:\Temp and run:

   perl Makefile.PL APACHE_SRC=C:/apache EAPI
   nmake
   nmake test
Normally this works fine, but occasionally I get errors from "nmake".
Under mp1.28 I sometimes get this at the start of the "msdev" command
run by "nmake":
   Compiling...
   Apache.c
   Apache.xs(1966) : error C2115: '=' : incompatible types
   [...]
   Error executing cl.exe.
   mod_perl.so - 1 error(s), 1 warning(s)
   NMAKE : fatal error U1077: 'msdev' : return code '0x1'
   

Hi Steve,
  This error is from an incompatibility with LARGEFILES
support - on about line 1966 of Apache.xs, there's a
  statcache = r->finfo
which is trying to relate two different things with
LARGEFILES support on Win32. This is fixed in the cvs.
Ah, right.  That makes sense, because the Perl in C:\perl5 does not have 
LFS while the one in C:\perl does.

And it being fixed in cvs (and therefore in 1.29-rc1) explains the 
different behaviour below.

 

while under mp1.29-rc1 I got this at the end of the "msdev" command:

   Linking...
  Creating library Release/mod_perl.lib and object Release/mod_perl.exp
   perl_util.obj : error LNK2001: unresolved external symbol
_Perl_croak_nocontext
   [...]
   Release/mod_perl.so : fatal error LNK1120: 42 unresolved externals
   Error executing link.exe.
   mod_perl.so - 180 error(s), 1 warning(s)
   NMAKE : fatal error U1077: 'msdev' : return code '0xb4'
I've finally tracked down what triggers this behaviour (it's the same
trigger in each case), but I can't explain why.  If I have another Perl
build installed in C:\perl then I get the above errors; if I haven't
then I don't!
I suspect that problem is also related to the fact that the other Perl
build that I sometimes have in C:\perl is a threaded Perl (a la
ActivePerl) whereas the one in C:\perl5 is non-threaded, hence the error
regarding "Perl_croak_nocontext" when the wrong Perl setup gets picked
up.  (Basically, I prefer the non-threaded Perl for my production work
with mp1, but I also have a threaded Perl kicking around for mp2 testing.)
Presumably something from the C:\perl installation is being used (in
preference to the C:\perl5 installation that should be being used) if it
is present.  Why does this happen?  All I have to do is move C:\perl out
of the way (say, to C:\perl.mp2) and it all works fine again.
Also, why is there a difference between 1.28 and 1.29-rc1 in the way in
which it breaks?  Is that related to this change in the ChangeLog?:
   For Win32, keep drive letters in mod_perl.dsp to fix bug, reported
   by DH <[EMAIL PROTECTED]>, when compiling mod_perl in
   cases where Apache and Perl are on different drives [Randy Kobes].
It's not an urgent problem for me since I have a workaround, but mp's
build process really shouldn't be picking up the "wrong" Perl.  All the
other CPAN modules I'm using (150+ !) build fine with the other Perl
sitting in C:\perl, so why shouldn't mod_perl?
   

It's something that should be fixed ... It sounds like
there's something askew in src\modules\win32\mod_perl.dsp;
could you send me (privately) this file, and also
src\modules\win32\mod_perl.dsp.orig? Thanks.
I think I've cracked it myself looking at those two files.

The original .dsp file has two lines like this:

   ADD CPP ... /I "\Perl\lib\CORE" ...

which get fixed up by Makefile.PL to this:

   ADD CPP ... /I "\Perl\lib\CORE" /I "C:\apache\include"  /I 
"C:\apache\include/../os/win32" /I "C:\perl5\lib\CORE" ...

Looks like that include path in the original ADD CPP lines should have 
been *changed* to "C:\perl5\lib\CORE", or at least have the correct path 
put *before* it -- it's no good having "\Perl\lib\CORE" and then 
"C:\perl5\lib\CORE" ;-)

The attached patch to Makefile.PL (against 1.29-rc1) fixes those lines 
up as this:

   ADD CPP ... /I "C:\apache\include"  /I 
"C:\apache\include/../os/win32" /I "C:\perl5\lib\CORE" ...

and I can now build mp1.29-rc1 with or without the extra C:\perl in place.

- Steve
--- Makefile.PL.orig2003-10-03 20:18:22.0 +0100
+++ Makefile.PL 2003-10-08 09:06:29.229298300 +0100
@@ -2229,7 +2229,7 @@
 }
 elsif (/ADD CPP/) {
   my $apache_inc = win32_fix_path_dsp($win32_path{APACHE_INC});
-  s!(/D "WIN32")!/I "$apache_inc"  /I "$apache_inc/../os/win32" /I "$perl_inc" 
$1!;
+  s!/I "\\Perl\\lib\\CORE"!/I "$apache_inc" /I "$apache_inc/../os/win32" /I 
"$perl_inc"!;
   s!(/D "WIN32")!$1 /D "EAPI" ! if $win32_args{EAPI}; 
   print NEWDSP $_;
 }

---

Re: t/SMOKE on win32

2003-10-08 Thread Steve Hay
Stas Bekman wrote:

Steve Hay wrote:

I've been trying to debug what goes on when I run this:

   perl t/SMOKE modperl\post_utf8

This single test is one of those that fails with the "Failed to dup 
STDOUT" error.
[...]


Continuing walking through Perl_do_openn() we visit line 389 (with 
dodup set to 2):

   fp = PerlIO_fdupopen(aTHX_ that_fp, NULL, dodup);


looks good

[which returns via line 536 of perlio.c] and later on line 555:

   fd = PerlIO_fileno(fp);

which sets fd to 35 (!),


sounds quite possible

.  We then go to line 561:

   if (PerlLIO_fstat(fd,&PL_statbuf) < 0) {

and then it all goes wrong because sadly that PerlLIO_fstat() fails 
and we return FALSE via lines 563, 564, 697, etc.  The 
PerlLIO_fstat() call actually winds up being _fstati64() on line 500 
of win32/win32sck.c, and returned -1.


I'd step through
  PerlIO_fdupopen(aTHX_ that_fp, NULL, dodup);
and see that the dup goes correctly. If stat fails that means that the 
above has failed. May be it needs a check? 
There's not very much to PerlIO_fdupopen():

=
PerlIO *
PerlIO_fdupopen(pTHX_ PerlIO *f, CLONE_PARAMS *param, int flags)
{
   if (PerlIOValid(f)) {
   PerlIO_funcs *tab = PerlIOBase(f)->tab;
   PerlIO_debug("fdupopen f=%p param=%p\n",(void*)f,(void*)param);
   if (tab && tab->Dup)
return (*tab->Dup)(aTHX_ PerlIO_allocate(aTHX), f, param, flags);
   else {
return PerlIOBase_dup(aTHX_ PerlIO_allocate(aTHX), f, param, 
flags);
   }
   }
   else
SETERRNO(EBADF, SS_IVCHAN);

   return NULL;
}
=
PerlIOValid(f) is true, and so is (tab && tab->Dup).  The 
PerlIO_allocate() returns f when i reaches 36; f is then some non-NULL 
value, param is 0x (is that alright or not?) and flags is 2.

We then call (*tab->Dup)(...), which is PerlIOBuf_dup(...), which calls 
PerlIOBase_dup(...).  We goto line 2170 of perlio.c in that function, 
which is another call to (*tab->Dup)(...), which is PerlIOUnix_dup(...) 
this time.

Line 2457 of perlio.c sets fd to 1 (which is STDOUT, right?)  flags is 
still 2, so (flags & PERLIO_DUP_FD) is true and we call PerlLIO_dup() on 
line 2459.  That calls win32_dup(), which calls dup(), which I can't 
step into ;-).  Each of dup(), win32_dup() and PerlLIO_dup() returns 35.

Then we call PerlIOBase_dup() at line 2462.  f and o are both non-NULL, 
param is 0x and flags is still 2.  This call skips over the 
PerlIOValid(nexto) test on line 2167.  self->Getarg is 0x at 
line 2180.

PerlIO_modestr() returns "wb", which is passed to PerlIO_push at line 
2185.  The (l && f) test on line 1158 is true.  (*l->tab->Pushed) is 
PerlIOUnix_pushed() which calls PerlIOBase_pushed().  tab->Set_ptrcnt is 
NULL at line 1965.  The 'w' and 'b' chars are handled correctly and we 
return 0.  Back on line 2366 in PerlIOUnix_pushed() now.  The 
*PerlIONext(f) test is false and we return 0.  Back on line 1166 in 
PerlIO_push() now.  The "if" test there is therefore false and we return 
f (which is non-NULL).

This f also gets returned from PerlIOBase_dup(), so back in 
PerlIOUnix_dup() we call PerlIOUnix_setfd() at line 2465 which looks 
good. f is non-NULL, fd is 35, os->oflags is 0.  The PerlLIO_fstat(35, 
&st) call at line 2348 is ultimately a call to _fstati64(35, ...) at 
win32/win32sck.c line 500 which returns -1.  I wasn't expecting that to 
have failed (_fstati64() returns 0 for success, -1 for failure) :-s (***)

Carrying on anyway, PerlIOUnix_dup() returns (non-NULL) f at line 2466.  
Back in PerlIOBase_dup(), f is true at line 2174.  self->Getarg is 
0x at line 2180.  This time PerlIO_modestr() returns "w", which 
is passed to PerlIO_push at line 2185.  Again the (l && f) test on line 
1158 is true.  Now (*l->tab->Pushed) is PerlIOCrlf_pushed() which calls 
PerlIOBuf_pushed().

PerlIO_fileno() ultimately calls PerlIOUnix_fileno(), which returns 35.  
The (fd >= 0 && PerlLIO_isatty(fd)) test is false; *PerlIONext(f) is true.

PerlIO_tell() ultimately calls _lseeki64(), which returns 29.  Is this 
just the number of bytes written to STDOUT at this point?  I assume 
that's OK.

Now we call PerlIOBase_pushed() (line 3418) again.  This time 
tab->Set_ptrcnt is PerlIOCrlf_set_ptrcnt at line 1965.  The 'w' char is 
handled correctly and we return 0, which PerlIOBuf_pushed() also returns.

The while loop at the end of PerlIOCrlf_pushed was only run through 
once, with b->tab == &PerlIO_unix.  I think that's OK - if I follow it 
correctly we have a unix layer already, and we've just pushed a crlf 
layer.  The while loop was looking for any crlf layers below what we've 
just pushed, and correctly didn't find any.

So we return 0 and the "if" test on line 1166 is false, and we return f 
(non-NULL) from PerlIO_push(), and from PerlIOBase_dup(), and from 
PerlIOBuf_dup(), and from PerlIO_fdupopen().

This sets fp to a non-NULL value at line 389 of doio.c.  Then as we saw 
before fd is 35 at line 555 and the PerlLIO_fstat() at line 561 returns 
-1 (failure) just like it did 

Re: [PATCH] add PERL5LIB to @INC in the correct order

2003-10-08 Thread Simon Flack
On Mon, 06 Oct 2003 15:37:12 -0400, Geoffrey Young wrote
> > Should we try to squeeze it into 1.29? If so do we need another release 
> > candidate? Since we need 1.29 out asap, because of the bug in 5.8.1, I 
> > suggest that we release 1.29 as it is now. Then hopefully we will 
> > release 1.30 some time soon (including the PERL5LIB fix). I know there 
> > are quite a few patches that Philippe has planned to put in and we want 
> > them to sit in the cvs for some time to get people time to test it out 
> > well.
> 
> my advice would be to get 1.29 out soon, as it will be the first 
> stable version of mod_perl specifically for 5.8.1.
> 
> release 1.30 can come with those patches, and be released at a much 
> more leisurely pace
> 
> --Geoff

I'm happy with that. There's also another problem with PERL5LIB that I've 
noticed, but not fixed because there's an easy workaround:

According to perlrun(1), architechture subfolders are added to @INC as well:

PERL5LIB

A colon-separated list of directories in which to look for
Perl library files before looking in the standard library
and the current directory. Any architecture-specific
directories under the specified locations are automatically
included if they exist.

For example on my dev box, setting PERL5LIB to '/home/simonflk/perllib' 
should add '/home/simonflk/perllib/i686-linux' if it exists.

The simple and obvious workaround is to add that to PERL5LIB. But it would be 
nice for it to behave as documented in perl. If the other fix to PERL5LIB is 
scheduled for v1.30, it makes sense to fix that for the same release. When I 
get some time I'll send in a patch.

Thanks

--simonflk

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: t/SMOKE on win32

2003-10-08 Thread Steve Hay
Steve Hay wrote:

So presumably it's that failure above, marked (***), which is where 
it's all gone wrong.  Here's a stack trace showing the path to 
my_stat() which calls _fstati64():

=
my_fstat(int 35, _stati64 * 0x02f3fb98) line 532
win32_fstat(int 35, _stati64 * 0x02f3fb98) line 2706 + 13 bytes
PerlLIOFileStat(IPerlLIO * 0x0141becc, int 35, _stati64 * 0x02f3fb98) 
line 974 + 13 bytes
PerlIOUnix_setfd(interpreter * 0x01b2004c, _PerlIO * * 0x01b20ee4, int 
35, int 0) line 2348 + 30 bytes
PerlIOUnix_dup(interpreter * 0x01b2004c, _PerlIO * * 0x01b20ee4, 
_PerlIO * * 0x0108ec8c, clone_params * 0x, int 2) line 2465 + 
24 bytes
PerlIOBase_dup(interpreter * 0x01b2004c, _PerlIO * * 0x01b20ee4, 
_PerlIO * * 0x01b20e5c, clone_params * 0x, int 2) line 2170 + 
26 bytes
PerlIOBuf_dup(interpreter * 0x01b2004c, _PerlIO * * 0x01b20ee4, 
_PerlIO * * 0x01b20e5c, clone_params * 0x, int 2) line 3871 + 
25 bytes
PerlIO_fdupopen(interpreter * 0x01b2004c, _PerlIO * * 0x01b20e5c, 
clone_params * 0x, int 2) line 536 + 35 bytes
Perl_do_openn(interpreter * 0x01b2004c, gv * 0x01fb0334, char * 
0x01ac46dc, long 8, int 0, int 1, int 0, _PerlIO * * 0x, sv * 
* 0x, long 0) line 389 + 19 bytes
Perl_do_open(interpreter * 0x01b2004c, gv * 0x01fb0334, char * 
0x1002c664 `string', long 8, int 0, int 1, int 0, _PerlIO * * 
0x) line 60 + 41 bytes
modperl_io_perlio_override_stdout(interpreter * 0x01b2004c, 
request_rec * 0x03386160) line 137 + 28 bytes
modperl_response_handler_cgi(request_rec * 0x03386160) line 918 + 13 
bytes
ap_run_handler(request_rec * 0x03386160) line 195 + 78 bytes
ap_invoke_handler(request_rec * 0x03386160) line 401 + 9 bytes
ap_process_request(request_rec * 0x03386160) line 288 + 9 bytes
ap_process_http_connection(conn_rec * 0x0084c650) line 293 + 9 bytes
ap_run_process_connection(conn_rec * 0x0084c650) line 85 + 78 bytes
ap_process_connection(conn_rec * 0x0084c650, void * 0x0084c588) line 213
worker_main(long 19) line 731
_threadstartex(void * 0x0028def0) line 212 + 13 bytes
KERNEL32! 77e7d33b()
=
I was just trying out your other suggestion of walking through a 
successful modperl_io_perlio_override_stdout() call, so I tried 
debugging a randomly chosen test that succeeds - api/lookup_uri - and I 
noticed that modperl_io_perlio_override_stdout() doesn't get run.  
Comparing the call stack to the above I have this from api/lookup_uri run:

=
[...]
modperl_response_handler(request_rec * 0x03386160) line 879 + 11 bytes
ap_run_handler(request_rec * 0x03386160) line 195 + 78 bytes
ap_invoke_handler(request_rec * 0x03386160) line 401 + 9 bytes
ap_process_request(request_rec * 0x03386160) line 288 + 9 bytes
ap_process_http_connection(conn_rec * 0x0084c650) line 293 + 9 bytes
ap_run_process_connection(conn_rec * 0x0084c650) line 85 + 78 bytes
ap_process_connection(conn_rec * 0x0084c650, void * 0x0084c588) line 213
worker_main(long 19) line 731
_threadstartex(void * 0x0028def0) line 212 + 13 bytes
KERNEL32! 77e7d33b()
=
i.e. the test that works (api/lookup_uri) calls 
modperl_response_handler() which calls modperl_response_handler_run() 
with little else first, whereas the test that fails (modperl\post_utf8) 
calls modperl_response_handler_cgi() which calls 
modperl_response_handler_run() after much more mucking around, including 
calls to modperl_io_override_stdin() and modperl_io_override_stdout().

This makes sense after reading

   http://perl.apache.org/docs/2.0/user/config/config.html#C_modperl_
   http://perl.apache.org/docs/2.0/user/config/config.html#C_perl_script_
and sure enough the various tests that fail, e.g.

   apache\scanhdrs
   api\rflush
   modperl\post_utf8
   modules\cgi
specify "SetHandler perl-script" in their .pm files, while the test 
above that succeeded (api\lookup_ui) specifies "SetHandler modperl" in 
its .pm file.

So it looks like rather than us having the redirection stuff oddly 
working for some tests and not for others, what we actually have is a 
case of it never working!  The tests that work only do so because they 
use the "modperl" response handler which skips the tie()'ing of 
STDIN/STDOUT.

- Steve

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: modperl_default_port_handler commit broke mp2's build

2003-10-08 Thread Geoffrey Young


Stas Bekman wrote:
Stas Bekman wrote:

Man, you should at least give us a chance to test patches if you post 
them for review and not rushing to commit them. mp2 now can't be even 
built. 


Mea Culpa,
that's ok, I'm used to it by now...

it can't be built only with:

  MP_CCOPTS='-Werror'

which you probably don't use. That's the same pragma for C as:

  use warnings FATAL => 'all';

for Perl, turning all warnings into fatal errors.

Any objections if we add this flag along with Wall when MP_MAINTAINER=1 
to avoid this kind of problems in the future?
please make maintainer mode include absolutely every possible check and 
verbosity level.

--Geoff

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: modperl_default_port_handler commit broke mp2's build

2003-10-08 Thread Geoffrey Young

Besides, while looking why this thing is broken, I saw that this is an 
HTTP hook and has little to do with protocols, since its defined in 
include/http_protocol.h. 
so?  apache isn't perfectly separated, and a few of the functions are 
clearly misnamed or misplaced - see ap_http_method which returns the default 
scheme (http) and _not_ the default http method (if there is such a thing, 
probably GET)

So it definitely doesn't belong to the 
protocols documentation, but should be put together with other HTTP 
hooks. 
right, there is no reason to hook into it if you're using http, where the 
defaults are specified via RFC 1700 (according to /etc/services).

but I don't see it as an http-only thing, regardless of where it happens to 
be defined.

it's use is in defining a default port so that Apache can create 
self-referential URLs - otherwise with UseCanonicalName Off and no port in 
the request line or headers there can be no way to determine the port for 
other parts of the API.

when would this happen?  with mod_pop3 for one, which listens on the 
standard POP3 port (110) - the default port should probably be set as well 
(although there isn't the concept of redirects in mail, so I doubt it 
actually matters).

but say you implement you own protocol for xslt transformations or something 
(xslt://foo/).  you can use a default port handler and default method 
handler for making sure that URL manipulation routines work properly for 
your application.

> Also I still fail to see its usability.

I think I've adequately answered that (and a few times, now).  and besides, 
you or I seeing it's usability isn't the issue.  in case you've lost sight

http://perl.apache.org/docs/2.0/user/design/design.html#Introduction

to me, that means this a no brainer: if it exists as a hook in apache we 
offer it to perl.

--Geoff



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: t/SMOKE on win32

2003-10-08 Thread Stas Bekman
Steve Hay wrote:
[...]
This makes sense after reading

   http://perl.apache.org/docs/2.0/user/config/config.html#C_modperl_
   http://perl.apache.org/docs/2.0/user/config/config.html#C_perl_script_
and sure enough the various tests that fail, e.g.

   apache\scanhdrs
   api\rflush
   modperl\post_utf8
   modules\cgi
specify "SetHandler perl-script" in their .pm files, while the test 
above that succeeded (api\lookup_ui) specifies "SetHandler modperl" in 
its .pm file.
Not a single test that uses 'perl-script' passes?

So it looks like rather than us having the redirection stuff oddly 
working for some tests and not for others, what we actually have is a 
case of it never working!  The tests that work only do so because they 
use the "modperl" response handler which skips the tie()'ing of 
STDIN/STDOUT.
But you said that they did work from 'make test'/ t/TEST, for some reason they 
only fail when run from t/SMOKE. Is that correct? So it's some sort of 
condition that t/SMOKE creates, creating the grounds for this problem.



__
Stas BekmanJAm_pH --> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: t/SMOKE on win32

2003-10-08 Thread Stas Bekman
Steve Hay wrote:
[...]
Line 2457 of perlio.c sets fd to 1 (which is STDOUT, right?)  flags is 
still 2, so (flags & PERLIO_DUP_FD) is true and we call PerlLIO_dup() on 
line 2459.  That calls win32_dup(), which calls dup(), which I can't 
step into ;-).  Each of dup(), win32_dup() and PerlLIO_dup() returns 35. 
[...]
This f also gets returned from PerlIOBase_dup(), so back in 
PerlIOUnix_dup() we call PerlIOUnix_setfd() at line 2465 which looks 
good. f is non-NULL, fd is 35, os->oflags is 0.  The PerlLIO_fstat(35, 
&st) call at line 2348 is ultimately a call to _fstati64(35, ...) at 
win32/win32sck.c line 500 which returns -1.  I wasn't expecting that to 
have failed (_fstati64() returns 0 for success, -1 for failure) :-s (***)
[...]

You said you have an app, that shows you the opened fds. So was fd 35 open 
after win32_dup() was called? And was it closed at the stat checkpoint above? 
If so could you monitor what caused to its closure?

Also I thought win32 doesn't work with fds, but its own HANDLE thingy. Is that 
not true?

__
Stas BekmanJAm_pH --> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: modperl_default_port_handler commit broke mp2's build

2003-10-08 Thread Stas Bekman
Stas Bekman wrote:
Man, you should at least give us a chance to test patches if you post 
them for review and not rushing to commit them. mp2 now can't be even 
built. Your patch has several problems. First of all the prototype is 
wrong, it should be:

  apr_port_t modperl_default_port_handler(const request_rec *r)

since the declaration is:

include/http_protocol.h:AP_DECLARE_HOOK(apr_port_t,default_port,(const 
request_rec *r))

without the const, the build warns and dies. But once you put the const 
in we have another warning, since

(apr_port_t)modperl_callback_per_srv(MP_DEFAULT_PORT_HANDLER, rr, 
MP_HOOK_RUN_FIRST);

discards 'const'. which is not trivial to fix, so I stopped there, 
removed your change and finished building the project.
So any suggestions to how to fix that?

__
Stas BekmanJAm_pH --> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: t/SMOKE on win32

2003-10-08 Thread Steve Hay
Stas Bekman wrote:

Steve Hay wrote:
[...]
This makes sense after reading

   http://perl.apache.org/docs/2.0/user/config/config.html#C_modperl_
   
http://perl.apache.org/docs/2.0/user/config/config.html#C_perl_script_

and sure enough the various tests that fail, e.g.

   apache\scanhdrs
   api\rflush
   modperl\post_utf8
   modules\cgi
specify "SetHandler perl-script" in their .pm files, while the test 
above that succeeded (api\lookup_ui) specifies "SetHandler modperl" 
in its .pm file.


Not a single test that uses 'perl-script' passes? 
Without trawling through all the tests trying each one individually 
under t/SMOKE and looking to see how it is run (perl-script/modperl) I 
couldn't say for sure.  I just happened to spot that the few failing 
ones that I've been looking at were all perl-script and a working one 
was modperl.



So it looks like rather than us having the redirection stuff oddly 
working for some tests and not for others, what we actually have is a 
case of it never working!  The tests that work only do so because 
they use the "modperl" response handler which skips the tie()'ing of 
STDIN/STDOUT.


But you said that they did work from 'make test'/ t/TEST, for some 
reason they only fail when run from t/SMOKE. Is that correct? So it's 
some sort of condition that t/SMOKE creates, creating the grounds for 
this problem. 
Yes, that's correct - "nmake test" runs the entire testsuite without 
error, "perl t/SMOKE" falls over on various tests.

I assumed it was something to do with the IPC::Run3 stuff in 
TestSmoke.pm that we've been playing with (specifically the fact that it 
introduces new redirections of its own), and I was under the impression 
that it is only t/SMOKE that uses this TestSmoke module.  Is that not 
the case?

My thinking was that maybe the redirections in TestSmoke clash with the 
redirections done by perl-script, but they're fine with the modperl 
handler because that doesn't do any redirections of its own.

- Steve

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: t/SMOKE on win32

2003-10-08 Thread Steve Hay
Stas Bekman wrote:

Steve Hay wrote:
[...]
Line 2457 of perlio.c sets fd to 1 (which is STDOUT, right?)  flags is 
still 2, so (flags & PERLIO_DUP_FD) is true and we call PerlLIO_dup() 
on line 2459.  That calls win32_dup(), which calls dup(), which I 
can't step into ;-).  Each of dup(), win32_dup() and PerlLIO_dup() 
returns 35. 
[...]

This f also gets returned from PerlIOBase_dup(), so back in 
PerlIOUnix_dup() we call PerlIOUnix_setfd() at line 2465 which looks 
good. f is non-NULL, fd is 35, os->oflags is 0.  The 
PerlLIO_fstat(35, &st) call at line 2348 is ultimately a call to 
_fstati64(35, ...) at win32/win32sck.c line 500 which returns -1.  I 
wasn't expecting that to have failed (_fstati64() returns 0 for 
success, -1 for failure) :-s (***)
[...]

You said you have an app, that shows you the opened fds. So was fd 35 
open after win32_dup() was called? And was it closed at the stat 
checkpoint above? If so could you monitor what caused to its closure? 
Sadly that app only shows files / devices etc that are open by name - it 
doesn't relate the open thingies to file descriptors or even Win32 
HANDLE's.  There are dozens of open thingies at this point, and loads of 
them have the same name (like \Device\Tcp and \Device\Afd\Endpoint).  I 
don't know how a dup() of STDOUT would even show up in the list, and I 
can't see anything that looks like it might be it.



Also I thought win32 doesn't work with fds, but its own HANDLE thingy. 
Is that not true? 
Win32 has FILE * filehandles and int file descriptors just like other 
OS's do, but also has a lower level HANDLE thing of its own too.  The MS 
STD C runtime library implements all the stdio (fopen et al) and lowio 
(open et al) functions in terms of calls to Win32 API functions 
(CreateFile et al); it is those API functions that use HANDLE's.

- Steve

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: t/SMOKE on win32

2003-10-08 Thread Stas Bekman
Steve Hay wrote:
Stas Bekman wrote:

Steve Hay wrote:
[...]
Line 2457 of perlio.c sets fd to 1 (which is STDOUT, right?)  flags is 


still 2, so (flags & PERLIO_DUP_FD) is true and we call PerlLIO_dup() 
on line 2459.  That calls win32_dup(), which calls dup(), which I 
can't step into ;-).  Each of dup(), win32_dup() and PerlLIO_dup() 
returns 35. 


[...]

This f also gets returned from PerlIOBase_dup(), so back in 
PerlIOUnix_dup() we call PerlIOUnix_setfd() at line 2465 which looks 
good. f is non-NULL, fd is 35, os->oflags is 0.  The 
PerlLIO_fstat(35, &st) call at line 2348 is ultimately a call to 
_fstati64(35, ...) at win32/win32sck.c line 500 which returns -1.  I 
wasn't expecting that to have failed (_fstati64() returns 0 for 
success, -1 for failure) :-s (***)


[...]

You said you have an app, that shows you the opened fds. So was fd 35 
open after win32_dup() was called? And was it closed at the stat 
checkpoint above? If so could you monitor what caused to its closure? 


Sadly that app only shows files / devices etc that are open by name - it 
doesn't relate the open thingies to file descriptors or even Win32 
HANDLE's.  There are dozens of open thingies at this point, and loads of 
them have the same name (like \Device\Tcp and \Device\Afd\Endpoint).  I 
don't know how a dup() of STDOUT would even show up in the list, and I 
can't see anything that looks like it might be it.
Could you look at that list just before the dup and then after it and see what 
was added? You could probably patch perl to tell you that info, though I don't 
know what function does that in the win32 land. It should be in one of the 
perl io functions.

Also I thought win32 doesn't work with fds, but its own HANDLE thingy. 
Is that not true? 


Win32 has FILE * filehandles and int file descriptors just like other 
OS's do, but also has a lower level HANDLE thing of its own too.  The MS 
STD C runtime library implements all the stdio (fopen et al) and lowio 
(open et al) functions in terms of calls to Win32 API functions 
(CreateFile et al); it is those API functions that use HANDLE's.
Thanks. Apache/apr are now exclusively using HANDLEs bypassing FILE*.

__
Stas BekmanJAm_pH --> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: t/SMOKE on win32

2003-10-08 Thread Stas Bekman
Steve Hay wrote:
Stas Bekman wrote:

Steve Hay wrote:
[...]
This makes sense after reading

   http://perl.apache.org/docs/2.0/user/config/config.html#C_modperl_
   
http://perl.apache.org/docs/2.0/user/config/config.html#C_perl_script_

and sure enough the various tests that fail, e.g.

   apache\scanhdrs
   api\rflush
   modperl\post_utf8
   modules\cgi
specify "SetHandler perl-script" in their .pm files, while the test 
above that succeeded (api\lookup_ui) specifies "SetHandler modperl" 
in its .pm file.


Not a single test that uses 'perl-script' passes? 


Without trawling through all the tests trying each one individually 
under t/SMOKE and looking to see how it is run (perl-script/modperl) I 
couldn't say for sure.  I just happened to spot that the few failing 
ones that I've been looking at were all perl-script and a working one 
was modperl.
It's easy to get the list of tests that use perl-script, since they specify 
that explicitly in their .pm:

% grep -Ilr 'perl-script' t | grep -v "t/conf"| grep "\.pm" | perl -p -e 
's/.*?Test(.*?).pm/lc($1)/e'
api/r_subclass
api/rflush
apache/cgihandler
apache/scanhdrs
apache/scanhdrs2
error/push_handlers
modperl/endav
modperl/env
modperl/print_utf8
modperl/post_utf8
modperl/cookie
modules/cgiupload
modules/cgi

So it looks like rather than us having the redirection stuff oddly 
working for some tests and not for others, what we actually have is a 
case of it never working!  The tests that work only do so because 
they use the "modperl" response handler which skips the tie()'ing of 
STDIN/STDOUT.


But you said that they did work from 'make test'/ t/TEST, for some 
reason they only fail when run from t/SMOKE. Is that correct? So it's 
some sort of condition that t/SMOKE creates, creating the grounds for 
this problem. 


Yes, that's correct - "nmake test" runs the entire testsuite without 
error, "perl t/SMOKE" falls over on various tests.
Can you confirm that it fails on all the 'perl-script' tests?

Do you have the same issue if you run them as:

  perl t/TEST testname

instead of using 'nmake'? If that's the case, it's not an issue with SMOKE.

I assumed it was something to do with the IPC::Run3 stuff in 
TestSmoke.pm that we've been playing with (specifically the fact that it 
introduces new redirections of its own), and I was under the impression 
that it is only t/SMOKE that uses this TestSmoke module.  Is that not 
the case?
Yes. But Smoke does nothing else but starting t/TEST and feeding tests to it, 
execution-wise. Anything that fails with t/SMOKE should fail the same way with 
t/TEST.

My thinking was that maybe the redirections in TestSmoke clash with the 
redirections done by perl-script, but they're fine with the modperl 
handler because that doesn't do any redirections of its own.
I doubt so, since the problem comes from the server, which has no idea what 
the client does, and has a totally different process environment, so I fail to 
see how the client can affect it. Unless there is something special about win32.

__
Stas BekmanJAm_pH --> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


current_callback return behavior

2003-10-08 Thread Geoffrey Young

the issue here isn't related to the new hook, but rather my use of 
setting the return status based on $r->args - I think $port was getting 
assigned the PV value and mod_perl isn't getting the associated IV.  
int() was just my workaround.
confirmed via Devel::Peek.

but yes, it's somewhat of a real issue (though apparently not, if nobody 
has reported it yet).

I'll do some investigation and see it I can't fix it for everyone.
attached is a first pass at fixing the problem, coupled with the removal of 
rc guessing that was already +1'd.

the fix involved checking for both SvIOK and SvPOK.  in the case of SvIOK we 
call SvIVX directly (no need for SvIVx, since we're already IOK) and return 
the int.  if SvPOK we call SvIVx to coerce the string to an integer and 
return that.

of course, neither of these protects against 'return "foo"', but I didn't 
see an easy way to trap that and I don't see that as a big issue, as strings 
were getting translated to OK already.

however, I'm especially interested if SvIV coerces all strings into 0 on all 
platforms, as I specifically test for that.  if not, we can just remove that 
 test.

as for the rest, the return status of the callback is passed directly back 
to apache.  the only exception are handlers with no return status, such as 
ModPerl::Util::exit, which gets translated as OK.

for the most part, I like the behavior.  the only thing that bothers me is 
whether or not to treat 1 as special, but then we get into the guessing game 
again, and I'd rather enforce good API use (all handlers ought to return a 
valid status) over trickery.  so we throw a meaningful error message there 
and leave it to the user to fix.

anyway, feedback welcome.

--Geoff
Index: src/modules/perl/modperl_callback.c
===
RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/modperl_callback.c,v
retrieving revision 1.61
diff -u -r1.61 modperl_callback.c
--- src/modules/perl/modperl_callback.c 7 Oct 2003 19:16:13 -   1.61
+++ src/modules/perl/modperl_callback.c 8 Oct 2003 18:47:43 -
@@ -4,6 +4,7 @@
  request_rec *r, server_rec *s, AV *args)
 {
 CV *cv=Nullcv;
+SV *status_sv;
 I32 flags = G_EVAL|G_SCALAR;
 dSP;
 int count, status = OK;
@@ -71,32 +72,48 @@
 SPAGAIN;
 
 if (count != 1) {
+/* XXX can this really happen with G_SCALAR? */
+MP_TRACE_h(MP_FUNC, "callback count not 1 - assuming OK\n");
 status = OK;
 }
 else {
-SV* status_sv = POPs;
+status_sv = POPs;
+
 if (SvIOK(status_sv)) {
-status = (IV)SvIVx(status_sv);
+status = SvIVX(status_sv);
+MP_TRACE_h(MP_FUNC, "callback returned valid integer status %d\n",
+   status);
 }
-else {
-/* ModPerl::Util::exit doesn't return an integer value */
-status = OK; 
+else if (SvPOK(status_sv)) {
+status = SvIVx(status_sv);
+MP_TRACE_h(MP_FUNC, "coerced callback return status to integer %d\n",
+   status);
 }
-/* assume OK for 200 (HTTP_OK) */
-if ((status == 200)) {
+else {
+/* ModPerl::Util::exit() and other void functions.
+ * also routines that die()d, which are caught by ERRSV  */
+MP_TRACE_h(MP_FUNC, "callback IV and PV return slots empty - assuming 
OK\n");
 status = OK;
 }
+
+/* XXX special error logging for possible Perl gotcha */
+if (status == 1) {
+ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
+ "Perl callback returned 1 - did you forget to return 
Apache::OK?\n");
+}
 }
 
 PUTBACK;
 }
-
+
 FREETMPS;LEAVE;
 
 if (SvTRUE(ERRSV)) {
 MP_TRACE_h(MP_FUNC, "$@ = %s", SvPVX(ERRSV));
 status = HTTP_INTERNAL_SERVER_ERROR;
 }
+
+MP_TRACE_h(MP_FUNC, "returning %d from callback\n", status);
 
 return status;
 }
Index: t/response/TestModperl/current_callback.pm
===
RCS file: /home/cvspublic/modperl-2.0/t/response/TestModperl/current_callback.pm,v
retrieving revision 1.3
diff -u -r1.3 current_callback.pm
--- t/response/TestModperl/current_callback.pm  31 Mar 2003 01:50:52 -  1.3
+++ t/response/TestModperl/current_callback.pm  8 Oct 2003 18:47:43 -
@@ -36,6 +36,8 @@
 die "expecting $expected callback, instead got $callback" 
 unless $callback eq $expected;
 #warn "in callback: $callback\n";
+
+return Apache::OK;
 }
 
 1;

--- /dev/null   2003-01-30 05:24:37.0 -0500
+++ t/modperl/status.t  2003-10-08 14:34:14.0 -0400
@@ -0,0 +1,145 @@
+use strict;
+use warnings FATAL => 'all';
+

Re: modperl_default_port_handler commit broke mp2's build

2003-10-08 Thread Geoffrey Young

(apr_port_t)modperl_callback_per_srv(MP_DEFAULT_PORT_HANDLER, rr, 
MP_HOOK_RUN_FIRST);

discards 'const'. which is not trivial to fix, so I stopped there, 
removed your change and finished building the project.


So any suggestions to how to fix that?
not the const request_rec *r part, no.  I tried playing with a few things 
but couldn't get anything to compile without warnings.  guess my C needs 
some polishing :)

--Geoff

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: current_callback return behavior

2003-10-08 Thread Stas Bekman
Geoffrey Young wrote:

the issue here isn't related to the new hook, but rather my use of 
setting the return status based on $r->args - I think $port was 
getting assigned the PV value and mod_perl isn't getting the 
associated IV.  int() was just my workaround.


confirmed via Devel::Peek.

but yes, it's somewhat of a real issue (though apparently not, if 
nobody has reported it yet).

I'll do some investigation and see it I can't fix it for everyone.


attached is a first pass at fixing the problem, coupled with the removal 
of rc guessing that was already +1'd.

the fix involved checking for both SvIOK and SvPOK.  in the case of 
SvIOK we call SvIVX directly (no need for SvIVx, since we're already 
IOK) and return the int.  if SvPOK we call SvIVx to coerce the string to 
an integer and return that.
is it possible that that later on we will have callbacks that return a string, 
which is a valid thing? Aren't we guessing here again? That's why I had the 
idea of doing a specific handling of return code, by the callback type where 
we know exactly what to expect. so if it's a default_port callback we know 
that we need to force pv->iv, but we don't need to run this code for all other 
handlers. If we prepare an array of supported callbacks with integer entries, 
a simple switch with a logic specific to each callback will be a much cleaner 
solution, IMHO.

of course, neither of these protects against 'return "foo"', but I 
didn't see an easy way to trap that and I don't see that as a big issue, 
as strings were getting translated to OK already.

however, I'm especially interested if SvIV coerces all strings into 0 on 
all platforms, as I specifically test for that.  if not, we can just 
remove that  test.
0 only if the string starts with non-digit, but otherwise it can be a number 
as well, perl -le 'print int "9bar"' prints 9.

as for the rest, the return status of the callback is passed directly 
back to apache.  the only exception are handlers with no return status, 
such as ModPerl::Util::exit, which gets translated as OK.
so we used to have all callbacks returning status (besides the exit() hack, 
which could probably be workarounded to return a special status as well), but 
now we get callbacks returning potentially anything.

for the most part, I like the behavior.  the only thing that bothers me 
is whether or not to treat 1 as special, but then we get into the 
guessing game again, and I'd rather enforce good API use (all handlers 
ought to return a valid status) over trickery.  so we throw a meaningful 
error message there and leave it to the user to fix.


+/* XXX special error logging for possible Perl gotcha */
+if (status == 1) {
+ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
+ "Perl callback returned 1 - did you forget to return 
Apache::OK?\n");
+}
why 1? it can be any value from the last evaluation if there is no explicit 
return.

__
Stas BekmanJAm_pH --> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: current_callback return behavior

2003-10-08 Thread Geoffrey Young

is it possible that that later on we will have callbacks that return a 
string, which is a valid thing? Aren't we guessing here again?
well, I suppose DIR_MAGIC_TYPE and DECLINE_CMD are both strings, but they 
managed to (somehow) work before.

other than that, if we declare that current_callback is for calling Perl 
handlers (as opposed to random Perl XSUBs) then I think it's an ok 
assumtion.  in my own XS routines that's all that I have ever wanted to do 
with it.

after all, the return value of modperl_callback itself is int, so it makes 
sense that we only accept integers back from Perl.  (maybe :)


That's 
why I had the idea of doing a specific handling of return code, by the 
callback type where we know exactly what to expect. so if it's a 
default_port callback we know that we need to force pv->iv,
well, don't confuse the issue.  this has nothing to do with default_port. 
the pv -> iv thing is a problem for any handler (as my tests show).  I think 
the real culprit was the last change from POPi to POPs - maybe POPi was 
already doing sv_2iv conversions behind the scenes.

but we don't 
need to run this code for all other handlers. If we prepare an array of 
supported callbacks with integer entries, a simple switch with a logic 
specific to each callback will be a much cleaner solution, IMHO.
again, that depends on whether modperl_callback is exclusively for Perl 
handlers.  if so, we need to decide whether we will ever need to accept a 
string from them.  personally, I don't see how, since it's either OK, 
DECLINED, DONE, or HTTP_* (or some later integer constants in 2.1).

can you think of situation where something other than an integer would be 
returned?


of course, neither of these protects against 'return "foo"', but I 
didn't see an easy way to trap that and I don't see that as a big 
issue, as strings were getting translated to OK already.

however, I'm especially interested if SvIV coerces all strings into 0 
on all platforms, as I specifically test for that.  if not, we can 
just remove that  test.


0 only if the string starts with non-digit, but otherwise it can be a 
number as well, perl -le 'print int "9bar"' prints 9.
cool, thanks.


as for the rest, the return status of the callback is passed directly 
back to apache.  the only exception are handlers with no return 
status, such as ModPerl::Util::exit, which gets translated as OK.


so we used to have all callbacks returning status (besides the exit() 
hack, which could probably be workarounded to return a special status as 
well), but now we get callbacks returning potentially anything.
with '9bar' yes.  is there a C routine to check whether a string is a valid 
integer?  if so, we could use it and throw 500.

but outside of strings, the intent (that I thought we had +1'd) was to allow 
handlers to return their own status and not superimpose HTTP status codes 
over it (which apache does anyway for HTTP).


+/* XXX special error logging for possible Perl gotcha */
+if (status == 1) {
+ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
+ "Perl callback returned 1 - did you 
forget to return Apache::OK?\n");
+}

why 1? it can be any value from the last evaluation if there is no 
explicit return.
just because perl uses 1 for true, and things like print() and other 
functions return 1.

but yes, we can't protect against everything.  I was just trying to make it 
a bit more obvious if users make an obvious mistake.

--Geoff

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: modperl_default_port_handler commit broke mp2's build

2003-10-08 Thread Stas Bekman
Geoffrey Young wrote:

(apr_port_t)modperl_callback_per_srv(MP_DEFAULT_PORT_HANDLER, rr, 
MP_HOOK_RUN_FIRST);

discards 'const'. which is not trivial to fix, so I stopped there, 
removed your change and finished building the project.


So any suggestions to how to fix that?


not the const request_rec *r part, no.  I tried playing with a few 
things but couldn't get anything to compile without warnings.  guess my 
C needs some polishing :)
The first argument to the default_port hook needs to be a function declared as:

  apr_port_t modperl_default_port_handler(const request_rec *r)

which is easy, the problem is that inside that function you call: 
modperl_callback_per_srv(), whose 'request_rec *r' argument is not 'const' and 
there is not much you can do about it, besides adding 
modperl_callback_per_srv_c with proto 'const request_rec *r'. Apache has a 
bunch of these dual functions, e.g. in include/httpd.h:

AP_DECLARE(char *) ap_strchr(char *s, int c);
AP_DECLARE(const char *) ap_strchr_c(const char *s, int c);
AP_DECLARE(char *) ap_strrchr(char *s, int c);
AP_DECLARE(const char *) ap_strrchr_c(const char *s, int c);
AP_DECLARE(char *) ap_strstr(char *s, const char *c);
AP_DECLARE(const char *) ap_strstr_c(const char *s, const char *c);
found with: grep -Ir const . | grep DECLARE | grep '_c('

__
Stas BekmanJAm_pH --> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: current_callback return behavior

2003-10-08 Thread Stas Bekman
Geoffrey Young wrote:

is it possible that that later on we will have callbacks that return a 
string, which is a valid thing? Aren't we guessing here again?


well, I suppose DIR_MAGIC_TYPE and DECLINE_CMD are both strings, but 
they managed to (somehow) work before.
I suppose, that's because we never return these?

other than that, if we declare that current_callback is for calling Perl 
handlers (as opposed to random Perl XSUBs) then I think it's an ok 
assumtion.  in my own XS routines that's all that I have ever wanted to 
do with it.

after all, the return value of modperl_callback itself is int, so it 
makes sense that we only accept integers back from Perl.  (maybe :)


That's why I had the idea of doing a specific handling of return code, 
by the callback type where we know exactly what to expect. so if it's 
a default_port callback we know that we need to force pv->iv,


well, don't confuse the issue.  this has nothing to do with 
default_port. the pv -> iv thing is a problem for any handler (as my 
tests show).  I think the real culprit was the last change from POPi to 
POPs - maybe POPi was already doing sv_2iv conversions behind the scenes.
right, sorry. So it should be easy to revert in the previous code, once we 
figure out what gets onto the stack when exit() is called (I'd guess that 
it'll be undef). I haven't really looked at it, thinking that we can just 
check whether the int slot is valid. So I really introduced a potential bug 
then if someone was returning "0" and it used to work.

Also for some reason I wasn't able to even reproduce the undef warning in the 
test suite, but easily reproduced outside of the test suite.

but we don't need to run this code for all other handlers. If we 
prepare an array of supported callbacks with integer entries, a simple 
switch with a logic specific to each callback will be a much cleaner 
solution, IMHO.


again, that depends on whether modperl_callback is exclusively for Perl 
handlers.  if so, we need to decide whether we will ever need to accept 
a string from them.  personally, I don't see how, since it's either OK, 
DECLINED, DONE, or HTTP_* (or some later integer constants in 2.1).

can you think of situation where something other than an integer would 
be returned?
not with the currently supported callbacks, but as you seem to plan to embrace 
all available hooks, I'm not familiar with all of them to tell ;)

as for the rest, the return status of the callback is passed directly 
back to apache.  the only exception are handlers with no return 
status, such as ModPerl::Util::exit, which gets translated as OK.


so we used to have all callbacks returning status (besides the exit() 
hack, which could probably be workarounded to return a special status 
as well), but now we get callbacks returning potentially anything.


with '9bar' yes.  is there a C routine to check whether a string is a 
valid integer?  if so, we could use it and throw 500.
Looks like Perl_grok_number is what you are after. Other than that isdigit(3) 
works on single chars.

But doing this in a general case would be expensive. I think a plan coercing 
into IV as before will do just as well. I suppose that it already uses 
grok_number internally.

but outside of strings, the intent (that I thought we had +1'd) was to 
allow handlers to return their own status and not superimpose HTTP 
status codes over it (which apache does anyway for HTTP).


+/* XXX special error logging for possible Perl gotcha */
+if (status == 1) {
+ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
+ "Perl callback returned 1 - did you 
forget to return Apache::OK?\n");
+}

why 1? it can be any value from the last evaluation if there is no 
explicit return.


just because perl uses 1 for true, and things like print() and other 
functions return 1.

but yes, we can't protect against everything.  I was just trying to make 
it a bit more obvious if users make an obvious mistake.
so we are guessing again?

__
Stas BekmanJAm_pH --> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: current_callback return behavior

2003-10-08 Thread Geoffrey Young


Stas Bekman wrote:
Geoffrey Young wrote:


is it possible that that later on we will have callbacks that return 
a string, which is a valid thing? Aren't we guessing here again?


well, I suppose DIR_MAGIC_TYPE and DECLINE_CMD are both strings, but 
they managed to (somehow) work before.


I suppose, that's because we never return these?
right about DIR_MAGIC_TYPE.  with DECLINE_CMD, modperl_module is using a 
custom callback routine instead of modperl_callback()  (I did some more 
research :)

well, don't confuse the issue.  this has nothing to do with 
default_port. the pv -> iv thing is a problem for any handler (as my 
tests show).  I think the real culprit was the last change from POPi 
to POPs - maybe POPi was already doing sv_2iv conversions behind the 
scenes.


right, sorry. So it should be easy to revert in the previous code, once 
we figure out what gets onto the stack when exit() is called (I'd guess 
that it'll be undef).
according to perlapi, yes: for Perl_croak count is 1 and undef is on the 
stack with G_EVAL.

I haven't really looked at it, thinking that we 
can just check whether the int slot is valid. So I really introduced a 
potential bug then if someone was returning "0" and it used to work.
yeah, it looks like "return values as strings" used to work and now they 
don't.  but that's ok, we'll fix it :)

Also for some reason I wasn't able to even reproduce the undef warning 
in the test suite, but easily reproduced outside of the test suite.
test suites are good for excercising functionality and some error 
conditions, but not always for reproducing real-world issues like that, 
unfortunately.

can you think of situation where something other than an integer would 
be returned?


not with the currently supported callbacks, but as you seem to plan to 
embrace all available hooks, I'm not familiar with all of them to tell ;)
me neither, but I'm learning as I go :)

everything I've seen (save DECLINE_CMD) is an integer, even the new auth 
constants in 2.1.  so, I'd venture to say we're safe (for the time being, 
anyway) to just use integers.  if we eventually need something that doesn't 
return an int, we can either hack modperl_callback, generate a new function 
for SVs, or split the logic up as needed.


with '9bar' yes.  is there a C routine to check whether a string is a 
valid integer?  if so, we could use it and throw 500.


Looks like Perl_grok_number is what you are after.
ah, cool.

Other than that 
isdigit(3) works on single chars.

But doing this in a general case would be expensive.
agreed.

I think a plan 
coercing into IV as before will do just as well. 
well, I'd like to add the integer check if we coerce the PV into an int, 
just to be safe - that's probably the (very) rare case, but I'd like to be 
able to say _something_ about it in the logs.

I suppose that it 
already uses grok_number internally.
I dunno.  I'll add an alphanumeric value to my tests and see what happens.


why 1? it can be any value from the last evaluation if there is no 
explicit return.


just because perl uses 1 for true, and things like print() and other 
functions return 1.

but yes, we can't protect against everything.  I was just trying to 
make it a bit more obvious if users make an obvious mistake.


so we are guessing again?
sorta.  with the patch I submitted, the 1 is allowed to continue on to 
Apache - only a warning is thrown.

I somehow feel that this will be a common mistake (it showed up in the test 
suite, in fact) that will be difficult to debug - when a return value is 
forgotten, the server merely sends a 500 with no errors logged at all.

but I'm not going to put up a fuss - I was just trying to anticipate user 
problems, which isn't always a good idea :)

at any rate, I'll come up with another patch tomorrow (if I have time - 
apachecon slides are looming).  however, after this discussion, I don't 
think it will be too much different - just the grok_integer and special 
treatment of 1 issues, right?

--Geoff

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: current_callback return behavior

2003-10-08 Thread Stas Bekman
Geoffrey Young wrote:
[...]
right, sorry. So it should be easy to revert in the previous code, 
once we figure out what gets onto the stack when exit() is called (I'd 
guess that it'll be undef).


according to perlapi, yes: for Perl_croak count is 1 and undef is on the 
stack with G_EVAL.
so why not POPs and then check status_sv == &PL_undef_sv and otherwise to go 
as before coercing SV into IV?

I haven't really looked at it, thinking that we can just check whether 
the int slot is valid. So I really introduced a potential bug then if 
someone was returning "0" and it used to work.


yeah, it looks like "return values as strings" used to work and now they 
don't.  but that's ok, we'll fix it :)
another reason to have a soonish release of 1.99_11 (the other is a bug in 
filters insertion (connection filter being inserted into the request filter queue)

Also for some reason I wasn't able to even reproduce the undef warning 
in the test suite, but easily reproduced outside of the test suite.


test suites are good for excercising functionality and some error 
conditions, but not always for reproducing real-world issues like that, 
unfortunately.
Frankly I don't think this applies to the undef-warn-on-exit problem, it's 
reproducable in a trivial cgi script. It must be something else in the test 
suite that masks the problem. I might take another look at it later on. As we 
mess with callback return statuses, it'd be nice to have it covered in the 
test suite.

I think a plan coercing into IV as before will do just as well. 


well, I'd like to add the integer check if we coerce the PV into an int, 
just to be safe - that's probably the (very) rare case, but I'd like to 
be able to say _something_ about it in the logs.
Checking the first char with isdigit should probably be good enough (though if 
you do that we need to use a crossplatform version of it, e.g. apr_isdigit)

just because perl uses 1 for true, and things like print() and other 
functions return 1.

but yes, we can't protect against everything.  I was just trying to 
make it a bit more obvious if users make an obvious mistake.


so we are guessing again?


sorta.  with the patch I submitted, the 1 is allowed to continue on to 
Apache - only a warning is thrown.
Do you think people will check their logs? And if for some reason they want it 
to be that way, they will just get annoyed by this log. I think we should 
either assert if we think it's a bug or stay quiet if we tolerate it, having a 
tracing tell us would be a good idea though if we get a bug report.

I somehow feel that this will be a common mistake (it showed up in the 
test suite, in fact) that will be difficult to debug - when a return 
value is forgotten, the server merely sends a 500 with no errors logged 
at all.
what if the return value happens to be an eval of some code which returns a 
valid status code, it's really guessing here...

but I'm not going to put up a fuss - I was just trying to anticipate 
user problems, which isn't always a good idea :)
Could be YAGNI as well.

at any rate, I'll come up with another patch tomorrow (if I have time - 
apachecon slides are looming).  however, after this discussion, I don't 
think it will be too much different - just the grok_integer and special 
treatment of 1 issues, right?
If you really insist. I'm +1 on fixing the bug introduced by exit() fix and 
keeping the rest as before the s/POPi/POPs/ change (quietly coercing defined 
SVs to IV with any additional checks).

__
Stas BekmanJAm_pH --> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: t/SMOKE on win32

2003-10-08 Thread Randy Kobes
On Wed, 8 Oct 2003, Stas Bekman wrote:

> Steve Hay wrote:
[ .. ]
> Do you have the same issue if you run them as:
>
>perl t/TEST testname
>
> instead of using 'nmake'? If that's the case, it's not an issue with SMOKE.

Running the tests that fail under t/SMOKE as
   perl t/TEST testname
reports them passing.

> > I assumed it was something to do with the IPC::Run3 stuff in
> > TestSmoke.pm that we've been playing with (specifically the fact that it
> > introduces new redirections of its own), and I was under the impression
> > that it is only t/SMOKE that uses this TestSmoke module.  Is that not
> > the case?
>
> Yes. But Smoke does nothing else but starting t/TEST and feeding tests to it,
> execution-wise. Anything that fails with t/SMOKE should fail the same way with
> t/TEST.
>
> > My thinking was that maybe the redirections in TestSmoke clash with the
> > redirections done by perl-script, but they're fine with the modperl
> > handler because that doesn't do any redirections of its own.
>
> I doubt so, since the problem comes from the server, which
> has no idea what the client does, and has a totally
> different process environment, so I fail to see how the
> client can affect it. Unless there is something special
> about win32.

I've attached a patch (for Steve's benefit, as it's Win32
specific) which, first of all, uses Win32::Process (to
rule out problems coming from IPC::Run3), and which defines
two functions:
   run_command - uses system() to run a command, and
 returns a flag indicating success;
   run_command_dup - uses Win32::Process to run a command,
 capturing the STDOUT and STDERR and
 then returning the text.

- if one uses run_command_dup to start/stop the server and
run the tests, then the error with failing to dup STDOUT
results, for those tests that use perl-script.
- if one uses run_command_dup to start/stop the server, but
uses run_command to run the tests, then the same error
results.
- if one uses run_command to start/stop the server and run
the tests, then all tests pass.

 --
best regards,
randyIndex: Apache-Test/lib/Apache/TestSmoke.pm
===
RCS file: /home/cvs/httpd-test/perl-framework/Apache-Test/lib/Apache/TestSmoke.pm,v
retrieving revision 1.23
diff -u -r1.23 TestSmoke.pm
--- Apache-Test/lib/Apache/TestSmoke.pm 12 Sep 2003 02:47:41 -  1.23
+++ Apache-Test/lib/Apache/TestSmoke.pm 9 Oct 2003 01:12:07 -
@@ -15,9 +15,16 @@
 use FindBin;
 use POSIX ();
 use Symbol ();
-
+#use IPC::Run3;
+use File::Temp qw(tempfile);
 #use constant DEBUG => 1;
 
+use constant WIN32 => Apache::TestConfig::WIN32;
+if (WIN32) {
+require Win32;
+require Win32::Process;
+}
+
 # how many times to run all tests at the first iteration
 use constant DEFAULT_TIMES  => 10;
 
@@ -111,7 +118,7 @@
 
 @{ $self->{tests} } = $self->get_tests($test_opts);
 
-$self->{base_command} = "./TEST";
+$self->{base_command} = "$^X $FindBin::Bin/TEST";
 
 # options common to all
 $self->{base_command} .= " -verbose" if $self->{verbose};
@@ -473,16 +480,14 @@
 # start server
 {
 my $command = $self->{start_command};
-open my $pipe, "$command 2>&1|" or die "cannot fork: $!";
-my $oldfh = select $pipe; $| = 1; select $oldfh;
-# XXX: check startup success?
 my $started_ok = 0;
 my $log = '';
-while (my $t = <$pipe>) {
-$started_ok = 1 if $t =~ /started/;
-$log .= $t;
+unless ($log = run_command($command, 0)) {
+error "Error running $command";
+exit 1;
 }
-close $pipe;
+#$started_ok = 1 if $log =~ /started/;
+$started_ok = 1;
 unless ($started_ok) {
 error "failed to start server\n $log";
 exit 1;
@@ -507,19 +512,15 @@
 my $fill = "." x ($max_len - length $test_name);
 $self->{total_tests_run}++;
 
-open my $pipe, "$command $test 2>&1|" or die "cannot fork: $!";
-my $oldfh = select $pipe; $| = 1; select $oldfh;
-
+my $test_command = "$command $test";
 my $ok = 0;
 my $log = '';
-while (<$pipe>) {
-$log .= $_;
-
-$ok = 1 if /All tests successful/;
+unless ($log = run_command($test_command, 1)) {
+error "Error running $test_command";
+exit 1;
 }
-# it's normal for $command to exit with a failure status if tests
-# fail, so we don't die/report it
-close $pipe;
+#$ok = 1 if $log =~ /All tests successful/;
+$ok = 1;
 
 my @core_files_msg = $self->Apache::TestRun::scan_core_incremental;
 
@@ -594,16 +595,14 @@
 # stop server
 {
 my $command = $self->{stop_command};
-open my $pipe, "$command 2>&1|" or die "cannot fork: $!";

Re: current_callback return behavior

2003-10-08 Thread Geoffrey Young

test suites are good for excercising functionality and some error 
conditions, but not always for reproducing real-world issues like 
that, unfortunately.


Frankly I don't think this applies to the undef-warn-on-exit problem, 
it's reproducable in a trivial cgi script. It must be something else in 
the test suite that masks the problem. I might take another look at it 
later on. As we mess with callback return statuses, it'd be nice to have 
it covered in the test suite.
indeed.  can you post the script?  I'll take a look at it too and see if I 
can figure out a way to reproduce it as well.

If you really insist. I'm +1 on fixing the bug introduced by exit() fix 
and keeping the rest as before the s/POPi/POPs/ change (quietly coercing 
defined SVs to IV with any additional checks).
"rest as before" in what sense?  do you mean keeping this part?

/* assume OK for non-http status codes and for 200 (HTTP_OK) */
if (((status > 0) && (status < 100)) ||
(status == 200) || (status > 600)) {
status = OK;
I thought you had agreed earlier to be rid of this as well?

at any rate, to avoid confusion, how about we address the issues separately. 
 I'll commit some version of status.t/status.pm tomottow that matches the 
current behavior and illustrates the pv/iv bug.  then we can review a fix 
for the bug and adjust the tests based on the model behavior we want for 
modperl_callback.

--Geoff

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


[Patch] Support for argument-less blocks

2003-10-08 Thread Philippe M. Chiasson
With apache-1.3, mod_perl users were able to do:


 some code


in httpd-2.0, the configuration parser doesn't like container directives
with no arguments (Syntax error on line nn of httpd.conf: 
directive missing  closing '>'). So, for now, users have to resort to
using this syntax (note the extra space):




The following patch fixes this little problem:

Index: server/config.c
===
RCS file: /home/cvspublic/httpd-2.0/server/config.c,v
retrieving revision 1.156.2.6
diff -u -I$Id: -r1.156.2.6 config.c
--- server/config.c 17 Sep 2003 10:30:47 -  1.156.2.6
+++ server/config.c 8 Oct 2003 22:10:40 -
@@ -926,6 +926,9 @@
 if (*lastc == '>') {
 *lastc = '\0' ;
 }
+if( cmd_name[0] == '<' && *args == '\0') {
+args = ">";
+}
 }
 
 newdir = apr_pcalloc(p, sizeof(ap_directive_t));


Philippe M. Chiasson /gozer\@(cpan|ectoplasm)\.org/ 88C3A5A5 (122FF51B/C634E37B)
http://gozer.ectoplasm.org/F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3 A5A5
Q: It is impossible to make anything foolproof because fools are so ingenious.
perl -e'$$=\${gozer};{$_=unpack(P7,pack(L,$$));/^JAm_pH\n$/&&print||$$++&&redo}'


signature.asc
Description: This is a digitally signed message part


Re: [PATCH] Re: Problem building mp1 on Win32

2003-10-08 Thread Randy Kobes
On Wed, 8 Oct 2003, Steve Hay wrote:

> I think I've cracked it myself looking at those two files.
> The original .dsp file has two lines like this:
> ADD CPP ... /I "\Perl\lib\CORE" ...
> which get fixed up by Makefile.PL to this:
> ADD CPP ... /I "\Perl\lib\CORE" /I "C:\apache\include"  /I
> "C:\apache\include/../os/win32" /I "C:\perl5\lib\CORE" ...
>
> Looks like that include path in the original ADD CPP lines should have
> been *changed* to "C:\perl5\lib\CORE", or at least have the correct path
> put *before* it -- it's no good having "\Perl\lib\CORE" and then
> "C:\perl5\lib\CORE" ;-)
>
> The attached patch to Makefile.PL (against 1.29-rc1) fixes those lines
> up as this:
>
> ADD CPP ... /I "C:\apache\include"  /I
> "C:\apache\include/../os/win32" /I "C:\perl5\lib\CORE" ...
>
> and I can now build mp1.29-rc1 with or without the extra C:\perl in place.

Thanks, Steve! I just committed your patch in the current cvs.

-- 
best regards,
randy

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: t/SMOKE on win32

2003-10-08 Thread Stas Bekman
Randy Kobes wrote:
On Wed, 8 Oct 2003, Stas Bekman wrote:


Steve Hay wrote:
[ .. ]

Do you have the same issue if you run them as:

  perl t/TEST testname

instead of using 'nmake'? If that's the case, it's not an issue with SMOKE.


Running the tests that fail under t/SMOKE as
   perl t/TEST testname
reports them passing.
Thanks, Randy. And if you run:

perl t/TEST -start
perl t/TEST -run testname
perl t/TEST -stop
if this still works, what if you run TestSmoke's test_run alone by ripping it 
off and just hardcoding things?

I've attached a patch (for Steve's benefit, as it's Win32
specific) which, first of all, uses Win32::Process (to
rule out problems coming from IPC::Run3), and which defines
two functions:
   run_command - uses system() to run a command, and
 returns a flag indicating success;
   run_command_dup - uses Win32::Process to run a command,
 capturing the STDOUT and STDERR and
 then returning the text.
- if one uses run_command_dup to start/stop the server and
run the tests, then the error with failing to dup STDOUT
results, for those tests that use perl-script.
- if one uses run_command_dup to start/stop the server, but
uses run_command to run the tests, then the same error
results.
- if one uses run_command to start/stop the server and run
the tests, then all tests pass.
What about the 4th option:

- if one uses run_command to start/stop the server and
uses run_command_dup to run the tests, ...
also isn't the third option exactly what we have now with IPC::Run? (i.e. 
using system everywhere?)

__
Stas BekmanJAm_pH --> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: current_callback return behavior

2003-10-08 Thread Stas Bekman
Geoffrey Young wrote:

test suites are good for excercising functionality and some error 
conditions, but not always for reproducing real-world issues like 
that, unfortunately.


Frankly I don't think this applies to the undef-warn-on-exit problem, 
it's reproducable in a trivial cgi script. It must be something else 
in the test suite that masks the problem. I might take another look at 
it later on. As we mess with callback return statuses, it'd be nice to 
have it covered in the test suite.


indeed.  can you post the script?  I'll take a look at it too and see if 
I can figure out a way to reproduce it as well.
package CGItest;

use strict;
use warnings;
use ModPerl::Util;

use Apache::RequestRec;
use Apache::RequestIO;
sub handler
{
my $r = shift;
$r->content_type( "text/plain" );
print "hey geoff\n";
ModPerl::Util::exit;
}
1;

If you really insist. I'm +1 on fixing the bug introduced by exit() 
fix and keeping the rest as before the s/POPi/POPs/ change (quietly 
coercing defined SVs to IV with any additional checks).


"rest as before" in what sense?  do you mean keeping this part?

/* assume OK for non-http status codes and for 200 (HTTP_OK) */
if (((status > 0) && (status < 100)) ||
(status == 200) || (status > 600)) {
status = OK;
I thought you had agreed earlier to be rid of this as well?
no,  I was talking about the change I did to fix the undef-on-exit problem 
(i.e. replacing POPi with POPs)

at any rate, to avoid confusion, how about we address the issues 
separately.  I'll commit some version of status.t/status.pm tomottow 
that matches the current behavior and illustrates the pv/iv bug.  then 
we can review a fix for the bug and adjust the tests based on the model 
behavior we want for modperl_callback.
+1

__
Stas BekmanJAm_pH --> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: t/SMOKE on win32

2003-10-08 Thread Randy Kobes
On Wed, 8 Oct 2003, Stas Bekman wrote:

> Randy Kobes wrote:
[ ... ]
> Thanks, Randy. And if you run:
>
> perl t/TEST -start
> perl t/TEST -run testname
> perl t/TEST -stop

This works ...

> if this still works, what if you run TestSmoke's test_run alone by ripping it
> off and just hardcoding things?

I'll take a look at this, but see below ...

> > I've attached a patch (for Steve's benefit, as it's Win32
> > specific) which, first of all, uses Win32::Process (to
> > rule out problems coming from IPC::Run3), and which defines
> > two functions:
> >run_command - uses system() to run a command, and
> >  returns a flag indicating success;
> >run_command_dup - uses Win32::Process to run a command,
> >  capturing the STDOUT and STDERR and
> >  then returning the text.
> >
> > - if one uses run_command_dup to start/stop the server and
> > run the tests, then the error with failing to dup STDOUT
> > results, for those tests that use perl-script.
> > - if one uses run_command_dup to start/stop the server, but
> > uses run_command to run the tests, then the same error
> > results.
> > - if one uses run_command to start/stop the server and run
> > the tests, then all tests pass.
>
> What about the 4th option:
>
> - if one uses run_command to start/stop the server and
> uses run_command_dup to run the tests, ...

That works! So the problem seems to be in starting the
server with a dup/redirect of STDOUT/STDERR; if one does
it with just a system call:
   my @args = split ' ', $start_command;
   system(@args);
then it seems OK to do the dup/redirect when running
the tests (I tested this with Win32::Process, but
I imagine the same thing would hold for IPC::Run3).

Like you mentioned earlier, though, Stas, there doesn't
seem to be any reason why doing something like this
on the client side would affect the server side. Might
there be though in starting the server?

> also isn't the third option exactly what we have now with
> IPC::Run? (i.e.  using system everywhere?)

The system() call I used was just that, with no STD
redirections.

-- 
best regards,
randy

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: t/SMOKE on win32

2003-10-08 Thread Stas Bekman
Randy Kobes wrote:
[...]
I've attached a patch (for Steve's benefit, as it's Win32
specific) which, first of all, uses Win32::Process (to
rule out problems coming from IPC::Run3), and which defines
two functions:
  run_command - uses system() to run a command, and
returns a flag indicating success;
  run_command_dup - uses Win32::Process to run a command,
capturing the STDOUT and STDERR and
then returning the text.
- if one uses run_command_dup to start/stop the server and
run the tests, then the error with failing to dup STDOUT
results, for those tests that use perl-script.
- if one uses run_command_dup to start/stop the server, but
uses run_command to run the tests, then the same error
results.
- if one uses run_command to start/stop the server and run
the tests, then all tests pass.
What about the 4th option:

- if one uses run_command to start/stop the server and
uses run_command_dup to run the tests, ...


That works! So the problem seems to be in starting the
server with a dup/redirect of STDOUT/STDERR; if one does
it with just a system call:
   my @args = split ' ', $start_command;
   system(@args);
then it seems OK to do the dup/redirect when running
the tests (I tested this with Win32::Process, but
I imagine the same thing would hold for IPC::Run3).
Like you mentioned earlier, though, Stas, there doesn't
seem to be any reason why doing something like this
on the client side would affect the server side. Might
there be though in starting the server?
Now, that makes much more sense, Randy. Looks like we are making a progress on 
this. Indeed as you said doing 't/TEST -run testname' doesn't affect the 
server side. But the way you start the server does.

I suppose this has something to do with the win32 implementation of the server 
startup. If the server start with stdout redirected, it's not available for 
the server at all. You should be able then to reproduce this outside 
Apache-Test, just doing:

httpd > /dev/null &

and then trying to run requests on it, should reproduce the problem. If you 
can, then in my opinion it's a bug in httpd, which meanwhile we certainly can 
workaround. Instead of parsing the output of startup, we could check the 
return status of the command and thus. though we will end up with startup 
noise which is not nice at all.

Also I'm bit lost in all the redirect attempts that you have tried. Have you 
tried doing this in smoke:

dup STDOUT
t/TEST -start | # redirected so we can parse it
restore STDOUT
close the dupped fh
t/TEST -run test
also isn't the third option exactly what we have now with
IPC::Run? (i.e.  using system everywhere?)


The system() call I used was just that, with no STD
redirections.
very good!

__
Stas BekmanJAm_pH --> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: t/SMOKE on win32

2003-10-08 Thread Randy Kobes
On Wed, 8 Oct 2003, Stas Bekman wrote:

> Now, that makes much more sense, Randy. Looks like we are
> making a progress on this. Indeed as you said doing
> 't/TEST -run testname' doesn't affect the server side. But
> the way you start the server does.
>
> I suppose this has something to do with the win32
> implementation of the server startup.

I think that's exactly it ... The problem seems to
be in Apache::TestServer, which starts the server
with Win32::Process as
   Win32::Process::Create($obj, $httpd, "$cmd $one_process",
  0,
  NORMAL_PRIORITY_CLASS, ',');
The "0" in there determines if the process inherits the
calling processes handles or not. Changing it to "1"
(so it does inherit) seems to fix things, in that I
can now run everything (starting and stopping the server,
and running the tests) with the run_command_dup function,
and the STDs are all properly captured.

I'll work on reverting back to IPC::Run3 and seeing if
that works.

-- 
best regards,
randy

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: t/SMOKE on win32

2003-10-08 Thread Stas Bekman
Stas Bekman wrote:
... You should be able then to reproduce
this outside Apache-Test, just doing:
httpd > /dev/null &

and then trying to run requests on it, should reproduce the problem.
It's probably should be reproducable by a simple attempt to do:

print STDOUT "It works";

from the startup.pl file. Most likely it won't print anything and if you check 
the return code from print, it'll indicate failure.

__
Stas BekmanJAm_pH --> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]