There is something I don't understand: We have trans->state and handle->trans->state. This is intentional? Probably you wanted to set handle->trans->state. No, if things are sane, I want to abort the *current* transaction. We are looking at that one's packages. In this case, because this is the add transaction, we should only have one? Or am I completely off and we still make another sub-transaction.
I may completely misunderstand something. So I guess that the purpose of this patch, that pacman should stop immediately after package upgrade if error occurs (not trying to commit the whole transaction, ie. installing the remaining packages.) This will work with -A/-U, where trans == handle->trans. However, if trans != handle->trans, setting trans->state has no effect, because add_commit watches the state of handle->trans only (in the first line of the loop). This is the situation with -S: handle->trans is a sync transaction, but sync_commit will create a new upgrade transaction, and it will commit it (this is passed to add_commit). So as I see, this patch has no effect on sync transaction. Maybe you forgot to insert a "break;" somewhere... Bye