Closed Bug 97290 Opened 23 years ago Closed 22 years ago

checksetup.pl should report about errors in localconfig

Categories

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

2.12
All
Other
defect

Tracking

()

RESOLVED FIXED
Bugzilla 2.16

People

(Reporter: alfred.spalt, Assigned: zach)

Details

Attachments

(1 file, 2 obsolete files)

If an error occurs when calling localconfig, none of the variables are defined. 
If checksetup.pl detects undefined variables, it simply adds them to 
localconfig. Thus an erroneous localconfig will grow with every call to 
checksetup.pl, without giving a notice to the user.
Planning this for 2.16
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → Bugzilla 2.16
We are currently trying to wrap up Bugzilla 2.16.  We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut.  Thus this is being retargetted at 2.18.  If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.
Target Milestone: Bugzilla 2.16 → Bugzilla 2.18
This is about 4 lines of code which I would like to see in 2.16. Simply run 
perl -c (the path to perl coming from the special variable which I don't 
know offhand) localconfig and check for errors. If we get them, print a 
message and exit. 
Target Milestone: Bugzilla 2.18 → Bugzilla 2.16
I did it like this:
do 'localconfig';
if ($@ ne "")     # errors with 'do' are reported in $@
{ print ("ERROR when evaluting localconfig: $@\n");
  exit 1;
}
ooooh! I forgot about $@ and would have done so crazy perl -c thing. 
Thanks! I'll get a patch soon.

Zach
Attached patch Patch to fix (obsolete) — Splinter Review
Comment on attachment 58720 [details] [diff] [review]
Patch to fix

- Should print to STDERR rather than STDOUT
- Should print the error message (contained in $@)
- Make text of error message less wordy, perhaps:

An error has occurred while reading your 
'localconfig' file.  The text of the error message is:

$@

Please fix the error in your 'localconfig' file.  
Alternately rename your 'localconfig' file, rerun 
checksetup.pl, and re-enter your answers.

  $ mv -f localconfig localconfig.orig
  $ ./checksetup.pl
Attachment #58720 - Flags: review-
A couple more thoughts:

  - Should this patch move localconfig out of the way on its own if
    it detects an error message (thereby forcing the next run of 
    checkconfig.pl to work successfully)?

  - It would be nice if we could tell users to run "perl -wc localconfig"
    on the file after they attempt to "fix" it, but it has a bunch of
    "variable used only once" warnings (which would only confuse most of
    the users).
$ perl -wc localconfig.bak 
Name "main::db_host" used only once: possible typo at localconfig.bak line 16.
Name "main::db_name" used only once: possible typo at localconfig.bak line 18.
Name "main::severities" used only once: possible typo at localconfig.bak line 41.
Name "main::webservergroup" used only once: possible typo at localconfig.bak line 9.
Name "main::db_user" used only once: possible typo at localconfig.bak line 19.
Name "main::platforms" used only once: possible typo at localconfig.bak line 107.
Name "main::db_check" used only once: possible typo at localconfig.bak line 34.
Name "main::db_pass" used only once: possible typo at localconfig.bak line 26.
Name "main::priorities" used only once: possible typo at localconfig.bak line 56.
Name "main::opsys" used only once: possible typo at localconfig.bak line 69.
Name "main::db_port" used only once: possible typo at localconfig.bak line 17.
localconfig.bak syntax OK
$ 
Zach: this has a needs-work patch that you wrote which is oh-so-close. :-)  Can
you finish it before I push this to 2.18?  ;)
I'll see what I can do with this.

Zach
ping Zach, take 2.  On Friday I push this to 2.18 if there's no patch.
I'll get a patch by monday
er, friday
Attached patch v2 (obsolete) — Splinter Review
Attachment #58720 - Attachment is obsolete: true
Attachment #69381 - Attachment is obsolete: true
Comment on attachment 69421 [details] [diff] [review]
v2.1, without the tabs

r=kiko

just clean up those tabs please
Attachment #69421 - Flags: review+
thanks kiko:

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.121; previous revision: 1.120
done
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.