I hate rename_ref :-) I have reworked the transaction code to special case the deletion of the old ref for n/n -> n and n -> n/n renames so that we can carefully avoid n/n.lock files to exist or prevent the directory <-> file transition for n during these renames. This should allow us to have rename_ref becoming reasonably implementation agnostic, aside for that it wants to lstat the ref to see if it is a soft link, and re-use the code for other refs backends. Please see the ref-transaction branch. On Thu, May 22, 2014 at 12:12 PM, Ronnie Sahlberg <sahlberg@xxxxxxxxxx> wrote: > On Thu, May 22, 2014 at 11:17 AM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote: >> Hi, >> >> Ronnie Sahlberg wrote: >> >>> Add a new flag REF_ISPACKONLY that we can use in ref_transaction_delete. >>> This flag indicates that the ref does not exist as a loose ref andf only as >>> a packed ref. If this is the case we then change the commit code so that >>> we skip taking out a lock file and we skip calling delete_ref_loose. >> >> This seems wrong. Can't someone else create a loose ref which will >> shadow the packed ref and break the serializability of updates to this >> ref? >> >> The above doesn't explain why we want to avoid locking the loose ref, >> but I assume it's for the sake of the "git branch -m foo/bar foo" >> case. For that case, wouldn't the following sequence of filesystem >> operations work? >> >> - create $GIT_DIR/refs/heads/foo/bar.lock >> - create $GIT_DIR/refs/heads/foo.lock >> - if $GIT_DIR/refs/heads/foo/bar exists, add the ref to >> packed-refs (using the usual lockfile-update mechanism) >> and unlink $GIT_DIR/refs/heads/foo/bar >> - verify that current foo and foo/bar state are okay. If >> not, roll back. >> - unlink $GIT_DIR/refs/heads/foo/bar.lock >> - rmdir $GIT_DIR/refs/heads/foo >> - rename $GIT_DIR/refs/heads/foo.lock into place >> >> Or: >> >> - create $GIT_DIR/refs/heads/foo/bar.lock >> - create $GIT_DIR/refs/heads/foo.lock >> - verify state of all relevant refs >> >> - update packed-refs to remove refs/heads/foo/bar and >> add refs/heads/foo >> >> - unlink $GIT_DIR/refs/heads/foo/bar >> - unlink $GIT_DIR/refs/heads/foo >> - unlink $GIT_DIR/refs/heads/foo/bar.lock >> - unlink $GIT_DIR/refs/heads/foo.lock >> > > > I removed all the rename_ref related patches for now. rename_ref is > probably best implemented specifically for each backend anyway. > > I will still produce a separate patch that will do something like this > you suggested > (since rename_ref is still racy and fragile) > >> - create $GIT_DIR/refs/heads/foo/bar.lock >> - create $GIT_DIR/refs/heads/foo.lock >> - verify state of all relevant refs >> >> - update packed-refs to remove refs/heads/foo/bar and >> add refs/heads/foo >> >> - unlink $GIT_DIR/refs/heads/foo/bar >> - unlink $GIT_DIR/refs/heads/foo >> - unlink $GIT_DIR/refs/heads/foo/bar.lock >> - unlink $GIT_DIR/refs/heads/foo.lock >> > > Thanks > ronnie sahlberg -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html