Fix Selectable implementation to honour causer.

diff --git a/VERSION.cmake b/VERSION.cmake
index f3fc64f..9a88115 100644 (file)
--- a/VERSION.cmake
+++ b/VERSION.cmake
@@ -61,7 +61,7 @@
 SET(LIBZYPP_MAJOR "5")
 SET(LIBZYPP_COMPATMINOR "5")
 SET(LIBZYPP_MINOR "5")
-SET(LIBZYPP_PATCH "1")
+SET(LIBZYPP_PATCH "2")
 #
 # LAST RELEASED: 5.5.1 (5)
 # (The number in parenthesis is LIBZYPP_COMPATMINOR)

diff --git a/zypp/ResStatus.h b/zypp/ResStatus.h
index cd078f7..ca76654 100644 (file)
--- a/zypp/ResStatus.h
+++ b/zypp/ResStatus.h
@@ -22,6 +22,7 @@ namespace zypp
   namespace resstatus
   {
     class UserLockQueryManip;
+    class StatusBackup;
   }
 
   ///////////////////////////////////////////////////////////////////
@@ -669,6 +670,7 @@ namespace zypp
          { return _bitfield.value<_Field>() < val_r; }
 
   private:
+    friend class resstatus::StatusBackup;
     BitFieldType _bitfield;
   };
   ///////////////////////////////////////////////////////////////////
@@ -684,7 +686,32 @@ namespace zypp
   inline bool operator!=( const ResStatus & lhs, const ResStatus & rhs )
   { return ! (lhs == rhs); }
 
-  /////////////////////////////////////////////////////////////////
+  ///////////////////////////////////////////////////////////////////
+
+  namespace resstatus
+  {
+    class StatusBackup
+    {
+      public:
+        StatusBackup()
+        : _status( 0 )
+        {}
+
+        StatusBackup( ResStatus & status_r )
+        : _status( &status_r )
+        , _bitfield( _status->_bitfield )
+        {}
+
+        void replay()
+        { if ( _status ) _status->_bitfield = _bitfield; }
+
+      private:
+        ResStatus *             _status;
+        ResStatus::BitFieldType _bitfield;
+    };
+  }
+
+ /////////////////////////////////////////////////////////////////
 } // namespace zypp
 ///////////////////////////////////////////////////////////////////
 #endif // ZYPP_RESSTATUS_H

diff --git a/zypp/ui/SelectableImpl.cc b/zypp/ui/SelectableImpl.cc
index a01221f..78e2ba4 100644 (file)
--- a/zypp/ui/SelectableImpl.cc
+++ b/zypp/ui/SelectableImpl.cc
@@ -35,6 +35,7 @@ namespace zypp
       : _impl( impl )
       , inst( impl.installedObj() )
       , cand( impl.candidateObj() )
+      , causer( causer_r )
       {}
 
       typedef Selectable::Impl::available_const_iterator available_const_iterator;
@@ -57,103 +58,148 @@ namespace zypp
       bool hasBoth() const
       { return inst && cand; }
 
-      //
-      // ResStatus manip
-      //
-      void resetTransactingCandidates() const
+      /** \name Topevel methods must restore status on failure. */
+      //@{
+      bool setInstall()
       {
-        for ( available_const_iterator it = _impl.availableBegin();
-              it != _impl.availableEnd(); ++it )
-          {
-            (*it).status().setTransact( false, ResStatus::USER );
+        if ( cand )
+        {
+          if ( inst ) {
+            ResStatus & inststatus( backup( inst.status() ) );
+            if ( ! inststatus.setTransact( false, causer ) ) return restore();
+            if ( ! inststatus.setLock    ( false, causer ) ) return restore();
+            if ( ! cand->installOnly() )
+            {
+              // This is what the solver most probabely will do.
+              // If we are wrong the solver will correct it. But
+              // this way we will get a better disk usage result,
+              // even if no autosolving is on.
+              inststatus.setTransact( true, ResStatus::SOLVER );
+            }
           }
+          if ( ! unlockCandidates() ) return restore();
+          ResStatus & candstatus( backup( cand.status() ) );
+          if ( ! candstatus.setTransact( true, causer ) ) return restore();
+          return true;
+        }
+        return false;
       }
-      void unlockCandidates() const
+
+      bool setDelete()
       {
-        for ( available_const_iterator it = _impl.availableBegin();
-              it != _impl.availableEnd(); ++it )
-          {
-            (*it).status().setTransact( false, ResStatus::USER );
-            (*it).status().setLock( false, ResStatus::USER );
-          }
+        if ( inst )
+        {
+          if ( ! resetTransactingCandidates() ) return restore();
+          ResStatus & inststatus( backup( inst.status() ) );
+          if ( ! inststatus.setLock( false, causer ) ) return restore();
+          if ( ! inststatus.setTransact( true, causer ) ) return restore();
+          return true;
+        }
+        return false;
       }
-      void lockCandidates() const
+
+      bool unset()
       {
-        for ( available_const_iterator it = _impl.availableBegin();
-              it != _impl.availableEnd(); ++it )
-          {
-            (*it).status().setTransact( false, ResStatus::USER );
-            (*it).status().setLock( true, ResStatus::USER );
-          }
+        if ( inst )
+        {
+          ResStatus & inststatus( backup( inst.status() ) );
+          if ( ! inststatus.setTransact( false, causer ) ) return restore();
+          if ( ! inststatus.setLock( false, causer ) ) return restore();
+        }
+        if ( ! unlockCandidates() ) return restore();
+        return true;
       }
 
-      bool setInstall() const
+      bool setProtected()
       {
-        if ( cand )
-          {
-             if ( inst ) {
-                 inst.status().setTransact( false, ResStatus::USER );
-                 inst.status().setLock    ( false, ResStatus::USER );
-                  if ( ! cand->installOnly() )
-                  {
-                    // This is what the solver most probabely will do.
-                    // If we are wrong the solver will correct it. But
-                    // this way we will get a better disk usage result,
-                    // even if no autosolving is on.
-                    inst.status().setTransact( true,  ResStatus::SOLVER );
-                  }
-             }
-              unlockCandidates();
-             return cand.status().setTransact( true, ResStatus::USER );
-          }
-        return false;
+        if ( causer != ResStatus::USER ) // by user only
+          return false;
+
+        if ( inst ) {
+          resetTransactingCandidates();
+          inst.status().setTransact( false, causer );
+          return inst.status().setLock( true, causer );
+        } else
+          return false;
       }
 
-      bool setDelete() const
+      bool setTaboo()
       {
-        if ( inst )
-          {
-            resetTransactingCandidates();
-           inst.status().setLock( false, ResStatus::USER );
-            return inst.status().setTransact( true, ResStatus::USER );
-          }
-        return false;
+        if ( causer != ResStatus::USER ) // by user only
+          return false;
+
+        if ( cand ) {
+          lockCandidates();
+          return true;
+        } else
+          return false;
       }
+      //@}
 
-      bool unset() const
+    private:
+      /** \name Helper methods backup status but do not replay. */
+      //@{
+      bool resetTransactingCandidates()
+      {
+        for_( it, _impl.availableBegin(), _impl.availableEnd() )
+        {
+          ResStatus & status( backup( (*it).status() ) );
+          if ( ! status.setTransact( false, causer ) ) return false;
+        }
+        return true;
+      }
+      bool unlockCandidates()
       {
-         if ( inst ) {
-             inst.status().setTransact( false, ResStatus::USER );
-             inst.status().setLock( false, ResStatus::USER );
-         }
-          unlockCandidates();
-         return true;
+        for_( it, _impl.availableBegin(), _impl.availableEnd() )
+        {
+          ResStatus & status( backup( (*it).status() ) );
+          if ( ! status.setTransact( false, causer ) ) return false;
+          if ( ! status.setLock( false, causer ) ) return false;
+        }
+        return true;
       }
-
-      bool setProtected() const
+      bool lockCandidates()
       {
-         if ( inst ) {
-              resetTransactingCandidates();
-             inst.status().setTransact( false, ResStatus::USER );
-             return inst.status().setLock( true, ResStatus::USER );
-         } else
-             return false;
+        for_( it, _impl.availableBegin(), _impl.availableEnd() )
+        {
+          ResStatus & status( backup( (*it).status() ) );
+          if ( ! status.setTransact( false, causer ) ) return false;
+          if ( ! status.setLock( true, causer ) ) return false;
+        }
+        return true;
       }
+      //@}
+
+    private:
+      const Selectable::Impl & _impl;
+      PoolItem                   inst;
+      PoolItem                   cand;
+      ResStatus::TransactByValue causer;
+
+    private:
+      // No backup replay needed if causer is user,
+      // because actions should always succeed.
 
-      bool setTaboo() const
+      ResStatus & backup( ResStatus & status_r )
       {
-         if ( cand ) {
-              lockCandidates();
-             return true;
-         } else
-             return false;
+        if ( causer != ResStatus::USER )
+          _backup.push_back( status_r );
+        return status_r;
       }
 
+      bool restore()
+      {
+        if ( causer != ResStatus::USER )
+        {
+          for_( rit, _backup.rbegin(), _backup.rend() )
+          {
+            rit->replay();
+          }
+        }
+        return false; // restore is done on error - return restore();
+      }
 
-    public:
-      const Selectable::Impl & _impl;
-      PoolItem inst;
-      PoolItem cand;
+      std::vector<resstatus::StatusBackup> _backup;
     };
     ///////////////////////////////////////////////////////////////////