From 73b6efc9289d6f7b6c147f4c2e2c62d2134fd7f3 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Sat, 9 Jan 2010 18:56:45 -0800 Subject: [PATCH] Replace writeSymref with RefUpdate.link 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 --- .../src/org/eclipse/jgit/pgm/Clone.java | 9 ++- .../tst/org/eclipse/jgit/lib/RefTest.java | 22 +++++-- .../org/eclipse/jgit/lib/RefUpdateTest.java | 27 ++++++--- .../src/org/eclipse/jgit/lib/RefDatabase.java | 13 ---- .../org/eclipse/jgit/lib/RefDirectory.java | 27 +++------ .../eclipse/jgit/lib/RefDirectoryRename.java | 12 +++- .../eclipse/jgit/lib/RefDirectoryUpdate.java | 27 ++++++++- .../src/org/eclipse/jgit/lib/RefUpdate.java | 59 ++++++++++++++++++- .../src/org/eclipse/jgit/lib/Repository.java | 18 ++---- 9 files changed, 147 insertions(+), 67 deletions(-) diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Clone.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Clone.java index 3fe50d668..571f34b05 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Clone.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Clone.java @@ -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); diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RefTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RefTest.java index 42c8a92b9..100b758e6 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RefTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RefTest.java @@ -1,4 +1,5 @@ /* + * Copyright (C) 2009-2010, Google Inc. * Copyright (C) 2009, Robin Rosenberg * Copyright (C) 2009, Robin Rosenberg * 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(); diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RefUpdateTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RefUpdateTest.java index cb9111758..c8a5b33f6 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RefUpdateTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RefUpdateTest.java @@ -1,5 +1,6 @@ /* * Copyright (C) 2008, Charles O'Farrell + * Copyright (C) 2009-2010, Google Inc. * Copyright (C) 2008-2009, Robin Rosenberg * 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 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); } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDatabase.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDatabase.java index 22d0c1ad3..21e7041b3 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDatabase.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDatabase.java @@ -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. * diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectory.java index 9fdb48fe8..90ac0bf47 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectory.java @@ -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 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 { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectoryRename.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectoryRename.java index e43d2a561..fec00d932 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectoryRename.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectoryRename.java @@ -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; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectoryUpdate.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectoryUpdate.java index 113b74b44..447be104a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectoryUpdate.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectoryUpdate.java @@ -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; + } } 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 33e12a110..553266284 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java @@ -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. + *

+ * 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; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java index ecae243a4..ca86e36a4 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java @@ -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() + "]"; }