[RFC] Failing to push to existing non-branch directories.

David Allouche david at allouche.net
Wed Sep 20 21:50:50 BST 2006


Apologies for replying so late. I overdosed on discussion a couple of
weeks ago then dived into an intense hacking binge.

Wouter van Heyst wrote:
> Different situations:
> 
> 1a) Dir does not exist (pretty obvious, create it, and fill it with the new contents)
> 
> 1b) Neither do parent directories. (create it only if create_prefix flag is set)
> 
> 2) Dir exists
> 
> 2a) Already a Branch present. Also pretty obvious. Just try to update the branch.
> 
> 2b) Directory is empty. Rather than puking, we probably should act like we created the directory.
> There is a race condition of 2 people pushing to the same location. Is it worth worrying about it?

I do not see that being qualitatively different, in terms of race
condition, than case 1a or 1b. In those cases we need to create the
branch directory and the .bzr directory, and in this case we only need
to create the .bzr directory: not an essential difference.

Ultimately, the race condition needs to be avoided by a dumb-fs locking
protocol. Remember that some filesystems allow concurrent mkdirs of the
same name to succeed.

> 2c) There exists a .bzr/ directory, but it doesn't seem to be valid.
>     Nuke it? Try to update it in place? Require a flag to avoid race
>     conditions?

I think we should nuke it.

Recent optimizations to initial knit creation use non-atomic file
writing to save on round-trips. It might be possible to detect
incompletely written knit files by checking the presence and contents of
all kndx files, but that seems more trouble than it is worth.

However, maybe should we avoid nuking an active lock at the same time. I
guess ideally we would first take a lock, then nuke everything but the
lock, then start filling in the data again. But IIRC the branch format
information is written last, so we cannot take the lock on an
incompletely created branch.

In the bug comments, I suggested adding a "--delete" or "--from-scratch"
option that just removes the .bzr directory. That would also be useful
in nuclear launch-codes/waste scenarios when somebody wants a "push
--overwrite" that leaves a target branch that has no more data than the
source branch.

Maybe we could avoid race conditions by creating the .bzr under a
temporary name (something like ".bzr-incomplete-<unique>", and then
renaming it to .bzr when it is complete. After the renaming was
successful we could delete all the existing directories that match the
temporary name pattern, so we would not leave cruft behind.

> 2d) There is no .bzr/ directory, but there are other files present.

That case makes no difference to Launchpad, since it only cares about
.bzr data, and arguably should even prevent creating anything else.

https://launchpad.net/products/launchpad-bazaar/+bug/53545

Note that if you decide to use a temporary name while constructing .bzr,
you should add a comment on that bug with a description of the temporary
name pattern.

I could imagine that branching/pushing/checking-out into an already
populated directory could be useful in some scenarios, and I see no
reason to forbid it. That would also help consistency with "bzr init".

> PS> See also:
> https://launchpad.net/products/launchpad-bazaar/+bug/53340
> https://launchpad.net/products/bzr/+bug/45504
> https://launchpad.net/products/bzr/+bug/30576

-- 
                                                            -- ddaa

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060920/ddde705e/attachment.pgp 


More information about the bazaar mailing list