History | Log In     View a printable version of the current page.  
Issue Details (XML | Word | Printable)

Key: IZPACK-73
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Critical Critical
Assignee: Dennis Reil
Reporter: Pavol Zibrita
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
IzPack

DynamicVariables does not work as expected/documented.

Created: 23/Apr/08 05:24 PM   Updated: 25/Apr/08 01:39 AM
Component/s: Installer, Compiler
Affects Version/s: 3.11.0
Fix Version/s: 4.0.0

Time Tracking:
Not Specified

File Attachments: 1. Text File izpack-dynamicvariables-3.11.0.patch (5 kb)
2. Text File izpack-dynamicvariables-trunk.patch (10 kb)


Patch Submitted: Yes


 Description  « Hide
DynamicVariables does not work as expected/documented. The dynamic variables are overwritten. As documented, you could have more dynamic variables with one name and different conditions. According the conditions, the right value should be set when you proceed in installer. However, only the last definition of the value with the last condition is take into the account.

Example:
<conditions>
<condition type="variable" id="isoracle">
<name>database.type</name>
<value>oracle</value>
</condition>
<condition type="variable" id="ismaxdb">
<name>database.type</name>
<value>maxdb</value>
</condition>
<condition type="variable" id="ismysql">
<name>database.type</name>
<value>mysql</value>
</condition>
</conditions>

<dynamicvariables>
<variable name="database.jdbcdriver" value="com.oracle.OracleDiver" condition="isoracle" />
<variable name="database.jdbcdriver" value="com.mysql.MysqlDiver" condition="ismysql" />
<variable name="database.jdbcdiver" value="com.maxdb.MaxdbDiver" condition="ismaxdb" />
</dynamicvariables>

In this example I would await, that after user input on one page and a choose of database.type by the user, the variable database.jdbcdiver would be set accordingly. However, as there can be only one definition of DynamicVariable per name in current implementation, only the last one (in this case maxdb) condition is taken.

I would await, that the concept of dynamic variable is able to handle this.
There are two patches available, for version 3.11 and current trunk.

The patches were not tested but the implementation should be clear. (no tests included)



 All   Comments   Work Log   Change History      Sort Order: Ascending order - Click to sort in descending order
Dennis Reil - 24/Apr/08 01:47 AM
I did not apply the patch, but changed the behaviour to the support of multiple values with different conditions.

Pavol Zibrita - 24/Apr/08 03:16 AM
Hi. Only one thing. I think the DynamicVariable's equals/hascode should be depended only on name and condition fields.

Pavol Zibrita - 24/Apr/08 03:17 AM
Will there be some 3.12 with this fix?

Dennis Reil - 24/Apr/08 04:06 AM
using value and conditionid for the hashcode is more general as these are the variables which can be different.

There will be no fix for 3.11.


Pavol Zibrita - 24/Apr/08 02:14 PM
Just my last 2 cents than:
What is then a "Dynamic variable overwritte?".
a) when you create a variable with same name and same condition but different value?
b) when you create a variable with same name, same condition and same value?

As this code in CompilerConfig is depended on DynamicVariable equals implementation:

if (dynamicValues.remove(dynamicVariable)){ parseWarn(var, "Dynamic Variable '" + name + "' will be overwritten"); }

for current implementation b) is the overwritte. Should not it be the a)?

This seems to be a mistake, but you will be not warned:
<variable name="database.jdbcdriver" value="com.oracle.OracleDiver" condition="isoracle" />
<variable name="database.jdbcdriver" value="com.mysql.MysqlDiver" condition="isoracle" />

Anyway, thanks for implementing it! good and fast work .
pavol


Dennis Reil - 25/Apr/08 12:55 AM
ok, your example convinced me
I changed the hashCode() implementation.

Thanks for your comments on that!


Pavol Zibrita - 25/Apr/08 01:39 AM
I'm glad for that, but, you needed to changed equals not hashCode (well this can be changed too) if you wanted to work it...

Thanks for all!