Bug 52791 - [PATCH] read windows installer default values from config file
Summary: [PATCH] read windows installer default values from config file
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 6
Classification: Unclassified
Component: Native:Packaging (show other bugs)
Version: 6.0.35
Hardware: PC All
: P2 enhancement (vote)
Target Milestone: default
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-28 19:40 UTC by Sandra Madden
Modified: 2017-07-04 14:19 UTC (History)
1 user (show)



Attachments
patch for the tomcat install project file (2.78 KB, patch)
2012-02-28 19:40 UTC, Sandra Madden
Details | Diff
sample config.ini. (179 bytes, text/plain)
2012-02-28 19:42 UTC, Sandra Madden
Details
Rev2 of the patch to add config file support. Incorporated changes suggested by Konstantin Kolinko (2.65 KB, patch)
2012-04-04 18:58 UTC, Sandra Madden
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sandra Madden 2012-02-28 19:40:35 UTC
Created attachment 28399 [details]
patch for the tomcat install project file

Hi,

I attached a patch for the tomcat nsis installer that enables the installer to read it's default values from an ini file (config.ini) instead of using the in script defined onces. If the ini file does not exist, the script values are used. The same happens if the ini file exists, but a value is not defined. E.g. maybe you want to go with all of the script defined values but one or so. Please look at the attached diff. 

This is how a sample config.ini file would look like (filename must be config.ini and reside in $EXEDIR)

---------------
config.ini:
---------------

TomcatPortHttp=80
TomcatMenuEntriesEnable=1
TomcatShortcutAllUsers=1
TomcatAdminEnable=1

---------------


It would be great if this enhancement could be merged into the next installer :-)

Thanks!
Sandra
Comment 1 Sandra Madden 2012-02-28 19:42:31 UTC
Created attachment 28400 [details]
sample config.ini.
Comment 2 Konstantin Kolinko 2012-04-03 20:40:29 UTC
-1 for hard-coded filename of "config.ini".

IMHO the name of the answer file should be passed on the command line. If no name is given then there should be no attempts to open and read the file.


(Is is possible to read the file just once, instead of re-opening and re-scanning it for each config option?  It is not as bad if this behaviour is off by default. it is just that it does not scale well.  Maybe read the file into memory and do string scans there?)

The names: _Call, _String, _Result, _Default -- are you sure that they are unique? They look like polluting the name space.
Comment 3 Sandra Madden 2012-04-04 18:58:31 UTC
Created attachment 28543 [details]
Rev2 of the patch to add config file support. Incorporated changes suggested by Konstantin Kolinko

Thanks for the constructive feedback Konstantin. I changed the patch to only read the ini file when it is passed in on the cmdline as /C=<fully qualified path to filename>.

I also improved the way the macro/function hybrid to read the values is written. I initially based it on sample code I found online, but now cleaned it all up to have less clutter and match the needs of the tomcat installer better. 

The patch is still not reading in the config file at once and parse it from memory. I wanted to keep using the established macro ${ConfigRead} instead of writing something new to minimize possible introduction of bugs.

-Sandra
Comment 4 Mark Thomas 2017-07-04 14:19:15 UTC
Apologies it has taken so long to get back to this and belated thanks for the updated patch.

Since installation is (broadly) a one time event, I'm not to concerned about multiple reads of the config file.

Fixed in:
- trunk for 9.0.0.M24 onwards
- 8.5.x for 8.5.18 onwards
- 8.0.x for 8.0.46 onwards
- 7.0.x for 7.0.80 onwards

Thanks again.