globals.pl and CGI.pl emit "subroutine redefined" messages

RESOLVED FIXED in Bugzilla 2.18

Status

()

defect
P2
trivial
RESOLVED FIXED
19 years ago
7 years ago

People

(Reporter: bugzilla, Assigned: kiko)

Tracking

unspecified
Bugzilla 2.18
Bug Flags:
approval +

Details

Attachments

(1 attachment, 4 obsolete attachments)

globals.pl and bug_form.pl emit "subroutine redefined" messages, and a huge
amount of other stuff when run.
Posted patch proposed patch (obsolete) — Splinter Review
This is caused by RelationSet.pm being used instead of required, as
RelationSet.pm uses these files, causing them to be run twice. This was
cluttering up the logs, and was not "perfect code". joy.
Keywords: patch
Keywords: review
Target Milestone: --- → Bugzilla 2.16
Priority: -- → P2
-> New Bugzilla Product
Assignee: tara → justdave
Component: Bugzilla → Bugzilla-General
Product: Webtools → Bugzilla
Version: other → unspecified
If you can tell me how to reproduce this I will gladly r= this harmless change.
Summary: sub redefined → globals.pl and bug_form.pl emit "subroutine redefined" messages
"perl -Wall -c bug_form.pl" will complain about confess croak and carp being
redefined. That's a symptom of this.

Also, when bug_form.pl is requested, the "subroutine redefined" error message
should show up in Apache's error log.
This patch is stale. Could you update it to the latest CVS bugzilla and repatch?
I'm getting:

bash-2.03# perl5.6.1 -Wall bug_form.pl
[Sun Oct  7 19:04:03 2001] bug_form.pl: Subroutine confess redefined at (eval 3)
line 1.
[Sun Oct  7 19:04:03 2001] bug_form.pl: Subroutine croak redefined at (eval 3)
line 2.
[Sun Oct  7 19:04:03 2001] bug_form.pl: Subroutine carp redefined at (eval 3)
line 3.
Content-type: text/html


<H1>Software error:</H1>
<PRE>Undefined subroutine &amp;main::quietly_check_login called at bug_form.pl
line 48.
</PRE>
<P>
For help, please send mail to this site's webmaster, giving this error message 
and the time and date of the error.


[Sun Oct  7 19:04:03 2001] bug_form.pl: Undefined subroutine
&main::quietly_check_login called at bug_form.pl line 48.

If you don't someday I might fix this :-)
While it is possible to fix the "huge amount of other stuff", which HAS been 
fixed, you can't fix those subroutine redefined warnings short of something 
spectacular.

A bit of debugger work shows that the warning are being thrown by CGI::Carp

CGI::Carp specifically uses Carp to initially create confess croak and carp, 
and then overwrites it shortly afterwards. The code that is causing the 
warnings is listed here for posterity...

# Avoid generating "subroutine redefined" warnings with the following
# hack:
{
    local $^W=0;
    eval <<EOF;
sub confess { CGI::Carp::die Carp::longmess \@_; }
sub croak   { CGI::Carp::die Carp::shortmess \@_; }
sub carp    { CGI::Carp::warn Carp::shortmess \@_; }
sub cluck   { CGI::Carp::warn Carp::longmess \@_; }
EOF
    ;
}

So for some reason the hack isn't working...
I'd suggest that the only way that you can fix this is to not use CGI::Carp...

CGI::Carp is mentioned on the following lines

bugzilla\Bug.pm(34): use CGI::Carp qw(fatalsToBrowser);
bugzilla\CGI.pl(49): use CGI::Carp qw(fatalsToBrowser);
bugzilla\RelationSet.pm(42): use CGI::Carp qw(fatalsToBrowser);
bugzilla\checksetup.pl(196): # If CGI::Carp was loaded successfully for version 
checking, it changes the
bugzilla\checksetup.pl(201): unless (have_vers("CGI::Carp",0))    { push 
@missing,"CGI::Carp" }
bugzilla\temp.pl(3): use CGI::Carp qw{fatalsToBrowser};


Now, given it's called in modules, which it really shouldn't... you can't do 
something like

BEGIN {
    if ( $developermode ) {
        eval { use CGI::Carp qw{fatalsToBrowser} };
    }
}

Which would only use it if you had some developer flag set...

Just to enforce that it's CGI::Carp doing it, try this

legba:/mnt/CVS/bugzilla# perl -Wall -e 'use CGI::Carp qw{fatalsToBrowser}'
[Sat Nov  3 01:03:36 2001] (eval 1): Subroutine confess redefined at (eval 1) 
line 1.
[Sat Nov  3 01:03:36 2001] (eval 1): Subroutine croak redefined at (eval 1) 
line 2.
[Sat Nov  3 01:03:36 2001] (eval 1): Subroutine carp redefined at (eval 1) line 
3.
legba:/mnt/CVS/bugzilla#

SO, in conclusion, your three options are

1) RESOLVED WONTFIX - Live with it
2) Stop using CGI::Carp qw{fatalsToBrowser};
3) Make your own version of CGI::Carp that doesn't throw that error

But if the CGI::Carp author can't stop the warnings, I don't think I want to go 
there....
All we really want from CGI::Carp here is the FatalsToBrowser override...  and
that's a trivial thing to create on our own just by hooking into $::SIG{__DIE__}
and $::SIG{__WARN__}.  My vote is to remove our dependency on CGI::Carp and do
our own routines to output the errors to the browser (and/or format them for web
logs).
Posted file CGI::FatalsToBrowser (obsolete) —
Added CGI::FatalsToBrowser

This needs to be added as bugzilla/CGI/FatalsToBrowser.pm

It's basically just CGI::Carp with the Following changes

1) NameSpace CGI::Carp -> CGI::FatalsToBrowser
2) use Carp  ->  BEGIN { require Carp; }
3) Removed the hack in my comment above
4) Move fatalsToBrowser from @EXPORT_OK to @EXPORT
5) Removed all that bloaty documentation, and replaced with a reference to 
CGI::Carp

I'll a patch to use the module instead of CGI::Carp shortly
Attachment #24422 - Attachment is obsolete: true
Posted patch Proposed patch (obsolete) — Splinter Review
Added patch to change to the new module

We only need to use the module in CGI.pl, since anything that is CGI will be 
using CGI.pl... right?

You should probably test this properly but it should be ok

perl -Wall -e 'use CGI::FatalsToBrowser; die "Blah";'

Status: NEW → ASSIGNED
OK

I've submitted a patch to Lincoln Stein to fix the subroutine redefined problem 
in the new version of CGI::Carp itself on CPAN, so in time this bug will go 
away. It should come in with version 2.79 of the CGI.pm package.

So... the patch is somewhat un-nescesary, although we might want to use it to 
put some other stuff to happen when something dies...
We are currently trying to wrap up Bugzilla 2.16.  We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut.  Thus this is being retargetted at 2.18.  If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.
Target Milestone: Bugzilla 2.16 → Bugzilla 2.18
Okie dokie.

Fix for this has been applied to the CPAN version of CGI::Carp.

This bug can be fixed by changing the required version of the CGI distribution 
to 2.79.
Attachment #56354 - Attachment is obsolete: true
Attachment #56358 - Attachment is obsolete: true
Adding patch to checksetup.pl to make sure the right version of CGI::Carp is on
the system.
This is right to go.

I'm guessing we will need to add the version requirement to Bundle::Bugzilla 
though right?

Since it's more likely someone will have an out of date and not just missing 
module now, I've added Bug 118953 to fix the message only mentioning missing 
modules, and not old.
Is the version really 1.22?
So is the only fix here for the later CGI version? We don't need the other patches?

Bundle::Bugzilla can be ignored; zach updates it just before a release.
Attachment #64129 - Flags: review?
Has this been fixed through other bugs? The fix for bug 147833 changed 
checksetup from checking CGI:Carp to checking for CGI v2.88, and the fix for 
bug 201816 pushed the check to v2.93.
Comment on attachment 64129 [details] [diff] [review]
Patch for checksetup.pl instead of CGIs

Patch has rotted, cancelling review request.
Attachment #64129 - Flags: review?
On CVS HEAD:

kiko@anthem:~/devel/bugzilla$ perl -Wall -c globals.pl 
[Wed Oct 29 14:23:28 2003] (eval 3): Subroutine confess redefined at (eval 3)
line 1.
[Wed Oct 29 14:23:28 2003] (eval 3): Subroutine croak redefined at (eval 3) line 2.
[Wed Oct 29 14:23:28 2003] (eval 3): Subroutine carp redefined at (eval 3) line 3.
[Wed Oct 29 14:23:28 2003] globals.pl: Name "main::SqlStateStack" used only
once: possible typo at globals.pl line 42.
globals.pl syntax OK

kiko@anthem:~/devel/bugzilla$ perl -Wall -c CGI.pl       
[Wed Oct 29 14:25:56 2003] (eval 2): Subroutine confess redefined at (eval 2)
line 1.
[Wed Oct 29 14:25:56 2003] (eval 2): Subroutine croak redefined at (eval 2) line 2.
[Wed Oct 29 14:25:56 2003] (eval 2): Subroutine carp redefined at (eval 2) line 3.
[Wed Oct 29 14:25:56 2003] CGI.pl: Name "main::buffer" used only once: possible
typo at CGI.pl line 422.
[Wed Oct 29 14:25:56 2003] CGI.pl: Name "main::dontchange" used only once:
possible typo at CGI.pl line 45.
CGI.pl syntax OK

The first 3 warnings are probably caused by the Carp problem (how do I check my
version?). The latter problem seems to derive from leftovers. I'll attach the
patch, and someone tell me if it's the wrong approach.

We don't have a bug_form.pl anymore, btw.
Assignee: justdave → kiko
Status: ASSIGNED → NEW
I'm not sure if the previous patch should be applied -- how widespread is that
version of CGI::Carp?
Attachment #134394 - Flags: review?(myk)
Comment on attachment 134394 [details] [diff] [review]
Fix remaining warnings

In case you want to help me out, since you recently touched this one. ;)
Attachment #134394 - Flags: review?(caduvall)
A suggestion on perlmonks to check a module version
     perl -MSomeModule -e'print $SomeModule::VERSION'

Replacing SomeModule with CGI and/or CGI::Carp

The carp problems with redefining subroutines should've been fixed (looking at 
changelogs) in CGI::Carp version 1.22, and CGI bundle 2.79, as Adam said. We 
require more recent than that (2.93) in checksetup.pl.

I'll take a closer look at this when I get home.

Am I actually allowed to review? :)
Summary: globals.pl and bug_form.pl emit "subroutine redefined" messages → globals.pl and CGI.pl emit "subroutine redefined" messages
Comment on attachment 64129 [details] [diff] [review]
Patch for checksetup.pl instead of CGIs

Per comments, obsoleted.
Attachment #64129 - Attachment is obsolete: true
As a secondary reviewer, yes, I'd like your opinion.

I may have an old version of CGI::Carp on Woody, but checksetup runs without
complaining, so I'm not sure it's fixed for sure. Checking...

kiko@anthem:~$ perl -MCGI -e'print $CGI::VERSION'
2.752

kiko@anthem:~$ perl -MCGI::Carp -e'print $CGI::Carp::VERSION'
1.20kiko

and yes, checksetup requires a newer version :-) So we need no additional patch
if a newer version of CGI::Carp fixes it.
Status: NEW → ASSIGNED
Comment on attachment 134394 [details] [diff] [review]
Fix remaining warnings

Patch clears up errors, doesn't seem to cause any ill effects. The two lines of
silliness being removed are trivially fine - they were missed when other
cleanups were done (SqlStateStack, for example, stopped being used when
Bugzilla::DB was created).

r=chaduv
Attachment #134394 - Flags: review?(caduvall) → review+
Newer version of CGI/CGI::Carp does fix it.

[mozdev@hermit bugzilla]$ perl -MCGI -e'print $CGI::VERSION'
3.00
[mozdev@hermit bugzilla]$ perl -MCGI::Carp -e'print $CGI::Carp::VERSION'
1.26

[mozdev@hermit bugzilla]$ perl -Wall -c globals.pl
globals.pl syntax OK
[mozdev@hermit bugzilla]$ perl -Wall -c CGI.pl
CGI.pl syntax OK

That's with patch. Doesn't checksetup force you to up your CGI package? There
was some weirdness with the 2.7x version numbers, but around line 190 is some
code that should work around it.
See comment 27 -- it does complain when I run checksetup.pl.
I have no idea how this installation was performed!
Comment on attachment 134394 [details] [diff] [review]
Fix remaining warnings

Looks good, works.
Attachment #134394 - Flags: review?(myk)
Flags: approval?
Flags: approval? → approval+
/cvsroot/mozilla/webtools/bugzilla/CGI.pl,v  <--  CGI.pl
new revision: 1.209; previous revision: 1.208
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.248; previous revision: 1.247

Thanks!
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.