Actually.. should pacman -Sc even be allowed if there is a lock file? If you were downloading a file, wouldn't it get deleted when uninstalled apps are removed from /var/cache?

I had the md5sum not pass for OOo, perhaps that's why.

Good catch Mr. Stonecrest. I (and hopefully Xavier) can take a look at this one.

$ sudo touch /var/lib/pacman/foo.bar

$ sudo pacman -Sc
Cache directory: /var/cache/pacman/pkg/
Do you want to remove uninstalled packages from cache? [Y/n] n
Database directory: /var/lib/pacman/
Do you want to remove unused repositories? [Y/n] y
Do you want to remove /var/lib/pacman/foo.bar? [Y/n] n
Database directory cleaned up

Looks like our friend alpm_option_get_syncdbs() is stupid. That should solve part of the problem, the other is requiring this cache clean to require a lock.
EDIT: scratch the above line, I can't read the code. However, I should be able to come up with a fix for this.

Here is a patch to fix part of the issue.

Ok, that looks good, I was also thinking about just checking directories. Perhaps we should use lstat here instead of stat, so that we skip directory symlinks too?

Now about the package clean part, yes, -Sc would have deleted everything you had just downloaded. All the first targets that were fully downloaded but not yet installed.
But even the one currently downloading will still look fine to pacman, because it only do a quick load of the archive (basically only reading .PKGINFO), instead of trying to
read the whole archive which really hurts the performance. And it will see that this package is not yet installed and should delete it too.

About locking the database for -Sc:
Do we need a public function in the backend as Nagy suggested, to allow the frontend to lock / unlock the database?
Because we have the same problem for -Sy and -Sc : both are not real transactions (no trans_addtarget, trans_prepare, trans_commit stuff),
but both require to lock the database.
We could do like I did with -Sy here : http://www.archlinux.org/pipermail/pacman-dev/2008-February/011201.html
I call trans_init and trans_release only for locking and unlocking the database. But I am not sure it's very nice :/
Maybe we should move these operations in the backend then, and implement them as real transactions (-Sy is already mostly in the backend, but -Sc is all in the frontend).

Or it might be better to just export lock / unlock functions. I have no idea..
So I'll trust Dan's judgment.

stat() will dereference the symlink, correct? So I think that behavior is OK.

Regarding the other stuff, I think we need a trans_init and trans_release that are lighter weight- in essence, true transaction operators. We'll worry about that for 3.2 though, and do the short-term fix for now for 3.1.2.

stat() will dereference the symlink, but then what will rmrf do? Just remove the symlink apparently. What about the directory it pointed to?
Anyway, that's a corner case, so not a big problem.

And so, what is the short-term fix? Using sync_trans_init and sync_trans_release like in my -Sy patch?
Can my -Sy patch be applied then ? Or only the part that contains these two functions?