Do not fake a SymbolicRef as an ObjectIdRef
When doing a detaching operation, JGit fakes a SymbolicRef as an ObjectIdRef. This is because RefUpdate#updateImpl dereferences the SymbolicRef when updating it. For example, assume that HEAD is pointing to refs/heads/master. If I try to make a detached HEAD pointing to a commit c0ffee, RefUpdate dereferences HEAD as refs/heads/master first and changes refs/heads/master to c0ffee. The detach argument of RefDatabase#newUpdate avoids this dereference by faking HEAD as ObjectIdRef. This faking is problematic for the linking operation of DfsRefDatabase. It does a compare-and-swap operation on every reference change because of its distributed systems nature. If a SymbolicRef is faked as an ObjectRef, it thinks that there is a racing change in the reference and rejects the update. Because of this, DFS based repositories cannot change the link target of symbolic refs. This has not been a problem for file-based repositories because they have a file-lock based semantics instead of the CAS based one. The reference implementation, InMemoryRepository, is not affected because it only compares ObjectIds. When [1] introduced this faking code, there was no way for RefUpdate to distinguish the detaching operation. When [2] fixed the detaching operation, it introduced a detachingSymbolicRef flag. This commit uses this flag to control whether it needs to dereference the symbolic refs by calling Ref#getLeaf. The same flag is used in the reflog update operation. This commit does not affect any operation that succeeds currently. In some DFS repository implementations, this fixes a ref linking operation, which is currently failing. [1]:01b5392cdb
[2]:3a86868c08
Change-Id: I118f85f0414dbfad02250944e28d74dddd59469b Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
This commit is contained in:
parent
ffbe03aa9b
commit
3e27fb3719
|
@ -217,10 +217,6 @@ public RefUpdate newUpdate(String refName, boolean detach)
|
||||||
else
|
else
|
||||||
detachingSymbolicRef = detach && ref.isSymbolic();
|
detachingSymbolicRef = detach && ref.isSymbolic();
|
||||||
|
|
||||||
if (detachingSymbolicRef) {
|
|
||||||
ref = new ObjectIdRef.Unpeeled(NEW, refName, ref.getObjectId());
|
|
||||||
}
|
|
||||||
|
|
||||||
DfsRefUpdate update = new DfsRefUpdate(this, ref);
|
DfsRefUpdate update = new DfsRefUpdate(this, ref);
|
||||||
if (detachingSymbolicRef)
|
if (detachingSymbolicRef)
|
||||||
update.setDetachingSymbolicRef();
|
update.setDetachingSymbolicRef();
|
||||||
|
|
|
@ -545,8 +545,6 @@ public RefDirectoryUpdate newUpdate(String name, boolean detach)
|
||||||
ref = new ObjectIdRef.Unpeeled(NEW, name, null);
|
ref = new ObjectIdRef.Unpeeled(NEW, name, null);
|
||||||
else {
|
else {
|
||||||
detachingSymbolicRef = detach && ref.isSymbolic();
|
detachingSymbolicRef = detach && ref.isSymbolic();
|
||||||
if (detachingSymbolicRef)
|
|
||||||
ref = new ObjectIdRef.Unpeeled(LOOSE, name, ref.getObjectId());
|
|
||||||
}
|
}
|
||||||
RefDirectoryUpdate refDirUpdate = new RefDirectoryUpdate(this, ref);
|
RefDirectoryUpdate refDirUpdate = new RefDirectoryUpdate(this, ref);
|
||||||
if (detachingSymbolicRef)
|
if (detachingSymbolicRef)
|
||||||
|
@ -579,7 +577,7 @@ private void putLooseRef(LooseRef ref) {
|
||||||
}
|
}
|
||||||
|
|
||||||
void delete(RefDirectoryUpdate update) throws IOException {
|
void delete(RefDirectoryUpdate update) throws IOException {
|
||||||
Ref dst = update.getRef().getLeaf();
|
Ref dst = update.getRef();
|
||||||
String name = dst.getName();
|
String name = dst.getName();
|
||||||
|
|
||||||
// Write the packed-refs file using an atomic update. We might
|
// Write the packed-refs file using an atomic update. We might
|
||||||
|
|
|
@ -56,6 +56,7 @@
|
||||||
class RefDirectoryUpdate extends RefUpdate {
|
class RefDirectoryUpdate extends RefUpdate {
|
||||||
private final RefDirectory database;
|
private final RefDirectory database;
|
||||||
|
|
||||||
|
private boolean shouldDeref;
|
||||||
private LockFile lock;
|
private LockFile lock;
|
||||||
|
|
||||||
RefDirectoryUpdate(final RefDirectory r, final Ref ref) {
|
RefDirectoryUpdate(final RefDirectory r, final Ref ref) {
|
||||||
|
@ -75,6 +76,7 @@ protected Repository getRepository() {
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
protected boolean tryLock(boolean deref) throws IOException {
|
protected boolean tryLock(boolean deref) throws IOException {
|
||||||
|
shouldDeref = deref;
|
||||||
Ref dst = getRef();
|
Ref dst = getRef();
|
||||||
if (deref)
|
if (deref)
|
||||||
dst = dst.getLeaf();
|
dst = dst.getLeaf();
|
||||||
|
@ -117,7 +119,7 @@ protected Result doUpdate(final Result status) throws IOException {
|
||||||
msg = strResult;
|
msg = strResult;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
database.log(this, msg, true);
|
database.log(this, msg, shouldDeref);
|
||||||
}
|
}
|
||||||
if (!lock.commit())
|
if (!lock.commit())
|
||||||
return Result.LOCK_FAILURE;
|
return Result.LOCK_FAILURE;
|
||||||
|
@ -140,7 +142,7 @@ private String toResultString(final Result status) {
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
protected Result doDelete(final Result status) throws IOException {
|
protected Result doDelete(final Result status) throws IOException {
|
||||||
if (getRef().getLeaf().getStorage() != Ref.Storage.NEW)
|
if (getRef().getStorage() != Ref.Storage.NEW)
|
||||||
database.delete(this);
|
database.delete(this);
|
||||||
return status;
|
return status;
|
||||||
}
|
}
|
||||||
|
|
|
@ -551,7 +551,9 @@ public Result delete() throws IOException {
|
||||||
* @throws IOException
|
* @throws IOException
|
||||||
*/
|
*/
|
||||||
public Result delete(final RevWalk walk) throws IOException {
|
public Result delete(final RevWalk walk) throws IOException {
|
||||||
final String myName = getRef().getLeaf().getName();
|
final String myName = detachingSymbolicRef
|
||||||
|
? getRef().getName()
|
||||||
|
: getRef().getLeaf().getName();
|
||||||
if (myName.startsWith(Constants.R_HEADS) && !getRepository().isBare()) {
|
if (myName.startsWith(Constants.R_HEADS) && !getRepository().isBare()) {
|
||||||
// Don't allow the currently checked out branch to be deleted.
|
// Don't allow the currently checked out branch to be deleted.
|
||||||
Ref head = getRefDatabase().getRef(Constants.HEAD);
|
Ref head = getRefDatabase().getRef(Constants.HEAD);
|
||||||
|
@ -628,7 +630,10 @@ private Result updateImpl(final RevWalk walk, final Store store)
|
||||||
if (oldValue == null && checkConflicting && getRefDatabase().isNameConflicting(getName()))
|
if (oldValue == null && checkConflicting && getRefDatabase().isNameConflicting(getName()))
|
||||||
return Result.LOCK_FAILURE;
|
return Result.LOCK_FAILURE;
|
||||||
try {
|
try {
|
||||||
if (!tryLock(true))
|
// If we're detaching a symbolic reference, we should update the reference
|
||||||
|
// itself. Otherwise, we will update the leaf reference, which should be
|
||||||
|
// an ObjectIdRef.
|
||||||
|
if (!tryLock(!detachingSymbolicRef))
|
||||||
return Result.LOCK_FAILURE;
|
return Result.LOCK_FAILURE;
|
||||||
if (expValue != null) {
|
if (expValue != null) {
|
||||||
final ObjectId o;
|
final ObjectId o;
|
||||||
|
|
Loading…
Reference in New Issue