343338 - Eliminate "my" variables from the root level of modules

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, enhancement)

Description

[Max Kanat-Alexander]

Certain modules have variables declared with "my" outside of any subroutine.

This is fine in CGI, but in mod_perl it could get weird.

Under mod_perl, "my" variables are only defined once, at the time Apache starts up. After that they're still defined, but they stick around. They live forever and never get reset.

So it's just safer to use constants, or something else, or just get rid of the variables if possible.

Comment 1

[Max Kanat-Alexander]

Attachment: v1

Just to make it clear, my intention with this bug is to make mod_cgi and mod_perl behave identically when it comes to these variables.

I did leave around the "my $columns" variables in certain bugs. Those will go away when we make those .pm files use Bugzilla::Object. In the mean time, it would have been a hassle to eliminate them, and it's not going to cause a big issue.

In fact, most of these would have been fine if left alone, but there's a chance that in the future somebody would come along and make some change, and then there would be a mysterious bug that only showed up on mod_perl, and not in mod_cgi. Or some customizer would have a problem that they couldn't figure out.

Comment 2

[Max Kanat-Alexander]

Attachment: v1.1

There was some bitrot; I fixed it.

Comment 3

[Frédéric Buclin]

Comment on attachment 228884 [details] [diff] [review]
v1.1

Looks good. Works correctly on a non-mod_perl installation. r=LpSolit

Comment 4

[Frédéric Buclin]

Comment on attachment 228884 [details] [diff] [review]
v1.1

>Index: Bugzilla/Template.pm

>+sub _load_constants {
>+    use Bugzilla::Constants ();

Nit: just one thing: it doesn't make sense to use it in a routine AFAIK.

Comment 5

[Max Kanat-Alexander]

Great. This puts mod_perl into a testable condition.

Checking in Bugzilla/BugMail.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/BugMail.pm,v  <--  BugMail.pm
new revision: 1.84; previous revision: 1.83
done
Checking in Bugzilla/Config.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Config.pm,v  <--  Config.pm
new revision: 1.65; previous revision: 1.64
done
Checking in Bugzilla/FlagType.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/FlagType.pm,v  <--  FlagType.pm
new revision: 1.31; previous revision: 1.30
done
Checking in Bugzilla/Template.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Template.pm,v  <--  Template.pm
new revision: 1.56; previous revision: 1.55
done
Checking in Bugzilla/Token.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Token.pm,v  <--  Token.pm
new revision: 1.46; previous revision: 1.45
done
Checking in Bugzilla/Search/Quicksearch.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search/Quicksearch.pm,v  <--  Quicksearch.pm
new revision: 1.11; previous revision: 1.10
done