[pacman-dev] Transaction failure handling

[Xavier]

On Thu, Aug 30, 2007 at 11:50:51PM +0100, Andrew Fyfe wrote:
> Xavier wrote:
> > 
> > That causes another problem though...
> > in _alpm_trans_release, we have :
> > 
> > 184         /* during a commit do not interrupt immediately, just after a target */
> > 185         if(trans->state == STATE_COMMITING || trans->state == STATE_INTERRUPTED) {
> > 186                 if(trans->state == STATE_COMMITING) {
> > 187                         trans->state = STATE_INTERRUPTED;
> > 188                 }
> > 189                 pm_errno = PM_ERR_TRANS_COMMITING;
> > 190                 return(-1);
> > 191         }
> > 
> > So since the state now stays on STATE_COMMITING (without going back to STATE_PREPARED)
> > when a transaction fails, this code is now run, and the function returns here,
> > without removing the lock.
> > 
> > Now, I'm totally unable to fix this correctly, the thing is, I really don't understand
> > the purpose of this part, what it's intended to do, in which cases, etc ...
> > 
> > It annoyed me that pacman didn't remove the lock on each error (for example,
> > conflicting targets), so I had to hack this, but that most likely kills totally what
> > the original author had in mind, making the code worse than it is.
> > 
> > I guess what I'm trying to say here is that I need someone to look at this,
> > and fix it correctly.
> > Maybe I could revert that previous change in sync.c, so that the state goes back to
> > STATE_PREPARED on failure, and then do the same in add.c / remove.c , but I
> > still don't like that idea..
> > 
> 
> I think this is another +1 for a rewrite. It seems for every bug we fix 
> another harder to fix one pops up to take its place.
> 
> Andrew
> 

Right, probably.
But well, it at least requires a temporary fix in the meantime.
I found where this part I don't understand was introduced.
Fortunately, it is a small commit. Maybe looking at the commit as the whole,
and the one line description can help understand it :
http://projects.archlinux.org/git/?p=pacman.git;a=commit;h=54008798efcc9646f622f6b052ecd83281d57cda