Hi!

While playing around with the skipbroken code I run into some problems when transaction members are removed and the packages is readded later on. Reason for this is that __cmp__ and __hash__ methods are overloaded in the TransactionMember class. This means that transaction members containing pkgs with the same pkgtup are treated as equal. Because of this the newly added transaction members are found in the already_seen(_removed) dicts if the old "equal" transaction members still linger there. A similar problem exists for TsInfo.removedmembers.

The current problem could be worked around by resetting already_seen* (as done by Jeremy's patch) and TsInfo.removedmembers (still missing) at the begin of the depsolving. But as soon as we try to move the skipbroken code deeper into the depsolving loop this will no longer be sufficient. Resetting the already_seen information also means restarting the depsolving from the start and loosing all the work we already did.

The attached patch works around this two problems by moving to a list (set) of transaction members that needs to be checked (TsInfo._unresolvedMembers) and keeps it up to date. It also removes the no longer needed already_seen* dicts and removedmembers attribute and related methods.

The new TsInfo.resetResolved() method feature a hard=False default behavior that keeps the already done work if this is better than starting from scratch which may be the case after big changes (GUI applications come to mind).

I am a bit unsure how this implementation would cope with reinstalls (adding install and remove transaction members for the same package) but IIRC these are not supported in yum anyway and it is currently not possible to do such things even from the yum shell. I also guess that some other code pieces would need work to support reinstalls.

So my main questions are:

 * Does this look sane?

* Are any of the removed pieces part of the "official" yum API and need to stay?


Florian
>From 4640fca641dba81501c29754b57dc2b221588d1e Mon Sep 17 00:00:00 2001
From: Florian Festi <[EMAIL PROTECTED]>
Date: Wed, 28 Nov 2007 17:33:25 +0100
Subject: [PATCH] Add tsInfo.getUnresolvedMembers() and use if for depsolving
 Drop tsInfo.removedmembers and DepCheck.already_seen*

---
 yum/depsolve.py        |   24 +++++-------------------
 yum/transactioninfo.py |   35 ++++++++++++++++++++---------------
 2 files changed, 25 insertions(+), 34 deletions(-)

diff --git a/yum/depsolve.py b/yum/depsolve.py
index 48dd48c..1160dd4 100644
--- a/yum/depsolve.py
+++ b/yum/depsolve.py
@@ -699,10 +699,7 @@ class Depsolve(object):
         # holder object for things from the check
         if not hasattr(self, '_dcobj'):
             self._dcobj = DepCheck()
-        else:
-            # reset what we've seen as things may have changed between calls
-            # to resolveDeps (rh#242368, rh#308321)
-            self._dcobj.reset()
+        self.tsInfo.resetResolved(hard=False)
 
         CheckDeps = True
         CheckRemoves = False
@@ -784,20 +781,17 @@ class Depsolve(object):
         CheckRemoves = False
         # we need to check the opposite of install and remove for regular
         # tsInfo members vs removed members
-        for (txmbr, inst, rem) in map(lambda x: (x, TS_INSTALL_STATES, TS_REMOVE_STATES), self.tsInfo.getMembers()) + map(lambda x: (x, TS_REMOVE_STATES, TS_INSTALL_STATES), self.tsInfo.getRemovedMembers()):
-            if (self._dcobj.already_seen_removed.has_key(txmbr) or
-                (txmbr.ts_state is not None and self._dcobj.already_seen.has_key(txmbr))):
-                continue
+        for txmbr in self.tsInfo.getUnresolvedMembers():
 
             if self.dsCallback and txmbr.ts_state:
                 self.dsCallback.pkgAdded(txmbr.pkgtup, txmbr.ts_state)
             self.verbose_logger.log(logginglevels.DEBUG_2,
                                     "Checking deps for %s" %(txmbr,))
 
-            if txmbr.output_state in inst:
+            if (txmbr.output_state in TS_INSTALL_STATES) == (txmbr.po.state != None):
                 thisneeds = self._checkInstall(txmbr)
                 CheckInstalls = True
-            elif txmbr.output_state in rem:
+            else:
                 thisneeds = self._checkRemove(txmbr)
                 CheckRemoves = True
 
@@ -809,10 +803,7 @@ class Depsolve(object):
                 missing_in_pkg |= missing
 
             if not missing_in_pkg:
-                if txmbr.ts_state is None:
-                    self._dcobj.already_seen_removed[txmbr] = 1
-                else:
-                    self._dcobj.already_seen[txmbr] = 1
+                self.tsInfo.markAsResolved(txmbr)
 
             any_missing |= missing_in_pkg
 
@@ -986,11 +977,6 @@ class DepCheck(object):
     def __init__(self):
         self.requires = []
         self.conflicts = []
-        self.reset()
-
-    def reset(self):
-        self.already_seen = {}
-        self.already_seen_removed = {}
 
     def addRequires(self, po, req_tuple_list):
         # fixme - do checking for duplicates or additions in here to zip things along
diff --git a/yum/transactioninfo.py b/yum/transactioninfo.py
index 8aed378..d29cd95 100644
--- a/yum/transactioninfo.py
+++ b/yum/transactioninfo.py
@@ -41,7 +41,7 @@ class TransactionData:
         self.root = '/'
         self.pkgdict = {} # key = pkgtup, val = list of TransactionMember obj
         self._namedict = {} # name -> list of TransactionMember obj
-        self.removedmembers = {}
+        self._unresolvedMembers = ReplaceSet()
         self.debug = 0
         self.changed = False
         
@@ -93,20 +93,18 @@ class TransactionData:
             returnlist.extend(self.pkgdict[pkgtup])
         return returnlist
             
-    def getRemovedMembers(self, pkgtup=None):
-        """takes an optional package tuple and returns all transaction members
-           matching, no pkgtup means it returns all transaction members"""
+    def getUnresolvedMembers(self):
+        return list(self._unresolvedMembers)
 
-        returnlist = []
-
-        if pkgtup is None:
-            for members in self.removedmembers.itervalues():
-                returnlist.extend(members)
-        elif self.removedmembers.has_key(pkgtup):
-            returnlist.extend(self.removedmembers[pkgtup])
-
-        return returnlist
+    def markAsResolved(self, txmbr):
+        self._unresolvedMembers.discard(txmbr)
 
+    def resetResolved(self, hard=False):
+        if hard or len(self) < len(self._unresolvedMembers):
+            self._unresolvedMembers.clear()
+            self._unresolvedMembers.update(self.getMembers())
+            return True
+        return False
 
     def getMode(self, name=None, arch=None, epoch=None, ver=None, rel=None):
         """returns the mode of the first match from the transaction set, 
@@ -183,7 +181,7 @@ class TransactionData:
             for po in self.conditionals[txmember.name]:
                 condtxmbr = self.addInstall(po)
                 condtxmbr.setAsDep(po=txmember.po)
-        
+        self._unresolvedMembers.add(txmember)
 
     def remove(self, pkgtup):
         """remove a package from the transaction"""
@@ -197,8 +195,8 @@ class TransactionData:
             elif isinstance(txmbr.po, YumAvailablePackageSqlite):
                 self.pkgSackPackages -= 1
             self._namedict[txmbr.name].remove(txmbr)
+            self._unresolvedMembers.add(txmbr)
         
-        self.removedmembers.setdefault(pkgtup, []).extend(self.pkgdict[pkgtup])
         del self.pkgdict[pkgtup]
         if not self._namedict[pkgtup[0]]:
             del self._namedict[pkgtup[0]]
@@ -556,3 +554,10 @@ class TransactionMember:
     # relationships are defined in constants
     # ts states are: u, i, e
     
+class ReplaceSet(set):
+
+    def add(self, obj):
+        """Make sure the added obj is in the set even
+        if an "identical" object is already included"""
+        self.discard(obj)
+        set.add(self, obj)
-- 
1.5.3.3

_______________________________________________
Yum-devel mailing list
[email protected]
https://lists.dulug.duke.edu/mailman/listinfo/yum-devel

Reply via email to