deprecate PHP3Script and PHPScript in twisted.web.twcgi, replace them with documentation for FilteredScript

[Jerub]

/usr/bin/php4 in debian unstable is the 'cli' version, a misguided product of a
push to see php in areas such as the command line.

the location of the cgi binary is in /usr/lib/cgi-bin/php4 - and in order for
the CGI version to function correctly, the env variable 'REDIRECT_STATUS' must
be '200'.

This patch probably isn't *optimal*, and will likely break other deployments
where /usr/bin/php4 is the cgi version and the other path doesn't actually
exist, but it demonstrates exactly what is required at a code level for this to
function on debian unstable.

[itamarst]

I suggest this patch get applied to debian packaged versions of Twisted when
they get generated.

[exarkun]

I suppose it would be good to have support for PHP4 CGIs on Debian. Did anyone contact Debian to discuss this issue? Also, couldn't we check for the existence of various executables instead of just assuming /usr/bin/php4 exists and is usable?

In any case, this isn't high priority, and the change needs test coverage.

[exarkun]

PHP3Script and PHPScript are nothing except platform-specific configuration. If we can't get this right, we shouldn't have it. We can have documentation which explains how to trivially create one of these which is correct for a particular location of the php cgi interpreter.

[exarkun]

This fruit hangs low.

[jesstess]

(In [28136]) Branching to 'phpscript-deprecation-516'

[jesstess]

(In [28146]) Deprecate PHPScript and PHP3Script and add unit tests for the deprecation.

refs #516

[jesstess]

I also added rather mundane docstrings for FilteredScript.runProcess and CGIScript.runProcess; more interesting descriptions are welcome.

[exarkun]

Thanks! The docstring additions look good. A couple other things:

• There's one pyflakes warning for the module, an unused local in CGIScript.render. Can you fix that?
• I think a brief section in the prose documentation for CGIScript and FilteredScript would be a good addition as well. I want to suggest adding it to using-twistedweb.xhtml, but it also occurs to me that it could be a section in the new sixty seconds documentation except that that branch hasn't been merged yet. Blocking this ticket on the sixty seconds ticket sounds lame. I do want to push us towards writing more prose documentation, though. Maybe I should do a sixty seconds post on twcgi and we can add it to the sixty seconds branch (or to another ticket after the sixty seconds branch is merged). (Sorry for the rambling non-committal review point.)

[jesstess]

(In [28172]) Remove unused local variable and clean up surrounding doc strings.

refs #516

[jesstess]

Ticket #4273 was opened to address exarkun's second review comment.

[exarkun]

Oops, I missed some test failures before. Looks like the new __init__ methods aren't compatible with the inherited __init__:

===============================================================================
[ERROR]: twisted.web.test.test_tap.ServiceTests.test_php3Processor

Traceback (most recent call last):
  File "/srv/bb-slave/Run/slave/reactors/Twisted/twisted/web/test/test_tap.py", line 80, in test_php3Processor
    self.assertIsInstance(root.getChild("foo.php3", None), PHP3Script)
  File "/srv/bb-slave/Run/slave/reactors/Twisted/twisted/web/static.py", line 311, in getChild
    return resource.IResource(processor(fpath.path, self.registry))
exceptions.TypeError: __init__() takes exactly 1 argument (3 given)
===============================================================================
[ERROR]: twisted.web.test.test_tap.ServiceTests.test_phpProcessor

Traceback (most recent call last):
  File "/srv/bb-slave/Run/slave/reactors/Twisted/twisted/web/test/test_tap.py", line 90, in test_phpProcessor
    self.assertIsInstance(root.getChild("foo.php", None), PHPScript)
  File "/srv/bb-slave/Run/slave/reactors/Twisted/twisted/web/static.py", line 311, in getChild
    return resource.IResource(processor(fpath.path, self.registry))
exceptions.TypeError: __init__() takes exactly 1 argument (3 given)
-------------------------------------------------------------------------------

One other thing. In the class docstring for FilteredScript, I think the sentence about customizing filter can be a bit broader - ie, this works for things other than PHP. :)

Thanks!

[jesstess]

(In [28177]) Make tap tests that check warnings be aware of the new PHPScript and PHP3Script warnings.

refs #516

[jesstess]

(In [28179]) FilteredScript is for more than PHP.

refs #516

[exarkun]

Hmm. I'm wondering about the warnings that get emitted by twistd -n web --path ... now. It'd be nice to avoid those.

[thijs]

See previous comment.

[jesstess]

(In [28380]) Delay calling PHP3Script and PHPScript in tap.Options.opt_path to avoid displaying deprecation warnings on something like twistd -n web --path .

refs #516

[exarkun]

1. test_tap.py has its copyright date bumped, but no other changes. This should be reverted.
2. test_PHP3ScriptIsDeprecated and test_PHPScriptIsDeprecated use getattr. Instead, they can just do twcgi.PHP3Script and twcgi.PHPScript.
3. 10.0 has sailed. Update the deprecation version numbers to 10.1.
4. As long as you're reformatting the CGIScript class docstring, can you switch it away from the I ... style? Something like L{CGIScript} is a resource which runs child processes according to the CGI specification. would be nice.

That's all. Please merge when the above points are addressed. Thanks.

[jesstess]

(In [28558]) Don't use getattr; we're up to 10.1.

refs #516

[jesstess]

(In [28559]) Merge phpscript-deprecation-516

Author: jesstess Reviewer: exarkun Fixes: #516

twisted.web.twcgi.PHP3Script and twisted.web.twcgi.PHPScript are now deprecated in favor of twisted.web.twcgi.FilterScript.