Re: [PATCH v8 41/44] refs.c: add a new flag for transaction delete for refs we know are packed only

On Thu, May 22, 2014 at 11:17 AM, Jonathan Nieder [off-list ref] wrote:

Hi,

Ronnie Sahlberg wrote:

quoted

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