121747 - query.cgi doesn't check for shutdownhtml

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, defect, P1)

Description

[Dave Miller [:justdave]]

Is this a regression because of the template conversion?  Are any other
templatized pages affected?

Comment 1

[Bradley Baetz (:bbaetz)]

Probably, because all the template stuff reads before anything happens.

We need a global func: checkForShutdown() which handles this for us.

Comment 2

[Jacob Steenhagen]

IIRC, we check for shutdownhtml in PutHeader().  It was done there because then
it only required a few lines of code change rather than changing *.cgi.  This
wasn't a really big deal originally, as all the .cgi's called PutHeader() [most
of them before doing anything critical, see bug 121741 for an exception].  As we
move to templates, this line of thinking changes in that you want to gather all
your data before you output anything.  Also, I'm pretty sure that the templates
don't use PutHeader() anymore.  I know they all include global/header, but I
highly doubt that checks for shutdownhtml and does the right thing.

So, it appears that Bradley is correct and we need to new global routine [such
as checkForShutdown()] that gets called at the top of every .cgi...  As we start
moving toward having Bugzilla be able to output data formats other than HTML
(such as xml.cgi and the patch in bug 103778) we need to be careful that we
output errors appropriate to the format in question, not just generically spit
out some HTML that says Bugzilla is down.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Why not do this in CGI.pl?  That script gets required by every CGI script, so we
could put a check at the top of it and stop every script dead in its tracks if
Bugzilla is currently shut down.

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: moves shut down message display to beginning of CGI.pl

Comment 5

[Myk Melez [:myk] [@mykmelez]]

Reassigning to myself.

Comment 6

[Myk Melez [:myk] [@mykmelez]]

Accepting.

Comment 7

[Dave Miller [:justdave]]

Comment on attachment 66925 [details] [diff] [review]
patch v1: moves shut down message display to beginning of CGI.pl

Looks like you have the attachment tracker patch mixed in with this...

Comment 8

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v2: same as v1 but without extra cruft from another patch

Comment 9

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v3: uses global variables instead of "our" declaration

This version removes the "our" variable declarations that Perl 5.005 doesn't
support and substitutes good ole "$::varname"s.

Comment 10

[Gervase Markham [:gerv]]

Comment on attachment 66959 [details] [diff] [review]
patch v3: uses global variables instead of "our" declaration

r=gerv. This would be very good to have :-)

Gerv

Comment 11

[Christian Reis]

Comment on attachment 66959 [details] [diff] [review]
patch v3: uses global variables instead of "our" declaration

r=kiko 

except for the Stash functions (they look ok, I just don't understand what they
are for)

The rest of the code should simplify work.

You can mark 2r= if you think it's ok to land (I do)

Comment 12

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 66959 [details] [diff] [review]
patch v3: uses global variables instead of "our" declaration

Apparently I kiko's r= if I agree with him that he should r= it.  I do agree
with about that, so checking it in.

Checking in CGI.pl;
/cvsroot/mozilla/webtools/bugzilla/CGI.pl,v  <--  CGI.pl
new revision: 1.130; previous revision: 1.129
done
Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.135; previous revision: 1.134
done
Checking in template/default/global/message.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/default/global/message.html.tmpl,v 
<--  message.html.tmpl
initial revision: 1.1
done
Checking in template/default/global/header;
/cvsroot/mozilla/webtools/bugzilla/template/default/global/header,v  <-- 
header
new revision: 1.5; previous revision: 1.4
done

Comment 13

[Myk Melez [:myk] [@mykmelez]]

Fix checked in.  Resolving fixed.

Comment 14

[Gervase Markham [:gerv]]

So, what's the officially-sanctioned way of using templates in a file now, then?
Here's my guess:

use vars qw($template $vars);
...
$vars->{'foo'} = \@bar;

Is that right? 

I'd very much like to avoid having to do a global s/$vars/$::vars/, if possible. 

Gerv

Comment 15

[Christian Reis]

Clarifying my drunken comment: I just didn't figure out if the Stash part did
what was expected; the code looked correct and good, so if Stash works r=kiko. :)

Comment 16

[Myk Melez [:myk] [@mykmelez]]

>I'd very much like to avoid having to do a global s/$vars/$::vars/, if possible. 

Sorry, but we had to revert to $::vars for compatibility with Perl 5.005 ("our"
was introduced in 5.6).  This nightware will all go away in modularization.  In
the meantime, squeeze your eyes shut and repeat after me: there's no place like
home, there's no place like home. ;-)

Comment 17

[Gervase Markham [:gerv]]

Will 

my $vars = $::vars;
my $template = $::template;

work? That would solve a lot of problems.

Gerv

Comment 18

[Myk Melez [:myk] [@mykmelez]]

>Will
>
>my $vars = $::vars;
>my $template = $::template;
>
>work? That would solve a lot of problems.

This probably won't work if you need to process multiple templates without the
variables from earlier templates polluting the variable space of later
templates, since $::vars (and $::template) are references to a hash and an
object, respectively.  Otherwise, they probably will work, but what problems do
$::vars and $::template have that creating local references will solve?

Comment 19

[Gervase Markham [:gerv]]

> Otherwise, they probably will work, but what problems do
> $::vars and $::template have that creating local references will solve?

a) They prevent a global search and replace
b) They look far, far nicer :-)

Gerv