Skip Menu |
 

Preferred bug tracker

Please visit the preferred bug tracker to report your issue.

This queue is for tickets about the CGI CPAN distribution.

Report information
The Basics
Id: 49943
Status: resolved
Priority: 0/
Queue: CGI

People
Owner: Nobody in particular
Requestors: MARKSTOS [...] cpan.org
Cc: miyagawa [...] bulknews.net
AdminCc:

Bug Information
Severity: Wishlist
Broken in: (no value)
Fixed in: (no value)



CC: miyagawa [...] bulknews.net
Subject: Needs discussion: wish: PSGI support in CGI.pm
Download (untitled) / with headers
text/plain 1.2k
Miyagawa has proposed that CGI.pm include direct support for the PSGI specification. An initial implementation has already been completed and is available in the 'pgsi_support' branch of my repo: http://github.com/markstos/CGI.pm/commits/psgi_support I support the theory of the project, which ports WSGI to Perl, which seems to have proven valuable in the Python and Ruby communities. This post explains more: http://bulknews.typepad.com/blog/2009/09/psgi-perl-wsgi.html Before this work is merged into the core, a few things need to happen: 1. We need to agree on the exact API 2. It needs to be tested and documented 3. Lincoln Stein needs to approve it. Here are my concerns regarding the API as it is proposed: 1. I'm concerned that the CGI::PSGI module exists primarily to set a variable: $CGI::PSGI. Directly setting the global or using a pragma seems more consistent with existing features. 2. I'm concerned about adding *any* significant features to CGI.pm at this point. However, there is no easy way to add this functionality externally. Ideally, if this moves forward, it would be ideal to have a similar update released to CGI::Simple at the same time. Next, I'm interested in feedback from other CGI.pm users about the proposed change
Subject: Re: [rt.cpan.org #49943] Needs discussion: wish: PSGI support in CGI.pm
Date: Thu, 24 Sep 2009 21:07:45 -0400
To: bug-CGI.pm [...] rt.cpan.org
From: Yanick Champoux <yanick [...] babyl.dyndns.org>
Download (untitled) / with headers
text/plain 734b
MARKSTOS via RT wrote: Show quoted text
> Miyagawa has proposed that CGI.pm include direct support for the PSGI > specification. [..]
Show quoted text
> Next, I'm interested in feedback from other CGI.pm users about the > proposed change
I share Mark's caution about adding new functionality to CGI. It's a module with a long history used by a lot of people, and in consequence we want to be extra-careful with any changes. On the other hand, the modifications to support PSGI are fairly minimal, and shouldn't impact any of the current functionality of the module. In this specific case, I think my vote would go for including the PSGI support, but only after making sure the implemented API is sound, and after having tested it through and through.
Subject: Re: [rt.cpan.org #49943] Needs discussion: wish: PSGI support in CGI.pm
Date: Fri, 25 Sep 2009 10:37:29 +0900
To: bug-CGI.pm [...] rt.cpan.org
From: Tatsuhiko Miyagawa <miyagawa [...] gmail.com>
Download (untitled) / with headers
text/plain 1.2k
Show quoted text
> 1. I'm concerned that the CGI::PSGI module exists primarily to set a > variable: $CGI::PSGI. Directly setting the global or using a pragma > seems more consistent with existing features.
CGI::PSGI module is there to help ease the migration in web frameworks, so "use CGI::PSGI" will fail if the installed CGI.pm doesn't support PSGI. The problem here is that current (unpatched) CGI.pm DOESN'T fail when called "$CGI::PSGI = 1" nor "use CGI qw(:psgi)" and silently doesn't do anything useful re: PSGI variables. Making CGI::PSGI module just to set the flag then makes sure that your application bails out if you need PSGI support but when your CGI.pm doesn't support it. Also, your frameworks can say "requires 'CGI::PSGI'" in Makefile.PL, instead of specifying the CGI.pm version number. the API's awkwardness is something separate worth discussing. I wanted to say use CGI::PSGI; my $q = CGI::PSGI->new($env); my($status, $headers) = $q->headers($content_type); without touching CGI.pm at all, but it was really hard since CGI.pm directly manipulates %ENV and there are so many methods to clone. (That means it's not technically impossible, so we still keep that as an option if a direct PSGI support is unapproved). Thanks, -- Tatsuhiko Miyagawa
Subject: Re: [rt.cpan.org #49943] Needs discussion: wish: PSGI support in CGI.pm
Date: Fri, 25 Sep 2009 11:09:47 +0900
To: bug-CGI.pm [...] rt.cpan.org
From: Tatsuhiko Miyagawa <miyagawa [...] gmail.com>
Download (untitled) / with headers
text/plain 657b
On Fri, Sep 25, 2009 at 10:37 AM, miyagawa@gmail.com via RT <bug-CGI.pm@rt.cpan.org> wrote: Show quoted text
> The problem here is that current (unpatched) CGI.pm DOESN'T fail when > called "$CGI::PSGI = 1" nor "use CGI qw(:psgi)" and silently doesn't > do anything useful re: PSGI variables. Making CGI::PSGI module just to > set the flag then makes sure that your application bails out if you > need PSGI support but when your CGI.pm doesn't support it.
The other approach for this would be to let app developers to say: use CGI qw(:psgi); # this would set $CGI::PSGI to 1 die "Your CGI.pm doesn't support PSGI!" unless defined $CGI::PSGI; -- Tatsuhiko Miyagawa
CC: MARKSTOS [...] cpan.org
Subject: Re: [rt.cpan.org #49943] Needs discussion: wish: PSGI support in CGI.pm
Date: Fri, 25 Sep 2009 11:16:26 +0900
To: bug-CGI.pm [...] rt.cpan.org
From: Tatsuhiko Miyagawa <miyagawa [...] gmail.com>
Download (untitled) / with headers
text/plain 1.4k
On Fri, Sep 25, 2009 at 10:37 AM, miyagawa@gmail.com via RT <bug-CGI.pm@rt.cpan.org> wrote: Show quoted text
>  use CGI::PSGI; >  my $q = CGI::PSGI->new($env); >  my($status, $headers) = $q->headers($content_type); > > without touching CGI.pm at all, but it was really hard since CGI.pm > directly manipulates %ENV and there are so many methods to clone. > (That means it's not technically impossible, so we still keep that as > an option if a direct PSGI support is unapproved).
The actual implementation for that would be: 1) in CGI::PSGI::new, save the current %ENV somewhere and populate %ENV with PSGI's $env 2) Override methods that reads from STDIN, like read_from_stdin and read_from_client to read from $env->{'psgi.input'} 3) Override header() method, or implement a different named method (like psgi_header) to return $status and $headers_ref 4) in DESTROY, restore the saved %ENV, and call CGI::initialize_globals() I *think* this can be done without touching CGI.pm at all, but a) a fair amount of code may need to be duplicated b) I'm not really sure that's "thread-safe" (or event-loop safe). If you run multiple PSGI apps in parallel, in our case with AnyEvent/Danga::Socket based event-loop server or with Coro server and if the app does streaming output (rather than direct response), global resetting and sharing of %ENV might cause issues. (Well, actually this might be problematic with the current patch as well though: local *ENV = $env gets reset when it goes out of scope -- same problem). -- Tatsuhiko Miyagawa
CC: MARKSTOS [...] cpan.org
Subject: Re: [rt.cpan.org #49943] Needs discussion: wish: PSGI support in CGI.pm
Date: Fri, 25 Sep 2009 13:24:23 +0900
To: bug-CGI.pm [...] rt.cpan.org
From: Tatsuhiko Miyagawa <miyagawa [...] gmail.com>
On Fri, Sep 25, 2009 at 11:16 AM, Tatsuhiko Miyagawa <miyagawa@gmail.com> wrote: Show quoted text
> The actual implementation for that would be: > > 1) in CGI::PSGI::new, save the current %ENV somewhere and populate > %ENV with PSGI's $env > 2) Override methods that reads from STDIN, like read_from_stdin and > read_from_client to read from $env->{'psgi.input'} > 3) Override header() method, or implement a different named method > (like psgi_header) to return $status and $headers_ref > 4) in DESTROY, restore the saved %ENV, and call CGI::initialize_globals()
Okay, I've done this: http://github.com/miyagawa/CGI-PSGI The implemenation is slightly modified, so PSGI $env is stored in the object and set to local *ENV whenever a method that touches %ENV is called. This might have a slight performance impact but I don't think it's really a problem. I also left $q->header as it was, and added a new $q->psgi_header method instead. This is not a big deal since app developers need to change wherever $q->header was called to do print(). The benefit of new implementation is, as stated in README and POD, that it doesn't require CGI.pm patched to support PSGI natively. Mark, let's remove CGI::PSGI from CGI.pm main repo as you suggested and use $CGI::PSGI variable's defined-ness to see if CGI.pm supports PSGI or not. I think I can ship this CGI::PSGI to CPAN, and then CGI.pm can individually support PSGI with $CGI::PSGI or use CGI qw(:psgi); and both can live on CPAN: application users can chose whichever they want to. a) If they don't want to upgrade CGI.pm and are fine with changing their code, use CGI::PSGI b) If they don't want to change their CGI.pm based code, then upgrade CGI.pm that fully supports PSGI By "fully supports PSGI" the current CGI.pm (my patch) is not complete since it only does the read side. The app should still update their code to do return [ $q->header, [ $body ] ] instead of print $q->header, $body; To enable that you need to tie STDOUT, which actually CGI::Emulate::PSGI generally solves, in which case CGI.pm doesn't really need to be patched. Hmm ... ;) -- Tatsuhiko Miyagawa
RT-Send-CC: miyagawa [...] bulknews.net
I'm not sure there's anything else to do here. Resolving.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.