Closed
Bug 302876
Opened 19 years ago
Closed 18 years ago
Database Version-Checking needs to be more modular and more generic
Categories
(Bugzilla :: Installation & Upgrading, enhancement)
Tracking
()
RESOLVED
FIXED
Bugzilla 3.0
People
(Reporter: mkanat, Assigned: mkanat)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
mkanat
:
review+
|
Details | Diff | Splinter Review |
Right now, when we check the version of the database in checksetup.pl, it's
pretty much a big hack. It works for MySQL and PostgreSQL, but it probably won't
work for other, future databases.
Also, it would be nice to be able to have the version-checking code do more
advanced things for certain databases. For example, Pg7 needs a different
version of DBD::Pg than Pg8 does, and checksetup should be able to detect that
and inform the user.
Assignee | ||
Updated•19 years ago
|
Assignee | ||
Comment 1•18 years ago
|
||
Okay, so what we do is we move all the data about DBD versions into Bugzilla::Constants.
Then, when we run checksetup pre-localconfig, we check for all possible DBDs, and only throw an error if *none* of them exist.
Then, right before we actually connect to the DB, we check if the *correct* DBD exists.
Assignee | ||
Updated•18 years ago
|
Attachment #222090 -
Flags: review?(wicked+bz)
Attachment #222090 -
Flags: review?(colin.ogilvie)
Attachment #222090 -
Flags: review?
Assignee | ||
Updated•18 years ago
|
Target Milestone: --- → Bugzilla 2.24
Comment 2•18 years ago
|
||
Comment on attachment 222090 [details] [diff] [review]
v1
Please fix smallish errors and nits listed below before commit.
I do hope we have a bug open about die's emitting HTML code to the screen..
>Index: checksetup.pl
>===================================================================
>+print "\nYou need one of the following DBD modules installed, depending on\n"
>+ , "which database you are using with Bugzilla:\n";
"unless $silent" missing so this is shown even in silent mode.
And what's with the lonely , there? You probably were trying to have a . and on previous line, right?
>@@ -433,8 +444,23 @@
>+if (!$have_one_dbd) {
> print "\n\n";
Nit: This print is not needed and causes extra blanks.
>+if (%missing) {
>+ print "\n";
Nit: This print is not needed and causes extra blanks.
If you want extra blank lines because that way users might see these errors more clearly then I suggest instead to fix the exit block to explicitly print a warning that user needs to fix above errors and rerun checksetup.
>+ die "$my_db_driver is not a valid choice for \$db_driver in",
>+ " localconfig.";
Remove dot after localconfig as die adds script name and line after it.
>+ print "For $sql_server, Bugzilla requires that perl's"
>+ . " $actual_dbd be installed.\nTo install this module,"
>+ . " you can do:\n " . install_command($actual_dbd);
Add newline after install_command call.
>Index: Bugzilla/DB.pm
>===================================================================
>@@ -1558,5 +1535,6 @@
> L<DBI>
>+L<Bugzilla::Constants> - The C<DB_MODULE> constant.
No newline between these entries in output so looks very confusing.
Attachment #222090 -
Flags: review?(wicked+bz) → review+
Assignee | ||
Comment 3•18 years ago
|
||
Okay, I fixed all of the nits, except for the extra newlines. Those are there on purpose, because they help make the output easier to read when there are multiple errors printed by checksetup. They were there before, also.
Attachment #222090 -
Attachment is obsolete: true
Attachment #223414 -
Flags: review+
Attachment #222090 -
Flags: review?(colin.ogilvie)
Assignee | ||
Updated•18 years ago
|
Flags: approval?
Updated•18 years ago
|
Flags: approval? → approval+
Assignee | ||
Comment 4•18 years ago
|
||
Thanks, everybody! I'm happy to have this in. :-)
Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v <-- checksetup.pl
new revision: 1.480; previous revision: 1.479
done
Checking in Bugzilla/Constants.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Constants.pm,v <-- Constants.pm
new revision: 1.39; previous revision: 1.38
done
Checking in Bugzilla/DB.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB.pm,v <-- DB.pm
new revision: 1.72; previous revision: 1.71
done
Checking in Bugzilla/DB/Mysql.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Mysql.pm,v <-- Mysql.pm
new revision: 1.37; previous revision: 1.36
done
Checking in Bugzilla/DB/Pg.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Pg.pm,v <-- Pg.pm
new revision: 1.20; previous revision: 1.19
done
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 5•18 years ago
|
||
*** Bug 300431 has been marked as a duplicate of this bug. ***
You need to log in
before you can comment on or make changes to this bug.
Description
•