From 18c57070565d1218935168b437f4adcfe9b38ce4 Mon Sep 17 00:00:00 2001 From: "mkanat%bugzilla.org" <> Date: Tue, 1 Aug 2006 05:15:55 +0000 Subject: Bug 346483: Fix up param-updating code in checksetup.pl to all be in a module. Patch By Max Kanat-Alexander (module owner) a=myk --- Bugzilla.pm | 24 +------- Bugzilla/Config.pm | 172 +++++++++++++++++++++++++++++++++++++---------------- checksetup.pl | 73 +++++------------------ editgroups.cgi | 2 +- editparams.cgi | 2 +- editvalues.cgi | 2 +- 6 files changed, 143 insertions(+), 132 deletions(-) diff --git a/Bugzilla.pm b/Bugzilla.pm index 0ffa1d1b9..3d8215937 100644 --- a/Bugzilla.pm +++ b/Bugzilla.pm @@ -25,6 +25,7 @@ package Bugzilla; use strict; +use Bugzilla::Config; use Bugzilla::Constants; use Bugzilla::Auth; use Bugzilla::Auth::Persist::Cookie; @@ -147,7 +148,7 @@ sub cgi { sub params { my $class = shift; - request_cache()->{params} ||= _load_param_values(); + request_cache()->{params} ||= Bugzilla::Config::read_param_file(); return request_cache()->{params}; } @@ -346,27 +347,6 @@ sub _cleanup { undef $_request_cache; } -sub _load_param_values { - my %params; - my $datadir = bz_locations()->{'datadir'}; - if (-e "$datadir/params") { - # Note that checksetup.pl sets file permissions on '$datadir/params' - - # Using Safe mode is _not_ a guarantee of safety if someone does - # manage to write to the file. However, it won't hurt... - # See bug 165144 for not needing to eval this at all - my $s = new Safe; - - $s->rdo("$datadir/params"); - die "Error reading $datadir/params: $!" if $!; - die "Error evaluating $datadir/params: $@" if $@; - - # Now read the param back out from the sandbox - %params = %{$s->varglob('param')}; - } - return \%params; -} - sub END { _cleanup(); } diff --git a/Bugzilla/Config.pm b/Bugzilla/Config.pm index 22bdeb1f5..7db033285 100644 --- a/Bugzilla/Config.pm +++ b/Bugzilla/Config.pm @@ -34,13 +34,15 @@ use strict; use base qw(Exporter); use Bugzilla::Constants; +use Data::Dumper; +use File::Temp; # Don't export localvars by default - people should have to explicitly # ask for it, as a (probably futile) attempt to stop code using it # when it shouldn't %Bugzilla::Config::EXPORT_TAGS = ( - admin => [qw(UpdateParams SetParam WriteParams)], + admin => [qw(update_params SetParam write_params)], db => [qw($db_driver $db_host $db_port $db_name $db_user $db_pass $db_sock $db_check)], localconfig => [qw($cvsbin $interdiffbin $diffpath $webservergroup)], @@ -49,16 +51,6 @@ Exporter::export_ok_tags('admin', 'db', 'localconfig'); use vars qw(@param_list); -# Data::Dumper is required as needed, below. The problem is that then when -# the code locally sets $Data::Dumper::Foo, this triggers 'used only once' -# warnings. -# We can't predeclare another package's vars, though, so just use them -{ - local $Data::Dumper::Sortkeys; - local $Data::Dumper::Terse; - local $Data::Dumper::Indent; -} - # INITIALISATION CODE # Perl throws a warning if we use bz_locations() directly after do. our $localconfig = bz_locations()->{'localconfig'}; @@ -111,13 +103,16 @@ sub SetParam { Bugzilla->params->{$name} = $value; } -sub UpdateParams { - # --- PARAM CONVERSION CODE --- +sub update_params { + my ($params) = @_; + my $answer = $params->{answer} || {}; + + my $param = read_param_file(); + + # If we didn't return any param values, then this is a new installation. + my $new_install = !(keys %$param); - # Note that this isn't particularly 'clean' in terms of separating - # the backend code (ie this) from the actual params. - # We don't care about that, though - my $param = Bugzilla->params; + # --- UPDATE OLD PARAMS --- # Old Bugzilla versions stored the version number in the params file # We don't want it, so get rid of it @@ -144,7 +139,9 @@ sub UpdateParams { # Change from old product groups to controls for group_control_map # 2002-10-14 bug 147275 bugreport@peshkin.net - if (exists $param->{'usebuggroups'} && !exists $param->{'makeproductgroups'}) { + if (exists $param->{'usebuggroups'} && + !exists $param->{'makeproductgroups'}) + { $param->{'makeproductgroups'} = $param->{'usebuggroups'}; } if (exists $param->{'usebuggroupsentry'} @@ -158,7 +155,9 @@ sub UpdateParams { } # set verify method to whatever loginmethod was - if (exists $param->{'loginmethod'} && !exists $param->{'user_verify_class'}) { + if (exists $param->{'loginmethod'} + && !exists $param->{'user_verify_class'}) + { $param->{'user_verify_class'} = $param->{'loginmethod'}; delete $param->{'loginmethod'}; } @@ -182,49 +181,96 @@ sub UpdateParams { _load_params unless %params; foreach my $item (@param_list) { my $name = $item->{'name'}; - $param->{$name} = $item->{'default'} unless exists $param->{$name}; + unless (exists $param->{$name}) { + print "New parameter: $name\n" unless $new_install; + $param->{$name} = $answer->{$name} || $item->{'default'}; + } } - # --- REMOVE OLD PARAMS --- + $param->{'utf8'} = 1 if $new_install; - my @oldparams = (); + # --- REMOVE OLD PARAMS --- - # Remove any old params + my @oldparams; + # Remove any old params, put them in old-params.txt foreach my $item (keys %$param) { if (!grep($_ eq $item, map ($_->{'name'}, @param_list))) { - require Data::Dumper; - - local $Data::Dumper::Terse = 1; + local $Data::Dumper::Terse = 1; local $Data::Dumper::Indent = 0; push (@oldparams, [$item, Data::Dumper->Dump([$param->{$item}])]); delete $param->{$item}; } } - return @oldparams; + if (@oldparams) { + my $op_file = new IO::File('old-params.txt', '>>', 0600) + || die "old-params.txt: $!"; + + print "The following parameters are no longer used in Bugzilla,", + " and so have been\nmoved from your parameters file into", + " old-params.txt:\n"; + + foreach my $p (@oldparams) { + my ($item, $value) = @$p; + print $op_file "\n\n$item:\n$value\n"; + print $item; + print ", " unless $item eq $oldparams[$#oldparams]->[0]; + } + print "\n"; + $op_file->close; + } + + if (ON_WINDOWS && !-e SENDMAIL_EXE + && $param->{'mail_delivery_method'} eq 'sendmail') + { + my $smtp = $answer->{'SMTP_SERVER'}; + if (!$smtp) { + print "\nBugzilla requires an SMTP server to function on", + " Windows.\nPlease enter your SMTP server's hostname: "; + $smtp = ; + chomp $smtp; + if ($smtp) { + $param->{'smtpserver'} = $smtp; + } + else { + print "\nWarning: No SMTP Server provided, defaulting to", + " localhost\n"; + } + } + + $param->{'mail_delivery_method'} = 'smtp'; + } + + write_params($param); } -sub WriteParams { - require Data::Dumper; - my $datadir = bz_locations()->{'datadir'}; +sub write_params { + my ($param_data) = @_; + $param_data ||= Bugzilla->params; + + my $datadir = bz_locations()->{'datadir'}; + my $param_file = "$datadir/params"; # This only has an affect for Data::Dumper >= 2.12 (ie perl >= 5.8.0) # Its just cosmetic, though, so that doesn't matter local $Data::Dumper::Sortkeys = 1; - require File::Temp; my ($fh, $tmpname) = File::Temp::tempfile('params.XXXXX', DIR => $datadir ); - print $fh (Data::Dumper->Dump([Bugzilla->params], ['*param'])) + print $fh (Data::Dumper->Dump([$param_data], ['*param'])) || die "Can't write param file: $!"; close $fh; - rename $tmpname, "$datadir/params" - || die "Can't rename $tmpname to $datadir/params: $!"; + rename $tmpname, $param_file + || die "Can't rename $tmpname to $param_file: $!"; + + ChmodDataFile($param_file, 0666); - ChmodDataFile("$datadir/params", 0666); + # And now we have to reset the params cache so that Bugzilla will re-read + # them. + delete Bugzilla->request_cache->{params}; } # Some files in the data directory must be world readable if and only if @@ -243,6 +289,27 @@ sub ChmodDataFile { chmod $perm,$file; } +sub read_param_file { + my %params; + my $datadir = bz_locations()->{'datadir'}; + if (-e "$datadir/params") { + # Note that checksetup.pl sets file permissions on '$datadir/params' + + # Using Safe mode is _not_ a guarantee of safety if someone does + # manage to write to the file. However, it won't hurt... + # See bug 165144 for not needing to eval this at all + my $s = new Safe; + + $s->rdo("$datadir/params"); + die "Error reading $datadir/params: $!" if $!; + die "Error evaluating $datadir/params: $@" if $@; + + # Now read the param back out from the sandbox + %params = %{$s->varglob('param')}; + } + return \%params; +} + 1; __END__ @@ -256,9 +323,9 @@ Bugzilla::Config - Configuration parameters for Bugzilla # Administration functions use Bugzilla::Config qw(:admin); - my @removed_params = UpgradeParams(); + update_params(); SetParam($param, $value); - WriteParams(); + write_params(); # Localconfig variables may also be imported use Bugzilla::Config qw(:db); @@ -281,30 +348,35 @@ Parameters can be set, retrieved, and updated. Sets the param named $name to $value. Values are checked using the checker function for the given param if one exists. -=item C +=item C Updates the parameters, by transitioning old params to new formats, setting -defaults for new params, and removing obsolete ones. +defaults for new params, and removing obsolete ones. Used by F +in the process of an installation or upgrade. -Any removed params are returned in a list, with elements [$item, $oldvalue] -where $item is the entry in the param list. +Prints out information about what it's doing, if it makes any changes. -=item C +May prompt the user for input, if certain required parameters are not +specified. -Writes the parameters to disk. +=item C -=back +Description: Writes the parameters to disk. -=over +Params: C<$params> (optional) - A hashref to write to the disk + instead of Cparams>. Used only by + C. -=item * +Returns: nothing -The new value for the parameter +=item C -=item * +Description: Most callers should never need this. This is used + by Cparams> to directly read C<$datadir/params> + and load it into memory. Use Cparams> instead. -A reference to the entry in the param list for this parameter +Params: none -Functions should return error text, or the empty string if there was no error. +Returns: A hashref containing the current params in C<$datadir/params>. =back diff --git a/checksetup.pl b/checksetup.pl index 485173fb2..00230e5c5 100755 --- a/checksetup.pl +++ b/checksetup.pl @@ -187,7 +187,9 @@ The format of that file is as follows: $answer{'db_user'} = 'mydbuser'; $answer{'db_pass'} = 'mydbpass'; - (Any localconfig variable can be specified as above.) + $answer{'urlbase'} = 'http://bugzilla.mydomain.com/'; + + (Any localconfig variable or parameter can be specified as above.) $answer{'ADMIN_OK'} = 'Y'; $answer{'ADMIN_EMAIL'} = 'myadmin@mydomain.net'; @@ -203,6 +205,14 @@ function calls needs to be specified in this file. L +L + +L + +L + +L + =cut ###################################################################### @@ -222,9 +232,7 @@ use lib "."; use Bugzilla::Constants; use Bugzilla::Install::Requirements; -if ($^O =~ /MSWin32/i) { - require 5.008001; # for CGI 2.93 or higher -} +require 5.008001 if ON_WINDOWS; # for CGI 2.93 or higher ###################################################################### # Subroutines @@ -356,62 +364,13 @@ create_htaccess() if $lc_hash->{'create_htaccess'}; my $datadir = bz_locations()->{'datadir'}; my $webdotdir = bz_locations()->{'webdotdir'}; -# Check for a new install -my $newinstall = !-e "$datadir/params"; - # Remove parameters from the params file that no longer exist in Bugzilla, # and set the defaults for new ones +update_params({ answer => \%answer}); -my @oldparams = UpdateParams(); - -if (@oldparams) { - open(PARAMFILE, '>>', 'old-params.txt') - || die "$0: Can't open old-params.txt for writing: $!\n"; - - print "The following parameters are no longer used in Bugzilla, " . - "and so have been\nmoved from your parameters file " . - "into old-params.txt:\n"; - - foreach my $p (@oldparams) { - my ($item, $value) = @{$p}; - - print PARAMFILE "\n\n$item:\n$value\n"; - - print $item; - print ", " unless $item eq $oldparams[$#oldparams]->[0]; - } - print "\n"; - close PARAMFILE; -} - -# Set mail_delivery_method to SMTP and prompt for SMTP server -# if running on Windows and no third party sendmail wrapper -# is available -if ($^O =~ /MSWin32/i - && Bugzilla->params->{'mail_delivery_method'} eq 'sendmail' - && !-e SENDMAIL_EXE) -{ - print "\nBugzilla requires an SMTP server to function on Windows.\n" . - "Please enter your SMTP server's hostname: "; - my $smtp = $answer{'SMTP_SERVER'} - || ($silent && die("cant preload SMTP_SERVER")) - || ; - chomp $smtp; - if (!$smtp) { - print "\nWarning: No SMTP Server provided, defaulting to localhost\n"; - $smtp = 'localhost'; - } - SetParam('mail_delivery_method', 'smtp'); - SetParam('smtpserver', $smtp); -} - -# Enable UTF-8 on new installs -if ($newinstall) { - SetParam('utf8', 1); -} - -# WriteParams will only write out still-valid entries -WriteParams(); +########################################################################### +# Pre-compile --TEMPLATE-- code +########################################################################### my $templatedir = bz_locations()->{'templatedir'}; unless ($switch{'no-templates'}) { diff --git a/editgroups.cgi b/editgroups.cgi index 57708cd3e..a7a608694 100755 --- a/editgroups.cgi +++ b/editgroups.cgi @@ -639,7 +639,7 @@ sub doGroupChanges { $update_params = 1; } } - WriteParams() if $update_params; + write_params() if $update_params; } if ($desc ne $cgi->param('olddesc')) { $chgs = 1; diff --git a/editparams.cgi b/editparams.cgi index 5ae2bcc25..79063271a 100755 --- a/editparams.cgi +++ b/editparams.cgi @@ -124,7 +124,7 @@ if ($action eq 'save' && $current_module) { $vars->{'message'} = 'parameters_updated'; $vars->{'param_changed'} = \@changes; - WriteParams(); + write_params(); } $template->process("admin/params/editparams.html.tmpl", $vars) diff --git a/editvalues.cgi b/editvalues.cgi index 109f8a29e..dc35c40e0 100755 --- a/editvalues.cgi +++ b/editvalues.cgi @@ -410,7 +410,7 @@ if ($action eq 'update') { && $valueold eq Bugzilla->params->{$defaults{$field}}) { SetParam($defaults{$field}, $value); - WriteParams(); + write_params(); $vars->{'default_value_updated'} = 1; } -- cgit v1.2.1