From 01dac80b344dd1c1752aec44fc79c2e0402382ef Mon Sep 17 00:00:00 2001 From: Michael Andres Date: Thu, 14 Aug 2008 19:58:30 +0000 Subject: [PATCH] Fix Selectable implementation to honour causer. --- VERSION.cmake | 2 +- zypp/ResStatus.h | 29 ++++++- zypp/ui/SelectableImpl.cc | 194 ++++++++++++++++++++++++++++------------------ 3 files changed, 149 insertions(+), 76 deletions(-) diff --git a/VERSION.cmake b/VERSION.cmake index f3fc64f..9a88115 100644 --- 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 --- 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 --- 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 _backup; }; /////////////////////////////////////////////////////////////////// -- 2.7.4