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

From: Jonathan Nieder <[email-masked]>
Date: Thu, 22 May 2014 11:17:22 -0700

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

Thanks,
Jonathan