checksetup.pl should report about errors in localconfig

RESOLVED FIXED in Bugzilla 2.16

Status

()

Bugzilla
Installation & Upgrading
P2
normal
RESOLVED FIXED
17 years ago
5 years ago

People

(Reporter: Alfred Spalt, Assigned: zach)

Tracking

2.12
Bugzilla 2.16
All
Other

Details

Attachments

(1 attachment, 2 obsolete attachments)

1.02 KB, patch
Christian Reis
: review+
Christian Reis
: review+
Details | Diff | Splinter Review
(Reporter)

Description

17 years ago
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.
(Assignee)

Comment 1

17 years ago
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
(Assignee)

Comment 3

17 years ago
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
(Reporter)

Comment 4

17 years ago
I did it like this:
do 'localconfig';
if ($@ ne "")     # errors with 'do' are reported in $@
{ print ("ERROR when evaluting localconfig: $@\n");
  exit 1;
}
(Assignee)

Comment 5

17 years ago
ooooh! I forgot about $@ and would have done so crazy perl -c thing. 
Thanks! I'll get a patch soon.

Zach
(Assignee)

Comment 6

17 years ago
Created attachment 58720 [details] [diff] [review]
Patch to fix

Comment 7

17 years ago
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-

Comment 8

17 years ago
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?  ;)
(Assignee)

Comment 10

16 years ago
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.
(Assignee)

Comment 12

16 years ago
I'll get a patch by monday
(Assignee)

Comment 13

16 years ago
er, friday
(Assignee)

Comment 14

16 years ago
Created attachment 69381 [details] [diff] [review]
v2
Attachment #58720 - Attachment is obsolete: true
(Assignee)

Comment 15

16 years ago
Created attachment 69421 [details] [diff] [review]
v2.1, without the tabs
Attachment #69381 - Attachment is obsolete: true

Comment 16

16 years ago
Comment on attachment 69421 [details] [diff] [review]
v2.1, without the tabs

r=kiko

just clean up those tabs please
Attachment #69421 - Flags: review+
(Assignee)

Comment 17

16 years ago
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
Last Resolved: 16 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.