Detaching HEAD when checking out the same commit.

Detaching HEAD didn't work in some corner checkout cases.  If, for example,
HEAD is symbolic ref to refs/heads/master, refs/heads/master is ref to commit
c0ffee... then:

    checkout c0ffee...

would leave the HEAD unchanged.

The same symptom occurs when checking out a remote tracking branch or a tag
that references the same commit as refs/heads/master.

In the above case, the RefUpdate class didn't have enough information to decide
if the update needed to detach symbolic ref because it dealt only with new/old
objectIDs. Therefore, this fix introduced the RefUpdate.detachingSymbolicRef
flag.

Bug: 315166
Change-Id: I085c98b77ea8f9104a213978ea0d4ac6fd58f49b
Signed-off-by: Sasa Zivkov <sasa.zivkov@sap.com>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CheckoutCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CheckoutCommandTest.java
index 21fbe0a1347e5010e1a6be98d8270077e01b0ac0..6e9f851793c31af9bb0bb2ab520dcbd0a1066892 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CheckoutCommandTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CheckoutCommandTest.java
@@ -47,6 +47,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -239,4 +240,20 @@ public class CheckoutCommandTest extends RepositoryTestCase {
                        fail(e.getMessage());
                }
        }
+
+       @Test
+       public void testDetachedHeadOnCheckout() throws JGitInternalException,
+                       RefAlreadyExistsException, RefNotFoundException,
+                       InvalidRefNameException, IOException {
+               CheckoutCommand co = git.checkout();
+               co.setName("master").call();
+
+               String commitId = db.getRef(Constants.MASTER).getObjectId().name();
+               co = git.checkout();
+               co.setName(commitId).call();
+
+               Ref head = db.getRef(Constants.HEAD);
+               assertFalse(head.isSymbolic());
+               assertSame(head, head.getTarget());
+       }
 }

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java
index e6f89333892c3b5a15f29b21646b20f18e56b4c8..24af944f2fe1c198d6e33ee1118459a3d90e1f94 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java
@@ -167,6 +167,17 @@ public abstract class RefUpdate {
 
        private final Ref ref;
 
+       /**
+        * Is this RefUpdate detaching a symbolic ref?
+        *
+        * We need this info since this.ref will normally be peeled of in case of
+        * detaching a symbolic ref (HEAD for example).
+        *
+        * Without this flag we cannot decide whether the ref has to be updated or
+        * not in case when it was a symbolic ref and the newValue == oldValue.
+        */
+       private boolean detachingSymbolicRef;
+
        /**
         * Construct a new update operation for the reference.
         * <p>
@@ -253,6 +264,13 @@ public abstract class RefUpdate {
                return newValue;
        }
 
+       /**
+        * Tells this RefUpdate that it is actually detaching a symbolic ref.
+        */
+       public void setDetachingSymbolicRef() {
+               detachingSymbolicRef = true;
+       }
+
        /**
         * Set the new value the ref will update to.
         *
@@ -596,7 +614,7 @@ public abstract class RefUpdate {
 
                        newObj = safeParse(walk, newValue);
                        oldObj = safeParse(walk, oldValue);
-                       if (newObj == oldObj)
+                       if (newObj == oldObj && !detachingSymbolicRef)
                                return store.execute(Result.NO_CHANGE);
 
                        if (newObj instanceof RevCommit && oldObj instanceof RevCommit) {

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectory.java
index e8ce2c546775ebd9f8ec6e412d4acaf0931ce426..c3402df672fe471625a0bb9cf03456f0fd11bb66 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectory.java
@@ -491,15 +491,22 @@ public class RefDirectory extends RefDatabase {
 
        public RefDirectoryUpdate newUpdate(String name, boolean detach)
                        throws IOException {
+               boolean detachingSymbolicRef = false;
                final RefList<Ref> packed = getPackedRefs();
                Ref ref = readRef(name, packed);
                if (ref != null)
                        ref = resolve(ref, 0, null, null, packed);
                if (ref == null)
                        ref = new ObjectIdRef.Unpeeled(NEW, name, null);
-               else if (detach && ref.isSymbolic())
-                       ref = new ObjectIdRef.Unpeeled(LOOSE, name, ref.getObjectId());
-               return new RefDirectoryUpdate(this, ref);
+               else {
+                       detachingSymbolicRef = detach && ref.isSymbolic();
+                       if (detachingSymbolicRef)
+                               ref = new ObjectIdRef.Unpeeled(LOOSE, name, ref.getObjectId());
+               }
+               RefDirectoryUpdate refDirUpdate = new RefDirectoryUpdate(this, ref);
+               if (detachingSymbolicRef)
+                       refDirUpdate.setDetachingSymbolicRef();
+               return refDirUpdate;
        }
 
        @Override