153291 - Add to config syntax the ability to install a configurable file

Status: VERIFIED FIXED

(SeaMonkey :: Installer, defect)

Description

[Curt Patrick (gone)]

To modify behavior of third party installers on the fly (specifically the Java
installer, but others work similarly) we need to be able to pass information
gathered from the user into a configuration file which the third party installer
reads.  For Java this file is called setup.iss.  To accomplish this, I propose
adding the following syntax to config.ini:
--------------------------------
[Configure FileX] or [Component ComponentName-Configure FileX]
Filename=NameOfConfigurableFile
VarX=KeyName
ValueX=ValueVariable
--------------------------------
Where:
Filename - identifies a file which exists in the Component xpi.
VarX - identifies a string in Filename which will be replaced with a value
determined by the DecryptableVariable associated with ValueX.
DecryptableVariable - Must be one of the decryptable variables defined in the
installer engine.


For the Java specific implementation we need to add the following two
decryptable variables:
[PROGRAM FOLDER DRIVE] - The letter of the drive the user has chosen to install to.
[PROGRAM FILES] - The name of the Program Files folder (this is differs across
international systems).

We also need to work out the details of where the configurable file would be
stored (in the components xpi, I suppose, or raw if the section is not component
specific) and where/when it would be placed and modified.  Do any tools (like
nztool.exe) have to be modified to support this?


So here is an Example:
----------------------------
[Component Java-Configure File0]
Filename=setup.iss
Var0=***InstallDrive***
Value0=[PROGRAM FOLDER DRIVE]
Var1=%ProgramFilesFolder%
Value1=[PROGRAM FILES]
-----------------------------
In this example, if Component Java is being installed the file setup.iss would
first be placed on the system and modified such that the line
"Install Path=***InstallDrive***:\%ProgramFilesFolder%\Java"
might (depending upon system and user input gathered by the installer) become
"Install Path=d:\Programme Stoffe\Java"  
(Well, that is a pretty contrived example because I can't remember what any
foreign countries really do call their Program Files folder.)

Comment 1

[dprice (gone)]

What about specifying each line of the configurable file in config.ini?  
You could do something like this:
[Component Java-Configure File0]
Filename=setup.iss
Line0="some text we don't change"
Line1="InstallPath="[PROGRAM FOLDER DRIVE][PROGRAM FOLDER PATH]
Line2="More text we don't change"

And then the installer would just call DecryptVariable on each line.  We would
run the risk of having a config.ini file that's a mile long.

How would we get information from the user?  

We could make DecryptVariable a little smarter.  If it sees [@Variable Name]
then it would know that it is expecting user input.  It would look for section
[Input @Variable Name]
Input TypeX=[text/folder pick/??]
Input MessageX=Where do you want to install the component?

Like the way things work in bug 139641

Then you would have 
[Input @Selected Path]
Input TypeX=[folder pick]
Input MessageX=Where do you want to install the component?

[Component Java-Configure File0]
Filename=setup.iss
Line0="some text we don't change"
Line1="InstallPath="[@Selected Path]
Line2="More text we don't change"

I guess we could reuse the folder picker in other parts of the installer.  When
would this happen in the install process?  pre runapp?

Comment 2

[dprice (gone)]

That would be...
[Input @Selected Path]
Input Type=[folder pick]
Input Message=Where do you want to install the component?

[Component Java-Configure File0]
Filename=setup.iss
Line0="some text we don't change"
Line1="InstallPath="[@Selected Path]
Line2="More text we don't change"

Comment 3

[dprice (gone)]

okay I was a little confused about this.  We've already aquired the user input,
we just need to take the data and use it to make an iss file for the executable.
 That means we wouldn't need the [Input @Selected Path] stuff that I suggested.  

Comment 4

[Curt Patrick (gone)]

I talked with Sean and he made some suggestions which would simplify this a
little, based on the observation that the .iss format is an INI format and
Windows has an API for manipulating key/value pairs in an INI format.  So the
config.ini format that Sean suggests is:

[Component Java-Configure FileX]
Filename=NameOfConfigurableFile
Type=INI
SectionX=SectionTitle
KeyX=KeyName
ValueX=ValueVariable

Using this information we can use SetPrivateProfileString() to manipulate the
.iss file.  (The Type= key isn't necassary now but could be used in the future
to expand functionality to support string substitution or some other format for
non-INI type config files.  But that is quite a lot more work.)  Another side
benefit of this approach is that SetPrivateProfileString() will create the file
if it doesn't already exist, so we would have the option of doing something kind
of like what Dave was suggesting, i.e., creating the entire .iss file on the fly
if it is simple enough.

I believe Dave is going to experiment with the Java installer using the
setup.iss file and work on further proving the feasability of doing what we are
suggesting here.  So --> Dave

Comment 5

[Curt Patrick (gone)]

Sean mentions that we might need to include path as well as filename for the
config file.  If we are always doing this in the temp folder, it might not be
necassary, but we need to keep this in mind as we're solving the problem.

Comment 6

[Curt Patrick (gone)]

Oops.  In Comment #4 above I said "SetPrivateProfileString()" when I meant
"WritePrivateProfileString()".

Comment 7

[Daniel Veditz [:dveditz]]

I agree with Sean, specify the whole path to the config file being edited and
call DecryptVariable on it. [WIZTEMP]\setup.iss

Also need a timing statement and to figure out a place to store the file to be
unpacked.

The new decrypt variables will be useful for Real as well.

Comment 8

[dprice (gone)]

These custom config files are going to be used by standalone setup executables
that ship with the installer.  So it seems like we could generate the custom
files during the pre launchapp phase.

Here is what it will take to define the custom config file for java.  Unless
someone complains, I'm going to stick these definitions into config.ini

[Component Java-Configuration File0]
FileName=[WIZTEMP]\Setup.iss
Type=INI

Section0=InstallShield Silent
Section0Key0=Version
Section0Value0=v6.00.000
Section0Key1=File
Section0Value1=Response File

Section1={7CF31609-270B-11D6-9445-000102308676}-DlgOrder
Section1Key0=Dlg0
Section1Value0={7CF31609-270B-11D6-9445-000102308676}-SdAskDestPath-0
Section1Key1=Count
Section1Value1=1

Section2={7CF31609-270B-11D6-9445-000102308676}-SdAskDestPath-0
Section2Key0=szDir
Section2Value0=[INSTALL DRIVE][PROGRAM FOLDER PATH]\Java\j2re1.4.0_01
Section2Key1=Result
Section2Value1=1

Section3=Application
Section3Key0=Name
Section3Value0=Java 2 Runtime Environment, SE v1.4.0_01
Section3Key1=Version
Section3Value1=1.4.0_01
Section3Key2=Company
Section3Value2=JavaSoft

Comment 9

[dprice (gone)]

Attachment: patch

Comment 10

[Curt Patrick (gone)]

Comment on attachment 90059 [details] [diff] [review]
patch

>Index: extra.c
>  if(lstrcmpi(szVariable, "PROGRAMFILESDIR") == 0)>     /* parse for the "c:\Program Files" directory */
>     GetWinReg(HKEY_LOCAL_MACHINE, szWRMSCurrentVersion, "ProgramFilesDir", szVariable, dwVariableSize);
>   }
>+  else if(lstrcmpi(szVariable, "PROGRAM FILES PATH") == 0)
>+  {
>+    /* parse for the "\Program Files" directory */
>+    GetWinReg(HKEY_LOCAL_MACHINE, szWRMSCurrentVersion, "ProgramFilesDir", szBuf, sizeof(szBuf));
>+    lstrcpy(szVariable, szBuf+2);

Assuming that we stick with the original GetWinReg() call above, which puts the
results into szVariable, can one safely do "lstrcpy(szVariable,
szVariable+2);"?  I'm afraid my rustiness with C is coming through.  If we can,
the above code could be cleaned up so we don't have to maintain two different
place where we grab the ProgramFilesDir.

Also, it would be cleare if the comments for PROGRAMFILESDIR and PROGRAM FILES
PATH were differentiated so it is clear why we have the two.  At first glance
it currently looks like they are going for the same info.

Lastest, and certainly leastest, it looks like the other related decrypt
variables have have not spaces so it would be more consistant to call this
PROGRAMFILESPATH, maybe?

Comment 11

[dprice (gone)]

I'll go ahead and rename it PROGRAMFILESPATH  and expand the documentation to say 
/* Parse for /Program files directory --NOTE this is the path without the drive
letter */

I don't think we should try to combine the "PROGRAMFILESDIR" and
"PROGRAMFILESPATH"  code together.  The logic is straight forward this way and
there won't be any real gains in efficiency

Comment 12

[dprice (gone)]

Attachment: patch with curt's changes

Comment 13

[Curt Patrick (gone)]

Comment on attachment 90132 [details] [diff] [review]
patch with curt's changes

Maintenance was the reason for my suggestion, not efficiency.  Granted the gain
is small, but not completely inconsequential.  None-the-less, I can buy off on
this.
So, r=curt

Comment 14

[Daniel Veditz [:dveditz]]

Comment on attachment 90132 [details] [diff] [review]
patch with curt's changes

sr=dveditz

Comment 15

[Daniel Veditz [:dveditz]]

Comment on attachment 90059 [details] [diff] [review]
patch

assuming curt's r= from comments

I'm OK with this for now, but it's pretty specific. I'd like
a bug filed to change this to a simple line-by-line file output
which has a higher likelyhood of being useful somewhere else.

>+    wsprintf(szSection,"%s-Configuration File%d"

In that *eventual* future (don't change now) this might be called "Write file"
or something rather than a configuration file

Then instead of worrying about section, key and value structure just

Line0=[section]
Line1=key=value

I think the second = is OK, but if not we'll have to invent an
escaping mechanism. (you just pointed out that we'll have to escape
the square brackets too since decryptVariable is run on these guys)

May not be able to tell the difference between a blank line an the end of the
list, in which case we'll need a marker for that, too

Line2=\
Line3=more stuff

'\' used for example, but we might as well use that unless there are other
issues.

So, sr=dveditz for now, but I'd like a bug to change this on the trunk to the
simpler method

Comment 16

[dprice (gone)]

Filed http://bugzilla.mozilla.org/show_bug.cgi?id=155694

Comment 17

[dprice (gone)]

on the trunk

Comment 18

[chris hofmann]

Comment on attachment 90059 [details] [diff] [review]
patch

a=chofmann for 1.0.1.  add the fixed1.0.1 keyword after checking in.

Comment 19

[chris hofmann]

Comment on attachment 90132 [details] [diff] [review]
patch with curt's changes

a=chofmann for 1.0.1.  add the fixed1.0.1 keyword after checking in.

Comment 20

[dprice (gone)]

http://bugscape.netscape.com/show_bug.cgi?id=16247 is verified, so this is too.

Comment 21

[Jaime Rodriguez, Jr.]

i am gonna assume "a=chofmann for 1.0.1", means mozilla1.0.1+, so i am marking
it as so ...

Comment 22

[Syd Logan]

adt1.0.1- per adt. 

Comment 23

[Randell Jesup [:jesup] (needinfo me)]

Retargeting for 1.0.2 and reopening so it doesn't get lost

If we never want to take this on the 1.0 branch, please re-close.

Changed summary to make it clear this is only open for branch

Comment 24

[Randell Jesup [:jesup] (needinfo me)]

Actually reopening this time.

Comment 25

[Sean Su]

over to me.

Comment 26

[Samir Gehani]

Installer triage team: -> FIXED since it's on the trunk and no need to do this
on the branch.

Comment 27

[Grace Bush]

verified code fix