286672 - Require correct DBD depending on the $db_driver in use

Status: RESOLVED FIXED

(Bugzilla :: Installation & Upgrading, enhancement, P2)

Description

[Max Kanat-Alexander]

Basically, we just need to move the DBD check down to around where we connect to
the database, so that we can appropriately check for DBD::Pg or DBD::mysql.

Comment 1

[Max Kanat-Alexander]

Oh, for the record, we require DBD::Pg 1.31 at the moment.

Comment 2

[Dave Miller [:justdave]]

Heh.  So we're requiring people using Postgres to have DBD::mysql installed, eh?
 Yeah, need to fix that :)

Comment 3

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

"If it's not a regression from 2.18 and it's not a critical problem with
something that's already landed, let's push it off." - Dave

Comment 4

[Max Kanat-Alexander]

This is a critical problem with something that's already landed. Why did it not
only have the blocking flag cleared, but have it set to *MINUS*, so I can't even
re-request blocking? 

Comment 5

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

(In reply to comment #4)
> This is a critical problem with something that's already landed.

Could you explain further?  The summary, description, and severity fields all
make it look like a minor issue.


> Why did it not only have the blocking flag cleared, but have it set to
*MINUS*, so I can't even re-request blocking?

Minus is better than cleared because it states intent.  Your inability to
re-request blocking is bug 291391, a fix for which has been checked in and will
appear on b.m.o as soon as I find out from Dave if I need to do anything special
for that.  In the meantime I've cleared the flag so you can re-request blocking.

Comment 6

[Tomas Kopal]

(In reply to comment #5)
> (In reply to comment #4)
> > This is a critical problem with something that's already landed.
> 
> Could you explain further?  The summary, description, and severity fields all
> make it look like a minor issue.

It's a minor to no issue for users running on MySQL, it's almost a showstopper
for running on Postgres. So if we want Postgres support (and we are *very*
close), we need this done.

Comment 7

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

> it's almost a showstopper for running on Postgres.

How so?

Comment 8

[Tomas Kopal]

(In reply to comment #7)
> > it's almost a showstopper for running on Postgres.
> 
> How so?

Well, how will a user feel if checksetup first complains about missing
DBD::Mysql module, which you know you don't need, and then, after checksetup is
happy about all modules, it fails with mysterious DB errors? (due to missing
DBD::Pg)

Comment 9

[Tomas Kopal]

Sorry Max, I couldn't find enough time to work on this, so it's going back to you.

Comment 10

[Max Kanat-Alexander]

Attachment: A hack for 2.20

Hey Kiko, welcome to the wonderful world of Bugzilla::DB. :-)

There were all kinds of problems in making this patch, mainly that we need
information from the Bugzilla::DB::<something> module, but we can't load it
because we don't yet know if we have the right DBD module for it.

This hack should work for 2.20. It's not a terrible hack; it might even keep
working beyond 2.20. :-)

Doing more than what I have here is almost impossible at this point, because it
would require a big reorganization of checksetup (the version-checking code is
all over the place, right now, and needs to be moved into a module), and that's
not something I want to do in a freeze.

Anyhow, what we do is we first check if we can load the module at all, and then
we connect to the datbase, and *then* we check the version of the DBD driver.

This means that in the rare case that somebody's DBD is so old that we can't
even connect, we'll fail. Once again, this is something that we can fix when we
don't have to live in hack-land. :-)

Comment 11

[Dave Miller [:justdave]]

blocks a blocker, so it's a blocker

Comment 12

[Frédéric Buclin]

Comment on attachment 183062 [details] [diff] [review]
A hack for 2.20

>+    # Unfortunately, $my_db_driver doesn't map perfectly between DBD
>+    # and Bugzilla::DB. We need to fix the case a bit.
>+    (my $dbd_name = trim($my_db_driver)) =~ s/(\w+)/\u\L$1/g;
>+    # And MySQL is special, because it's all lowercase in DBD.
>+    $dbd_name = 'mysql' if $dbd_name eq 'Mysql';

Nit (for 2.20): MySQL is not so special, see below:

DBD/XBase.pm  DBD/SQLite2.pm  DBD/SQLite.pm  DBD/PgSPI.pm  DBD/Pg.pm 
DBD/CSV.pm  DBD/mysql.pm  DBD/ExampleP.pm  DBD/DBM.pm  DBD/NullP.pm 
DBD/Sponge.pm

These are some examples I could find on my system. Maybe Bugzilla should be
consistent with these names so that =~ s/(\w+)/\u\L$1/g; would not be required
anymore.

Anyway, the patch works fine. r=LpSolit

Comment 13

[Max Kanat-Alexander]

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.402; previous revision: 1.401
done
Checking in Bugzilla/DB.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB.pm,v  <--  DB.pm
new revision: 1.54; previous revision: 1.53
done
Checking in Bugzilla/DB/Mysql.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Mysql.pm,v  <--  Mysql.pm
new revision: 1.21; previous revision: 1.20
done
Checking in Bugzilla/DB/Pg.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Pg.pm,v  <--  Pg.pm
new revision: 1.9; previous revision: 1.8
done

Comment 14

[Max Kanat-Alexander]

Added to the release notes in bug 286274, as a note that if your DBI is really
old, we might just die with a strange error message, and people should try
upgrading their DBI in that case.