Closed Bug 277502 Opened 21 years ago Closed 19 years ago

Re-organize checksetup.pl to be understood more easily (make it a short series of subroutines)

Categories

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

2.19.1
enhancement

Tracking

()

RESOLVED FIXED
Bugzilla 3.0

People

(Reporter: mkanat, Assigned: mkanat)

References

(Blocks 1 open bug)

Details

(Keywords: meta)

Attachments

(2 files)

The other morning I was thinking about how to make checksetup more logical and easy to understand without risking major code changes (since checksetup is hard to test when we change things). Basically, I think that the basic code of checksetup.pl should look like the classic main() function in an application, and just be a simple series of subroutines. Something like: check_versions() check_schema() create_admin() check_sanity() Or something like that. I'll have to investigate checksetup a bit more to get the actual list of functions that it will consist of. It will also make it a lot easier to futher improve the checksetup code, and move some things out of that script that don't need to be there.
I just spent a few hours making this. It's an English description, step-by-step, of what checksetup does, in detail. This should help a lot with any re-organization.
Blocks: bz-majorarch
Blocks: 160297
Depends on: 133169
Blocks: 133169
No longer depends on: 133169
Attchment 170860 looks like the right idea. I suggest that each line (numbered item) also become a subroutine as well as having a subroutine for each major category.
(In reply to comment #2) > Attchment 170860 looks like the right idea. I suggest that each line (numbered > item) also become a subroutine as well as having a subroutine for each major > category. Exactly my plan. :-)
So, I'm thinking that as I do this, I might move these subroutines into perl modules, and checksetup would just call them: Bugzilla::Requirements Bugzilla::Install Bugzilla::Requirements would contain everything that's in the BEGIN block, now. This would also allow other code (like runtests) to check that the modules exist.
Keywords: meta
Depends on: 281494
Priority: -- → P3
I'm going to start some work on this in 2.22, but will probably not finish until 2.24.
Target Milestone: --- → Bugzilla 2.24
And please, don't forget a routine (or, better, pluggable mechanism with overrides/plug-ins) to check bz_schema against the real DB. Then we'll be able to remove that wierd comment in localconfig for db_check parameter. I mean a phrase about the "moon phase". :) I've ran into it last week, so had to drop all the problem tables along with bz_schema itself as a workaround. Thanks God, it was just a test DB.
(In reply to comment #6) > And please, don't forget a routine (or, better, pluggable mechanism with > overrides/plug-ins) to check bz_schema against the real DB. Then we'll be able > to remove that wierd comment in localconfig for db_check parameter. I mean a > phrase about the "moon phase". :) Hahahaha. Actually, that "moon phase" comment is totally unrelated to that part of checksetup. The "moon phase" stuff is involved in checking the DB version numbers and so forth. > I've ran into it last week, so had to drop all the problem tables along with > bz_schema itself as a workaround. Thanks God, it was just a test DB. That's hopefully also addressed by bug 299230.
(In reply to comment #7) > (In reply to comment #6) > > And please, don't forget a routine (or, better, pluggable mechanism with > > overrides/plug-ins) to check bz_schema against the real DB. Then we'll be able > > to remove that wierd comment in localconfig for db_check parameter. I mean a > > phrase about the "moon phase". :) > > Hahahaha. Actually, that "moon phase" comment is totally unrelated to that > part of checksetup. The "moon phase" stuff is involved in checking the DB > version numbers and so forth. Well, so we also have, say, "Venus phases" in there :) > > I've ran into it last week, so had to drop all the problem tables along with > > bz_schema itself as a workaround. Thanks God, it was just a test DB. > > That's hopefully also addressed by bug 299230. No, that's not the case. I've been forced to drop the tables (or manually check for correctness) to make their structure consistent with schema description. Though I did not wish to. Should I fill in the bug on this and add it into blockers of this one?
(In reply to comment #8) > Should I fill in the bug on this and add it into blockers of this one? Well, actually, our whole discussion here is really unrelated to this bug. So anything we're talking about should go into another bug. Actually *changing* the way checksetup works would be a totally separate bug from this one, and certainly any problems you're experiencing with checksetup would be a different bug from this one. But yes, I'd certainly appreciate a bug report with the details of the situation. :-)
Added bug 300452 in dependency list
Depends on: 300452
No longer depends on: 300452
Depends on: 302876
QA Contact: mattyt-bugzilla → default-qa
Depends on: 344855
Depends on: 346212
Alias: bz-checksetup
Depends on: 346265
Depends on: 346270
Depends on: 346275
Depends on: 346343
Depends on: 346344
Depends on: 346375
Blocks: 289036
Depends on: 346410
Depends on: 346414
Depends on: 346483
Depends on: 346493
Depends on: 346552
Blocks: 190188
Depends on: 346594
Depends on: 346597
Depends on: 346815
Depends on: 346932
Depends on: 347083
Depends on: 347116
Depends on: 347290
No longer blocks: 160297
Depends on: 351983
Depends on: 352593
Okay, here's the very final step of this bug, this final cleanup of checksetup.pl. You'll notice that there's now a very nice "How Checksetup Works" section in the POD. It should be useful to developers especially. Granting myself review, as module owner.
Attachment #238349 - Flags: review+
Requesting approval directly, as module owner.
Status: NEW → ASSIGNED
Flags: approval?
Flags: approval? → approval+
*Cheering can be heard!* Checking in checksetup.pl; /cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v <-- checksetup.pl new revision: 1.543; previous revision: 1.542 done Checking in Bugzilla/Install.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install.pm,v <-- Install.pm new revision: 1.8; previous revision: 1.7 done Checking in Bugzilla/Install/Requirements.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/Requirements.pm,v <-- Requirements.pm new revision: 1.18; previous revision: 1.17 done Checking in template/en/default/global/messages.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/messages.html.tmpl,v <-- messages.html.tmpl new revision: 1.43; previous revision: 1.42 don
Alias: bz-checksetup
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Blocks: 319354
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: