]> source.dussan.org Git - jgit.git/commitdiff
Replace writeSymref with RefUpdate.link 95/195/6
authorShawn O. Pearce <spearce@spearce.org>
Sun, 10 Jan 2010 02:56:45 +0000 (18:56 -0800)
committerShawn O. Pearce <spearce@spearce.org>
Sat, 23 Jan 2010 19:10:56 +0000 (11:10 -0800)
By using RefUpdate for symbolic reference creation we can reuse
the logic related to updating the reflog with the event, without
needing to expose something such as the legacy ReflogWriter class
(which we no longer have).

Applications using writeSymref must update their code to use the
new pattern of changing the reference through the updateRef method:

    String refName = "refs/heads/master";
    RefUpdate u = repository.updateRef(Constants.HEAD);
    u.setRefLogMessage("checkout: moving to " + refName, false);
    switch (u.link(refName)) {
    case NEW:
    case FORCED:
    case NO_CHANGE:
        // A successful update of the reference
        break;
    default:
        // Handle the failure, e.g. for older behavior
        throw new IOException(u.getResult());
    }

Change-Id: I1093e1ec2970147978a786cfdd0a75d0aebf8010
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Clone.java
org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RefTest.java
org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RefUpdateTest.java
org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDatabase.java
org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectory.java
org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectoryRename.java
org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectoryUpdate.java
org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java
org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java

index 3fe50d668242c56e9f055208894e119de33d4e43..571f34b05dc6dc95a665a16b840ab59cec6b1446 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008-2009, Google Inc.
+ * Copyright (C) 2008-2010, Google Inc.
  * and other copyright owners as documented in the project's IP log.
  *
  * This program and the accompanying materials are made available
@@ -164,8 +164,11 @@ class Clone extends AbstractFetchCommand {
        private void doCheckout(final Ref branch) throws IOException {
                if (branch == null)
                        throw die("cannot checkout; no HEAD advertised by remote");
-               if (!Constants.HEAD.equals(branch.getName()))
-                       db.writeSymref(Constants.HEAD, branch.getName());
+               if (!Constants.HEAD.equals(branch.getName())) {
+                       RefUpdate u = db.updateRef(Constants.HEAD);
+                       u.disableRefLog();
+                       u.link(branch.getName());
+               }
 
                final Commit commit = db.mapCommit(branch.getObjectId());
                final RefUpdate u = db.updateRef(Constants.HEAD);
index 42c8a92b933dc9ab939ac038156877d8441d68d1..100b758e6500f0eb46fea4f62e491f0e952e85ca 100644 (file)
@@ -1,4 +1,5 @@
 /*
+ * Copyright (C) 2009-2010, Google Inc.
  * Copyright (C) 2009, Robin Rosenberg
  * Copyright (C) 2009, Robin Rosenberg <robin.rosenberg@dewire.com>
  * and other copyright owners as documented in the project's IP log.
@@ -58,15 +59,26 @@ import org.eclipse.jgit.lib.RefUpdate.Result;
  */
 public class RefTest extends SampleDataRepositoryTestCase {
 
+       private void writeSymref(String src, String dst) throws IOException {
+               RefUpdate u = db.updateRef(src);
+               switch (u.link(dst)) {
+               case NEW:
+               case FORCED:
+               case NO_CHANGE:
+                       break;
+               default:
+                       fail("link " + src + " to " + dst);
+               }
+       }
+
        public void testReadAllIncludingSymrefs() throws Exception {
                ObjectId masterId = db.resolve("refs/heads/master");
                RefUpdate updateRef = db.updateRef("refs/remotes/origin/master");
                updateRef.setNewObjectId(masterId);
                updateRef.setForceUpdate(true);
                updateRef.update();
-               db
-                               .writeSymref("refs/remotes/origin/HEAD",
-                                               "refs/remotes/origin/master");
+               writeSymref("refs/remotes/origin/HEAD",
+                                       "refs/remotes/origin/master");
 
                ObjectId r = db.resolve("refs/remotes/origin/HEAD");
                assertEquals(masterId, r);
@@ -85,7 +97,7 @@ public class RefTest extends SampleDataRepositoryTestCase {
        }
 
        public void testReadSymRefToPacked() throws IOException {
-               db.writeSymref("HEAD", "refs/heads/b");
+               writeSymref("HEAD", "refs/heads/b");
                Ref ref = db.getRef("HEAD");
                assertEquals(Ref.Storage.LOOSE, ref.getStorage());
                assertTrue("is symref", ref.isSymbolic());
@@ -102,7 +114,7 @@ public class RefTest extends SampleDataRepositoryTestCase {
                Result update = updateRef.update();
                assertEquals(Result.FORCED, update); // internal
 
-               db.writeSymref("HEAD", "refs/heads/master");
+               writeSymref("HEAD", "refs/heads/master");
                Ref ref = db.getRef("HEAD");
                assertEquals(Ref.Storage.LOOSE, ref.getStorage());
                ref = ref.getTarget();
index cb9111758af98abe794d5015a46662b8d1aa33ee..c8a5b33f68e9477a45783f08936ba3971a07797a 100644 (file)
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2008, Charles O'Farrell <charleso@charleso.org>
+ * Copyright (C) 2009-2010, Google Inc.
  * Copyright (C) 2008-2009, Robin Rosenberg <robin.rosenberg@dewire.com>
  * and other copyright owners as documented in the project's IP log.
  *
@@ -56,6 +57,18 @@ import org.eclipse.jgit.revwalk.RevWalk;
 
 public class RefUpdateTest extends SampleDataRepositoryTestCase {
 
+       private void writeSymref(String src, String dst) throws IOException {
+               RefUpdate u = db.updateRef(src);
+               switch (u.link(dst)) {
+               case NEW:
+               case FORCED:
+               case NO_CHANGE:
+                       break;
+               default:
+                       fail("link " + src + " to " + dst);
+               }
+       }
+
        private RefUpdate updateRef(final String name) throws IOException {
                final RefUpdate ref = db.updateRef(name);
                ref.setNewObjectId(db.resolve(Constants.HEAD));
@@ -328,7 +341,7 @@ public class RefUpdateTest extends SampleDataRepositoryTestCase {
         */
        public void testUpdateRefDetachedUnbornHead() throws Exception {
                ObjectId ppid = db.resolve("refs/heads/master^");
-               db.writeSymref("HEAD", "refs/heads/unborn");
+               writeSymref("HEAD", "refs/heads/unborn");
                RefUpdate updateRef = db.updateRef("HEAD", true);
                updateRef.setForceUpdate(true);
                updateRef.setNewObjectId(ppid);
@@ -437,7 +450,7 @@ public class RefUpdateTest extends SampleDataRepositoryTestCase {
                // Do not use the defalt repo for this case.
                Map<String, Ref> allRefs = db.getAllRefs();
                ObjectId oldValue = db.resolve("HEAD");
-               db.writeSymref(Constants.HEAD, "refs/heads/newref");
+               writeSymref(Constants.HEAD, "refs/heads/newref");
                RefUpdate updateRef = db.updateRef(Constants.HEAD);
                updateRef.setForceUpdate(true);
                updateRef.setNewObjectId(oldValue);
@@ -601,7 +614,7 @@ public class RefUpdateTest extends SampleDataRepositoryTestCase {
 
        public void testRenameCurrentBranch() throws IOException {
                ObjectId rb = db.resolve("refs/heads/b");
-               db.writeSymref(Constants.HEAD, "refs/heads/b");
+               writeSymref(Constants.HEAD, "refs/heads/b");
                ObjectId oldHead = db.resolve(Constants.HEAD);
                assertTrue("internal test condition, b == HEAD", rb.equals(oldHead));
                writeReflog(db, rb, rb, "Just a message", "refs/heads/b");
@@ -659,7 +672,7 @@ public class RefUpdateTest extends SampleDataRepositoryTestCase {
        public void tryRenameWhenLocked(String toLock, String fromName,
                        String toName, String headPointsTo) throws IOException {
                // setup
-               db.writeSymref(Constants.HEAD, headPointsTo);
+               writeSymref(Constants.HEAD, headPointsTo);
                ObjectId oldfromId = db.resolve(fromName);
                ObjectId oldHeadId = db.resolve(Constants.HEAD);
                writeReflog(db, oldfromId, oldfromId, "Just a message",
@@ -753,7 +766,7 @@ public class RefUpdateTest extends SampleDataRepositoryTestCase {
        public void testRenameRefNameColission1avoided() throws IOException {
                // setup
                ObjectId rb = db.resolve("refs/heads/b");
-               db.writeSymref(Constants.HEAD, "refs/heads/a");
+               writeSymref(Constants.HEAD, "refs/heads/a");
                RefUpdate updateRef = db.updateRef("refs/heads/a");
                updateRef.setNewObjectId(rb);
                updateRef.setRefLogMessage("Setup", false);
@@ -785,7 +798,7 @@ public class RefUpdateTest extends SampleDataRepositoryTestCase {
        public void testRenameRefNameColission2avoided() throws IOException {
                // setup
                ObjectId rb = db.resolve("refs/heads/b");
-               db.writeSymref(Constants.HEAD, "refs/heads/prefix/a");
+               writeSymref(Constants.HEAD, "refs/heads/prefix/a");
                RefUpdate updateRef = db.updateRef("refs/heads/prefix/a");
                updateRef.setNewObjectId(rb);
                updateRef.setRefLogMessage("Setup", false);
@@ -823,6 +836,6 @@ public class RefUpdateTest extends SampleDataRepositoryTestCase {
                RefDirectoryUpdate update = refs.newUpdate(refName, true);
                update.setOldObjectId(oldId);
                update.setNewObjectId(newId);
-               refs.log(update, msg);
+               refs.log(update, msg, true);
        }
 }
index 22d0c1ad3bec92d6872de87003a30fea190bef71..21e7041b322e9ff11618991f5063373629320274 100644 (file)
@@ -113,19 +113,6 @@ public abstract class RefDatabase {
         */
        public abstract boolean isNameConflicting(String name) throws IOException;
 
-       /**
-        * Create a symbolic reference from one name to another.
-        *
-        * @param name
-        *            the name of the reference. Should be {@link Constants#HEAD} or
-        *            starting with {@link Constants#R_REFS}.
-        * @param target
-        *            the target of the reference.
-        * @throws IOException
-        *             the reference could not be created or overwritten.
-        */
-       public abstract void link(String name, String target) throws IOException;
-
        /**
         * Create a new update command to create, modify or delete a reference.
         *
index 9fdb48fe83e28d2bb1e0fdf5a84b411e02ad9449..90ac0bf47e6f9c888b9a7e1b8529ba2994a26495 100644 (file)
@@ -439,24 +439,11 @@ public class RefDirectory extends RefDatabase {
                return leaf;
        }
 
-       @Override
-       public void link(String name, String target) throws IOException {
-               LockFile lck = new LockFile(fileFor(name));
-               if (!lck.lock())
-                       throw new IOException("Cannot lock " + name);
-               lck.setNeedStatInformation(true);
-               try {
-                       lck.write(encode(SYMREF + target + '\n'));
-                       if (!lck.commit())
-                               throw new IOException("Cannot write " + name);
-               } finally {
-                       lck.unlock();
-               }
-               putLooseRef(newSymbolicRef(lck.getCommitLastModified(), name, target));
+       void storedSymbolicRef(RefDirectoryUpdate u, long modified, String target) {
+               putLooseRef(newSymbolicRef(modified, u.getRef().getName(), target));
                fireRefsChanged();
        }
 
-       @Override
        public RefDirectoryUpdate newUpdate(String name, boolean detach)
                        throws IOException {
                final RefList<Ref> packed = getPackedRefs();
@@ -536,7 +523,8 @@ public class RefDirectory extends RefDatabase {
                fireRefsChanged();
        }
 
-       void log(final RefUpdate update, final String msg) throws IOException {
+       void log(final RefUpdate update, final String msg, final boolean deref)
+                       throws IOException {
                final ObjectId oldId = update.getOldObjectId();
                final ObjectId newId = update.getNewObjectId();
                final Ref ref = update.getRef();
@@ -558,9 +546,12 @@ public class RefDirectory extends RefDatabase {
                r.append('\n');
                final byte[] rec = encode(r.toString());
 
-               if (ref.isSymbolic())
+               if (deref && ref.isSymbolic()) {
+                       log(ref.getName(), rec);
+                       log(ref.getLeaf().getName(), rec);
+               } else {
                        log(ref.getName(), rec);
-               log(ref.getLeaf().getName(), rec);
+               }
        }
 
        private void log(final String refName, final byte[] rec) throws IOException {
index e43d2a561727395b2e711da9413f823ae78674d6..fec00d93257dac8791f5aca34e954aa8d5aacd42 100644 (file)
@@ -208,8 +208,16 @@ class RefDirectoryRename extends RefRename {
 
        private boolean linkHEAD(RefUpdate target) {
                try {
-                       refdb.link(Constants.HEAD, target.getName());
-                       return true;
+                       RefUpdate u = refdb.newUpdate(Constants.HEAD, false);
+                       u.disableRefLog();
+                       switch (u.link(target.getName())) {
+                       case NEW:
+                       case FORCED:
+                       case NO_CHANGE:
+                               return true;
+                       default:
+                               return false;
+                       }
                } catch (IOException e) {
                        return false;
                }
index 113b74b44efe58bab980515abde06f267bd4e500..447be104ab2225d52c8fcdc580cb6d76895206ae 100644 (file)
@@ -44,6 +44,8 @@
 
 package org.eclipse.jgit.lib;
 
+import static org.eclipse.jgit.lib.Constants.encode;
+
 import java.io.IOException;
 
 /** Updates any reference stored by {@link RefDirectory}. */
@@ -68,8 +70,10 @@ class RefDirectoryUpdate extends RefUpdate {
        }
 
        @Override
-       protected boolean tryLock() throws IOException {
-               Ref dst = getRef().getLeaf();
+       protected boolean tryLock(boolean deref) throws IOException {
+               Ref dst = getRef();
+               if (deref)
+                       dst = dst.getLeaf();
                String name = dst.getName();
                lock = new LockFile(database.fileFor(name));
                if (lock.lock()) {
@@ -105,7 +109,7 @@ class RefDirectoryUpdate extends RefUpdate {
                                                msg = strResult;
                                }
                        }
-                       database.log(this, msg);
+                       database.log(this, msg, true);
                }
                if (!lock.commit())
                        return Result.LOCK_FAILURE;
@@ -132,4 +136,21 @@ class RefDirectoryUpdate extends RefUpdate {
                        database.delete(this);
                return status;
        }
+
+       @Override
+       protected Result doLink(final String target) throws IOException {
+               lock.setNeedStatInformation(true);
+               lock.write(encode(RefDirectory.SYMREF + target + '\n'));
+
+               String msg = getRefLogMessage();
+               if (msg != null)
+                       database.log(this, msg, false);
+               if (!lock.commit())
+                       return Result.LOCK_FAILURE;
+               database.storedSymbolicRef(this, lock.getCommitLastModified(), target);
+
+               if (getRef().getStorage() == Ref.Storage.NEW)
+                       return Result.NEW;
+               return Result.FORCED;
+       }
 }
index 33e12a110df740fe08afd6844420ab25fb2b84aa..553266284bda0562737d405cad7bc60b6d17bee5 100644 (file)
@@ -183,13 +183,17 @@ public abstract class RefUpdate {
         * If the locking was successful the implementor must set the current
         * identity value by calling {@link #setOldObjectId(ObjectId)}.
         *
+        * @param deref
+        *            true if the lock should be taken against the leaf level
+        *            reference; false if it should be taken exactly against the
+        *            current reference.
         * @return true if the lock was acquired and the reference is likely
         *         protected from concurrent modification; false if it failed.
         * @throws IOException
         *             the lock couldn't be taken due to an unexpected storage
         *             failure, and not because of a concurrent update.
         */
-       protected abstract boolean tryLock() throws IOException;
+       protected abstract boolean tryLock(boolean deref) throws IOException;
 
        /** Releases the lock taken by {@link #tryLock} if it succeeded. */
        protected abstract void unlock();
@@ -208,6 +212,13 @@ public abstract class RefUpdate {
         */
        protected abstract Result doDelete(Result desiredResult) throws IOException;
 
+       /**
+        * @param target
+        * @return {@link Result#NEW} on success.
+        * @throws IOException
+        */
+       protected abstract Result doLink(String target) throws IOException;
+
        /**
         * Get the name of the ref this update will operate on.
         *
@@ -499,6 +510,50 @@ public abstract class RefUpdate {
                }
        }
 
+       /**
+        * Replace this reference with a symbolic reference to another reference.
+        * <p>
+        * This exact reference (not its traversed leaf) is replaced with a symbolic
+        * reference to the requested name.
+        *
+        * @param target
+        *            name of the new target for this reference. The new target name
+        *            must be absolute, so it must begin with {@code refs/}.
+        * @return {@link Result#NEW} or {@link Result#FORCED} on success.
+        * @throws IOException
+        */
+       public Result link(String target) throws IOException {
+               if (!target.startsWith(Constants.R_REFS))
+                       throw new IllegalArgumentException("Not " + Constants.R_REFS);
+               if (getRefDatabase().isNameConflicting(getName()))
+                       return Result.LOCK_FAILURE;
+               try {
+                       if (!tryLock(false))
+                               return Result.LOCK_FAILURE;
+
+                       final Ref old = getRefDatabase().getRef(getName());
+                       if (old != null && old.isSymbolic()) {
+                               final Ref dst = old.getTarget();
+                               if (target.equals(dst.getName()))
+                                       return result = Result.NO_CHANGE;
+                       }
+
+                       if (old != null && old.getObjectId() != null)
+                               setOldObjectId(old.getObjectId());
+
+                       final Ref dst = getRefDatabase().getRef(target);
+                       if (dst != null && dst.getObjectId() != null)
+                               setNewObjectId(dst.getObjectId());
+
+                       return result = doLink(target);
+               } catch (IOException x) {
+                       result = Result.IO_FAILURE;
+                       throw x;
+               } finally {
+                       unlock();
+               }
+       }
+
        private Result updateImpl(final RevWalk walk, final Store store)
                        throws IOException {
                RevObject newObj;
@@ -507,7 +562,7 @@ public abstract class RefUpdate {
                if (getRefDatabase().isNameConflicting(getName()))
                        return Result.LOCK_FAILURE;
                try {
-                       if (!tryLock())
+                       if (!tryLock(true))
                                return Result.LOCK_FAILURE;
                        if (expValue != null) {
                                final ObjectId o;
index ecae243a41dc1e4f26522383c3a2e5463a8a6577..ca86e36a47598ace2881f7a0ab6269dc2cedfcdb 100644 (file)
@@ -276,8 +276,10 @@ public class Repository {
                objectDatabase.create();
 
                new File(gitDir, "branches").mkdir();
-               final String master = Constants.R_HEADS + Constants.MASTER;
-               refs.link(Constants.HEAD, master);
+
+               RefUpdate head = updateRef(Constants.HEAD);
+               head.disableRefLog();
+               head.link(Constants.R_HEADS + Constants.MASTER);
 
                cfg.setInt("core", null, "repositoryformatversion", 0);
                cfg.setBoolean("core", null, "filemode", true);
@@ -899,18 +901,6 @@ public class Repository {
                objectDatabase.openPack(pack, idx);
        }
 
-    /**
-     * Writes a symref (e.g. HEAD) to disk
-     *
-     * @param name symref name
-     * @param target pointed to ref
-     * @throws IOException
-     */
-    public void writeSymref(final String name, final String target)
-                       throws IOException {
-               refs.link(name, target);
-       }
-
        public String toString() {
                return "Repository[" + getDirectory() + "]";
        }