599539 - Update the mod_perl.pl code for Apache2::SizeLimit 0.92

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect)

Description

[Max Kanat-Alexander]

In SizeLimit 0.92, the syntax for Apache2::SizeLimit changed. The old syntax remains for backwards-compatibility, so we don't have to update old branches, but we should update our latest code and require 0.92.

Also, the docs seem to indicate that Apache2::SizeLimit now works on Windows.

Comment 1

[Max Kanat-Alexander]

I ran into this bug while I was working on this:

  https://rt.cpan.org/Ticket/Display.html?id=61622

Comment 2

[Max Kanat-Alexander]

Attachment: v1

This should be all that's required. Not asking for review yet, because I'm waiting for the bug report to be addressed.

Comment 3

[Max Kanat-Alexander]

The SizeLimit maintainers have fixed the bug and it will be released as part of 0.93.

Comment 4

[Frédéric Buclin]

(In reply to comment #3)
> The SizeLimit maintainers have fixed the bug and it will be released as part of
> 0.93.

0.93 has been released 2 months ago.

Comment 5

[Max Kanat-Alexander]

Comment on attachment 478472 [details] [diff] [review]
v1

All right, then this patch is ready.

Comment 6

[Frédéric Buclin]

(In reply to comment #0)
> but we should update our latest code and require 0.92.

Shouldn't your patch also require 0.92?

Comment 7

[Max Kanat-Alexander]

Ah, probably yes it should. :-)

Comment 8

[Max Kanat-Alexander]

Attachment: v2

Comment 9

[:glob ✱]

Comment on attachment 495510 [details] [diff] [review]
v2

The docs state that in order for this to work on windows, Win32::API needs to be installed.  If we're to include support for Windows with this patch, we also need to ensure that module is installed.

Comment 10

[Max Kanat-Alexander]

Comment on attachment 495510 [details] [diff] [review]
v2

Actually, Bugzilla already requires Win32::API in order for checksetup to even run. (See Bugzilla::Install::Util.) We can't even check requirements safely on Windows without having Win32::API already installed.

Comment 11

[:glob ✱]

> Actually, Bugzilla already requires Win32::API in order for checksetup to even
> run. (See Bugzilla::Install::Util.)

the code in Bugzilla::Install::Util treats Win32::API as optional:

  # Win32::API ships with ActiveState by default, though there could 
  # theoretically be a Windows installation without it, I suppose.
  if (ON_WINDOWS and eval { require Win32::API }) {

Win32::API isn't referenced anywhere else.

i'd be happy to just make Win32::API mandatory :)

Comment 12

[Max Kanat-Alexander]

Attachment: v3

Ah, you're totally right. Okay, here's a patch that adds Win32::API to the optional requirements if we're on Windows. I figure we don't need to make it mandatory unless it really does become mandatory for us.

Comment 13

[:glob ✱]

Comment on attachment 497293 [details] [diff] [review]
v3

r=glob

Comment 14

[Max Kanat-Alexander]

Awesome, thanks!

Comment 15

[Max Kanat-Alexander]

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified mod_perl.pl
modified Bugzilla/Install/Requirements.pm
modified template/en/default/pages/release-notes.html.tmpl
Committed revision 7636.

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/4.0/
modified mod_perl.pl
modified Bugzilla/Install/Requirements.pm
modified template/en/default/pages/release-notes.html.tmpl
Committed revision 7503.