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

RESOLVED FIXED in Bugzilla 4.0

Status

()

defect
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: mkanat, Assigned: mkanat)

Tracking

Bugzilla 4.0
Bug Flags:
approval +
approval4.0 +

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

9 years ago
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.
(Assignee)

Comment 1

9 years ago
I ran into this bug while I was working on this:

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

Comment 2

9 years ago
Posted patch v1 (obsolete) — Splinter Review
This should be all that's required. Not asking for review yet, because I'm waiting for the bug report to be addressed.
Assignee: general → mkanat
Status: NEW → ASSIGNED

Updated

9 years ago
Blocks: 600708

Updated

9 years ago
No longer blocks: 600708
(Assignee)

Comment 3

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

Comment 4

9 years ago
(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.
(Assignee)

Comment 5

9 years ago
Comment on attachment 478472 [details] [diff] [review]
v1

All right, then this patch is ready.
Attachment #478472 - Flags: review?(LpSolit)
(Assignee)

Updated

9 years ago
Target Milestone: --- → Bugzilla 4.0

Comment 6

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

Shouldn't your patch also require 0.92?
(Assignee)

Comment 7

9 years ago
Ah, probably yes it should. :-)
(Assignee)

Comment 8

9 years ago
Posted patch v2 (obsolete) — Splinter Review
Attachment #478472 - Attachment is obsolete: true
Attachment #495510 - Flags: review?(LpSolit)
Attachment #478472 - Flags: review?(LpSolit)
(Assignee)

Updated

9 years ago
Attachment #495510 - Flags: review?(LpSolit) → review?(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.
Attachment #495510 - Flags: review?(glob) → review-
(Assignee)

Comment 10

9 years ago
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.
Attachment #495510 - Flags: review- → review?(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 :)
(Assignee)

Comment 12

9 years ago
Posted patch v3Splinter Review
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.
Attachment #495510 - Attachment is obsolete: true
Attachment #497293 - Flags: review?(glob)
Attachment #495510 - Flags: review?(glob)
Comment on attachment 497293 [details] [diff] [review]
v3

r=glob
Attachment #497293 - Flags: review?(glob) → review+
Flags: approval?
(Assignee)

Comment 14

9 years ago
Awesome, thanks!
Flags: approval? → approval+
(Assignee)

Updated

9 years ago
Flags: approval4.0+
(Assignee)

Comment 15

8 years ago
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.