[wpkg-users] [Bug 175] New: Test return value of removeSettingsNode function
http://bugzilla.wpkg.org/show_bug.cgi?id=175 Summary: Test return value of removeSettingsNode function Product: WPKG Version: other Platform: PC OS/Version: Windows XP Status: NEW Severity: enhancement Priority: P2 Component: wpkg.js AssignedTo: man...@wpkg.org ReportedBy: siwalt...@hotmail.com QAContact: wpkg-users@lists.wpkg.org removeSettingsNode is used at 4 places in V1.1.2 (Line 614 , Line 4015,Line 4093 and Line 4119) but no test is made of its return falue (true/false) and usually WPKG just prints a package removal success message. I think it would be better to test its return value and suppress the successful message and output a debug message if it returns false. regards Simon -- Configure bugmail: http://bugzilla.wpkg.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. - wpkg-users mailing list archives http://lists.wpkg.org/pipermail/wpkg-users/ ___ wpkg-users mailing list wpkg-users@lists.wpkg.org http://lists.wpkg.org/mailman/listinfo/wpkg-users
[wpkg-users] [Bug 175] Test return value of removeSettingsNode function
http://bugzilla.wpkg.org/show_bug.cgi?id=175 Rainer Meier r.me...@wpkg.org changed: What|Removed |Added Status|NEW |RESOLVED CC||r.me...@wpkg.org Resolution||WONTFIX --- Comment #1 from Rainer Meier r.me...@wpkg.org 2009-10-04 15:21:11 --- Hi Simon, It's true that this method returns true/ralse but in no place where it is currently used it is required to check the return value. The function returns false only in case the node could not be removed from the settings node which means it is not part of the current settings (wpkg.xml). As the intention of calling this method is to remove the package from settings it is successful in any case: - case 1: setting node exists and has been removed, return value true - case 2: setting node does not exist and does not need to be removed, return value false In both cases the state after calling the method is the same: The setting node is not part of wpkg.xml (or not any more). So in all cases where removeSettingsNode is currently used the action to be performed is done. So I don't see a reason to introduce some code to print some useless debug messages even if the return value of the method is completely irrelevant. More over on line 614 for example it will enter this area only if the settings node exists (see line 610) and therefore the return value of removeSettings node can only be true. And even if it would be false it would not matter (see above). So personally I don't see a reason to insert some bloat code to catch events never happening and confuse users by useless output messages. For example in the removePackage function removeSettingNode is called just to make sure the node is not part of the settings any more (I don't care if it was before, I just want it to be removed after I call the function). So checking the return value would be useful only in case I care about the return result. I.e. I have no clue if the node to be removed is part of the settings file at this time and I want to know if it has been before or not. But this is not the case for any of the calls currently used. By the way in case of failure or unexpected error an exception will be thrown and this will of course trigger error handling. -- Configure bugmail: http://bugzilla.wpkg.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. - wpkg-users mailing list archives http://lists.wpkg.org/pipermail/wpkg-users/ ___ wpkg-users mailing list wpkg-users@lists.wpkg.org http://lists.wpkg.org/mailman/listinfo/wpkg-users
[wpkg-users] [Bug 175] Test return value of removeSettingsNode function
http://bugzilla.wpkg.org/show_bug.cgi?id=175
--- Comment #2 from Rainer Meier r.me...@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
Re: [wpkg-users] [Bug 175] Test return value of removeSettingsNode function
I hadn't realised that WPKG removed an wpkg.xml entry before upgrading it - (e.g line 614) so my initial modification request is no longer valid :( I'll have to learn more and try again in the future :) If this is not what you want to achieve please explain your use-case in more detail. I am trying to stop WPKG removing wpkg.xml entries if there are no remove instructions in a package 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 Yes - thats one way of doing it. 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? No - just trying to make WPKG simpler :) regards Simon -- View this message in context: http://www.nabble.com/-Bug-175--New%3A-Test-return-value-of-removeSettingsNode-function-tp25737803p25740605.html Sent from the WPKG - Users mailing list archive at Nabble.com. - wpkg-users mailing list archives http://lists.wpkg.org/pipermail/wpkg-users/ ___ wpkg-users mailing list wpkg-users@lists.wpkg.org http://lists.wpkg.org/mailman/listinfo/wpkg-users
Re: [wpkg-users] [Bug 175] Test return value of removeSettingsNode function
Hello Simplesi, 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. please let us have a look at your changes or send a patch, so we can see what and why you need a modded version. Thanx, Falko -- Falko Trojahn fon +49-341-3581294 Dipl-Ingenieur Netzwerke/Support fax +49-341-3581295 SMI Softmark Informationstechnologien GmbH Sitz: D-04416 Markkleeberg, Friedrich-Ebert-Str. 51 Registergericht: Amtsgericht Leipzig HRB 164 Geschäftsführer: Andreas Griesmann - wpkg-users mailing list archives http://lists.wpkg.org/pipermail/wpkg-users/ ___ wpkg-users mailing list wpkg-users@lists.wpkg.org http://lists.wpkg.org/mailman/listinfo/wpkg-users
Re: [wpkg-users] [Bug 175] Test return value of removeSettingsNode function
Starting from the beginning again :)
I adminster several small school networks and use wpkg to deploy packages to
several machines if it is quicker to use wpkg than to go around all
computers and do a manual install.
I want to be able to test a new package out or a modified one on an existing
live system.
I do this by trying to allocate/modify a package on one computer.
In practice, I have found this difficult to achieve this cleanly without
breaking something sometimes :(
Currently, WPKG will remove a package entry from WPKG.xml if it decides that
the package is no longer required for that computer.
Even with the two exisiting flags (noremove and noforced remove) WPKG will
still remove the package entry from WPKG.xml under certain circumstances.
If a mistake is made, and then later rectified, then WPKG thinks it hasn't
installed a package and attempts re-install.
IF I programmed a package with checks - I could probably work around this
but I don't. I don't use checks and I dont use remove commands. I just have
an install command to install and an update command to update -very simple
:)
If I could have a flag to stop WPKG from removing package entries, then when
my mistake is rectified, wpkg would not attempt a re-install.
Its that simple :)
I don't use WPKG to keep track of everything on my network - I just use it
if it might save me time and effort.
Obviously if I had a separate testing setup, virtual/real testing machines
or didn't make mistooks then I wouldn't have a problem - but I dont have
these and therefore I have a problem :)
regards
Simon
PS
Just for reference my basic code change proposal is
function removeSettingsNodeSW(packageNode, saveImmediately) {
// sw modified to stop removal if no remove instructions found
if (getPackageCmdRemove(packageNode).length 0) {
// make sure the settigngs node is selected
var packageID = getPackageID(packageNode);
dinfo(Removing package id ' + packageID + ' from settings.);
var settingsNode = getSettingNode(packageID);
var success = false;
if(settingsNode != null) {
success = removeNode(getSettings(), settingsNode);
}
// save settings if remove was successful
if (success saveImmediately) {
saveSettings();
}
return success;
} else {
var packageID = getPackageID(packageNode);
dinfo(package id ' + packageID + ' not removed as no remove
instructions found.);
var success = false;
return success;
}
}
and then the calling code should check the return success value
--
View this message in context:
http://www.nabble.com/-Bug-175--New%3A-Test-return-value-of-removeSettingsNode-function-tp25737803p25741942.html
Sent from the WPKG - Users mailing list archive at Nabble.com.
-
wpkg-users mailing list archives http://lists.wpkg.org/pipermail/wpkg-users/
___
wpkg-users mailing list
wpkg-users@lists.wpkg.org
http://lists.wpkg.org/mailman/listinfo/wpkg-users
Re: [wpkg-users] [Bug 175] Test return value of removeSettingsNode function
Rainer Meier wrote: Could you explain this certain circumstances in a bit more detail because it might be a bug. I've checked the code and wpkg.xml entries are only removed in the removePackage function. And all of them are completely skipped if the /noremove flag (or noremove config) is set. In this case no remove commands are executed at all and wpkg.xml is not touched at all. Are you saying the documentation is wrong and that /noremove will not remove packages under any circumstances? from wpkg.js 1.1.2 /noremove \n + Disable the removal of all packages. If used in conjunction with the \n + /synchronize parameter it will just add packages but never remove them. \n + Instead of removing a list of packages which would have been removed \n + during that session is printed on exit. Packages are not removed from \n + the local settings database (wpkg.xml). Therefore it will contain a list \n + of all packages ever installed. \n + and this is the bit that causes me problems Note that each package which is requested to be removed (manually or by \n + a synchronization request) will be checked for its state by executing its \n + included package checks. If the package has been removed manually it will \n + also be removed from the settings database. Due to the fact that packages \n + whithout checks always return 'false' for during the install-status check \n + these packages will disappear from the local wpkg.xml. \n + \n regards Simon -- View this message in context: http://www.nabble.com/-Bug-175--New%3A-Test-return-value-of-removeSettingsNode-function-tp25737803p25742689.html Sent from the WPKG - Users mailing list archive at Nabble.com. - wpkg-users mailing list archives http://lists.wpkg.org/pipermail/wpkg-users/ ___ wpkg-users mailing list wpkg-users@lists.wpkg.org http://lists.wpkg.org/mailman/listinfo/wpkg-users
Re: [wpkg-users] [Bug 175] Test return value of removeSettingsNode function
Hi Simon, simplesi wrote: Are you saying the documentation is wrong and that /noremove will not remove packages under any circumstances? from wpkg.js 1.1.2 Which part of the documentation is wrong? The one you quote below... /noremove \n + Disable the removal of all packages. If used in conjunction with the \n + /synchronize parameter it will just add packages but never remove them. \n + Instead of removing a list of packages which would have been removed \n + during that session is printed on exit. Packages are not removed from \n + the local settings database (wpkg.xml). Therefore it will contain a list \n + of all packages ever installed. \n + and this is the bit that causes me problems I don't think this causes any problem - this is just the printout of the usage screen ;-) Well I think you want to point out something within the text. Which parts do you think are wrong? I was under the impression that you would like WPKG not to remove any package at any point in time. This is what the /noremove flag is supposed to do I think (unless there is some other misunderstanding). Especially it reads Packages are not removed from the local settings database (wpkg.xml). Therefore it will contain a list of all packages ever installed. Which seems to be what you like to achieve, isn't it? br, Rainer - wpkg-users mailing list archives http://lists.wpkg.org/pipermail/wpkg-users/ ___ wpkg-users mailing list wpkg-users@lists.wpkg.org http://lists.wpkg.org/mailman/listinfo/wpkg-users
Re: [wpkg-users] [Bug 175] Test return value of removeSettingsNode function
I don't know how you are viewing my message but when I view it on Nabble - it shows the relavent part of the documentation in bold :) Please just look at the documentation in wpkg.js about noremove especially the last bit regards Simon -- View this message in context: http://www.nabble.com/-Bug-175--New%3A-Test-return-value-of-removeSettingsNode-function-tp25737803p25743078.html Sent from the WPKG - Users mailing list archive at Nabble.com. - wpkg-users mailing list archives http://lists.wpkg.org/pipermail/wpkg-users/ ___ wpkg-users mailing list wpkg-users@lists.wpkg.org http://lists.wpkg.org/mailman/listinfo/wpkg-users
[wpkg-users] [Bug 176] New: WPKG Service crashes when using Offline mode
http://bugzilla.wpkg.org/show_bug.cgi?id=176 Summary: WPKG Service crashes when using Offline mode Product: WPKG Client Version: 1.3.9 Platform: PC OS/Version: Windows XP Status: NEW Severity: major Priority: P2 Component: WPKG Client AssignedTo: man...@wpkg.org ReportedBy: b...@tsl.com.au QAContact: wpkg-users@lists.wpkg.org When offline mode is enabled the WPKG service crashes with the following event log message: Faulting application WPKGSrv.exe, version 1.0.0.13, faulting module WPKGSrv.exe, version 1.0.0.13, fault address 0xa704. WPKG Service log file: 2009-10-05 12:08:24 - Starting WPKG on startup 2009-10-05 12:08:24 - Before create shared memory: successfuly done. 2009-10-05 12:08:24 - Create shared memory: successfuly done. 2009-10-05 12:08:24 - Set script security context: successfuly done. 2009-10-05 12:08:24 - Offline mode enabled: successfuly done. 2009-10-05 12:08:24 - Offline mode: server connecting method selected. This did not occur with 1.3.6 -- Configure bugmail: http://bugzilla.wpkg.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. - wpkg-users mailing list archives http://lists.wpkg.org/pipermail/wpkg-users/ ___ wpkg-users mailing list wpkg-users@lists.wpkg.org http://lists.wpkg.org/mailman/listinfo/wpkg-users
