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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]