Re: [PATCH] pack-refs: remove newly empty directories

From: Greg Price <price@ksplice.com>
Date: Jul 6, 2010, 19:13 UTC
Message-ID: <AANLkTilsfURYKOQC-kAtNrr1cTmULOXDTn0zxicgb9-S@mail.gmail.com>
In-Reply-To: <m2y6doqwch.fsf@igel.home>

On Tue, Jul 6, 2010 at 2:49 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:

> What happens if a parallel running git wants to update a ref in one of
> the now-empty directories?  Can it get a spurious error after it has
> called safe_create_leading_directories?

Good question! I asked the same question. =) It can get the same kind of error that happens if two parallel running git processes try to update the same ref.

Anything that tries to update a ref will call lock_ref_sha1_basic(), which calls safe_create_leading_directories() to make the directory and then hold_lock_file_for_update() to open(O_CREAT|O_EXCL) a lock file inside the directory. So if the prune happens after the open(), it will simply not remove the directory and all is well. If it happens between the safe_create_leading_directories() and the open(), then the open() will fail. This is also what happens if the lock has been taken by another git process updating the same ref. Currently lock_ref_sha1_basic() chooses to have the process die in this case and print an error message.

Similarly the prune could remove a directory just before safe_create_leading_directories() creates one inside it. Then safe_create_leading_directories() will fail and loc_ref_sha1_basic() will print an error message and return failure. In both cases, since we are exposed to a similar failure if a parallel process is updating the same ref (a common operation) rather than packing refs (an uncommon operation), I don't think this is a problem.

Greg