999296 - checksetup.pl complains about variables you need to check even if you explicitly set them via an answers file

Status: RESOLVED FIXED

(Bugzilla :: Installation & Upgrading, defect)

Description

[Dave Miller [:justdave]]

This was reported on IRC the other day, and the reporter never filed a bug, but I don't want to lose it.

When deploying a Bugzilla with an answers file which explicitly contains everything that needs to go into localconfig, checksetup.pl stops after creating the localconfig file, complaining that new values were added to it that you might want to check before continuing, and then listing off everything in localconfig.  Thus when deploying in an automated fashion (the reporter was writing a puppet module to do the deployment) he had to run checksetup.pl twice for the initial install to work.

checksetup shouldn't count the addition of a variable to localconfig as a new variable if the value was explicitly provided by an answers file.

I have a patch coming momentarily.

Comment 1

[Dave Miller [:justdave]]

Attachment: Patch v1

Don't push the variable name onto @new_vars unless we actually use a default value to set it.

Comment 2

[Dave Miller [:justdave]]

Attachment: Patch v2

Comment 3

[Dave Miller [:justdave]]

er, I touched something funny, I didn't mean to submit yet there.

New patch also doesn't push onto @new_vars if an answer file was supplied but didn't include a site_wide_secret, because it's good form to let the installer generate that anyway and not supply it, and we don't want to trip up installation if the user does that.  We still notify the user if we generated it in interactive mode though.

Comment 4

[Dave Miller [:justdave]]

(In reply to Dave Miller [:justdave] ([e-mail]) from comment #0)
> This was reported on IRC the other day, and the reporter never filed a bug,
> but I don't want to lose it.

[e-mail] was the reporter for what it's worth, he didn't have a bugzilla account at the time so I couldn't CC him.

Comment 5

[Frédéric Buclin]

Comment on attachment 8410495 [details] [diff] [review]
Patch v2

>+                if (!(($name eq 'site_wide_secret') && (scalar(keys %$answer) > 0)))  {

I think: if (!scalar(keys %$answer) || $name ne 'site_wide_secret') would be more readable. I had to ask on IRC to understand it correctly.



>+                    push @new_vars, $var);

Problem with parens.

Comment 6

[Frédéric Buclin]

Don't know if you want it for 4.4 too, so I will let you set the target milestone.

Comment 7

[Dave Miller [:justdave]]

Attachment: Patch v3

Yeah, I agree, that's easier to parse.  Updated.

Comment 8

[Dave Miller [:justdave]]

(In reply to Frédéric Buclin from comment #6)
> Don't know if you want it for 4.4 too

Yeah, probably should.  Set as so.

Comment 9

[Frédéric Buclin]

Comment on attachment 8415360 [details] [diff] [review]
Patch v3

>-            push(@new_vars, $name);
>+                    push(@new_vars, $var);

Err... you want to push $name, not $var, else you get:

This version of Bugzilla contains some variables that you may want to
change and adapt to your local settings. The following variables are
new to ./localconfig since you last ran checksetup.pl:

HASH(0xc269838), HASH(0xc28b570), HASH(0xc28b5b0), HASH(0xc28b600),
HASH(0xc28bf60), HASH(0xc28bfa0), HASH(0xc28bfe0), HASH(0xc295c18),

Comment 10

[Dave Miller [:justdave]]

Attachment: Patch v4

Doh.  where the hell was my brain?

Comment 11

[Frédéric Buclin]

Comment on attachment 8415363 [details] [diff] [review]
Patch v4

r=LpSolit

Comment 12

[Dave Miller [:justdave]]

Trunk:

To ssh://[e-mail]/bugzilla/bugzilla.git
   7c6963d..082cea0  master -> master

4.4:

To ssh://[e-mail]/bugzilla/bugzilla.git
   3b4ddd1..6be81e8  4.4 -> 4.4