[wpkg-users] [Bug 175] Test return value of removeSettingsNode function

[bugzilla-daemon at bugzilla.wpkg.org]

http://bugzilla.wpkg.org/show_bug.cgi?id=175





--- Comment #2 from Rainer Meier <r.meier at wpkg.org>  2009-10-04 17:52:24 ---
In answer to the following reply on the mailing list:

Simon wrote:

If it never happens - then users won't see the messages :)

This is just part of my endless attempts to get WPKG to not remove packages
from wpkg.xml if there are no positive instructions to do.

If you add a few if/else statements (I'm more than willing to supply the
code changes from my modded version) then all I'd have to do everytime the
main codebase changes is to redo my mods for the removeSettingsNode function
itself.

I thought that my request was harmless, trivial to implement and was what
I'd call good defensive programming practice :)

I know you think my philosophy on "no remove if not told to do so" is wrong
but with a bit of goodwill, we could maybe come up with compromise that
wouldn't harm your concept of how WPKG should work but at the same time let
it be used in a different way :)

------

> If it never happens - then users won't see the messages :)

I thought you request to evaluate the result and print some message. For
example changing line 614 from

removeSettingsNode(settingsNode, false);

to

if (result == true) {
    dinfo("Successfully removed package from settings.");
} else {
    dinfo("No settings node removed. Probably the entry did not exist.");
}

Obviously this does not change anything in the script behavior. However in WPKG
the "else" case will never ever be executed since line 614 is executed only if
there IS an entry in the settings node to be removed.

If you intend to modify the removeSettingsNode function then you might return
"false" even if there is a settings node to be removed. But why should WPKG
include code to add output in case of modification? You could add this directly
to the removeSettingsNode function which you're going to modify anyway.

For example:
function removeSettingsNode(packageNode, saveImmediately) {
    dinfo("Settings node not removed - keep all entries");
    return false;
}

If this is not what you want to achieve please explain your use-case in more
detail.

In any case this modification of not removing any nodes from the settings node
will break functionality which relies on knowing the current state of the
machine. Which basically means it will lead to unexpected results in case of
install, upgrade, downgrade, remove and synchronization. The only use-case
where it could be useful is if you're using /force all the time so WPKG ignores
the current system state. In this case a modified removeSettingsNode method
which does not remove nodes would make wpkg.xml grow and grow  containing all
packages ever installed to the system I guess. For this to work you might have
to modify the addSettingsNode function too since it first uses
removeSettingsNode to make sure the node is not already inserted (provides
replace functionality). 

In any case I think it's a bad idea to abuse wpkg.xml for this use-case since
it will also make wpkg.xml contain multiple times the same package (in
different revisions). And WPKG currently uses the package ID as the "primary"
key and expects entries to be unique by ID.


What exactly do you try to achieve by this modification? I was under the
impression that you might like to have an XML file which includes the history
of installations - don't you?
Currently this might be possible already by analyzing the log files - but I
agree it's not very convenient.

What about the following proposal:
WPKG might write such a "system change log" for you - optionally. So this would
write a new XML file which contains the XML nodes of every package applied to
the system. This would provide some kind of "installation history" within this
XML file (excluding package removals).

The structure could look as follows when using the functionality:
<?xml version="1.0" encoding="utf-8" ?>
<packages>

    <package id='Firefox' name='Mozilla Firefox' revision='3.5.0' priority='50'
reboot='false' >
      <check type='uninstall' condition='exists' path='Mozilla Firefox (3.5.0)'
/>
      <install cmd='"%SOFTWARE%\firefox.cmd"' />
      <remove cmd='"%SOFTWARE%\firefox.cmd"' />
      <upgrade cmd='"%SOFTWARE%\firefox-remove.cmd"' />
    </package>

    <package id='Thunderbird' name='Mozilla Thunderbird' revision='2.0.0.20'
priority='50' reboot='false' >
      <check type='uninstall' condition='exists' path='Mozilla Thunderbird
(2.0.0.20)' />
      <install cmd='"%SOFTWARE%\thunderbird.cmd"' />
      <remove cmd='"%SOFTWARE%\thunderbird.cmd"' />
      <upgrade cmd='"%SOFTWARE%\thunderbird-remove.cmd"' />
    </package>

    <package id='Thunderbird' name='Mozilla Thunderbird' revision='2.0.0.21'
priority='50' reboot='false' >
      <check type='uninstall' condition='exists' path='Mozilla Thunderbird
(2.0.0.21)' />
      <install cmd='"%SOFTWARE%\thunderbird.cmd"' />
      <remove cmd='"%SOFTWARE%\thunderbird.cmd"' />
      <upgrade cmd='"%SOFTWARE%\thunderbird-remove.cmd"' />
    </package>

    <package id='Firefox' name='Mozilla Firefox' revision='3.5.1' priority='50'
reboot='false' >
      <check type='uninstall' condition='exists' path='Mozilla Firefox (3.5.1)'
/>
      <install cmd='"%SOFTWARE%\firefox.cmd"' />
      <remove cmd='"%SOFTWARE%\firefox.cmd"' />
      <upgrade cmd='"%SOFTWARE%\firefox-remove.cmd"' />
    </package>

    <package id='Firefox' name='Mozilla Firefox' revision='3.5.4' priority='50'
reboot='false' >
      <check type='uninstall' condition='exists' path='Mozilla Firefox (3.5.4)'
/>
      <install cmd='"%SOFTWARE%\firefox.cmd"' />
      <remove cmd='"%SOFTWARE%\firefox.cmd"' />
      <upgrade cmd='"%SOFTWARE%\firefox-remove.cmd"' />
    </package>

    <package id='Thunderbird' name='Mozilla Thunderbird' revision='2.0.0.23'
priority='50' reboot='false' >
      <check type='uninstall' condition='exists' path='Mozilla Thunderbird
(2.0.0.23)' />
      <install cmd='"%SOFTWARE%\thunderbird.cmd"' />
      <remove cmd='"%SOFTWARE%\thunderbird.cmd"' />
      <upgrade cmd='"%SOFTWARE%\thunderbird-remove.cmd"' />
    </package>
</packages>


Just as an example - so you could see all packages eve installed on the system
by WPKG by viewing this XML file.

-- 
Configure bugmail: http://bugzilla.wpkg.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.