344647 - checksetup.pl can create two database connections instead of one

Status: RESOLVED FIXED

(Bugzilla :: Installation & Upgrading, defect)

Description

[Max Kanat-Alexander]

Instead of using Bugzilla->dbh to connect to the database, the first time checksetup connects it uses Bugzilla::DB::connect_main(). It *really* shouldn't do this. This means that if anybody uses "Bugzilla->dbh" in checksetup.pl, that's actually a *different* database object! This can cause all sorts of trouble, including messing up the Schema (because one object will have a different Schema than another).

Comment 1

[Max Kanat-Alexander]

Attachment: v1

Okay, here's a one-line patch that fixes it.

Thankfully, this bug doesn't affect us at all, yet, because we don't use Bugzilla->dbh inside of a subroutine or anything. But it could affect any future patch, and it can cause havoc with customizations.

In fact, twice I've spent hours trying to track down this exact problem, when tracking down issues in custom code. So this time I'm patching it. :-)

Comment 2

[Frédéric Buclin]

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

Looks good, but I didn't test with a fresh install. But I don't see how this could be wrong. r=LpSolit

2nd review still required, just to be sure I'm not missing something.

Comment 3

[Olav Vitters]

Patch was checked in as part of bug 346375 and works ok.

*** This bug has been marked as a duplicate of 346375 ***

Comment 4

[Olav Vitters]

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

Cancelling review as patch was checked in.

Comment 5

[Olav Vitters]

I missed the target milestone :-(

Comment 6

[Olav Vitters]

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

r=bkor

tested on 2.20: existing+empty db
tested on 2.22: existing+empty db+upgrade from 2.20

Comment 7

[Frédéric Buclin]

don't you want to commit this one?

Comment 8

[Max Kanat-Alexander]

Ah yes, I do! I missed it because it was only approved for the branch. :-)

2.22:

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.469.2.11; previous revision: 1.469.2.10
done

Comment 9

[Frédéric Buclin]

This patch didn't land on the 2.20 branch. Either do it or retarget the bug to 2.22 and remove the approval2.20+ flag.

Comment 10

[Max Kanat-Alexander]

Oh, you're right! Thanks.

2.20:

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.412.2.24; previous revision: 1.412.2.23
done