I'd like to point out that a recent trunk change took a block of code 
and changed the indentation.  It might be "right" but it's "different 
from everything else in that function".

Also, the changes did not include the extra precautions about package 
query failure -- which could happen legitimately, and not just due to 
the recent bug.  Please accept this patch:

Index: do.c
===================================================================
--- do.c        (revision 428)
+++ do.c        (working copy)
@@ -2794,12 +2794,21 @@

     match = PackageCheck(ptr->name, ptr->pkgmgr, ptr->ver, ptr->cmp);

+   /* Check for a problem executing the command */
+   if ( (match != 1) && (match != 0) )
+      {
+      snprintf(OUTPUT,CF_BUFSIZE,"Error: Package manager query failed, 
skipping %s\n", ptr->name);
+      CfLog(cferror,OUTPUT,"");
+      ptr->done = 'y';
+      continue;
+      }
+
     /* Process any queued actions (install/remove). */
     if ((pending_pkgs != NULL) && ((ptr->action != prev_action) || 
(ptr->pkgmgr != prev_pkgmgr)))
        {

Also note that there is a bug in package deletion which apparently was 
overlooked until I fixed the output to tell us what the package manager 
said, and now everyone noticed ;-)

We don't need to call AppendItem, that function is handled by the 
PackageList() directly.

@@ -2809,7 +2818,6 @@
        if (ptr->action == pkgaction_remove)
           {
           PackageList(ptr->name, ptr->pkgmgr, ptr->ver, ptr->cmp, 
&pending_pkgs);
-         AppendItem(&pending_pkgs, ptr->name, NULL);

           /* Some package managers operate best doing things one at a 
time. */

Also I have now gotten numerous requests to have even freebsd and sun 
defer package deletions because it turns out both systems have 
overlapping compatibility problems and sometimes you have to remove 
three packages together to get them all to remove.  So you could instead 
adopt this:

@@ -2809,16 +2818,6 @@
        if (ptr->action == pkgaction_remove)
           {
           PackageList(ptr->name, ptr->pkgmgr, ptr->ver, ptr->cmp, 
&pending_pkgs);
-         AppendItem(&pending_pkgs, ptr->name, NULL);
-
-         /* Some package managers operate best doing things one at a 
time. */
-
-         if ((ptr->pkgmgr == pkgmgr_freebsd) || (ptr->pkgmgr == 
pkgmgr_sun))
-            {
-            RemovePackage( ptr->pkgmgr, &pending_pkgs );
-            DeleteItemList( pending_pkgs );
-            pending_pkgs = NULL;
-            }
           }
        else if (ptr->action == pkgaction_upgrade)
           {


-- 
Jo Rhett
senior geek
Silicon Valley Colocation
_______________________________________________
Bug-cfengine mailing list
[email protected]
https://cfengine.org/mailman/listinfo/bug-cfengine

Reply via email to