[EGIT PATCH 0/3] Rename refs

[EGIT PATCH 0/3] Rename refs

[Robin Rosenberg]

Not very pretty, but my alternative design was much worse, Having the
refs in a databae would make things simpler.

-- robin

Robin Rosenberg (3):
  Add ref rename support to JGit
  Use Constants.R_* in Branch dialog
  Add ref rename support to the branch dialog

 .../src/org/spearce/egit/ui/UIText.java            |    6 +
 .../ui/internal/dialogs/BranchSelectionDialog.java |  107 ++++++++++++++-----
 .../src/org/spearce/egit/ui/uitext.properties      |    3 +
 .../tst/org/spearce/jgit/lib/RefUpdateTest.java    |  109 +++++++++++++++++++
 .../src/org/spearce/jgit/lib/RefDatabase.java      |   32 +++++-
 .../src/org/spearce/jgit/lib/RefLogWriter.java     |   21 ++++-
 .../src/org/spearce/jgit/lib/RefUpdate.java        |  112 +++++++++++++++++++-
 .../src/org/spearce/jgit/lib/Repository.java       |   33 ++++++
 8 files changed, 390 insertions(+), 33 deletions(-)

[EGIT PATCH 1/3] Add ref rename support to JGit

[Robin Rosenberg]

Now refs can be renamed. The intent is that should be safe. Only the named
refs and associated logs are updated. Any symbolic refs referring to the renames
branches are unaffected, except HEAD.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../tst/org/spearce/jgit/lib/RefUpdateTest.java    |  109 +++++++++++++++++++
 .../src/org/spearce/jgit/lib/RefDatabase.java      |   32 +++++-
 .../src/org/spearce/jgit/lib/RefLogWriter.java     |   21 ++++-
 .../src/org/spearce/jgit/lib/RefUpdate.java        |  112 +++++++++++++++++++-
 .../src/org/spearce/jgit/lib/Repository.java       |   33 ++++++
 5 files changed, 303 insertions(+), 4 deletions(-)

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
index 55d7441..b02773d 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
@@ -42,6 +42,7 @@
 import java.util.Map;
 import java.util.Map.Entry;
 
+import org.spearce.jgit.lib.RefUpdate.RenameRefUpdates;
 import org.spearce.jgit.lib.RefUpdate.Result;
 import org.spearce.jgit.revwalk.RevCommit;
 
@@ -155,4 +156,112 @@ public void testRefKeySameAsOrigName() {
 
 		}
 	}
+
+	public void testRenameBranchNoPreviousLog() throws IOException {
+		assertFalse(new File(db.getDirectory(),"logs/refs/heads/b").exists()); // no log on old branch
+		ObjectId rb = db.resolve("refs/heads/b");
+		ObjectId oldHead = db.resolve(Constants.HEAD);
+		assertFalse(rb.equals(oldHead)); // assumption for this test
+		RenameRefUpdates renameRef = db.renameRef("refs/heads/b",
+				"refs/heads/new/name");
+		Result result = renameRef.rename();
+		assertEquals(Result.RENAMED, result);
+		assertEquals(rb, db.resolve("refs/heads/new/name"));
+		assertNull(db.resolve("refs/heads/b"));
+		assertTrue(new File(db.getDirectory(),"logs/refs/heads/new/name").exists());
+		assertFalse(new File(db.getDirectory(),"logs/refs/heads/b").exists());
+		assertEquals(oldHead, db.resolve(Constants.HEAD));
+		// TODO: test coprivate void assertNotEquals(ObjectId rb, ObjectId oldHead) {
+		// TODO Auto-generated method stub
+	}
+
+	public void testRenameBranchHasPreviousLog() throws IOException {
+		ObjectId rb = db.resolve("refs/heads/b");
+		ObjectId oldHead = db.resolve(Constants.HEAD);
+		assertFalse(rb.equals(oldHead)); // assumption for this test
+		RefLogWriter.writeReflog(db, rb, rb, "Just a message", "refs/heads/b");
+		assertTrue(new File(db.getDirectory(),"logs/refs/heads/b").exists()); // no log on old branch
+		RenameRefUpdates renameRef = db.renameRef("refs/heads/b",
+				"refs/heads/new/name");
+		Result result = renameRef.rename();
+		assertEquals(Result.RENAMED, result);
+		assertEquals(rb, db.resolve("refs/heads/new/name"));
+		assertNull(db.resolve("refs/heads/b"));
+		assertTrue(new File(db.getDirectory(),"logs/refs/heads/new/name").exists());
+		assertFalse(new File(db.getDirectory(),"logs/refs/heads/b").exists());
+		assertEquals(oldHead, db.resolve(Constants.HEAD));
+		// TODO: test content of log file
+	}
+
+	public void testRenameCurrentBranch() throws IOException {
+		ObjectId rb = db.resolve("refs/heads/b");
+		db.link(Constants.HEAD, "refs/heads/b");
+		ObjectId oldHead = db.resolve(Constants.HEAD);
+		assertTrue(rb.equals(oldHead)); // assumption for this test
+		RefLogWriter.writeReflog(db, rb, rb, "Just a message", "refs/heads/b");
+		assertTrue(new File(db.getDirectory(),"logs/refs/heads/b").exists()); // no log on old branch
+		RenameRefUpdates renameRef = db.renameRef("refs/heads/b",
+				"refs/heads/new/name");
+		Result result = renameRef.rename();
+		assertEquals(Result.RENAMED, result);
+		assertEquals(rb, db.resolve("refs/heads/new/name"));
+		assertNull(db.resolve("refs/heads/b"));
+		assertTrue(new File(db.getDirectory(),"logs/refs/heads/new/name").exists());
+		assertFalse(new File(db.getDirectory(),"logs/refs/heads/b").exists());
+		assertEquals(rb, db.resolve(Constants.HEAD));
+		// TODO: test content of log file
+	}
+
+	public void testRenameBranchCannotLockFirstBranch() throws IOException {
+		// "someone" has branch b locked
+		assertTrue(new LockFile(new File(db.getDirectory(), "refs/heads/b")).lock());
+
+		// setup
+		ObjectId rb = db.resolve("refs/heads/b");
+		db.link(Constants.HEAD, "refs/heads/b");
+		ObjectId oldHead = db.resolve(Constants.HEAD);
+		assertTrue(rb.equals(oldHead)); // assumption for this test
+		RefLogWriter.writeReflog(db, rb, rb, "Just a message", "refs/heads/b");
+		assertTrue(new File(db.getDirectory(),"logs/refs/heads/b").exists()); // no log on old branch
+
+		// Now this is our test
+		RenameRefUpdates renameRef = db.renameRef("refs/heads/b",
+				"refs/heads/new/name");
+		Result result = renameRef.rename();
+		assertEquals(Result.LOCK_FAILURE, result);
+
+		// Check that the involved refs are sane despite the failure
+		assertFalse(new File(db.getDirectory(), "refs/heads/new/name").exists());
+		assertFalse(new File(db.getDirectory(), "refs/heads/new/name.lock").exists());
+		assertFalse(new File(db.getDirectory(), "logs/refs/heads/b/lock").exists());
+		assertFalse(new File(db.getDirectory(), "logs/refs/heads/new/name.lock").exists());
+		assertEquals(rb, db.resolve(Constants.HEAD));
+	}
+
+	public void testRenameBranchCannotLockHEAD() throws IOException {
+		// setup
+		ObjectId rb = db.resolve("refs/heads/b");
+		db.link(Constants.HEAD, "refs/heads/b");
+
+		// "someone" has branch b locked
+		assertTrue(new LockFile(new File(db.getDirectory(), "HEAD")).lock());
+
+		ObjectId oldHead = db.resolve(Constants.HEAD);
+		assertTrue(rb.equals(oldHead)); // assumption for this test
+		RefLogWriter.writeReflog(db, rb, rb, "Just a message", "refs/heads/b");
+		assertTrue(new File(db.getDirectory(),"logs/refs/heads/b").exists()); // no log on old branch
+
+		// Now this is our test
+		RenameRefUpdates renameRef = db.renameRef("refs/heads/b",
+				"refs/heads/new/name");
+		Result result = renameRef.rename();
+		assertEquals(Result.LOCK_FAILURE, result);
+
+		// Check that the involved refs are sane despite the failure
+		assertFalse(new File(db.getDirectory(), "refs/heads/new/name").exists());
+		assertFalse(new File(db.getDirectory(), "refs/heads/new/name.lock").exists());
+		assertFalse(new File(db.getDirectory(), "logs/refs/heads/b/lock").exists());
+		assertFalse(new File(db.getDirectory(), "logs/refs/heads/new/name.lock").exists());
+		assertEquals(rb, db.resolve(Constants.HEAD));
+	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
index 87f26bf..a73467a 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
@@ -52,6 +52,7 @@
 
 import org.spearce.jgit.errors.ObjectWritingException;
 import org.spearce.jgit.lib.Ref.Storage;
+import org.spearce.jgit.lib.RefUpdate.RenameRefUpdates;
 import org.spearce.jgit.util.FS;
 import org.spearce.jgit.util.NB;
 
@@ -148,6 +149,25 @@ synchronized (this) {
 	}
 
 	/**
+	 * An set of update operations for renaming a ref
+	 *
+	 * @param fromRef Old ref name
+	 * @param toRef New ref name
+	 * @return a RefUpdate operation to rename a ref
+	 * @throws IOException
+	 */
+	public RenameRefUpdates newRename(String fromRef, String toRef) throws IOException {
+		refreshPackedRefs();
+		Ref f = readRefBasic(fromRef, 0);
+		Ref t = readRefBasic(toRef, 0);
+		if (t != null)
+			throw new IOException("Ref rename target exists: " + t.getName());
+		RefUpdate refUpdateFrom = new RefUpdate(this, f, fileForRef(f.getName()));
+		RefUpdate refUpdateTo = db.updateRef(toRef);
+		return new RenameRefUpdates(refUpdateTo, refUpdateFrom, null);
+	}
+
+	/**
 	 * Writes a symref (e.g. HEAD) to disk
 	 * 
 	 * @param name
@@ -160,11 +180,19 @@ void link(final String name, final String target) throws IOException {
 		final byte[] content = Constants.encode("ref: " + target + "\n");
 		lockAndWriteFile(fileForRef(name), content);
 		synchronized (this) {
+			looseSymRefs.remove(name);
 			setModified();
 		}
 		db.fireRefsMaybeChanged();
 	}
 
+	void uncacheSymRef(String name) {
+		synchronized(this) {
+			looseSymRefs.remove(name);
+			setModified();
+		}
+	}
+
 	private void setModified() {
 		lastRefModification = refModificationCounter++;
 	}
@@ -484,8 +512,8 @@ private void lockAndWriteFile(File file, byte[] content) throws IOException {
 	}
 
 	synchronized void removePackedRef(String name) throws IOException {
-		packedRefs.remove(name);
-		writePackedRefs();
+		if (packedRefs.remove(name) != null)
+			writePackedRefs();
 	}
 
 	private void writePackedRefs() throws IOException {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java
index a077051..bbf26eb 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java
@@ -44,7 +44,7 @@
 import java.io.IOException;
 
 /**
- * Utility class to add reflog entries
+ * Utility class to work with reflog files
  * 
  * @author Dave Watson
  */
@@ -58,6 +58,25 @@ static void append(final RefUpdate u, final String msg) throws IOException {
 		appendOneRecord(oldId, newId, ident, msg, db, u.getName());
 	}
 
+	static boolean renameTo(final Repository db, final RefUpdate from,
+			final RefUpdate to) throws IOException {
+		final File logdir = new File(db.getDirectory(), Constants.LOGS);
+		final File reflogFrom = new File(logdir, from.getName());
+		if (!reflogFrom.exists())
+			return true;
+		final File reflogTo = new File(logdir, to.getName());
+		final File refdirTo = reflogTo.getParentFile();
+		if (!refdirTo.exists() && !refdirTo.mkdirs()) {
+			throw new IOException("Cannot create directory " + refdirTo);
+		}
+		if (!reflogFrom.renameTo(reflogTo)) {
+			reflogTo.delete(); // try
+			throw new IOException("Cannot rename " + reflogFrom + " to "
+					+ reflogTo);
+		}
+		return true;
+	}
+
 	private static void appendOneRecord(final ObjectId oldId,
 			final ObjectId newId, PersonIdent ident, final String msg,
 			final Repository db, final String refName) throws IOException {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
index a9ab73b..8ecccfe 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
@@ -50,6 +50,100 @@
  * Updates any locally stored ref.
  */
 public class RefUpdate {
+	/**
+	 * A RefUpdate combination for renaming a ref
+	 */
+	public static class RenameRefUpdates {
+		RenameRefUpdates(final RefUpdate toUpdate, final RefUpdate fromUpdate,
+				final RefUpdate headUpdate) {
+			newToUpdate = toUpdate;
+			oldFromDelete = fromUpdate;
+		}
+		private RefUpdate newToUpdate;
+
+		private RefUpdate oldFromDelete;
+
+		private Result renameResult;
+
+		/**
+		 * @return result of rename operation
+		 */
+		public Result getResult() {
+			return renameResult;
+		}
+
+		/**
+		 * @return the result of the new ref update
+		 * @throws IOException
+		 */
+		public Result rename() throws IOException {
+			LockFile lockFileFrom = new LockFile(oldFromDelete.looseFile);
+			boolean lockFrom = lockFileFrom.lock();
+			if (!lockFrom)
+				return Result.LOCK_FAILURE;
+			LockFile lockFileTo = null;
+			try {
+				lockFileTo = new LockFile(newToUpdate.looseFile);
+				boolean lockTo = lockFileTo.lock();
+				if (!lockTo) {
+					lockFileFrom.unlock();
+					return renameResult = Result.LOCK_FAILURE;
+				}
+			} catch (IOException e) {
+				lockFileFrom.unlock();
+				throw e;
+			}
+			LockFile lockFileHEAD = new LockFile(new File(oldFromDelete.db.getRepository().getDirectory(), Constants.HEAD));
+			boolean renameHEADtoo;
+			try {
+				boolean lockHEAD = lockFileHEAD.lock();
+				renameHEADtoo = oldFromDelete.db.readRef(Constants.HEAD).getName().equals(oldFromDelete.getName());
+				if (!renameHEADtoo)
+					lockFileHEAD.unlock();
+				else {
+					if (!lockHEAD) {
+						lockFileFrom.unlock();
+						lockFileTo.unlock();
+						return renameResult = Result.LOCK_FAILURE;
+					}
+				}
+			} catch (IOException e) {
+				lockFileHEAD.unlock();
+				lockFileFrom.unlock();
+				lockFileTo.unlock();
+				throw e;
+			}
+			try {
+				UpdateStore toStore = newToUpdate.newUpdateStore();
+				if (RefLogWriter.renameTo(oldFromDelete.getRepository(), oldFromDelete, newToUpdate)) {
+					newToUpdate.setNewObjectId(oldFromDelete.getOldObjectId());
+					newToUpdate.setExpectedOldObjectId(oldFromDelete.getOldObjectId());
+					newToUpdate.setRefLogMessage("jgit branch: renamed " + oldFromDelete.getName() + " to " + oldFromDelete.getName(), false);
+					newToUpdate.result = toStore.store(lockFileTo, Result.RENAMED);
+					DeleteStore fromStore = oldFromDelete.newDeleteStore();
+					Result store = fromStore.store(lockFileFrom, Result.RENAMED);
+					if (renameHEADtoo) {
+						final byte[] content = Constants.encode("ref: " + newToUpdate.getName() + "\n");
+						lockFileHEAD.write(content);
+						synchronized (this) {
+							oldFromDelete.db.getRepository().uncacheSymRef(Constants.HEAD);
+						}
+						if (!lockFileHEAD.commit())
+							throw new IOException("Failed to commit HEAD during rename");
+					}
+					oldFromDelete.db.getRepository().fireRefsMaybeChanged();
+					return store;
+				} else {
+					return Result.IO_FAILURE;
+				}
+			} finally {
+				lockFileFrom.unlock();
+				lockFileTo.unlock();
+				lockFileHEAD.unlock();
+			}
+		}
+	}
+
 	/** Status of an update request. */
 	public static enum Result {
 		/** The ref update/delete has not been attempted by the caller. */
@@ -125,7 +219,13 @@
 		 * This kind of error doesn't include {@link #LOCK_FAILURE}, which is a
 		 * different case.
 		 */
-		IO_FAILURE
+		IO_FAILURE,
+
+		/**
+		 * The ref was renamed from another name
+		 * <p>
+		 */
+		RENAMED
 	}
 
 	/** Repository the ref is stored in. */
@@ -478,6 +578,8 @@ else if (status == Result.FAST_FORWARD)
 				msg += ": fast forward";
 			else if (status == Result.NEW)
 				msg += ": created";
+			else if (status == Result.RENAMED)
+				msg += ": renamed";
 		}
 		RefLogWriter.append(this, msg);
 		if (!lock.commit())
@@ -553,4 +655,12 @@ private static int count(final String s, final char c) {
 		}
 		return count;
 	}
+
+	private UpdateStore newUpdateStore() {
+		return new UpdateStore();
+	}
+
+	private DeleteStore newDeleteStore() {
+		return new DeleteStore();
+	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
index 5baa7a0..e704aeb 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
@@ -468,6 +468,22 @@ public RefUpdate updateRef(final String ref) throws IOException {
 	}
 
 	/**
+	 * Create a command to rename a ref in this repository
+	 *
+	 * @param fromRef
+	 *            name of ref to rename from
+	 * @param toRef
+	 *            name of ref to rename to
+	 * @return an update command that knows how to rename a branch to another.
+	 * @throws IOException
+	 *             the rename could not be performed.
+	 *
+	 */
+	public RefUpdate.RenameRefUpdates renameRef(final String fromRef, final String toRef) throws IOException {
+		return refs.newRename(fromRef, toRef);
+	}
+
+	/**
 	 * Parse a git revision string and return an object id.
 	 *
 	 * Currently supported is combinations of these.
@@ -1067,4 +1083,21 @@ public void scanForRepoChanges() throws IOException {
 		getAllRefs(); // This will look for changes to refs
 		getIndex(); // This will detect changes in the index
 	}
+
+	/**
+	 * Writes a symref (e.g. HEAD) to disk
+	 *
+	 * @param name
+	 *            symref name
+	 * @param target
+	 *            pointed to ref
+	 * @throws IOException
+	 */
+	public void link(final String name, final String target) throws IOException {
+		refs.link(name, target);
+	}
+
+	void uncacheSymRef(String name) {
+		refs.uncacheSymRef(name);
+	}
 }
-- 
1.6.3.rc2.1.g868b6

[EGIT PATCH 2/3] Use Constants.R_* in Branch dialog

[Robin Rosenberg]

There were a few places that didn't use these constants

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../ui/internal/dialogs/BranchSelectionDialog.java |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
index 1866086..9aad95b 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
@@ -154,19 +154,19 @@ private void fillTreeWithBranches(String select) throws IOException {
 
 		for (String ref : branches) {
 			String shortName = ref;
-			if (ref.startsWith("refs/heads/")) {
+			if (ref.startsWith(Constants.R_HEADS)) {
 				shortName = ref.substring(11);
-				if (!"refs/heads/".equals(curPrefix)) {
-					curPrefix = "refs/heads/";
+				if (!Constants.R_HEADS.equals(curPrefix)) {
+					curPrefix = Constants.R_HEADS;
 					curSubPrefix = null;
 					curSubItem = null;
 					curItem = new TreeItem(branchTree, SWT.NONE);
 					curItem.setText(UIText.BranchSelectionDialog_LocalBranches);
 				}
-			} else if (ref.startsWith("refs/remotes/")) {
+			} else if (ref.startsWith(Constants.R_REMOTES)) {
 				shortName = ref.substring(13);
-				if (!"refs/remotes/".equals(curPrefix)) {
-					curPrefix = "refs/remotes/";
+				if (!Constants.R_REMOTES.equals(curPrefix)) {
+					curPrefix = Constants.R_REMOTES;
 					curItem = new TreeItem(branchTree, SWT.NONE);
 					curItem.setText(UIText.BranchSelectionDialog_RemoteBranches);
 					curSubItem = null;
@@ -186,10 +186,10 @@ private void fillTreeWithBranches(String select) throws IOException {
 					curSubItem = null;
 					curSubPrefix = null;
 				}
-			} else if (ref.startsWith("refs/tags/")) {
+			} else if (ref.startsWith(Constants.R_TAGS)) {
 				shortName = ref.substring(10);
-				if (!"refs/tags/".equals(curPrefix)) {
-					curPrefix = "refs/tags/";
+				if (!Constants.R_HEADS.equals(curPrefix)) {
+					curPrefix = Constants.R_TAGS;
 					curSubPrefix = null;
 					curSubItem = null;
 					curItem = new TreeItem(branchTree, SWT.NONE);
-- 
1.6.3.rc2.1.g868b6

[EGIT PATCH 3/3] Add ref rename support to the branch dialog

[Robin Rosenberg]

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../src/org/spearce/egit/ui/UIText.java            |    6 ++
 .../ui/internal/dialogs/BranchSelectionDialog.java |   89 +++++++++++++++-----
 .../src/org/spearce/egit/ui/uitext.properties      |    3 +
 3 files changed, 78 insertions(+), 20 deletions(-)

diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
index 654e155..1bf2dd3 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
@@ -893,6 +893,12 @@
 	public static String BranchSelectionDialog_ErrorCouldNotRefresh;
 
 	/** */
+	public static String BranchSelectionDialog_ErrorCouldNotRenameRef;
+
+	/** */
+	public static String BranchSelectionDialog_ErrorCouldNotRenameRef2;
+
+	/** */
 	public static String BranchSelectionDialog_BranchSuffix_Current;
 
 	/** */
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
index 9aad95b..a518c2a 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
@@ -50,6 +50,8 @@
 import org.spearce.jgit.lib.ObjectId;
 import org.spearce.jgit.lib.RefUpdate;
 import org.spearce.jgit.lib.Repository;
+import org.spearce.jgit.lib.RefUpdate.RenameRefUpdates;
+import org.spearce.jgit.lib.RefUpdate.Result;
 
 /**
  * The branch and reset selection dialog
@@ -277,6 +279,30 @@ private void refNameFromDialog() {
 		}
 	}
 
+	private InputDialog getRefNameInputDialog() {
+		InputDialog labelDialog = new InputDialog(
+				getShell(),
+				UIText.BranchSelectionDialog_QuestionNewBranchTitle,
+				UIText.BranchSelectionDialog_QuestionNewBranchMessage,
+				null, new IInputValidator() {
+					public String isValid(String newText) {
+						String testFor = Constants.R_HEADS + newText;
+						try {
+							if (repo.resolve(testFor) != null)
+								return UIText.BranchSelectionDialog_ErrorAlreadyExists;
+						} catch (IOException e1) {
+							Activator.logError(NLS.bind(
+									UIText.BranchSelectionDialog_ErrorCouldNotResolve, testFor), e1);
+						}
+						if (!Repository.isValidRefName(testFor))
+							return UIText.BranchSelectionDialog_ErrorInvalidRefName;
+						return null;
+					}
+				});
+		labelDialog.setBlockOnOpen(true);
+		return labelDialog;
+	}
+
 	@Override
 	protected void createButtonsForButtonBar(Composite parent) {
 		if (!showResetType) {
@@ -284,32 +310,54 @@ protected void createButtonsForButtonBar(Composite parent) {
 			newButton.setFont(JFaceResources.getDialogFont());
 			newButton.setText(UIText.BranchSelectionDialog_NewBranch);
 			((GridLayout)parent.getLayout()).numColumns++;
+			Button renameButton = new Button(parent, SWT.PUSH);
+			renameButton.setText("&Rename");
+			renameButton.addSelectionListener(new SelectionListener() {
+				public void widgetSelected(SelectionEvent e) {
+					// check what ref name the user selected, if any.
+					refNameFromDialog();
+
+					InputDialog labelDialog = getRefNameInputDialog();
+					if (labelDialog.open() == Window.OK) {
+						String newRefName = Constants.R_HEADS + labelDialog.getValue();
+						try {
+							RenameRefUpdates renameRef = repo.renameRef(refName, newRefName);
+							if (renameRef.rename() != Result.RENAMED) {
+								MessageDialog.openError(getShell(),
+										"Rename failed",
+										NLS.bind(UIText.BranchSelectionDialog_ErrorCouldNotRenameRef,
+												new Object[] { refName, newRefName, renameRef.getResult() }));
+								Activator.logError(NLS.bind(
+										UIText.BranchSelectionDialog_ErrorCouldNotRenameRef2,
+												new Object[] { refName, newRefName }), null);
+							}
+							// FIXME: Update HEAD
+						} catch (IOException e1) {
+							Activator.logError(NLS.bind(
+									UIText.BranchSelectionDialog_ErrorCouldNotRenameRef2,
+															newRefName), e1);
+						}
+						try {
+							branchTree.removeAll();
+							fillTreeWithBranches(newRefName);
+						} catch (IOException e1) {
+							Activator.logError(
+									UIText.BranchSelectionDialog_ErrorCouldNotRefreshBranchList,
+											e1);
+						}
+					}
+				}
+				public void widgetDefaultSelected(SelectionEvent e) {
+					widgetSelected(e);
+				}
+			});
 			newButton.addSelectionListener(new SelectionListener() {
 
 				public void widgetSelected(SelectionEvent e) {
 					// check what ref name the user selected, if any.
 					refNameFromDialog();
 
-					InputDialog labelDialog = new InputDialog(
-							getShell(),
-							UIText.BranchSelectionDialog_QuestionNewBranchTitle,
-							UIText.BranchSelectionDialog_QuestionNewBranchMessage,
-							null, new IInputValidator() {
-								public String isValid(String newText) {
-									String testFor = Constants.R_HEADS + newText;
-									try {
-										if (repo.resolve(testFor) != null)
-											return UIText.BranchSelectionDialog_ErrorAlreadyExists;
-									} catch (IOException e1) {
-										Activator.logError(NLS.bind(
-												UIText.BranchSelectionDialog_ErrorCouldNotResolve, testFor), e1);
-									}
-									if (!Repository.isValidRefName(testFor))
-										return UIText.BranchSelectionDialog_ErrorInvalidRefName;
-									return null;
-								}
-							});
-					labelDialog.setBlockOnOpen(true);
+					InputDialog labelDialog = getRefNameInputDialog();
 					if (labelDialog.open() == Window.OK) {
 						String newRefName = Constants.R_HEADS + labelDialog.getValue();
 						RefUpdate updateRef;
@@ -338,6 +386,7 @@ public String isValid(String newText) {
 					}
 				}
 
+
 				public void widgetDefaultSelected(SelectionEvent e) {
 					widgetSelected(e);
 				}
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties b/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
index 1d21c81..193f3ad 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
@@ -340,6 +340,9 @@ BranchSelectionDialog_ErrorCouldNotRefresh=Could not refresh list of branches
 BranchSelectionDialog_ErrorCouldNotRefreshBranchList=Could not refresh list of branches
 BranchSelectionDialog_ErrorCouldNotResolve=Could not attempt to resolve {0}
 BranchSelectionDialog_ErrorInvalidRefName=Invalid ref name
+BranchSelectionDialog_ErrorCouldNotRenameRef=Failed to rename branch {0} -> {1}, status={2}
+BranchSelectionDialog_ErrorCouldNotRenameRef2=Failed to rename branch {0} -> {1}
+
 BranchSelectionDialog_LocalBranches=Local Branches
 BranchSelectionDialog_NewBranch=&New branch
 BranchSelectionDialog_NoBranchSeletectMessage=You must select a valid ref.
-- 
1.6.3.rc2.1.g868b6

Re: [EGIT PATCH 1/3] Add ref rename support to JGit

[Shawn O. Pearce]

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> Now refs can be renamed. The intent is that should be safe. Only the named
> refs and associated logs are updated. Any symbolic refs referring to the renames
> branches are unaffected, except HEAD.

See below.  I'm not going to apply as-is, because its really quite
messy, as you stated in the cover letter...
 
> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
> index 87f26bf..a73467a 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
> @@ -148,6 +149,25 @@ synchronized (this) {
>  	}
>  
>  	/**
> +	 * An set of update operations for renaming a ref
> +	 *
> +	 * @param fromRef Old ref name
> +	 * @param toRef New ref name
> +	 * @return a RefUpdate operation to rename a ref
> +	 * @throws IOException
> +	 */
> +	public RenameRefUpdates newRename(String fromRef, String toRef) throws IOException {

This class is not public; none of its methods are public; please
don't mark this public.

> +		refreshPackedRefs();
> +		Ref f = readRefBasic(fromRef, 0);
> +		Ref t = readRefBasic(toRef, 0);
> +		if (t != null)
> +			throw new IOException("Ref rename target exists: " + t.getName());
> +		RefUpdate refUpdateFrom = new RefUpdate(this, f, fileForRef(f.getName()));
> +		RefUpdate refUpdateTo = db.updateRef(toRef);

db.updateRef is just a redirection to this.newUpdate(), which scans
the refs (done above) and then makes a new RefUpdate instance.
Why not just reproduce the three lines that matter and avoid this
messy indirection?

  if (t == null)
    t = new Ref(Ref.Storage.NEW, name, null)
  RefUpdate refUpdateTo = new RefUpdate(this, t, fileForRef(t.getName()));

> @@ -484,8 +512,8 @@ private void lockAndWriteFile(File file, byte[] content) throws IOException {
>  	}
>  
>  	synchronized void removePackedRef(String name) throws IOException {
> -		packedRefs.remove(name);
> -		writePackedRefs();
> +		if (packedRefs.remove(name) != null)
> +			writePackedRefs();
>  	}

How did we get here when Ref.Storage.isPacked() == false?  IMHO this
shouldn't have been invoked if isPacked is false.

> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java
> index a077051..bbf26eb 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java
> @@ -58,6 +58,25 @@ static void append(final RefUpdate u, final String msg) throws IOException {
>  		appendOneRecord(oldId, newId, ident, msg, db, u.getName());
>  	}
>  
> +	static boolean renameTo(final Repository db, final RefUpdate from,
> +			final RefUpdate to) throws IOException {

Hmm, this method returns only true, or throws an IOException.
I wonder, why bother returning boolean?  Why not make it void?

> +		final File logdir = new File(db.getDirectory(), Constants.LOGS);
> +		final File reflogFrom = new File(logdir, from.getName());
> +		if (!reflogFrom.exists())
> +			return true;
> +		final File reflogTo = new File(logdir, to.getName());
> +		final File refdirTo = reflogTo.getParentFile();
> +		if (!refdirTo.exists() && !refdirTo.mkdirs()) {
> +			throw new IOException("Cannot create directory " + refdirTo);
> +		}
> +		if (!reflogFrom.renameTo(reflogTo)) {
> +			reflogTo.delete(); // try
> +			throw new IOException("Cannot rename " + reflogFrom + " to "
> +					+ reflogTo);

So if the rename fails, we delete the target and then throw an
exception anyway?  Why delete the target in this case?  I think we
should just fail here.

> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
> index a9ab73b..8ecccfe 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
> @@ -50,6 +50,100 @@
>   * Updates any locally stored ref.
>   */
>  public class RefUpdate {
> +	/**
> +	 * A RefUpdate combination for renaming a ref
> +	 */
> +	public static class RenameRefUpdates {

I would prefer this to be called "RefRename", and to make it a
top level class in the lib package, rather than a nested class
of RefUpdate.

> +		RenameRefUpdates(final RefUpdate toUpdate, final RefUpdate fromUpdate,
> +				final RefUpdate headUpdate) {
> +			newToUpdate = toUpdate;
> +			oldFromDelete = fromUpdate;

What happened to headUpdate?

> +		}
> +		private RefUpdate newToUpdate;
> +
> +		private RefUpdate oldFromDelete;
> +
> +		private Result renameResult;

Constructor after instance member field decls please.

> +		/**
> +		 * @return the result of the new ref update
> +		 * @throws IOException
> +		 */
> +		public Result rename() throws IOException {
> +			LockFile lockFileFrom = new LockFile(oldFromDelete.looseFile);
> +			boolean lockFrom = lockFileFrom.lock();
> +			if (!lockFrom)
> +				return Result.LOCK_FAILURE;
> +			LockFile lockFileTo = null;
> +			try {
> +				lockFileTo = new LockFile(newToUpdate.looseFile);
> +				boolean lockTo = lockFileTo.lock();
> +				if (!lockTo) {
> +					lockFileFrom.unlock();
> +					return renameResult = Result.LOCK_FAILURE;
> +				}
> +			} catch (IOException e) {
> +				lockFileFrom.unlock();
> +				throw e;
> +			}
> +			LockFile lockFileHEAD = new LockFile(new File(oldFromDelete.db.getRepository().getDirectory(), Constants.HEAD));
> +			boolean renameHEADtoo;
> +			try {
> +				boolean lockHEAD = lockFileHEAD.lock();
> +				renameHEADtoo = oldFromDelete.db.readRef(Constants.HEAD).getName().equals(oldFromDelete.getName());
> +				if (!renameHEADtoo)
> +					lockFileHEAD.unlock();
> +				else {
> +					if (!lockHEAD) {
> +						lockFileFrom.unlock();
> +						lockFileTo.unlock();
> +						return renameResult = Result.LOCK_FAILURE;
> +					}
> +				}
> +			} catch (IOException e) {
> +				lockFileHEAD.unlock();
> +				lockFileFrom.unlock();
> +				lockFileTo.unlock();

I wonder if this code would be easier to follow if we made a
Set<LockFile> as an instace member, and made some utility methods
like:

  private final Set<LockFile> allLocks = new HashSet<LockFile>();

  private boolean takeLock(final LockFile f) {
    if (f.lock()) {
      allLocks.add(f);
      return true;
    }
    return false;
  }

  private void unlock(final LockFile f) {
    f.unlock();
    allLocks.remove(f);
  }

  private void abort() {
    for (final LockFile f : allLocks)
      f.unlock();
  }

Then a lot of this local variable management code might simplify out.

Another approach might be to make LockFile a member of RefUpdate,
and allow the RefUpdate members for oldFromDelete and newToUpdate
to keep track of their associated LockFile for you.  This may clean
up the code related to constructing the store operation.

> +				throw e;
> +			}
> +			try {
> +				UpdateStore toStore = newToUpdate.newUpdateStore();
> +				if (RefLogWriter.renameTo(oldFromDelete.getRepository(), oldFromDelete, newToUpdate)) {
> +					newToUpdate.setNewObjectId(oldFromDelete.getOldObjectId());
> +					newToUpdate.setExpectedOldObjectId(oldFromDelete.getOldObjectId());
> +					newToUpdate.setRefLogMessage("jgit branch: renamed " + oldFromDelete.getName() + " to " + oldFromDelete.getName(), false);

I would prefer if the caller can set the prefix part of the reflog
message ("jgit branch" portion) so we can set it by command.

> +					newToUpdate.result = toStore.store(lockFileTo, Result.RENAMED);
> +					DeleteStore fromStore = oldFromDelete.newDeleteStore();
> +					Result store = fromStore.store(lockFileFrom, Result.RENAMED);
> +					if (renameHEADtoo) {
> +						final byte[] content = Constants.encode("ref: " + newToUpdate.getName() + "\n");
> +						lockFileHEAD.write(content);
> +						synchronized (this) {

What effect could "synchronized (this)" really have here, other
than to waste CPU cycles?  Who else could have a reference to this
RefRenames instance and be able to lock on it?

> @@ -478,6 +578,8 @@ else if (status == Result.FAST_FORWARD)
>  				msg += ": fast forward";
>  			else if (status == Result.NEW)
>  				msg += ": created";
> +			else if (status == Result.RENAMED)
> +				msg += ": renamed";
>  		}

I don't think this is useful.  We probably shouldn't append
status during a rename as the rename method had already done
setRefLogMessage("renamed A to B") earlier.  Doing status here
would yield "renamed A to B: renamed" in the reflog; ugly.

> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
> index 5baa7a0..e704aeb 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
> @@ -1067,4 +1083,21 @@ public void scanForRepoChanges() throws IOException {
>  		getAllRefs(); // This will look for changes to refs
>  		getIndex(); // This will detect changes in the index
>  	}
> +
> +	/**
> +	 * Writes a symref (e.g. HEAD) to disk
> +	 *
> +	 * @param name
> +	 *            symref name
> +	 * @param target
> +	 *            pointed to ref
> +	 * @throws IOException
> +	 */
> +	public void link(final String name, final String target) throws IOException {
> +		refs.link(name, target);
> +	}

Uhm, isn't this already called writeSymref() ?

> +	void uncacheSymRef(String name) {
> +		refs.uncacheSymRef(name);
> +	}

Why is this in Repository?

-- 
Shawn.

[EGIT PATCH 0/6] Ref rename

[Robin Rosenberg]

Ok, an update with most of your comments and a mix of other fixes
founding since then during testing.

Robin Rosenberg (6):
  Make sure we get the right storage for loose/pack/loose and packed
    refs
  Add ref rename support to JGit
  Add ref rename support to the branch dialog
  Allow non-ASCII ref names when writing the packed-refs file
  Use Constants.PACKED_REFS in RefWriter
  Improve error reporting in the branch dialog

 .../src/org/spearce/egit/ui/UIText.java            |   15 ++
 .../ui/internal/dialogs/BranchSelectionDialog.java |  119 ++++++++++++----
 .../src/org/spearce/egit/ui/uitext.properties      |    6 +
 .../tst/org/spearce/jgit/lib/RefTest.java          |   94 ++++++++++++
 .../tst/org/spearce/jgit/lib/RefUpdateTest.java    |  152 ++++++++++++++++++++
 .../src/org/spearce/jgit/lib/RefDatabase.java      |   56 +++++++-
 .../src/org/spearce/jgit/lib/RefLogWriter.java     |   20 +++-
 .../src/org/spearce/jgit/lib/RefRename.java        |  101 +++++++++++++
 .../src/org/spearce/jgit/lib/RefUpdate.java        |   29 +++-
 .../src/org/spearce/jgit/lib/RefWriter.java        |    2 +-
 .../src/org/spearce/jgit/lib/Repository.java       |   16 ++
 11 files changed, 567 insertions(+), 43 deletions(-)
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/RefRename.java

[EGIT PATCH 1/6] Make sure we get the right storage for loose/pack/loose and packed refs

[Robin Rosenberg]

Also adds a few some more test for refs, though not complete.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../tst/org/spearce/jgit/lib/RefTest.java          |   94 ++++++++++++++++++++
 .../src/org/spearce/jgit/lib/RefDatabase.java      |   26 ++++--
 2 files changed, 114 insertions(+), 6 deletions(-)

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefTest.java
index cae5143..7b0c9a9 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefTest.java
@@ -36,8 +36,14 @@
  */
 package org.spearce.jgit.lib;
 
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
 import java.util.Map;
 
+import org.spearce.jgit.lib.Ref.Storage;
+import org.spearce.jgit.lib.RefUpdate.Result;
+
 /**
  * Misc tests for refs. A lot of things are tested elsewhere so not having a
  * test for a ref related method, does not mean it is untested.
@@ -69,4 +75,92 @@ public void testReadAllIncludingSymrefs() throws Exception {
 		assertFalse(refmaster.isPeeled());
 		assertNull(refmaster.getPeeledObjectId());
 	}
+
+	public void testReadSymRefToPacked() throws IOException {
+		db.writeSymref("HEAD", "refs/heads/master");
+		Ref ref = db.getRef("HEAD");
+		assertEquals(Ref.Storage.LOOSE, ref.getStorage());
+	}
+
+	public void testReadSymRefToLoosePacked() throws IOException {
+		ObjectId pid = db.resolve("refs/heads/master^");
+		RefUpdate updateRef = db.updateRef("refs/heads/master");
+		updateRef.setNewObjectId(pid);
+		updateRef.setForceUpdate(true);
+		Result update = updateRef.update();
+		assertEquals(Result.FORCED, update); // internal
+
+		db.writeSymref("HEAD", "refs/heads/master");
+		Ref ref = db.getRef("HEAD");
+		assertEquals(Ref.Storage.LOOSE, ref.getStorage());
+	}
+
+	public void testReadLooseRef() throws IOException {
+		RefUpdate updateRef = db.updateRef("ref/heads/new");
+		updateRef.setNewObjectId(db.resolve("refs/heads/master"));
+		Result update = updateRef.update();
+		assertEquals(Result.NEW, update);
+		Ref ref = db.getRef("ref/heads/new");
+		assertEquals(Storage.LOOSE, ref.getStorage());
+	}
+
+	/**
+	 * Let an "outsider" create a loose ref with the same name as a packed one
+	 *
+	 * @throws IOException
+	 * @throws InterruptedException
+	 */
+	public void testReadLoosePackedRef() throws IOException,
+			InterruptedException {
+		Ref ref = db.getRef("refs/heads/master");
+		assertEquals(Storage.PACKED, ref.getStorage());
+		FileOutputStream os = new FileOutputStream(new File(db.getDirectory(),
+				"refs/heads/master"));
+		os.write(ref.getObjectId().name().getBytes());
+		os.write('\n');
+		os.close();
+
+		ref = db.getRef("refs/heads/master");
+		assertEquals(Storage.LOOSE_PACKED, ref.getStorage());
+	}
+
+	/**
+	 * Modify a packed ref using the API. This creates a loose ref too, ie.
+	 * LOOSE_PACKED
+	 *
+	 * @throws IOException
+	 */
+	public void testReadSimplePackedRefSameRepo() throws IOException {
+		Ref ref = db.getRef("refs/heads/master");
+		ObjectId pid = db.resolve("refs/heads/master^");
+		assertEquals(Storage.PACKED, ref.getStorage());
+		RefUpdate updateRef = db.updateRef("refs/heads/master");
+		updateRef.setNewObjectId(pid);
+		updateRef.setForceUpdate(true);
+		Result update = updateRef.update();
+		assertEquals(Result.FORCED, update);
+
+		ref = db.getRef("refs/heads/master");
+		assertEquals(Storage.LOOSE_PACKED, ref.getStorage());
+	}
+
+	/**
+	 * Delete a ref that exists both as packed and loose. Make sure the ref
+	 * cannot be resolved after delete.
+	 *
+	 * @throws IOException
+	 */
+	public void testDeleteLoosePacked() throws IOException {
+		ObjectId pid = db.resolve("refs/heads/master^");
+		RefUpdate updateRef = db.updateRef("refs/heads/master");
+		updateRef.setNewObjectId(pid);
+		updateRef.setForceUpdate(true);
+		Result update = updateRef.update();
+		assertEquals(Result.FORCED, update); // internal
+
+		RefUpdate updateRef2 = db.updateRef("refs/heads/master");
+		Result delete = updateRef2.delete();
+		assertEquals(Result.FORCED, delete);
+		assertNull(db.resolve("refs/heads/master"));
+	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
index 155ed9a..573fcac 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
@@ -283,8 +283,14 @@ private synchronized Ref readRefBasic(final String origName,
 		final long mtime = loose.lastModified();
 		if (ref != null) {
 			Long cachedlastModified = looseRefsMTime.get(name);
-			if (cachedlastModified != null && cachedlastModified == mtime)
-				return ref;
+			if (cachedlastModified != null && cachedlastModified == mtime) {
+				if (packedRefs.containsKey(origName))
+					return new Ref(Storage.LOOSE_PACKED, origName, ref
+							.getObjectId(), ref.getPeeledObjectId(), ref
+							.isPeeled());
+				else
+					return ref;
+			}
 			looseRefs.remove(origName);
 			looseRefsMTime.remove(origName);
 		}
@@ -349,12 +355,20 @@ private synchronized Ref readRefBasic(final String origName,
 			throw new IOException("Not a ref: " + name + ": " + line);
 		}
 
-		ref = new Ref(Ref.Storage.LOOSE, origName, name, id);
-
-		looseRefs.put(origName, ref);
-		ref = new Ref(Ref.Storage.LOOSE, origName, id);
+		Storage storage;
+		if (packedRefs.containsKey(name))
+			storage = Ref.Storage.LOOSE_PACKED;
+		else
+			storage = Ref.Storage.LOOSE;
+		ref = new Ref(storage, name, id);
 		looseRefs.put(name, ref);
 		looseRefsMTime.put(name, mtime);
+
+		if (!origName.equals(name)) {
+			ref = new Ref(Ref.Storage.LOOSE, origName, name, id);
+			looseRefs.put(origName, ref);
+		}
+
 		return ref;
 	}
 
-- 
1.6.3.dirty

[EGIT PATCH 2/6] Add ref rename support to JGit

[Robin Rosenberg]

Now refs can be renamed. The intent is that should be safe. Only the named
refs and associated logs are updated. Any symbolic refs referring to the renames
branches are unaffected, except HEAD.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../tst/org/spearce/jgit/lib/RefUpdateTest.java    |  152 ++++++++++++++++++++
 .../src/org/spearce/jgit/lib/RefDatabase.java      |   28 ++++
 .../src/org/spearce/jgit/lib/RefLogWriter.java     |   20 +++-
 .../src/org/spearce/jgit/lib/RefRename.java        |  101 +++++++++++++
 .../src/org/spearce/jgit/lib/RefUpdate.java        |   29 +++-
 .../src/org/spearce/jgit/lib/Repository.java       |   16 ++
 6 files changed, 339 insertions(+), 7 deletions(-)
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/RefRename.java

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
index 55d7441..cd66eb1 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
@@ -155,4 +155,156 @@ public void testRefKeySameAsOrigName() {
 
 		}
 	}
+
+	public void testRenameBranchNoPreviousLog() throws IOException {
+		assertFalse("precondition, no log on old branchg", new File(db
+				.getDirectory(), "logs/refs/heads/b").exists());
+		ObjectId rb = db.resolve("refs/heads/b");
+		ObjectId oldHead = db.resolve(Constants.HEAD);
+		assertFalse(rb.equals(oldHead)); // assumption for this test
+		RefRename renameRef = db.renameRef("refs/heads/b",
+				"refs/heads/new/name");
+		Result result = renameRef.rename();
+		assertEquals(Result.RENAMED, result);
+		assertEquals(rb, db.resolve("refs/heads/new/name"));
+		assertNull(db.resolve("refs/heads/b"));
+		assertTrue(new File(db.getDirectory(), "logs/refs/heads/new/name")
+				.exists());
+		assertFalse(new File(db.getDirectory(), "logs/refs/heads/b").exists());
+		assertEquals(oldHead, db.resolve(Constants.HEAD));
+	}
+
+	public void testRenameBranchHasPreviousLog() throws IOException {
+		ObjectId rb = db.resolve("refs/heads/b");
+		ObjectId oldHead = db.resolve(Constants.HEAD);
+		assertFalse("precondition for this test, branch b != HEAD", rb
+				.equals(oldHead));
+		RefLogWriter.writeReflog(db, rb, rb, "Just a message", "refs/heads/b");
+		assertTrue("no log on old branch", new File(db.getDirectory(),
+				"logs/refs/heads/b").exists());
+		RefRename renameRef = db.renameRef("refs/heads/b",
+				"refs/heads/new/name");
+		Result result = renameRef.rename();
+		assertEquals(Result.RENAMED, result);
+		assertEquals(rb, db.resolve("refs/heads/new/name"));
+		assertNull(db.resolve("refs/heads/b"));
+		assertTrue(new File(db.getDirectory(), "logs/refs/heads/new/name")
+				.exists());
+		assertFalse(new File(db.getDirectory(), "logs/refs/heads/b").exists());
+		assertEquals(oldHead, db.resolve(Constants.HEAD));
+		// TODO: test content of log file
+	}
+
+	public void testRenameCurrentBranch() throws IOException {
+		ObjectId rb = db.resolve("refs/heads/b");
+		db.writeSymref(Constants.HEAD, "refs/heads/b");
+		ObjectId oldHead = db.resolve(Constants.HEAD);
+		assertTrue("internal test condition, b == HEAD", rb.equals(oldHead));
+		RefLogWriter.writeReflog(db, rb, rb, "Just a message", "refs/heads/b");
+		assertTrue("no log on old branch", new File(db.getDirectory(),
+				"logs/refs/heads/b").exists());
+		RefRename renameRef = db.renameRef("refs/heads/b",
+				"refs/heads/new/name");
+		Result result = renameRef.rename();
+		assertEquals(Result.RENAMED, result);
+		assertEquals(rb, db.resolve("refs/heads/new/name"));
+		assertNull(db.resolve("refs/heads/b"));
+		assertTrue(new File(db.getDirectory(), "logs/refs/heads/new/name")
+				.exists());
+		assertFalse(new File(db.getDirectory(), "logs/refs/heads/b").exists());
+		assertEquals(rb, db.resolve(Constants.HEAD));
+		// TODO: test content of log file
+	}
+
+	public void testRenameBranchAlsoInPack() throws IOException {
+		ObjectId rb = db.resolve("refs/heads/b");
+		ObjectId rb2 = db.resolve("refs/heads/b~1");
+		assertEquals(Ref.Storage.PACKED, db.getRef("refs/heads/b").getStorage());
+		RefUpdate updateRef = db.updateRef("refs/heads/b");
+		updateRef.setNewObjectId(rb2);
+		updateRef.setForceUpdate(true);
+		Result update = updateRef.update();
+		assertEquals("internal check new ref is loose", Result.FORCED, update);
+		assertEquals(Ref.Storage.LOOSE_PACKED, db.getRef("refs/heads/b").getStorage());
+		RefLogWriter.writeReflog(db, rb, rb, "Just a message", "refs/heads/b");
+		assertTrue("no log on old branch", new File(db.getDirectory(),
+				"logs/refs/heads/b").exists());
+		RefRename renameRef = db.renameRef("refs/heads/b",
+				"refs/heads/new/name");
+		Result result = renameRef.rename();
+		assertEquals(Result.RENAMED, result);
+		assertEquals(rb2, db.resolve("refs/heads/new/name"));
+		assertNull(db.resolve("refs/heads/b"));
+		assertTrue(new File(db.getDirectory(), "logs/refs/heads/new/name")
+				.exists());
+		assertFalse(new File(db.getDirectory(), "logs/refs/heads/b").exists());
+
+		// Create new Repository instance, to reread caches and make sure our
+		// assumptions are persistent.
+		Repository ndb = new Repository(db.getDirectory());
+		assertEquals(rb2, ndb.resolve("refs/heads/new/name"));
+		assertNull(ndb.resolve("refs/heads/b"));
+	}
+
+	public void testRenameBranchCannotLockFirstBranch() throws IOException {
+		// "someone" has branch b locked
+		assertTrue(new LockFile(new File(db.getDirectory(), "refs/heads/b"))
+				.lock());
+
+		// setup
+		ObjectId rb = db.resolve("refs/heads/b");
+		db.writeSymref(Constants.HEAD, "refs/heads/b");
+		ObjectId oldHead = db.resolve(Constants.HEAD);
+		assertTrue(rb.equals(oldHead)); // assumption for this test
+		RefLogWriter.writeReflog(db, rb, rb, "Just a message", "refs/heads/b");
+		assertTrue("internal check, we have a log", new File(db.getDirectory(),
+				"logs/refs/heads/b").exists());
+
+		// Now this is our test
+		RefRename renameRef = db.renameRef("refs/heads/b",
+				"refs/heads/new/name");
+		Result result = renameRef.rename();
+		assertEquals(Result.LOCK_FAILURE, result);
+
+		// Check that the involved refs are sane despite the failure
+		assertFalse(new File(db.getDirectory(), "refs/heads/new/name").exists());
+		assertFalse(new File(db.getDirectory(), "refs/heads/new/name.lock")
+				.exists());
+		assertFalse(new File(db.getDirectory(), "logs/refs/heads/b/lock")
+				.exists());
+		assertFalse(new File(db.getDirectory(), "logs/refs/heads/new/name.lock")
+				.exists());
+		assertEquals(rb, db.resolve(Constants.HEAD));
+	}
+
+	public void testRenameBranchCannotLockHEAD() throws IOException {
+		// setup
+		ObjectId rb = db.resolve("refs/heads/b");
+		db.writeSymref(Constants.HEAD, "refs/heads/b");
+
+		// "someone" has branch b locked
+		assertTrue(new LockFile(new File(db.getDirectory(), "HEAD")).lock());
+
+		ObjectId oldHead = db.resolve(Constants.HEAD);
+		assertTrue("internal test branch b == HEAD", rb.equals(oldHead));
+		RefLogWriter.writeReflog(db, rb, rb, "Just a message", "refs/heads/b");
+		assertTrue("no log on old branch", new File(db.getDirectory(),
+				"logs/refs/heads/b").exists());
+
+		// Now this is our test
+		RefRename renameRef = db.renameRef("refs/heads/b",
+				"refs/heads/new/name");
+		Result result = renameRef.rename();
+		assertEquals(Result.LOCK_FAILURE, result);
+
+		// Check that the involved refs are sane despite the failure
+		assertFalse(new File(db.getDirectory(), "refs/heads/new/name").exists());
+		assertFalse(new File(db.getDirectory(), "refs/heads/new/name.lock")
+				.exists());
+		assertFalse(new File(db.getDirectory(), "logs/refs/heads/b/lock")
+				.exists());
+		assertFalse(new File(db.getDirectory(), "logs/refs/heads/new/name.lock")
+				.exists());
+		assertEquals(rb, db.resolve(Constants.HEAD));
+	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
index 573fcac..9c0d114 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
@@ -146,6 +146,26 @@ synchronized (this) {
 	}
 
 	/**
+	 * An set of update operations for renaming a ref
+	 *
+	 * @param fromRef Old ref name
+	 * @param toRef New ref name
+	 * @return a RefUpdate operation to rename a ref
+	 * @throws IOException
+	 */
+	RefRename newRename(String fromRef, String toRef) throws IOException {
+		refreshPackedRefs();
+		Ref f = readRefBasic(fromRef, 0);
+		Ref t = readRefBasic(toRef, 0);
+		if (t != null)
+			throw new IOException("Ref rename target exists: " + t.getName());
+		t = new Ref(Ref.Storage.NEW, toRef, null);
+		RefUpdate refUpdateFrom = new RefUpdate(this, f, fileForRef(f.getName()));
+		RefUpdate refUpdateTo = new RefUpdate(this, t, fileForRef(t.getName()));
+		return new RefRename(refUpdateTo, refUpdateFrom);
+	}
+
+	/**
 	 * Writes a symref (e.g. HEAD) to disk
 	 * 
 	 * @param name
@@ -158,11 +178,19 @@ void link(final String name, final String target) throws IOException {
 		final byte[] content = Constants.encode("ref: " + target + "\n");
 		lockAndWriteFile(fileForRef(name), content);
 		synchronized (this) {
+			looseSymRefs.remove(name);
 			setModified();
 		}
 		db.fireRefsMaybeChanged();
 	}
 
+	void uncacheSymRef(String name) {
+		synchronized(this) {
+			looseSymRefs.remove(name);
+			setModified();
+		}
+	}
+
 	private void setModified() {
 		lastRefModification = refModificationCounter++;
 	}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java
index a077051..2efa12d 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java
@@ -44,7 +44,7 @@
 import java.io.IOException;
 
 /**
- * Utility class to add reflog entries
+ * Utility class to work with reflog files
  * 
  * @author Dave Watson
  */
@@ -58,6 +58,24 @@ static void append(final RefUpdate u, final String msg) throws IOException {
 		appendOneRecord(oldId, newId, ident, msg, db, u.getName());
 	}
 
+	static void renameTo(final Repository db, final RefUpdate from,
+			final RefUpdate to) throws IOException {
+		final File logdir = new File(db.getDirectory(), Constants.LOGS);
+		final File reflogFrom = new File(logdir, from.getName());
+		if (!reflogFrom.exists())
+			return;
+		final File reflogTo = new File(logdir, to.getName());
+		final File refdirTo = reflogTo.getParentFile();
+		if (!refdirTo.exists() && !refdirTo.mkdirs()) {
+			throw new IOException("Cannot create directory " + refdirTo);
+		}
+		if (!reflogFrom.renameTo(reflogTo)) {
+			reflogTo.delete(); // try
+			throw new IOException("Cannot rename " + reflogFrom + " to "
+					+ reflogTo);
+		}
+	}
+
 	private static void appendOneRecord(final ObjectId oldId,
 			final ObjectId newId, PersonIdent ident, final String msg,
 			final Repository db, final String refName) throws IOException {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefRename.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefRename.java
new file mode 100644
index 0000000..4ea9cfa
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefRename.java
@@ -0,0 +1,101 @@
+package org.spearce.jgit.lib;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.spearce.jgit.lib.RefUpdate.DeleteStore;
+import org.spearce.jgit.lib.RefUpdate.Result;
+import org.spearce.jgit.lib.RefUpdate.UpdateStore;
+
+/**
+ * A RefUpdate combination for renaming a ref
+ */
+public class RefRename {
+	private RefUpdate newToUpdate;
+
+	private RefUpdate oldFromDelete;
+
+	private Result renameResult;
+
+	RefRename(final RefUpdate toUpdate, final RefUpdate fromUpdate) {
+		newToUpdate = toUpdate;
+		oldFromDelete = fromUpdate;
+	}
+
+	/**
+	 * @return result of rename operation
+	 */
+	public Result getResult() {
+		return renameResult;
+	}
+
+	/**
+	 * @return the result of the new ref update
+	 * @throws IOException
+	 */
+	public Result rename() throws IOException {
+		List<LockFile> lockFiles = new ArrayList<LockFile>();
+		LockFile lockFileFrom = new LockFile(oldFromDelete.looseFile);
+		LockFile lockFileTo = new LockFile(newToUpdate.looseFile);
+		LockFile lockFileHEAD = new LockFile(new File(oldFromDelete.db
+				.getRepository().getDirectory(), Constants.HEAD));
+		lockFiles.add(lockFileTo);
+		lockFiles.add(lockFileFrom);
+		lockFiles.add(lockFileHEAD);
+		try {
+			for (LockFile l : lockFiles) {
+				if (!l.lock()) {
+					unlock(lockFiles);
+					return Result.LOCK_FAILURE;
+				}
+			}
+		} catch (RuntimeException e) {
+			unlock(lockFiles);
+			throw e;
+		} catch (IOException e) {
+			unlock(lockFiles);
+			throw e;
+		}
+		boolean renameHEADtoo = oldFromDelete.db.readRef(Constants.HEAD)
+				.getName().equals(oldFromDelete.getName());
+		try {
+			UpdateStore toStore = newToUpdate.newUpdateStore();
+			RefLogWriter.renameTo(oldFromDelete.getRepository(), oldFromDelete,
+					newToUpdate);
+			newToUpdate.setNewObjectId(oldFromDelete.getOldObjectId());
+			newToUpdate.setExpectedOldObjectId(oldFromDelete.getOldObjectId());
+			newToUpdate.setRefLogMessage("jgit branch: renamed "
+					+ oldFromDelete.getName() + " to "
+					+ oldFromDelete.getName(), false);
+			newToUpdate.result = toStore.store(lockFileTo, Result.RENAMED);
+			DeleteStore fromStore = oldFromDelete.newDeleteStore();
+			Result store = fromStore.store(lockFileFrom, Result.RENAMED);
+			if (store == Result.RENAMED && renameHEADtoo) {
+				final byte[] content = Constants.encode("ref: "
+						+ newToUpdate.getName() + "\n");
+				lockFileHEAD.write(content);
+				oldFromDelete.db.uncacheSymRef(Constants.HEAD);
+				if (!lockFileHEAD.commit())
+					throw new IOException("Failed to commit HEAD during rename");
+			}
+			oldFromDelete.db.getRepository().fireRefsMaybeChanged();
+			return store;
+		} catch (RuntimeException e) {
+			unlock(lockFiles);
+			throw e;
+		} catch (IOException e) {
+			System.err.println(e);
+			return Result.IO_FAILURE;
+		} finally {
+			unlock(lockFiles);
+		}
+	}
+
+	private void unlock(List<LockFile> lockFiles) {
+		for (LockFile lockFile : lockFiles) {
+			lockFile.unlock();
+		}
+	}
+}
\ No newline at end of file
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
index a9ab73b..7a1074d 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
@@ -125,14 +125,20 @@
 		 * This kind of error doesn't include {@link #LOCK_FAILURE}, which is a
 		 * different case.
 		 */
-		IO_FAILURE
+		IO_FAILURE,
+
+		/**
+		 * The ref was renamed from another name
+		 * <p>
+		 */
+		RENAMED
 	}
 
 	/** Repository the ref is stored in. */
-	private final RefDatabase db;
+	final RefDatabase db;
 
 	/** Location of the loose file holding the value of this ref. */
-	private final File looseFile;
+	final File looseFile;
 
 	/** New value the caller wants this ref to have. */
 	private ObjectId newValue;
@@ -156,7 +162,7 @@
 	private ObjectId expValue;
 
 	/** Result of the update operation. */
-	private Result result = Result.NOT_ATTEMPTED;
+	Result result = Result.NOT_ATTEMPTED;
 
 	private final Ref ref;
 
@@ -478,6 +484,9 @@ else if (status == Result.FAST_FORWARD)
 				msg += ": fast forward";
 			else if (status == Result.NEW)
 				msg += ": created";
+			else if (status == Result.RENAMED) {
+				// don't amend the message here
+			}
 		}
 		RefLogWriter.append(this, msg);
 		if (!lock.commit())
@@ -495,7 +504,7 @@ abstract Result store(final LockFile lock, final Result status)
 				throws IOException;
 	}
 
-	private class UpdateStore extends Store {
+	class UpdateStore extends Store {
 
 		@Override
 		Result store(final LockFile lock, final Result status)
@@ -504,7 +513,7 @@ Result store(final LockFile lock, final Result status)
 		}
 	}
 
-	private class DeleteStore extends Store {
+	class DeleteStore extends Store {
 
 		@Override
 		Result store(LockFile lock, Result status) throws IOException {
@@ -553,4 +562,12 @@ private static int count(final String s, final char c) {
 		}
 		return count;
 	}
+
+	UpdateStore newUpdateStore() {
+		return new UpdateStore();
+	}
+
+	DeleteStore newDeleteStore() {
+		return new DeleteStore();
+	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
index 9bed1b7..08539fa 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
@@ -468,6 +468,22 @@ public RefUpdate updateRef(final String ref) throws IOException {
 	}
 
 	/**
+	 * Create a command to rename a ref in this repository
+	 *
+	 * @param fromRef
+	 *            name of ref to rename from
+	 * @param toRef
+	 *            name of ref to rename to
+	 * @return an update command that knows how to rename a branch to another.
+	 * @throws IOException
+	 *             the rename could not be performed.
+	 *
+	 */
+	public RefRename renameRef(final String fromRef, final String toRef) throws IOException {
+		return refs.newRename(fromRef, toRef);
+	}
+
+	/**
 	 * Parse a git revision string and return an object id.
 	 *
 	 * Currently supported is combinations of these.
-- 
1.6.3.dirty

[EGIT PATCH 3/6] Add ref rename support to the branch dialog

[Robin Rosenberg]

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../src/org/spearce/egit/ui/UIText.java            |    9 ++
 .../ui/internal/dialogs/BranchSelectionDialog.java |   92 +++++++++++++++----
 .../src/org/spearce/egit/ui/uitext.properties      |    4 +
 3 files changed, 85 insertions(+), 20 deletions(-)

diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
index 654e155..1d52973 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
@@ -893,6 +893,12 @@
 	public static String BranchSelectionDialog_ErrorCouldNotRefresh;
 
 	/** */
+	public static String BranchSelectionDialog_ErrorCouldNotRenameRef;
+
+	/** */
+	public static String BranchSelectionDialog_ErrorCouldNotRenameRef2;
+
+	/** */
 	public static String BranchSelectionDialog_BranchSuffix_Current;
 
 	/** */
@@ -929,6 +935,9 @@
 	public static String BranchSelectionDialog_QuestionNewBranchTitle;
 
 	/** */
+	public static String BranchSelectionDialog_QuestionNewBranchNameMessage;
+
+	/** */
 	public static String BranchSelectionDialog_QuestionNewBranchMessage;
 
 	/** */
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
index 9aad95b..6700768 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
@@ -48,8 +48,10 @@
 import org.spearce.egit.ui.UIText;
 import org.spearce.jgit.lib.Constants;
 import org.spearce.jgit.lib.ObjectId;
+import org.spearce.jgit.lib.RefRename;
 import org.spearce.jgit.lib.RefUpdate;
 import org.spearce.jgit.lib.Repository;
+import org.spearce.jgit.lib.RefUpdate.Result;
 
 /**
  * The branch and reset selection dialog
@@ -277,6 +279,30 @@ private void refNameFromDialog() {
 		}
 	}
 
+	private InputDialog getRefNameInputDialog(String prompt) {
+		InputDialog labelDialog = new InputDialog(
+				getShell(),
+				UIText.BranchSelectionDialog_QuestionNewBranchTitle,
+				prompt,
+				null, new IInputValidator() {
+					public String isValid(String newText) {
+						String testFor = Constants.R_HEADS + newText;
+						try {
+							if (repo.resolve(testFor) != null)
+								return UIText.BranchSelectionDialog_ErrorAlreadyExists;
+						} catch (IOException e1) {
+							Activator.logError(NLS.bind(
+									UIText.BranchSelectionDialog_ErrorCouldNotResolve, testFor), e1);
+						}
+						if (!Repository.isValidRefName(testFor))
+							return UIText.BranchSelectionDialog_ErrorInvalidRefName;
+						return null;
+					}
+				});
+		labelDialog.setBlockOnOpen(true);
+		return labelDialog;
+	}
+
 	@Override
 	protected void createButtonsForButtonBar(Composite parent) {
 		if (!showResetType) {
@@ -284,32 +310,57 @@ protected void createButtonsForButtonBar(Composite parent) {
 			newButton.setFont(JFaceResources.getDialogFont());
 			newButton.setText(UIText.BranchSelectionDialog_NewBranch);
 			((GridLayout)parent.getLayout()).numColumns++;
+			Button renameButton = new Button(parent, SWT.PUSH);
+			renameButton.setText("&Rename");
+			renameButton.addSelectionListener(new SelectionListener() {
+				public void widgetSelected(SelectionEvent e) {
+					// check what ref name the user selected, if any.
+					refNameFromDialog();
+
+					InputDialog labelDialog = getRefNameInputDialog(NLS
+							.bind(
+									UIText.BranchSelectionDialog_QuestionNewBranchNameMessage,
+									refName));
+					if (labelDialog.open() == Window.OK) {
+						String newRefName = Constants.R_HEADS + labelDialog.getValue();
+						try {
+							RefRename renameRef = repo.renameRef(refName, newRefName);
+							if (renameRef.rename() != Result.RENAMED) {
+								MessageDialog.openError(getShell(),
+										"Rename failed",
+										NLS.bind(UIText.BranchSelectionDialog_ErrorCouldNotRenameRef,
+												new Object[] { refName, newRefName, renameRef.getResult() }));
+								Activator.logError(NLS.bind(
+										UIText.BranchSelectionDialog_ErrorCouldNotRenameRef2,
+												new Object[] { refName, newRefName }), null);
+							}
+							// FIXME: Update HEAD
+						} catch (IOException e1) {
+							Activator.logError(NLS.bind(
+									UIText.BranchSelectionDialog_ErrorCouldNotRenameRef2,
+															newRefName), e1);
+						}
+						try {
+							branchTree.removeAll();
+							fillTreeWithBranches(newRefName);
+						} catch (IOException e1) {
+							Activator.logError(
+									UIText.BranchSelectionDialog_ErrorCouldNotRefreshBranchList,
+											e1);
+						}
+					}
+				}
+				public void widgetDefaultSelected(SelectionEvent e) {
+					widgetSelected(e);
+				}
+			});
 			newButton.addSelectionListener(new SelectionListener() {
 
 				public void widgetSelected(SelectionEvent e) {
 					// check what ref name the user selected, if any.
 					refNameFromDialog();
 
-					InputDialog labelDialog = new InputDialog(
-							getShell(),
-							UIText.BranchSelectionDialog_QuestionNewBranchTitle,
-							UIText.BranchSelectionDialog_QuestionNewBranchMessage,
-							null, new IInputValidator() {
-								public String isValid(String newText) {
-									String testFor = Constants.R_HEADS + newText;
-									try {
-										if (repo.resolve(testFor) != null)
-											return UIText.BranchSelectionDialog_ErrorAlreadyExists;
-									} catch (IOException e1) {
-										Activator.logError(NLS.bind(
-												UIText.BranchSelectionDialog_ErrorCouldNotResolve, testFor), e1);
-									}
-									if (!Repository.isValidRefName(testFor))
-										return UIText.BranchSelectionDialog_ErrorInvalidRefName;
-									return null;
-								}
-							});
-					labelDialog.setBlockOnOpen(true);
+					InputDialog labelDialog = getRefNameInputDialog(UIText.BranchSelectionDialog_QuestionNewBranchMessage);
 					if (labelDialog.open() == Window.OK) {
 						String newRefName = Constants.R_HEADS + labelDialog.getValue();
 						RefUpdate updateRef;
@@ -338,6 +389,7 @@ public String isValid(String newText) {
 					}
 				}
 
+
 				public void widgetDefaultSelected(SelectionEvent e) {
 					widgetSelected(e);
 				}
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties b/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
index 1d21c81..2a40439 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
@@ -340,6 +340,9 @@ BranchSelectionDialog_ErrorCouldNotRefresh=Could not refresh list of branches
 BranchSelectionDialog_ErrorCouldNotRefreshBranchList=Could not refresh list of branches
 BranchSelectionDialog_ErrorCouldNotResolve=Could not attempt to resolve {0}
 BranchSelectionDialog_ErrorInvalidRefName=Invalid ref name
+BranchSelectionDialog_ErrorCouldNotRenameRef=Failed to rename branch {0} -> {1}, status={2}
+BranchSelectionDialog_ErrorCouldNotRenameRef2=Failed to rename branch {0} -> {1}
+
 BranchSelectionDialog_LocalBranches=Local Branches
 BranchSelectionDialog_NewBranch=&New branch
 BranchSelectionDialog_NoBranchSeletectMessage=You must select a valid ref.
@@ -347,6 +350,7 @@ BranchSelectionDialog_NoBranchSeletectTitle=No branch/tag selected
 BranchSelectionDialog_OkCheckout=&Checkout
 BranchSelectionDialog_OkReset=&Reset
 BranchSelectionDialog_QuestionNewBranchMessage=Enter name of new branch. It will branch from the selected branch. refs/heads/ will be prepended to the name you type
+BranchSelectionDialog_QuestionNewBranchNameMessage=Enter new name of the {0} branch. refs/heads/ will be prepended to the name you type
 BranchSelectionDialog_QuestionNewBranchTitle=New branch
 BranchSelectionDialog_ReallyResetMessage=Resetting will overwrite any changes in your working directory.\n\nDo you wish to continue?
 BranchSelectionDialog_ReallyResetTitle=Really reset?
-- 
1.6.3.dirty

[EGIT PATCH 4/6] Allow non-ASCII ref names when writing the packed-refs file

[Robin Rosenberg]

Previously we crashed. Now we encode using UTF-8. This may possibly
change the encoding in the packed-refs file as we try to be smart about
reading the file when converting to Java strings.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../src/org/spearce/jgit/lib/RefWriter.java        |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefWriter.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefWriter.java
index 2d39713..0a1bac8 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefWriter.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefWriter.java
@@ -145,7 +145,7 @@ public void writePackedRefs() throws IOException {
 				w.write('\n');
 			}
 		}
-		writeFile(Constants.PACKED_REFS, Constants.encodeASCII(w.toString()));
+		writeFile(Constants.PACKED_REFS, Constants.encode(w.toString()));
 	}
 
 	/**
-- 
1.6.3.dirty

[EGIT PATCH 5/6] Use Constants.PACKED_REFS in RefWriter

[Robin Rosenberg]

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../src/org/spearce/jgit/lib/RefDatabase.java      |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
index 9c0d114..de987ab 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
@@ -87,7 +87,7 @@ RefDatabase(final Repository r) {
 		db = r;
 		gitDir = db.getDirectory();
 		refsDir = FS.resolve(gitDir, "refs");
-		packedRefsFile = FS.resolve(gitDir, "packed-refs");
+		packedRefsFile = FS.resolve(gitDir, Constants.PACKED_REFS);
 		clearCache();
 	}
 
-- 
1.6.3.dirty

[EGIT PATCH 6/6] Improve error reporting in the branch dialog

[Robin Rosenberg]

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../src/org/spearce/egit/ui/UIText.java            |    6 ++
 .../ui/internal/dialogs/BranchSelectionDialog.java |   59 ++++++++++++--------
 .../src/org/spearce/egit/ui/uitext.properties      |    2 +
 3 files changed, 43 insertions(+), 24 deletions(-)

diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
index 1d52973..7fdd470 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
@@ -899,6 +899,12 @@
 	public static String BranchSelectionDialog_ErrorCouldNotRenameRef2;
 
 	/** */
+	public static String BranchSelectionDialog_BranchSelectionDialog_CreateFailedTitle;
+
+	/** */
+	public static String BranchSelectionDialog_BranchSelectionDialog_RenamedFailedTitle;
+
+	/** */
 	public static String BranchSelectionDialog_BranchSuffix_Current;
 
 	/** */
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
index 6700768..e8c083a 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
@@ -103,7 +103,7 @@ getShell().setText(
 
 		try {
 			fillTreeWithBranches(null);
-		} catch (IOException e) {
+		} catch (Throwable e) {
 			Activator.logError(UIText.BranchSelectionDialog_ErrorCouldNotRefresh, e);
 		}
 
@@ -293,6 +293,7 @@ public String isValid(String newText) {
 						} catch (IOException e1) {
 							Activator.logError(NLS.bind(
 									UIText.BranchSelectionDialog_ErrorCouldNotResolve, testFor), e1);
+							return e1.getMessage();
 						}
 						if (!Repository.isValidRefName(testFor))
 							return UIText.BranchSelectionDialog_ErrorInvalidRefName;
@@ -326,27 +327,28 @@ public void widgetSelected(SelectionEvent e) {
 						try {
 							RefRename renameRef = repo.renameRef(refName, newRefName);
 							if (renameRef.rename() != Result.RENAMED) {
-								MessageDialog.openError(getShell(),
-										"Rename failed",
-										NLS.bind(UIText.BranchSelectionDialog_ErrorCouldNotRenameRef,
-												new Object[] { refName, newRefName, renameRef.getResult() }));
-								Activator.logError(NLS.bind(
-										UIText.BranchSelectionDialog_ErrorCouldNotRenameRef2,
-												new Object[] { refName, newRefName }), null);
+								reportError(
+										null,
+										UIText.BranchSelectionDialog_BranchSelectionDialog_RenamedFailedTitle,
+										UIText.BranchSelectionDialog_ErrorCouldNotRenameRef,
+										refName, newRefName, renameRef
+												.getResult());
 							}
-							// FIXME: Update HEAD
-						} catch (IOException e1) {
-							Activator.logError(NLS.bind(
-									UIText.BranchSelectionDialog_ErrorCouldNotRenameRef2,
-															newRefName), e1);
+						} catch (Throwable e1) {
+							reportError(
+									e1,
+									UIText.BranchSelectionDialog_BranchSelectionDialog_RenamedFailedTitle,
+									UIText.BranchSelectionDialog_ErrorCouldNotRenameRef,
+									refName, newRefName, e1.getMessage());
 						}
 						try {
 							branchTree.removeAll();
 							fillTreeWithBranches(newRefName);
-						} catch (IOException e1) {
-							Activator.logError(
-									UIText.BranchSelectionDialog_ErrorCouldNotRefreshBranchList,
-											e1);
+						} catch (Throwable e1) {
+							reportError(
+									e1,
+									UIText.BranchSelectionDialog_BranchSelectionDialog_RenamedFailedTitle,
+									UIText.BranchSelectionDialog_ErrorCouldNotRefreshBranchList);
 						}
 					}
 				}
@@ -373,18 +375,20 @@ public void widgetSelected(SelectionEvent e) {
 								startAt = repo.resolve(refName);
 							updateRef.setNewObjectId(startAt);
 							updateRef.update();
-						} catch (IOException e1) {
-							Activator.logError(NLS.bind(
+						} catch (Throwable e1) {
+							reportError(
+									e1,
+									UIText.BranchSelectionDialog_BranchSelectionDialog_CreateFailedTitle,
 									UIText.BranchSelectionDialog_ErrorCouldNotCreateNewRef,
-															newRefName), e1);
+									newRefName);
 						}
 						try {
 							branchTree.removeAll();
 							fillTreeWithBranches(newRefName);
-						} catch (IOException e1) {
-							Activator.logError(
-									UIText.BranchSelectionDialog_ErrorCouldNotRefreshBranchList,
-											e1);
+						} catch (Throwable e1) {
+							reportError(e1,
+									UIText.BranchSelectionDialog_BranchSelectionDialog_CreateFailedTitle,
+									UIText.BranchSelectionDialog_ErrorCouldNotRefreshBranchList);
 						}
 					}
 				}
@@ -405,4 +409,11 @@ createButton(parent, IDialogConstants.OK_ID,
 	protected int getShellStyle() {
 		return super.getShellStyle() | SWT.RESIZE;
 	}
+
+	private void reportError(Throwable e, String title, String message,
+			Object... args) {
+		String msg = NLS.bind(message, args);
+		MessageDialog.openError(getShell(), title, msg);
+		Activator.logError(msg, e);
+	}
 }
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties b/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
index 2a40439..2edef25 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
@@ -333,6 +333,8 @@ WindowCachePreferencePage_packedGitMMAP=Use virtual memory mapping
 
 BranchSelectionDialog_TitleCheckout=Checkout: {0}
 BranchSelectionDialog_TitleReset=Reset: {0}
+BranchSelectionDialog_BranchSelectionDialog_CreateFailedTitle=New branch creation failed
+BranchSelectionDialog_BranchSelectionDialog_RenamedFailedTitle=Rename failed
 BranchSelectionDialog_BranchSuffix_Current=\ (current)
 BranchSelectionDialog_ErrorAlreadyExists=Already exists
 BranchSelectionDialog_ErrorCouldNotCreateNewRef=Could not create new ref {0}
-- 
1.6.3.dirty

Re: [EGIT PATCH 1/6] Make sure we get the right storage for loose/pack/loose and packed refs

[Shawn O. Pearce]

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> Also adds a few some more test for refs, though not complete.

Hmm, tests fail, wrong expected values?
 
> +	public void testReadSymRefToPacked() throws IOException {
> +		assertEquals(Ref.Storage.LOOSE, ref.getStorage());

Actual was LOOSE_PACKED.

> +	public void testReadSymRefToLoosePacked() throws IOException {
> +		assertEquals(Ref.Storage.LOOSE, ref.getStorage());

Actual was LOOSE_PACKED.

> +	public void testDeleteLoosePacked() throws IOException {
> +		ObjectId pid = db.resolve("refs/heads/master^");
> +		RefUpdate updateRef = db.updateRef("refs/heads/master");
> +		updateRef.setNewObjectId(pid);
> +		updateRef.setForceUpdate(true);
> +		Result update = updateRef.update();
> +		assertEquals(Result.FORCED, update); // internal
> +
> +		RefUpdate updateRef2 = db.updateRef("refs/heads/master");
> +		Result delete = updateRef2.delete();
> +		assertEquals(Result.FORCED, delete);

Actual was Result.REJECTED_CURRENT_BRANCH.

> @@ -349,12 +355,20 @@ private synchronized Ref readRefBasic(final String origName,
> +
> +		if (!origName.equals(name)) {
> +			ref = new Ref(Ref.Storage.LOOSE, origName, name, id);
> +			looseRefs.put(origName, ref);
> +		}

Seems fine, but I'm starting to hate our current way of handling
symrefs.  Not for this series.  But its starting to worry me.

-- 
Shawn.

Re: [EGIT PATCH 2/6] Add ref rename support to JGit

[Shawn O. Pearce]

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> Now refs can be renamed. The intent is that should be safe. Only the named
> refs and associated logs are updated. Any symbolic refs referring to the renames
> branches are unaffected, except HEAD.
...
> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java
> index a077051..2efa12d 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java
> @@ -58,6 +58,24 @@ static void append(final RefUpdate u, final String msg) throws IOException {
>  		appendOneRecord(oldId, newId, ident, msg, db, u.getName());
>  	}
>  
> +	static void renameTo(final Repository db, final RefUpdate from,
> +			final RefUpdate to) throws IOException {
> +		final File logdir = new File(db.getDirectory(), Constants.LOGS);
> +		final File reflogFrom = new File(logdir, from.getName());
> +		if (!reflogFrom.exists())
> +			return;
> +		final File reflogTo = new File(logdir, to.getName());
> +		final File refdirTo = reflogTo.getParentFile();
> +		if (!refdirTo.exists() && !refdirTo.mkdirs()) {
> +			throw new IOException("Cannot create directory " + refdirTo);
> +		}
> +		if (!reflogFrom.renameTo(reflogTo)) {

What happens if from = "refs/heads/a" and to = "refs/heads/a/b" ?

> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefRename.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefRename.java
> new file mode 100644
> index 0000000..4ea9cfa
> --- /dev/null
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefRename.java
> @@ -0,0 +1,101 @@
> +package org.spearce.jgit.lib;

Copyright header?

> +
> +import java.io.File;
> +import java.io.IOException;
> +import java.util.ArrayList;
> +import java.util.List;
> +
> +import org.spearce.jgit.lib.RefUpdate.DeleteStore;
> +import org.spearce.jgit.lib.RefUpdate.Result;
> +import org.spearce.jgit.lib.RefUpdate.UpdateStore;
> +
> +/**
> + * A RefUpdate combination for renaming a ref
> + */
> +public class RefRename {
> +	private RefUpdate newToUpdate;
> +
> +	private RefUpdate oldFromDelete;
> +
> +	private Result renameResult;
> +
> +	RefRename(final RefUpdate toUpdate, final RefUpdate fromUpdate) {
> +		newToUpdate = toUpdate;
> +		oldFromDelete = fromUpdate;
> +	}
> +
> +	/**
> +	 * @return result of rename operation
> +	 */
> +	public Result getResult() {
> +		return renameResult;
> +	}
> +
> +	/**
> +	 * @return the result of the new ref update
> +	 * @throws IOException
> +	 */
> +	public Result rename() throws IOException {
> +		List<LockFile> lockFiles = new ArrayList<LockFile>();
> +		LockFile lockFileFrom = new LockFile(oldFromDelete.looseFile);
> +		LockFile lockFileTo = new LockFile(newToUpdate.looseFile);
> +		LockFile lockFileHEAD = new LockFile(new File(oldFromDelete.db
> +				.getRepository().getDirectory(), Constants.HEAD));
> +		lockFiles.add(lockFileTo);
> +		lockFiles.add(lockFileFrom);
> +		lockFiles.add(lockFileHEAD);
> +		try {
> +			for (LockFile l : lockFiles) {
> +				if (!l.lock()) {
> +					unlock(lockFiles);
> +					return Result.LOCK_FAILURE;
> +				}
> +			}
> +		} catch (RuntimeException e) {
> +			unlock(lockFiles);
> +			throw e;
> +		} catch (IOException e) {
> +			unlock(lockFiles);
> +			throw e;
> +		}
> +		boolean renameHEADtoo = oldFromDelete.db.readRef(Constants.HEAD)
> +				.getName().equals(oldFromDelete.getName());
> +		try {
> +			UpdateStore toStore = newToUpdate.newUpdateStore();
> +			RefLogWriter.renameTo(oldFromDelete.getRepository(), oldFromDelete,
> +					newToUpdate);
> +			newToUpdate.setNewObjectId(oldFromDelete.getOldObjectId());
> +			newToUpdate.setExpectedOldObjectId(oldFromDelete.getOldObjectId());

The dst ref can't expect its current value to be the src ref's
current value, the dst ref shouldn't exist, right?  So I'm not sure
how your code is working with this particular assignment occurring
in it.  Shouldn't the update store be failing over the compare on
expected old id?

I think we always want

  newToUpdate.setExpectedOldObjectId(ObjectId.zeroId())

as the JavaDoc for that method says zeroId should be used if the
ref isn't supposed to exist, which should be the case for the
destination side of a rename.

Ooooh.  I see.  Here UpdateStore is bypassing the CAS check, as
it runs inside of the RefUpdate.update() method, before the Store
instance is constructed.

Post taking the LockFile successfully we need to validate that

  ObjectId.zeroId().equals(db.idOf(newToUpdate.getName()))

is still true, otherwise it means someone else created the
destination ref and we're about to overwrite it.

> +			newToUpdate.setRefLogMessage("jgit branch: renamed "
> +					+ oldFromDelete.getName() + " to "
> +					+ oldFromDelete.getName(), false);

Typo, you meant newToUpdate here in the 2nd case.

-- 
Shawn.

Re: [EGIT PATCH 1/6] Make sure we get the right storage for loose/pack/loose and packed refs

[Robin Rosenberg]

onsdag 20 maj 2009 23:43:59 skrev "Shawn O. Pearce" <spearce@spearce.org>:
> Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> > Also adds a few some more test for refs, though not complete.
> 
> Hmm, tests fail, wrong expected values?

Gah, not sure how that passed. It's not random failures...

> Actual was Result.REJECTED_CURRENT_BRANCH.
> 
> > @@ -349,12 +355,20 @@ private synchronized Ref readRefBasic(final String origName,
> > +
> > +		if (!origName.equals(name)) {
> > +			ref = new Ref(Ref.Storage.LOOSE, origName, name, id);
> > +			looseRefs.put(origName, ref);
> > +		}
> 
> Seems fine, but I'm starting to hate our current way of handling
> symrefs.  Not for this series.  But its starting to worry me.

I agree. For decorating the history it's convenient, but it's inconsistent. For some reason
it was harder to see without the unit tests. All kinds of things slip by when testing is
done on the surface only.

I'll update, and and even more tests. 

Should we use multiple Ref objects for symrefs? I.e. a Ref referring to another in a chain,
with all symrefs in between visible?

-- robin

Re: [EGIT PATCH 1/6] Make sure we get the right storage for loose/pack/loose and packed refs

[Shawn O. Pearce]

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> onsdag 20 maj 2009 23:43:59 skrev "Shawn O. Pearce" <spearce@spearce.org>:
> 
> I agree. For decorating the history it's convenient, but it's inconsistent. For some reason
> it was harder to see without the unit tests. All kinds of things slip by when testing is
> done on the surface only.
> 
> I'll update, and and even more tests. 

Thanks.
 
> Should we use multiple Ref objects for symrefs? I.e. a Ref referring to another in a chain,
> with all symrefs in between visible?

Yea, I'm starting to think that we should expose a symref as a Ref that
contains a nested Ref.  The getObjectId() / getPeeledObjectId() of any
of those Refs in that chain should return the leaf node value, but the
name of each Ref should be the name of the symref.

So readRef(HEAD) yields:

  Ref {
    name = "HEAD"
    next = Ref {
      name = "refs/heads/master"
      next = null
      objectId = ....
    }
  }

That may break some callers though, as they'd now get HEAD instead
of master as the name of the return value.

-- 
Shawn.

[EGIT PATCH 00/10] Rename support

[Robin Rosenberg]

This is basically a rewrite. While adding tests to handle some of
the case mentioned in Shawns comments, I had to give up the locking
strategy and go for something more like git, i.e. rename proceeds
in small steps and the I try to undo if a problem is encountered.

Add to that some small enchancments to ref support. Not the big
change we discussed earlier.

The branch command is touched by one of the patches, but it does
not include rename.

-- robin

Robin Rosenberg (10):
  Make sure we get the right storage for loose/pack/loose and packed
    refs
  Add a toString for debugging to RemoteRefUpdate
  Add a toString to LockFile
  Add more tests for RefUpdate
  Do not write to the reflog unless the refupdate logmessage is set
  Add a utility method for shortening long ref names to short ones.
  Set a nice reflog message in the branch command
  Add ref rename support to JGit
  Add ref rename support to the branch dialog
  Improve error reporting in the branch dialog

 .../src/org/spearce/egit/ui/UIText.java            |   18 +
 .../ui/internal/dialogs/BranchSelectionDialog.java |  133 +++++--
 .../src/org/spearce/egit/ui/uitext.properties      |    7 +
 .../src/org/spearce/jgit/pgm/Branch.java           |   15 +-
 .../org/spearce/jgit/test/resources/packed-refs    |    1 +
 .../tst/org/spearce/jgit/lib/RefTest.java          |   75 +++
 .../tst/org/spearce/jgit/lib/RefUpdateTest.java    |  483 ++++++++++++++++++--
 .../org/spearce/jgit/transport/TransportTest.java  |    2 +-
 .../src/org/spearce/jgit/lib/LockFile.java         |    5 +
 .../src/org/spearce/jgit/lib/RefDatabase.java      |   60 +++-
 .../src/org/spearce/jgit/lib/RefLogWriter.java     |   34 ++-
 .../src/org/spearce/jgit/lib/RefRename.java        |  163 +++++++
 .../src/org/spearce/jgit/lib/RefUpdate.java        |   65 ++-
 .../src/org/spearce/jgit/lib/Repository.java       |   31 ++
 .../spearce/jgit/transport/RemoteRefUpdate.java    |   13 +-
 15 files changed, 1015 insertions(+), 90 deletions(-)
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/RefRename.java

[EGIT PATCH 01/10] Make sure we get the right storage for loose/pack/loose and packed refs

[Robin Rosenberg]

Also adds a few some more test for refs, though not complete.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../tst/org/spearce/jgit/lib/RefTest.java          |   75 ++++++++++++++++++++
 .../src/org/spearce/jgit/lib/RefDatabase.java      |   26 +++++--
 2 files changed, 95 insertions(+), 6 deletions(-)

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefTest.java
index cae5143..440686d 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefTest.java
@@ -36,8 +36,14 @@
  */
 package org.spearce.jgit.lib;
 
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
 import java.util.Map;
 
+import org.spearce.jgit.lib.Ref.Storage;
+import org.spearce.jgit.lib.RefUpdate.Result;
+
 /**
  * Misc tests for refs. A lot of things are tested elsewhere so not having a
  * test for a ref related method, does not mean it is untested.
@@ -69,4 +75,73 @@ public void testReadAllIncludingSymrefs() throws Exception {
 		assertFalse(refmaster.isPeeled());
 		assertNull(refmaster.getPeeledObjectId());
 	}
+
+	public void testReadSymRefToPacked() throws IOException {
+		db.writeSymref("HEAD", "refs/heads/b");
+		Ref ref = db.getRef("HEAD");
+		assertEquals(Ref.Storage.LOOSE_PACKED, ref.getStorage());
+	}
+
+	public void testReadSymRefToLoosePacked() throws IOException {
+		ObjectId pid = db.resolve("refs/heads/master^");
+		RefUpdate updateRef = db.updateRef("refs/heads/master");
+		updateRef.setNewObjectId(pid);
+		updateRef.setForceUpdate(true);
+		Result update = updateRef.update();
+		assertEquals(Result.FORCED, update); // internal
+
+		db.writeSymref("HEAD", "refs/heads/master");
+		Ref ref = db.getRef("HEAD");
+		assertEquals(Ref.Storage.LOOSE_PACKED, ref.getStorage());
+	}
+
+	public void testReadLooseRef() throws IOException {
+		RefUpdate updateRef = db.updateRef("ref/heads/new");
+		updateRef.setNewObjectId(db.resolve("refs/heads/master"));
+		Result update = updateRef.update();
+		assertEquals(Result.NEW, update);
+		Ref ref = db.getRef("ref/heads/new");
+		assertEquals(Storage.LOOSE, ref.getStorage());
+	}
+
+	/**
+	 * Let an "outsider" create a loose ref with the same name as a packed one
+	 *
+	 * @throws IOException
+	 * @throws InterruptedException
+	 */
+	public void testReadLoosePackedRef() throws IOException,
+			InterruptedException {
+		Ref ref = db.getRef("refs/heads/master");
+		assertEquals(Storage.PACKED, ref.getStorage());
+		FileOutputStream os = new FileOutputStream(new File(db.getDirectory(),
+				"refs/heads/master"));
+		os.write(ref.getObjectId().name().getBytes());
+		os.write('\n');
+		os.close();
+
+		ref = db.getRef("refs/heads/master");
+		assertEquals(Storage.LOOSE_PACKED, ref.getStorage());
+	}
+
+	/**
+	 * Modify a packed ref using the API. This creates a loose ref too, ie.
+	 * LOOSE_PACKED
+	 *
+	 * @throws IOException
+	 */
+	public void testReadSimplePackedRefSameRepo() throws IOException {
+		Ref ref = db.getRef("refs/heads/master");
+		ObjectId pid = db.resolve("refs/heads/master^");
+		assertEquals(Storage.PACKED, ref.getStorage());
+		RefUpdate updateRef = db.updateRef("refs/heads/master");
+		updateRef.setNewObjectId(pid);
+		updateRef.setForceUpdate(true);
+		Result update = updateRef.update();
+		assertEquals(Result.FORCED, update);
+
+		ref = db.getRef("refs/heads/master");
+		assertEquals(Storage.LOOSE_PACKED, ref.getStorage());
+	}
+
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
index 8a47412..869d012 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
@@ -283,8 +283,14 @@ private synchronized Ref readRefBasic(final String origName,
 		final long mtime = loose.lastModified();
 		if (ref != null) {
 			Long cachedlastModified = looseRefsMTime.get(name);
-			if (cachedlastModified != null && cachedlastModified == mtime)
-				return ref;
+			if (cachedlastModified != null && cachedlastModified == mtime) {
+				if (packedRefs.containsKey(origName))
+					return new Ref(Storage.LOOSE_PACKED, origName, ref
+							.getObjectId(), ref.getPeeledObjectId(), ref
+							.isPeeled());
+				else
+					return ref;
+			}
 			looseRefs.remove(origName);
 			looseRefsMTime.remove(origName);
 		}
@@ -349,12 +355,20 @@ private synchronized Ref readRefBasic(final String origName,
 			throw new IOException("Not a ref: " + name + ": " + line);
 		}
 
-		ref = new Ref(Ref.Storage.LOOSE, origName, name, id);
-
-		looseRefs.put(origName, ref);
-		ref = new Ref(Ref.Storage.LOOSE, origName, id);
+		Storage storage;
+		if (packedRefs.containsKey(name))
+			storage = Ref.Storage.LOOSE_PACKED;
+		else
+			storage = Ref.Storage.LOOSE;
+		ref = new Ref(storage, name, id);
 		looseRefs.put(name, ref);
 		looseRefsMTime.put(name, mtime);
+
+		if (!origName.equals(name)) {
+			ref = new Ref(Ref.Storage.LOOSE, origName, name, id);
+			looseRefs.put(origName, ref);
+		}
+
 		return ref;
 	}
 
-- 
1.6.3.dirty

[EGIT PATCH 02/10] Add a toString for debugging to RemoteRefUpdate

[Robin Rosenberg]

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>

---
 .../spearce/jgit/transport/RemoteRefUpdate.java    |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteRefUpdate.java b/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteRefUpdate.java
index 9b054eb..8af7d50 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteRefUpdate.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteRefUpdate.java
@@ -299,7 +299,8 @@ public Status getStatus() {
 
 	/**
 	 * Check whether update was fast-forward. Note that this result is
-	 * meaningful only after successful update (when status is {@link Status#OK}).
+	 * meaningful only after successful update (when status is {@link Status#OK}
+	 * ).
 	 *
 	 * @return true if update was fast-forward; false otherwise.
 	 */
@@ -341,4 +342,14 @@ protected void updateTrackingRef(final RevWalk walk) throws IOException {
 		else
 			trackingRefUpdate.update(walk);
 	}
+
+	@Override
+	public String toString() {
+		return "RemoteRefUpdate[remoteName=" + remoteName + ", " + status
+				+ ", " + (expectedOldObjectId!=null?expectedOldObjectId.abbreviate(localDb).name() :"(null)")
+				+ "..." + (newObjectId != null ? newObjectId.abbreviate(localDb).name() : "(null)")
+				+ (fastForward ? ", fastForward" : "")
+				+ ", srcRef=" + srcRef + (forceUpdate ? ", forceUpdate" : "") + ", message=" + (message != null ? "\""
+				+ message + "\"" : "null") + ", " + localDb.getDirectory() + "]";
+	}
 }
-- 
1.6.3.dirty

[EGIT PATCH 03/10] Add a toString to LockFile

[Robin Rosenberg]

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../src/org/spearce/jgit/lib/LockFile.java         |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/LockFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/LockFile.java
index 9f19efe..abe8b0d 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/LockFile.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/LockFile.java
@@ -397,4 +397,9 @@ public void unlock() {
 			lck.delete();
 		}
 	}
+
+	@Override
+	public String toString() {
+		return "LockFile[" + lck + ", haveLck=" + haveLck + "]";
+	}
 }
-- 
1.6.3.dirty

[EGIT PATCH 04/10] Add more tests for RefUpdate

[Robin Rosenberg]

Also checks more in one of the old tests

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../tst/org/spearce/jgit/lib/RefUpdateTest.java    |  189 ++++++++++++++++----
 1 files changed, 156 insertions(+), 33 deletions(-)

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
index 55d7441..6b1975a 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
@@ -72,7 +72,8 @@ public void testNoCacheObjectIdSubclass() throws IOException {
 			// empty
 		};
 		ru.setNewObjectId(newid);
-		ru.update();
+		Result update = ru.update();
+		assertEquals(Result.NEW, update);
 		final Ref r = db.getAllRefs().get(newRef);
 		assertNotNull(r);
 		assertEquals(newRef, r.getName());
@@ -82,6 +83,25 @@ public void testNoCacheObjectIdSubclass() throws IOException {
 		assertEquals(newid.copy(), r.getObjectId());
 	}
 
+	/**
+	 * Delete a ref that is pointed to by HEAD
+	 *
+	 * @throws IOException
+	 */
+	public void testDeleteHEADreferencedRef() throws IOException {
+		ObjectId pid = db.resolve("refs/heads/master^");
+		RefUpdate updateRef = db.updateRef("refs/heads/master");
+		updateRef.setNewObjectId(pid);
+		updateRef.setForceUpdate(true);
+		Result update = updateRef.update();
+		assertEquals(Result.FORCED, update); // internal
+
+		RefUpdate updateRef2 = db.updateRef("refs/heads/master");
+		Result delete = updateRef2.delete();
+		assertEquals(Result.REJECTED_CURRENT_BRANCH, delete);
+		assertEquals(pid, db.resolve("refs/heads/master"));
+	}
+
 	public void testLooseDelete() throws IOException {
 		final String newRef = "refs/heads/abc";
 		RefUpdate ref = updateRef(newRef);
@@ -95,21 +115,32 @@ public void testDeleteHead() throws IOException {
 		delete(ref, Result.REJECTED_CURRENT_BRANCH, true, false);
 	}
 
-	public void testLogDeleted() throws IOException {
-		String refName = "refs/heads/a";
-		final File log = createLog(refName);
-		assertTrue(log.exists());
-		final RefUpdate ref = updateRef(refName);
-		delete(ref, Result.FAST_FORWARD);
-		assertFalse(log.exists());
-	}
+	/**
+	 * Delete a loose ref and make sure the directory in refs is deleted too,
+	 * and the reflog dir too
+	 *
+	 * @throws IOException
+	 */
+	public void testDeleteLooseAndItsDirectory() throws IOException {
+		ObjectId pid = db.resolve("refs/heads/c^");
+		RefUpdate updateRef = db.updateRef("refs/heads/z/c");
+		updateRef.setNewObjectId(pid);
+		updateRef.setForceUpdate(true);
+		Result update = updateRef.update();
+		assertEquals(Result.NEW, update); // internal
+		assertTrue(new File(db.getDirectory(), Constants.R_HEADS + "z")
+				.exists());
+		assertTrue(new File(db.getDirectory(), "logs/refs/heads/z").exists());
 
-	private File createLog(String name) throws IOException {
-		final File log = new File(db.getDirectory(), Constants.LOGS + "/"
-				+ name);
-		log.getParentFile().mkdirs();
-		log.createNewFile();
-		return log;
+		// The real test here
+		RefUpdate updateRef2 = db.updateRef("refs/heads/z/c");
+		updateRef2.setForceUpdate(true);
+		Result delete = updateRef2.delete();
+		assertEquals(Result.FORCED, delete);
+		assertNull(db.resolve("refs/heads/z/c"));
+		assertFalse(new File(db.getDirectory(), Constants.R_HEADS + "z")
+				.exists());
+		assertFalse(new File(db.getDirectory(), "logs/refs/heads/z").exists());
 	}
 
 	public void testDeleteNotFound() throws IOException {
@@ -130,24 +161,6 @@ public void testDeleteForce() throws IOException {
 		delete(ref, Result.FORCED);
 	}
 
-	public void testDeleteEmptyDirs() throws IOException {
-		final String top = "refs/heads/a";
-		final String newRef = top + "/b/c";
-		final String newRef2 = top + "/d";
-		updateRef(newRef).update();
-		updateRef(newRef2).update();
-		delete(updateRef(newRef2), Result.NO_CHANGE);
-		assertExists(true, top);
-		createLog(newRef);
-		delete(updateRef(newRef), Result.NO_CHANGE);
-		assertExists(false, top);
-		assertExists(false, Constants.LOGS + "/" + top);
-	}
-
-	private void assertExists(final boolean expected, final String name) {
-		assertEquals(expected, new File(db.getDirectory(), name).exists());
-	}
-
 	public void testRefKeySameAsOrigName() {
 		Map<String, Ref> allRefs = db.getAllRefs();
 		for (Entry<String, Ref> e : allRefs.entrySet()) {
@@ -155,4 +168,114 @@ public void testRefKeySameAsOrigName() {
 
 		}
 	}
+
+	/**
+	 * Try modify a ref forward, fast forward
+	 *
+	 * @throws IOException
+	 */
+	public void testUpdateRefForward() throws IOException {
+		ObjectId ppid = db.resolve("refs/heads/master^");
+		ObjectId pid = db.resolve("refs/heads/master");
+
+		RefUpdate updateRef = db.updateRef("refs/heads/master");
+		updateRef.setNewObjectId(ppid);
+		updateRef.setForceUpdate(true);
+		Result update = updateRef.update();
+		assertEquals(Result.FORCED, update);
+		assertEquals(ppid, db.resolve("refs/heads/master"));
+
+		// real test
+		RefUpdate updateRef2 = db.updateRef("refs/heads/master");
+		updateRef2.setNewObjectId(pid);
+		Result update2 = updateRef2.update();
+		assertEquals(Result.FAST_FORWARD, update2);
+		assertEquals(pid, db.resolve("refs/heads/master"));
+	}
+
+	/**
+	 * Delete a ref that exists both as packed and loose. Make sure the ref
+	 * cannot be resolved after delete.
+	 *
+	 * @throws IOException
+	 */
+	public void testDeleteLoosePacked() throws IOException {
+		ObjectId pid = db.resolve("refs/heads/c^");
+		RefUpdate updateRef = db.updateRef("refs/heads/c");
+		updateRef.setNewObjectId(pid);
+		updateRef.setForceUpdate(true);
+		Result update = updateRef.update();
+		assertEquals(Result.FORCED, update); // internal
+
+		// The real test here
+		RefUpdate updateRef2 = db.updateRef("refs/heads/c");
+		updateRef2.setForceUpdate(true);
+		Result delete = updateRef2.delete();
+		assertEquals(Result.FORCED, delete);
+		assertNull(db.resolve("refs/heads/c"));
+	}
+
+	/**
+	 * Try modify a ref to same
+	 *
+	 * @throws IOException
+	 */
+	public void testUpdateRefNoChange() throws IOException {
+		ObjectId pid = db.resolve("refs/heads/master");
+		RefUpdate updateRef = db.updateRef("refs/heads/master");
+		updateRef.setNewObjectId(pid);
+		Result update = updateRef.update();
+		assertEquals(Result.NO_CHANGE, update);
+		assertEquals(pid, db.resolve("refs/heads/master"));
+	}
+
+	/**
+	 * Try modify a ref, but get wrong expected old value
+	 *
+	 * @throws IOException
+	 */
+	public void testUpdateRefLockFailureWrongOldValue() throws IOException {
+		ObjectId pid = db.resolve("refs/heads/master");
+		RefUpdate updateRef = db.updateRef("refs/heads/master");
+		updateRef.setNewObjectId(pid);
+		updateRef.setExpectedOldObjectId(db.resolve("refs/heads/master^"));
+		Result update = updateRef.update();
+		assertEquals(Result.LOCK_FAILURE, update);
+		assertEquals(pid, db.resolve("refs/heads/master"));
+	}
+
+	/**
+	 * Try modify a ref that is locked
+	 *
+	 * @throws IOException
+	 */
+	public void testUpdateRefLockFailureLocked() throws IOException {
+		ObjectId opid = db.resolve("refs/heads/master");
+		ObjectId pid = db.resolve("refs/heads/master^");
+		RefUpdate updateRef = db.updateRef("refs/heads/master");
+		updateRef.setNewObjectId(pid);
+		LockFile lockFile1 = new LockFile(new File(db.getDirectory(),"refs/heads/master"));
+		assertTrue(lockFile1.lock()); // precondition to test
+		Result update = updateRef.update();
+		assertEquals(Result.LOCK_FAILURE, update);
+		assertEquals(opid, db.resolve("refs/heads/master"));
+		LockFile lockFile2 = new LockFile(new File(db.getDirectory(),"refs/heads/master"));
+		assertFalse(lockFile2.lock()); // was locked, still is
+	}
+
+	/**
+	 * Try to delete a ref. Delete requires force.
+	 *
+	 * @throws IOException
+	 */
+	public void testDeleteLoosePackedRejected() throws IOException {
+		ObjectId pid = db.resolve("refs/heads/c^");
+		ObjectId oldpid = db.resolve("refs/heads/c");
+		RefUpdate updateRef = db.updateRef("refs/heads/c");
+		updateRef.setNewObjectId(pid);
+		Result update = updateRef.update();
+		assertEquals(Result.REJECTED, update);
+		assertEquals(oldpid, db.resolve("refs/heads/c"));
+	}
+
 }
-- 
1.6.3.dirty

[EGIT PATCH 05/10] Do not write to the reflog unless the refupdate logmessage is set

[Robin Rosenberg]

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../tst/org/spearce/jgit/lib/RefUpdateTest.java    |    1 +
 .../src/org/spearce/jgit/lib/RefUpdate.java        |    3 ++-
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
index 6b1975a..84653c8 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
@@ -126,6 +126,7 @@ public void testDeleteLooseAndItsDirectory() throws IOException {
 		RefUpdate updateRef = db.updateRef("refs/heads/z/c");
 		updateRef.setNewObjectId(pid);
 		updateRef.setForceUpdate(true);
+		updateRef.setRefLogMessage("new test ref", false);
 		Result update = updateRef.update();
 		assertEquals(Result.NEW, update); // internal
 		assertTrue(new File(db.getDirectory(), Constants.R_HEADS + "z")
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
index a9ab73b..79d9f2d 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
@@ -479,7 +479,8 @@ else if (status == Result.FAST_FORWARD)
 			else if (status == Result.NEW)
 				msg += ": created";
 		}
-		RefLogWriter.append(this, msg);
+		if (msg != null)
+			RefLogWriter.append(this, msg);
 		if (!lock.commit())
 			return Result.LOCK_FAILURE;
 		db.stored(this.ref.getOrigName(),  ref.getName(), newValue, lock.getCommitLastModified());
-- 
1.6.3.dirty

[EGIT PATCH 06/10] Add a utility method for shortening long ref names to short ones.

[Robin Rosenberg]

refs/heads/x -> x etc.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../src/org/spearce/jgit/lib/Repository.java       |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
index 9bed1b7..55dae27 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
@@ -1074,4 +1074,19 @@ public void scanForRepoChanges() throws IOException {
 		getAllRefs(); // This will look for changes to refs
 		getIndex(); // This will detect changes in the index
 	}
+
+	/**
+	 * @param refName
+	 * 
+	 * @return a more user friendly ref name
+	 */
+	public String shortenRefName(String refName) {
+		if (refName.startsWith(Constants.R_HEADS))
+			return refName.substring(Constants.R_HEADS.length());
+		if (refName.startsWith(Constants.R_TAGS))
+			return refName.substring(Constants.R_TAGS.length());
+		if (refName.startsWith(Constants.R_REMOTES))
+			return refName.substring(Constants.R_REMOTES.length());
+		return refName;
+	}
 }
-- 
1.6.3.dirty

[EGIT PATCH 07/10] Set a nice reflog message in the branch command

[Robin Rosenberg]

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../src/org/spearce/jgit/pgm/Branch.java           |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Branch.java b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Branch.java
index 11002f0..dbe297e 100644
--- a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Branch.java
+++ b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Branch.java
@@ -98,12 +98,18 @@ protected void run() throws Exception {
 
 			if (branches.size() > 0) {
 				String newHead = branches.get(0);
-				ObjectId startAt;
+				String startBranch;
 				if (branches.size() == 2)
-					startAt = db.resolve(branches.get(1) + "^0");
+					startBranch = branches.get(1);
 				else
-					startAt = db.resolve(Constants.HEAD + "^0");
-
+					startBranch = Constants.HEAD;
+				Ref startRef = db.getRef(startBranch);
+				ObjectId startAt = db.resolve(startBranch + "^0");
+				if (startRef != null)
+					startBranch = startRef.getName();
+				else
+					startBranch = startAt.name();
+				startBranch = db.shortenRefName(startBranch);
 				String newRefName = newHead;
 				if (!newRefName.startsWith(Constants.R_HEADS))
 					newRefName = Constants.R_HEADS + newRefName;
@@ -114,6 +120,7 @@ protected void run() throws Exception {
 				RefUpdate updateRef = db.updateRef(newRefName);
 				updateRef.setNewObjectId(startAt);
 				updateRef.setForceUpdate(createForce);
+				updateRef.setRefLogMessage("branch: Created from " + startBranch, false);
 				Result update = updateRef.update();
 				if (update == Result.REJECTED)
 					throw die(String.format("Could not create branch %s: %s", newHead, update.toString()));
-- 
1.6.3.dirty

[EGIT PATCH 08/10] Add ref rename support to JGit

[Robin Rosenberg]

Now refs can be renamed. The intent is that should be safe. Only the named
refs and associated logs are updated. Any symbolic refs referring to the renames
branches are unaffected, except HEAD, which is updated if the branch it refers
to is being renamed.

In order to perform operations like rename from a to a/b and vice verse, we
perform the rename similar to the way C git does it, i.e. one ref update at
a time using the same temporary intermediate names as the C version does.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../org/spearce/jgit/test/resources/packed-refs    |    1 +
 .../tst/org/spearce/jgit/lib/RefUpdateTest.java    |  293 ++++++++++++++++++++
 .../org/spearce/jgit/transport/TransportTest.java  |    2 +-
 .../src/org/spearce/jgit/lib/RefDatabase.java      |   34 +++
 .../src/org/spearce/jgit/lib/RefLogWriter.java     |   34 +++-
 .../src/org/spearce/jgit/lib/RefRename.java        |  163 +++++++++++
 .../src/org/spearce/jgit/lib/RefUpdate.java        |   62 +++-
 .../src/org/spearce/jgit/lib/Repository.java       |   16 +
 8 files changed, 588 insertions(+), 17 deletions(-)
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/RefRename.java

diff --git a/org.spearce.jgit.test/tst-rsrc/org/spearce/jgit/test/resources/packed-refs b/org.spearce.jgit.test/tst-rsrc/org/spearce/jgit/test/resources/packed-refs
index 38a70ec..a6a14f2 100644
--- a/org.spearce.jgit.test/tst-rsrc/org/spearce/jgit/test/resources/packed-refs
+++ b/org.spearce.jgit.test/tst-rsrc/org/spearce/jgit/test/resources/packed-refs
@@ -6,6 +6,7 @@ f73b95671f326616d66b2afb3bdfcdbbce110b44 refs/heads/d
 d0114ab8ac326bab30e3a657a0397578c5a1af88 refs/heads/e
 47d3697c3747e8184e0dc479ccbd01e359023577 refs/heads/f
 175d5b80bd9768884d8fced02e9bd33488174396 refs/heads/g
+175d5b80bd9768884d8fced02e9bd33488174396 refs/heads/prefix/a
 49322bb17d3acc9146f98c97d078513228bbf3c0 refs/heads/master
 d86a2aada2f5e7ccf6f11880bfb9ab404e8a8864 refs/heads/pa
 6db9c2ebf75590eef973081736730a9ea169a0c4 refs/tags/A
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
index 84653c8..8c6ca0c 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
@@ -83,6 +83,68 @@ public void testNoCacheObjectIdSubclass() throws IOException {
 		assertEquals(newid.copy(), r.getObjectId());
 	}
 
+	public void testNewNamespaceConflictWithLoosePrefixNameExists() throws IOException {
+		final String newRef = "refs/heads/z";
+		final RefUpdate ru = updateRef(newRef);
+		final RevCommit newid = new RevCommit(ru.getNewObjectId()) {
+			// empty
+		};
+		ru.setNewObjectId(newid);
+		Result update = ru.update();
+		assertEquals(Result.NEW, update);
+		// end setup
+		final String newRef2 = "refs/heads/z/a";
+		final RefUpdate ru2 = updateRef(newRef2);
+		final RevCommit newid2 = new RevCommit(ru2.getNewObjectId()) {
+			// empty
+		};
+		ru.setNewObjectId(newid2);
+		Result update2 = ru2.update();
+		assertEquals(Result.LOCK_FAILURE, update2);
+	}
+
+	public void testNewNamespaceConflictWithPackedPrefixNameExists() throws IOException {
+		final String newRef = "refs/heads/master/x";
+		final RefUpdate ru = updateRef(newRef);
+		final RevCommit newid = new RevCommit(ru.getNewObjectId()) {
+			// empty
+		};
+		ru.setNewObjectId(newid);
+		Result update = ru.update();
+		assertEquals(Result.LOCK_FAILURE, update);
+	}
+
+	public void testNewNamespaceConflictWithLoosePrefixOfExisting() throws IOException {
+		final String newRef = "refs/heads/z/a";
+		final RefUpdate ru = updateRef(newRef);
+		final RevCommit newid = new RevCommit(ru.getNewObjectId()) {
+			// empty
+		};
+		ru.setNewObjectId(newid);
+		Result update = ru.update();
+		assertEquals(Result.NEW, update);
+		// end setup
+		final String newRef2 = "refs/heads/z";
+		final RefUpdate ru2 = updateRef(newRef2);
+		final RevCommit newid2 = new RevCommit(ru2.getNewObjectId()) {
+			// empty
+		};
+		ru.setNewObjectId(newid2);
+		Result update2 = ru2.update();
+		assertEquals(Result.LOCK_FAILURE, update2);
+	}
+
+	public void testNewNamespaceConflictWithPackedPrefixOfExisting() throws IOException {
+		final String newRef = "refs/heads/prefix";
+		final RefUpdate ru = updateRef(newRef);
+		final RevCommit newid = new RevCommit(ru.getNewObjectId()) {
+			// empty
+		};
+		ru.setNewObjectId(newid);
+		Result update = ru.update();
+		assertEquals(Result.LOCK_FAILURE, update);
+	}
+
 	/**
 	 * Delete a ref that is pointed to by HEAD
 	 *
@@ -279,4 +341,235 @@ public void testDeleteLoosePackedRejected() throws IOException {
 		assertEquals(oldpid, db.resolve("refs/heads/c"));
 	}
 
+	public void testRenameBranchNoPreviousLog() throws IOException {
+		assertFalse("precondition, no log on old branchg", new File(db
+				.getDirectory(), "logs/refs/heads/b").exists());
+		ObjectId rb = db.resolve("refs/heads/b");
+		ObjectId oldHead = db.resolve(Constants.HEAD);
+		assertFalse(rb.equals(oldHead)); // assumption for this test
+		RefRename renameRef = db.renameRef("refs/heads/b",
+				"refs/heads/new/name");
+		Result result = renameRef.rename();
+		assertEquals(Result.RENAMED, result);
+		assertEquals(rb, db.resolve("refs/heads/new/name"));
+		assertNull(db.resolve("refs/heads/b"));
+		assertTrue(new File(db.getDirectory(), "logs/refs/heads/new/name")
+				.exists());
+		assertFalse(new File(db.getDirectory(), "logs/refs/heads/b").exists());
+		assertEquals(oldHead, db.resolve(Constants.HEAD));
+	}
+
+	public void testRenameBranchHasPreviousLog() throws IOException {
+		ObjectId rb = db.resolve("refs/heads/b");
+		ObjectId oldHead = db.resolve(Constants.HEAD);
+		assertFalse("precondition for this test, branch b != HEAD", rb
+				.equals(oldHead));
+		RefLogWriter.writeReflog(db, rb, rb, "Just a message", "refs/heads/b");
+		assertTrue("no log on old branch", new File(db.getDirectory(),
+				"logs/refs/heads/b").exists());
+		RefRename renameRef = db.renameRef("refs/heads/b",
+				"refs/heads/new/name");
+		Result result = renameRef.rename();
+		assertEquals(Result.RENAMED, result);
+		assertEquals(rb, db.resolve("refs/heads/new/name"));
+		assertNull(db.resolve("refs/heads/b"));
+		assertTrue(new File(db.getDirectory(), "logs/refs/heads/new/name")
+				.exists());
+		assertFalse(new File(db.getDirectory(), "logs/refs/heads/b").exists());
+		assertEquals(oldHead, db.resolve(Constants.HEAD));
+		// TODO: test content of log file
+	}
+
+	public void testRenameCurrentBranch() throws IOException {
+		ObjectId rb = db.resolve("refs/heads/b");
+		db.writeSymref(Constants.HEAD, "refs/heads/b");
+		ObjectId oldHead = db.resolve(Constants.HEAD);
+		assertTrue("internal test condition, b == HEAD", rb.equals(oldHead));
+		RefLogWriter.writeReflog(db, rb, rb, "Just a message", "refs/heads/b");
+		assertTrue("no log on old branch", new File(db.getDirectory(),
+				"logs/refs/heads/b").exists());
+		RefRename renameRef = db.renameRef("refs/heads/b",
+				"refs/heads/new/name");
+		Result result = renameRef.rename();
+		assertEquals(Result.RENAMED, result);
+		assertEquals(rb, db.resolve("refs/heads/new/name"));
+		assertNull(db.resolve("refs/heads/b"));
+		assertTrue(new File(db.getDirectory(), "logs/refs/heads/new/name")
+				.exists());
+		assertFalse(new File(db.getDirectory(), "logs/refs/heads/b").exists());
+		assertEquals(rb, db.resolve(Constants.HEAD));
+		// TODO: test content of log file
+	}
+
+	public void testRenameBranchAlsoInPack() throws IOException {
+		ObjectId rb = db.resolve("refs/heads/b");
+		ObjectId rb2 = db.resolve("refs/heads/b~1");
+		assertEquals(Ref.Storage.PACKED, db.getRef("refs/heads/b").getStorage());
+		RefUpdate updateRef = db.updateRef("refs/heads/b");
+		updateRef.setNewObjectId(rb2);
+		updateRef.setForceUpdate(true);
+		Result update = updateRef.update();
+		assertEquals("internal check new ref is loose", Result.FORCED, update);
+		assertEquals(Ref.Storage.LOOSE_PACKED, db.getRef("refs/heads/b").getStorage());
+		RefLogWriter.writeReflog(db, rb, rb, "Just a message", "refs/heads/b");
+		assertTrue("no log on old branch", new File(db.getDirectory(),
+				"logs/refs/heads/b").exists());
+		RefRename renameRef = db.renameRef("refs/heads/b",
+				"refs/heads/new/name");
+		Result result = renameRef.rename();
+		assertEquals(Result.RENAMED, result);
+		assertEquals(rb2, db.resolve("refs/heads/new/name"));
+		assertNull(db.resolve("refs/heads/b"));
+		assertTrue(new File(db.getDirectory(), "logs/refs/heads/new/name")
+				.exists());
+		assertFalse(new File(db.getDirectory(), "logs/refs/heads/b").exists());
+
+		// Create new Repository instance, to reread caches and make sure our
+		// assumptions are persistent.
+		Repository ndb = new Repository(db.getDirectory());
+		assertEquals(rb2, ndb.resolve("refs/heads/new/name"));
+		assertNull(ndb.resolve("refs/heads/b"));
+	}
+
+	public void tryRenameWhenLocked(String toLock, String fromName,
+			String toName, String headPointsTo) throws IOException {
+		// setup
+		db.writeSymref(Constants.HEAD, headPointsTo);
+		ObjectId oldfromId = db.resolve(fromName);
+		ObjectId oldHeadId = db.resolve(Constants.HEAD);
+
+		RefLogWriter.writeReflog(db, oldfromId, oldfromId, "Just a message", fromName);
+		assertTrue("internal check, we have a log", new File(db.getDirectory(),
+				"logs/" + fromName).exists());
+
+		// "someone" has branch X locked
+		assertTrue(new LockFile(new File(db.getDirectory(), toLock)).lock());
+
+		// Now this is our test
+		RefRename renameRef = db.renameRef(fromName, toName);
+		Result result = renameRef.rename();
+		assertEquals(Result.LOCK_FAILURE, result);
+
+		// Check that the involved refs are sane despite the failure
+		assertExists(false, toName);
+		if (!toLock.equals(toName))
+			assertExists(false, toName + ".lock");
+		assertExists(true, toLock + ".lock");
+		if (!toLock.equals(fromName))
+			assertExists(false, "logs/" + fromName + ".lock");
+		assertExists(false, "logs/" + toName + ".lock");
+		assertEquals(oldHeadId, db.resolve(Constants.HEAD));
+		assertEquals(oldfromId, db.resolve(fromName));
+		assertNull(db.resolve(toName));
+	}
+
+	private void assertExists(boolean positive, String toName) {
+		assertEquals(toName + (positive ? " " : " does not ") + "exist",
+				positive, new File(db.getDirectory(), toName).exists());
+	}
+
+	public void testRenameBranchCannotLockAFileHEADisFromLockHEAD() throws IOException {
+		tryRenameWhenLocked("HEAD", "refs/heads/b", "refs/heads/new/name",
+				"refs/heads/b");
+	}
+	public void testRenameBranchCannotLockAFileHEADisFromLockFrom() throws IOException {
+		tryRenameWhenLocked("refs/heads/b", "refs/heads/b",
+				"refs/heads/new/name", "refs/heads/b");
+	}
+	public void testRenameBranchCannotLockAFileHEADisFromLockTo() throws IOException {
+		tryRenameWhenLocked("refs/heads/new/name", "refs/heads/b",
+				"refs/heads/new/name", "refs/heads/b");
+	}
+	public void testRenameBranchCannotLockAFileHEADisFromLockTmp() throws IOException {
+		tryRenameWhenLocked("RENAMED-REF", "refs/heads/b",
+				"refs/heads/new/name", "refs/heads/b");
+	}
+
+//	public void testRenameBranchCannotLockAFileHEADisToLockHead() throws IOException {
+//		an example following the rename failure pattern, but this one is ok!
+//		tryRenameWhenLocked("HEAD", "refs/heads/b", "refs/heads/new/name",
+//				"refs/heads/new/name");
+//	}
+
+	public void testRenameBranchCannotLockAFileHEADisToLockFrom() throws IOException {
+		tryRenameWhenLocked("refs/heads/b", "refs/heads/b",
+				"refs/heads/new/name", "refs/heads/new/name");
+	}
+
+	public void testRenameBranchCannotLockAFileHEADisToLockTo() throws IOException {
+		tryRenameWhenLocked("refs/heads/new/name", "refs/heads/b",
+				"refs/heads/new/name", "refs/heads/new/name");
+	}
+
+	public void testRenameBranchCannotLockAFileHEADisToLockTmp() throws IOException {
+		tryRenameWhenLocked("RENAMED-REF", "refs/heads/b",
+				"refs/heads/new/name", "refs/heads/new/name");
+	}
+
+//	public void testRenameBranchCannotLockAFileHEADisOtherLockHead() throws IOException {
+//		an example following the rename failure pattern, but this one is ok!
+//		tryRenameWhenLocked("HEAD", "refs/heads/b", "refs/heads/new/name",
+//				"refs/heads/a");
+//	}
+
+	public void testRenameBranchCannotLockAFileHEADisOtherLockFrom() throws IOException {
+		tryRenameWhenLocked("refs/heads/b", "refs/heads/b",
+				"refs/heads/new/name", "refs/heads/a");
+	}
+
+	public void testRenameBranchCannotLockAFileHEADisOtherLockTo() throws IOException {
+		tryRenameWhenLocked("refs/heads/new/name", "refs/heads/b",
+				"refs/heads/new/name", "refs/heads/a");
+	}
+
+	public void testRenameBranchCannotLockAFileHEADisOtherLockTmp() throws IOException {
+		tryRenameWhenLocked("RENAMED-REF", "refs/heads/b",
+				"refs/heads/new/name", "refs/heads/a");
+	}
+
+	public void testRenameRefNameColission1avoided() throws IOException {
+		// setup
+		ObjectId rb = db.resolve("refs/heads/b");
+		db.writeSymref(Constants.HEAD, "refs/heads/a");
+		RefUpdate updateRef = db.updateRef("refs/heads/a");
+		updateRef.setNewObjectId(rb);
+		assertEquals(Result.FAST_FORWARD, updateRef.update());
+		ObjectId oldHead = db.resolve(Constants.HEAD);
+		assertTrue(rb.equals(oldHead)); // assumption for this test
+		RefLogWriter.writeReflog(db, rb, rb, "Just a message", "refs/heads/a");
+		assertTrue("internal check, we have a log", new File(db.getDirectory(),
+				"logs/refs/heads/a").exists());
+
+		// Now this is our test
+		RefRename renameRef = db.renameRef("refs/heads/a",
+				"refs/heads/a/b");
+		Result result = renameRef.rename();
+		assertEquals(Result.RENAMED, result);
+		assertNull(db.resolve("refs/heads/a"));
+		assertEquals(rb, db.resolve("refs/heads/a/b"));
+	}
+
+	public void testRenameRefNameColission2avoided() throws IOException {
+		// setup
+		ObjectId rb = db.resolve("refs/heads/b");
+		db.writeSymref(Constants.HEAD, "refs/heads/prefix/a");
+		RefUpdate updateRef = db.updateRef("refs/heads/prefix/a");
+		updateRef.setNewObjectId(rb);
+		updateRef.setForceUpdate(true);
+		assertEquals(Result.FORCED, updateRef.update());
+		ObjectId oldHead = db.resolve(Constants.HEAD);
+		assertTrue(rb.equals(oldHead)); // assumption for this test
+		RefLogWriter.writeReflog(db, rb, rb, "Just a message", "refs/heads/prefix/a");
+		assertTrue("internal check, we have a log", new File(db.getDirectory(),
+				"logs/refs/heads/prefix/a").exists());
+
+		// Now this is our test
+		RefRename renameRef = db.renameRef("refs/heads/prefix/a",
+				"refs/heads/prefix");
+		Result result = renameRef.rename();
+		assertEquals(Result.RENAMED, result);
+
+		assertNull(db.resolve("refs/heads/prefix/a"));
+		assertEquals(rb, db.resolve("refs/heads/prefix"));
+	}
 }
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/transport/TransportTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/transport/TransportTest.java
index c6e3335..fcf3f5c 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/transport/TransportTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/transport/TransportTest.java
@@ -123,7 +123,7 @@ public void testFindRemoteRefUpdatesWildcardNoTracking() throws IOException {
 				.findRemoteRefUpdatesFor(Collections.nCopies(1, new RefSpec(
 						"+refs/heads/*:refs/heads/test/*")));
 
-		assertEquals(9, result.size());
+		assertEquals(10, result.size());
 		boolean foundA = false;
 		boolean foundB = false;
 		for (final RemoteRefUpdate rru : result) {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
index 869d012..babef0b 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
@@ -146,6 +146,26 @@ synchronized (this) {
 	}
 
 	/**
+	 * An set of update operations for renaming a ref
+	 *
+	 * @param fromRef Old ref name
+	 * @param toRef New ref name
+	 * @return a RefUpdate operation to rename a ref
+	 * @throws IOException
+	 */
+	RefRename newRename(String fromRef, String toRef) throws IOException {
+		refreshPackedRefs();
+		Ref f = readRefBasic(fromRef, 0);
+		Ref t = readRefBasic(toRef, 0);
+		if (t != null)
+			throw new IOException("Ref rename target exists: " + t.getName());
+		t = new Ref(Ref.Storage.NEW, toRef, null);
+		RefUpdate refUpdateFrom = new RefUpdate(this, f, fileForRef(f.getName()));
+		RefUpdate refUpdateTo = new RefUpdate(this, t, fileForRef(t.getName()));
+		return new RefRename(refUpdateTo, refUpdateFrom);
+	}
+
+	/**
 	 * Writes a symref (e.g. HEAD) to disk
 	 * 
 	 * @param name
@@ -158,11 +178,25 @@ void link(final String name, final String target) throws IOException {
 		final byte[] content = Constants.encode("ref: " + target + "\n");
 		lockAndWriteFile(fileForRef(name), content);
 		synchronized (this) {
+			looseSymRefs.remove(name);
 			setModified();
 		}
 		db.fireRefsMaybeChanged();
 	}
 
+	void uncacheSymRef(String name) {
+		synchronized(this) {
+			looseSymRefs.remove(name);
+			setModified();
+		}
+	}
+
+	void uncacheRef(String name) {
+		looseRefs.remove(name);
+		looseRefsMTime.remove(name);
+		packedRefs.remove(name);
+	}
+
 	private void setModified() {
 		lastRefModification = refModificationCounter++;
 	}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java
index a077051..18cf2ed 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java
@@ -44,7 +44,7 @@
 import java.io.IOException;
 
 /**
- * Utility class to add reflog entries
+ * Utility class to work with reflog files
  * 
  * @author Dave Watson
  */
@@ -58,6 +58,38 @@ static void append(final RefUpdate u, final String msg) throws IOException {
 		appendOneRecord(oldId, newId, ident, msg, db, u.getName());
 	}
 
+	static void append(RefRename refRename, String msg) throws IOException {
+		final ObjectId id = refRename.getObjectId();
+		final Repository db = refRename.getRepository();
+		final PersonIdent ident = refRename.getRefLogIdent();
+		appendOneRecord(id, id, ident, msg, db, refRename.getToName());
+	}
+
+	static void renameTo(final Repository db, final RefUpdate from,
+			final RefUpdate to) throws IOException {
+		final File logdir = new File(db.getDirectory(), Constants.LOGS);
+		final File reflogFrom = new File(logdir, from.getName());
+		if (!reflogFrom.exists())
+			return;
+		final File reflogTo = new File(logdir, to.getName());
+		final File reflogToDir = reflogTo.getParentFile();
+		File tmp = new File(logdir,"tmp-renamed-log");
+		tmp.delete(); // if any exists
+		if (!reflogFrom.renameTo(tmp)) {
+			throw new IOException("Cannot rename " + reflogFrom + " to (" + tmp
+					+ ")" + reflogTo);
+		}
+		RefUpdate.deleteEmptyDir(reflogFrom, RefUpdate.count(from.getName(),
+				'/'));
+		if (!reflogToDir.exists() && !reflogToDir.mkdirs()) {
+			throw new IOException("Cannot create directory " + reflogToDir);
+		}
+		if (!tmp.renameTo(reflogTo)) {
+			throw new IOException("Cannot rename (" + tmp + ")" + reflogFrom
+					+ " to " + reflogTo);
+		}
+	}
+
 	private static void appendOneRecord(final ObjectId oldId,
 			final ObjectId newId, PersonIdent ident, final String msg,
 			final Repository db, final String refName) throws IOException {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefRename.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefRename.java
new file mode 100644
index 0000000..589d752
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefRename.java
@@ -0,0 +1,163 @@
+/*
+ * Copyright (C) 2009, Robin Rosenberg
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.spearce.jgit.lib;
+
+import java.io.IOException;
+
+import org.spearce.jgit.lib.RefUpdate.Result;
+
+/**
+ * A RefUpdate combination for renaming a ref
+ */
+public class RefRename {
+	private RefUpdate newToUpdate;
+
+	private RefUpdate oldFromDelete;
+
+	private Result renameResult = Result.NOT_ATTEMPTED;
+
+	RefRename(final RefUpdate toUpdate, final RefUpdate fromUpdate) {
+		newToUpdate = toUpdate;
+		oldFromDelete = fromUpdate;
+	}
+
+	/**
+	 * @return result of rename operation
+	 */
+	public Result getResult() {
+		return renameResult;
+	}
+
+	/**
+	 * @return the result of the new ref update
+	 * @throws IOException
+	 */
+	public Result rename() throws IOException {
+		Ref oldRef = oldFromDelete.db.readRef(Constants.HEAD);
+		boolean renameHEADtoo = oldRef != null
+				&& oldRef.getName().equals(oldFromDelete.getName());
+		try {
+			Repository db = oldFromDelete.getRepository();
+			RefLogWriter.renameTo(db, oldFromDelete,
+					newToUpdate);
+			newToUpdate.setRefLogMessage(null, false);
+			RefUpdate tmpUpdateRef = oldFromDelete.db.getRepository().updateRef(
+					"RENAMED-REF");
+			if (renameHEADtoo) {
+				try {
+					oldFromDelete.db.link(Constants.HEAD, "RENAMED-REF");
+				} catch (IOException e) {
+					RefLogWriter.renameTo(db,
+							newToUpdate, oldFromDelete);
+					return renameResult = Result.LOCK_FAILURE;
+				}
+			}
+			tmpUpdateRef.setNewObjectId(oldFromDelete.getOldObjectId());
+			tmpUpdateRef.setForceUpdate(true);
+			Result update = tmpUpdateRef.update();
+			if (update != Result.FORCED && update != Result.NEW && update != Result.NO_CHANGE) {
+				RefLogWriter.renameTo(db,
+						newToUpdate, oldFromDelete);
+				if (renameHEADtoo) {
+					oldFromDelete.db.link(Constants.HEAD, oldFromDelete.getName());
+				}
+				return renameResult = update;
+			}
+
+			oldFromDelete.setForceUpdate(true);
+			Result delete = oldFromDelete.delete();
+			if (delete != Result.FORCED) {
+				if (db.getRef(
+						oldFromDelete.getName()) != null) {
+					RefLogWriter.renameTo(db,
+							newToUpdate, oldFromDelete);
+					if (renameHEADtoo) {
+						oldFromDelete.db.link(Constants.HEAD, oldFromDelete
+								.getName());
+					}
+				}
+				return renameResult = delete;
+			}
+
+			newToUpdate.setNewObjectId(tmpUpdateRef.getNewObjectId());
+			Result updateResult = newToUpdate.update();
+			if (updateResult != Result.NEW) {
+				RefLogWriter.renameTo(db,
+						newToUpdate, oldFromDelete);
+				if (renameHEADtoo) {
+					oldFromDelete.db.link(Constants.HEAD, oldFromDelete.getName());
+				}
+				oldFromDelete.setNewObjectId(oldFromDelete.getOldObjectId());
+				Result undelete = oldFromDelete.update();
+				if (undelete != Result.NEW)
+					return renameResult = Result.IO_FAILURE;
+				return renameResult = Result.LOCK_FAILURE;
+			}
+
+			if (renameHEADtoo) {
+				oldFromDelete.db.link(Constants.HEAD, newToUpdate.getName());
+			} else {
+				oldFromDelete.db.getRepository().fireRefsMaybeChanged();
+			}
+			RefLogWriter.append(this, "jgit branch: renamed "
+					+ db.shortenRefName(oldFromDelete.getName()) + " to "
+					+ db.shortenRefName(newToUpdate.getName()));
+			return renameResult = Result.RENAMED;
+		} catch (RuntimeException e) {
+			throw e;
+		} catch (IOException e) {
+			System.err.println(e);
+			return renameResult = Result.IO_FAILURE;
+		}
+	}
+
+	ObjectId getObjectId() {
+		return oldFromDelete.getOldObjectId();
+	}
+
+	Repository getRepository() {
+		return oldFromDelete.getRepository();
+	}
+
+	PersonIdent getRefLogIdent() {
+		return newToUpdate.getRefLogIdent();
+	}
+
+	String getToName() {
+		return newToUpdate.getName();
+	}
+}
\ No newline at end of file
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
index 79d9f2d..a2041de 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
@@ -125,14 +125,20 @@
 		 * This kind of error doesn't include {@link #LOCK_FAILURE}, which is a
 		 * different case.
 		 */
-		IO_FAILURE
+		IO_FAILURE,
+
+		/**
+		 * The ref was renamed from another name
+		 * <p>
+		 */
+		RENAMED
 	}
 
 	/** Repository the ref is stored in. */
-	private final RefDatabase db;
+	final RefDatabase db;
 
 	/** Location of the loose file holding the value of this ref. */
-	private final File looseFile;
+	final File looseFile;
 
 	/** New value the caller wants this ref to have. */
 	private ObjectId newValue;
@@ -156,7 +162,7 @@
 	private ObjectId expValue;
 
 	/** Result of the update operation. */
-	private Result result = Result.NOT_ATTEMPTED;
+	Result result = Result.NOT_ATTEMPTED;
 
 	private final Ref ref;
 
@@ -418,6 +424,15 @@ private Result updateImpl(final RevWalk walk, final Store store)
 		RevObject newObj;
 		RevObject oldObj;
 
+		int lastSlash = getName().lastIndexOf('/');
+		if (lastSlash > 0)
+			if (db.getRepository().getRef(getName().substring(0, lastSlash)) != null)
+				return Result.LOCK_FAILURE;
+		String rName = getName() + "/";
+		for (Ref r : db.getAllRefs().values()) {
+			if (r.getName().startsWith(rName))
+				return Result.LOCK_FAILURE;
+		}
 		lock = new LockFile(looseFile);
 		if (!lock.lock())
 			return Result.LOCK_FAILURE;
@@ -478,6 +493,9 @@ else if (status == Result.FAST_FORWARD)
 				msg += ": fast forward";
 			else if (status == Result.NEW)
 				msg += ": created";
+			else if (status == Result.RENAMED) {
+				// don't amend the message here
+			}
 		}
 		if (msg != null)
 			RefLogWriter.append(this, msg);
@@ -496,7 +514,7 @@ abstract Result store(final LockFile lock, final Result status)
 				throws IOException;
 	}
 
-	private class UpdateStore extends Store {
+	class UpdateStore extends Store {
 
 		@Override
 		Result store(final LockFile lock, final Result status)
@@ -505,7 +523,7 @@ Result store(final LockFile lock, final Result status)
 		}
 	}
 
-	private class DeleteStore extends Store {
+	class DeleteStore extends Store {
 
 		@Override
 		Result store(LockFile lock, Result status) throws IOException {
@@ -526,28 +544,42 @@ Result store(LockFile lock, Result status) throws IOException {
 			lock.unlock();
 			if (storage.isLoose())
 				deleteFileAndEmptyDir(looseFile, levels);
+			db.uncacheRef(ref.getName());
 			return status;
 		}
 
 		private void deleteFileAndEmptyDir(final File file, final int depth)
 				throws IOException {
-			if (file.exists()) {
+			if (file.isFile()) {
 				if (!file.delete())
 					throw new IOException("File cannot be deleted: " + file);
-				deleteEmptyDir(file.getParentFile(), depth);
+				File dir = file.getParentFile();
+				for  (int i = 0; i < depth; ++i) {
+					if (!dir.delete())
+						break; // ignore problem here
+					dir = dir.getParentFile();
+				}
 			}
 		}
+	}
 
-		private void deleteEmptyDir(File dir, int depth) {
-			for (; depth > 0 && dir != null; depth--) {
-				if (!dir.delete())
-					break;
-				dir = dir.getParentFile();
-			}
+	UpdateStore newUpdateStore() {
+		return new UpdateStore();
+	}
+
+	DeleteStore newDeleteStore() {
+		return new DeleteStore();
+	}
+
+	static void deleteEmptyDir(File dir, int depth) {
+		for (; depth > 0 && dir != null; depth--) {
+			if (dir.exists() && !dir.delete())
+				break;
+			dir = dir.getParentFile();
 		}
 	}
 
-	private static int count(final String s, final char c) {
+	static int count(final String s, final char c) {
 		int count = 0;
 		for (int p = s.indexOf(c); p >= 0; p = s.indexOf(c, p + 1)) {
 			count++;
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
index 55dae27..06e0c7b 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
@@ -468,6 +468,22 @@ public RefUpdate updateRef(final String ref) throws IOException {
 	}
 
 	/**
+	 * Create a command to rename a ref in this repository
+	 *
+	 * @param fromRef
+	 *            name of ref to rename from
+	 * @param toRef
+	 *            name of ref to rename to
+	 * @return an update command that knows how to rename a branch to another.
+	 * @throws IOException
+	 *             the rename could not be performed.
+	 *
+	 */
+	public RefRename renameRef(final String fromRef, final String toRef) throws IOException {
+		return refs.newRename(fromRef, toRef);
+	}
+
+	/**
 	 * Parse a git revision string and return an object id.
 	 *
 	 * Currently supported is combinations of these.
-- 
1.6.3.dirty

[EGIT PATCH 09/10] Add ref rename support to the branch dialog

[Robin Rosenberg]

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../src/org/spearce/egit/ui/UIText.java            |   12 ++
 .../ui/internal/dialogs/BranchSelectionDialog.java |  106 ++++++++++++++++----
 .../src/org/spearce/egit/ui/uitext.properties      |    5 +
 3 files changed, 103 insertions(+), 20 deletions(-)

diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
index 654e155..aa31a93 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
@@ -893,6 +893,12 @@
 	public static String BranchSelectionDialog_ErrorCouldNotRefresh;
 
 	/** */
+	public static String BranchSelectionDialog_ErrorCouldNotRenameRef;
+
+	/** */
+	public static String BranchSelectionDialog_ErrorCouldNotRenameRef2;
+
+	/** */
 	public static String BranchSelectionDialog_BranchSuffix_Current;
 
 	/** */
@@ -929,6 +935,12 @@
 	public static String BranchSelectionDialog_QuestionNewBranchTitle;
 
 	/** */
+	public static String BranchSelectionDialog_QuestionNewBranchNameTitle;
+
+	/** */
+	public static String BranchSelectionDialog_QuestionNewBranchNameMessage;
+
+	/** */
 	public static String BranchSelectionDialog_QuestionNewBranchMessage;
 
 	/** */
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
index 9aad95b..28105af 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
@@ -48,8 +48,11 @@
 import org.spearce.egit.ui.UIText;
 import org.spearce.jgit.lib.Constants;
 import org.spearce.jgit.lib.ObjectId;
+import org.spearce.jgit.lib.Ref;
+import org.spearce.jgit.lib.RefRename;
 import org.spearce.jgit.lib.RefUpdate;
 import org.spearce.jgit.lib.Repository;
+import org.spearce.jgit.lib.RefUpdate.Result;
 
 /**
  * The branch and reset selection dialog
@@ -277,6 +280,30 @@ private void refNameFromDialog() {
 		}
 	}
 
+	private InputDialog getRefNameInputDialog(String prompt) {
+		InputDialog labelDialog = new InputDialog(
+				getShell(),
+				UIText.BranchSelectionDialog_QuestionNewBranchTitle,
+				prompt,
+				null, new IInputValidator() {
+					public String isValid(String newText) {
+						String testFor = Constants.R_HEADS + newText;
+						try {
+							if (repo.resolve(testFor) != null)
+								return UIText.BranchSelectionDialog_ErrorAlreadyExists;
+						} catch (IOException e1) {
+							Activator.logError(NLS.bind(
+									UIText.BranchSelectionDialog_ErrorCouldNotResolve, testFor), e1);
+						}
+						if (!Repository.isValidRefName(testFor))
+							return UIText.BranchSelectionDialog_ErrorInvalidRefName;
+						return null;
+					}
+				});
+		labelDialog.setBlockOnOpen(true);
+		return labelDialog;
+	}
+
 	@Override
 	protected void createButtonsForButtonBar(Composite parent) {
 		if (!showResetType) {
@@ -284,43 +311,81 @@ protected void createButtonsForButtonBar(Composite parent) {
 			newButton.setFont(JFaceResources.getDialogFont());
 			newButton.setText(UIText.BranchSelectionDialog_NewBranch);
 			((GridLayout)parent.getLayout()).numColumns++;
+			Button renameButton = new Button(parent, SWT.PUSH);
+			renameButton.setText("&Rename");
+			renameButton.addSelectionListener(new SelectionListener() {
+				public void widgetSelected(SelectionEvent e) {
+					// check what ref name the user selected, if any.
+					refNameFromDialog();
+
+					String branchName;
+					if (refName.startsWith(Constants.R_HEADS))
+						branchName = refName.substring(Constants.R_HEADS.length());
+					else
+						branchName = refName;	
+					InputDialog labelDialog = getRefNameInputDialog(NLS
+							.bind(
+									UIText.BranchSelectionDialog_QuestionNewBranchNameMessage,
+									branchName));
+					if (labelDialog.open() == Window.OK) {
+						String newRefName = Constants.R_HEADS + labelDialog.getValue();
+						try {
+							RefRename renameRef = repo.renameRef(refName, newRefName);
+							if (renameRef.rename() != Result.RENAMED) {
+								MessageDialog.openError(getShell(),
+										"Rename failed",
+										NLS.bind(UIText.BranchSelectionDialog_ErrorCouldNotRenameRef,
+												new Object[] { refName, newRefName, renameRef.getResult() }));
+								Activator.logError(NLS.bind(
+										UIText.BranchSelectionDialog_ErrorCouldNotRenameRef2,
+												new Object[] { refName, newRefName }), null);
+							}
+							// FIXME: Update HEAD
+						} catch (IOException e1) {
+							Activator.logError(NLS.bind(
+									UIText.BranchSelectionDialog_ErrorCouldNotRenameRef2,
+															newRefName), e1);
+						}
+						try {
+							branchTree.removeAll();
+							fillTreeWithBranches(newRefName);
+						} catch (IOException e1) {
+							Activator.logError(
+									UIText.BranchSelectionDialog_ErrorCouldNotRefreshBranchList,
+											e1);
+						}
+					}
+				}
+				public void widgetDefaultSelected(SelectionEvent e) {
+					widgetSelected(e);
+				}
+			});
 			newButton.addSelectionListener(new SelectionListener() {
 
 				public void widgetSelected(SelectionEvent e) {
 					// check what ref name the user selected, if any.
 					refNameFromDialog();
 
-					InputDialog labelDialog = new InputDialog(
-							getShell(),
-							UIText.BranchSelectionDialog_QuestionNewBranchTitle,
-							UIText.BranchSelectionDialog_QuestionNewBranchMessage,
-							null, new IInputValidator() {
-								public String isValid(String newText) {
-									String testFor = Constants.R_HEADS + newText;
-									try {
-										if (repo.resolve(testFor) != null)
-											return UIText.BranchSelectionDialog_ErrorAlreadyExists;
-									} catch (IOException e1) {
-										Activator.logError(NLS.bind(
-												UIText.BranchSelectionDialog_ErrorCouldNotResolve, testFor), e1);
-									}
-									if (!Repository.isValidRefName(testFor))
-										return UIText.BranchSelectionDialog_ErrorInvalidRefName;
-									return null;
-								}
-							});
-					labelDialog.setBlockOnOpen(true);
+					InputDialog labelDialog = getRefNameInputDialog(UIText.BranchSelectionDialog_QuestionNewBranchNameMessage);
 					if (labelDialog.open() == Window.OK) {
 						String newRefName = Constants.R_HEADS + labelDialog.getValue();
 						RefUpdate updateRef;
 						try {
 							updateRef = repo.updateRef(newRefName);
+							Ref startRef = repo.getRef(refName);
 							ObjectId startAt;
 							if (refName == null)
 								startAt = repo.resolve(Constants.HEAD);
 							else
 								startAt = repo.resolve(refName);
+							String startBranch;
+							if (startRef != null)
+								startBranch = refName;
+							else
+								startBranch = startAt.name();
+							startBranch = repo.shortenRefName(startBranch);
 							updateRef.setNewObjectId(startAt);
+							updateRef.setRefLogMessage("branch: Created from " + startBranch, false);
 							updateRef.update();
 						} catch (IOException e1) {
 							Activator.logError(NLS.bind(
@@ -338,6 +403,7 @@ public String isValid(String newText) {
 					}
 				}
 
+
 				public void widgetDefaultSelected(SelectionEvent e) {
 					widgetSelected(e);
 				}
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties b/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
index 1d21c81..f19db60 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
@@ -340,6 +340,9 @@ BranchSelectionDialog_ErrorCouldNotRefresh=Could not refresh list of branches
 BranchSelectionDialog_ErrorCouldNotRefreshBranchList=Could not refresh list of branches
 BranchSelectionDialog_ErrorCouldNotResolve=Could not attempt to resolve {0}
 BranchSelectionDialog_ErrorInvalidRefName=Invalid ref name
+BranchSelectionDialog_ErrorCouldNotRenameRef=Failed to rename branch {0} -> {1}, status={2}
+BranchSelectionDialog_ErrorCouldNotRenameRef2=Failed to rename branch {0} -> {1}
+
 BranchSelectionDialog_LocalBranches=Local Branches
 BranchSelectionDialog_NewBranch=&New branch
 BranchSelectionDialog_NoBranchSeletectMessage=You must select a valid ref.
@@ -347,7 +350,9 @@ BranchSelectionDialog_NoBranchSeletectTitle=No branch/tag selected
 BranchSelectionDialog_OkCheckout=&Checkout
 BranchSelectionDialog_OkReset=&Reset
 BranchSelectionDialog_QuestionNewBranchMessage=Enter name of new branch. It will branch from the selected branch. refs/heads/ will be prepended to the name you type
+BranchSelectionDialog_QuestionNewBranchNameMessage=Enter new name of the {0} branch. refs/heads/ will be prepended to the name you type
 BranchSelectionDialog_QuestionNewBranchTitle=New branch
+BranchSelectionDialog_QuestionNewBranchNameTitle=Rename branch
 BranchSelectionDialog_ReallyResetMessage=Resetting will overwrite any changes in your working directory.\n\nDo you wish to continue?
 BranchSelectionDialog_ReallyResetTitle=Really reset?
 BranchSelectionDialog_RemoteBranches=Remote Branches
-- 
1.6.3.dirty

[EGIT PATCH 10/10] Improve error reporting in the branch dialog

[Robin Rosenberg]

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../src/org/spearce/egit/ui/UIText.java            |    6 ++
 .../ui/internal/dialogs/BranchSelectionDialog.java |   59 ++++++++++++--------
 .../src/org/spearce/egit/ui/uitext.properties      |    2 +
 3 files changed, 43 insertions(+), 24 deletions(-)

diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
index aa31a93..c120327 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
@@ -899,6 +899,12 @@
 	public static String BranchSelectionDialog_ErrorCouldNotRenameRef2;
 
 	/** */
+	public static String BranchSelectionDialog_BranchSelectionDialog_CreateFailedTitle;
+
+	/** */
+	public static String BranchSelectionDialog_BranchSelectionDialog_RenamedFailedTitle;
+
+	/** */
 	public static String BranchSelectionDialog_BranchSuffix_Current;
 
 	/** */
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
index 28105af..43588ae 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
@@ -104,7 +104,7 @@ getShell().setText(
 
 		try {
 			fillTreeWithBranches(null);
-		} catch (IOException e) {
+		} catch (Throwable e) {
 			Activator.logError(UIText.BranchSelectionDialog_ErrorCouldNotRefresh, e);
 		}
 
@@ -294,6 +294,7 @@ public String isValid(String newText) {
 						} catch (IOException e1) {
 							Activator.logError(NLS.bind(
 									UIText.BranchSelectionDialog_ErrorCouldNotResolve, testFor), e1);
+							return e1.getMessage();
 						}
 						if (!Repository.isValidRefName(testFor))
 							return UIText.BranchSelectionDialog_ErrorInvalidRefName;
@@ -332,27 +333,28 @@ public void widgetSelected(SelectionEvent e) {
 						try {
 							RefRename renameRef = repo.renameRef(refName, newRefName);
 							if (renameRef.rename() != Result.RENAMED) {
-								MessageDialog.openError(getShell(),
-										"Rename failed",
-										NLS.bind(UIText.BranchSelectionDialog_ErrorCouldNotRenameRef,
-												new Object[] { refName, newRefName, renameRef.getResult() }));
-								Activator.logError(NLS.bind(
-										UIText.BranchSelectionDialog_ErrorCouldNotRenameRef2,
-												new Object[] { refName, newRefName }), null);
+								reportError(
+										null,
+										UIText.BranchSelectionDialog_BranchSelectionDialog_RenamedFailedTitle,
+										UIText.BranchSelectionDialog_ErrorCouldNotRenameRef,
+										refName, newRefName, renameRef
+												.getResult());
 							}
-							// FIXME: Update HEAD
-						} catch (IOException e1) {
-							Activator.logError(NLS.bind(
-									UIText.BranchSelectionDialog_ErrorCouldNotRenameRef2,
-															newRefName), e1);
+						} catch (Throwable e1) {
+							reportError(
+									e1,
+									UIText.BranchSelectionDialog_BranchSelectionDialog_RenamedFailedTitle,
+									UIText.BranchSelectionDialog_ErrorCouldNotRenameRef,
+									refName, newRefName, e1.getMessage());
 						}
 						try {
 							branchTree.removeAll();
 							fillTreeWithBranches(newRefName);
-						} catch (IOException e1) {
-							Activator.logError(
-									UIText.BranchSelectionDialog_ErrorCouldNotRefreshBranchList,
-											e1);
+						} catch (Throwable e1) {
+							reportError(
+									e1,
+									UIText.BranchSelectionDialog_BranchSelectionDialog_RenamedFailedTitle,
+									UIText.BranchSelectionDialog_ErrorCouldNotRefreshBranchList);
 						}
 					}
 				}
@@ -387,18 +389,20 @@ public void widgetSelected(SelectionEvent e) {
 							updateRef.setNewObjectId(startAt);
 							updateRef.setRefLogMessage("branch: Created from " + startBranch, false);
 							updateRef.update();
-						} catch (IOException e1) {
-							Activator.logError(NLS.bind(
+						} catch (Throwable e1) {
+							reportError(
+									e1,
+									UIText.BranchSelectionDialog_BranchSelectionDialog_CreateFailedTitle,
 									UIText.BranchSelectionDialog_ErrorCouldNotCreateNewRef,
-															newRefName), e1);
+									newRefName);
 						}
 						try {
 							branchTree.removeAll();
 							fillTreeWithBranches(newRefName);
-						} catch (IOException e1) {
-							Activator.logError(
-									UIText.BranchSelectionDialog_ErrorCouldNotRefreshBranchList,
-											e1);
+						} catch (Throwable e1) {
+							reportError(e1,
+									UIText.BranchSelectionDialog_BranchSelectionDialog_CreateFailedTitle,
+									UIText.BranchSelectionDialog_ErrorCouldNotRefreshBranchList);
 						}
 					}
 				}
@@ -419,4 +423,11 @@ createButton(parent, IDialogConstants.OK_ID,
 	protected int getShellStyle() {
 		return super.getShellStyle() | SWT.RESIZE;
 	}
+
+	private void reportError(Throwable e, String title, String message,
+			Object... args) {
+		String msg = NLS.bind(message, args);
+		MessageDialog.openError(getShell(), title, msg);
+		Activator.logError(msg, e);
+	}
 }
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties b/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
index f19db60..93ce3c9 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
@@ -333,6 +333,8 @@ WindowCachePreferencePage_packedGitMMAP=Use virtual memory mapping
 
 BranchSelectionDialog_TitleCheckout=Checkout: {0}
 BranchSelectionDialog_TitleReset=Reset: {0}
+BranchSelectionDialog_BranchSelectionDialog_CreateFailedTitle=New branch creation failed
+BranchSelectionDialog_BranchSelectionDialog_RenamedFailedTitle=Rename failed
 BranchSelectionDialog_BranchSuffix_Current=\ (current)
 BranchSelectionDialog_ErrorAlreadyExists=Already exists
 BranchSelectionDialog_ErrorCouldNotCreateNewRef=Could not create new ref {0}
-- 
1.6.3.dirty

Re: [EGIT PATCH 05/10] Do not write to the reflog unless the refupdate logmessage is set

[Shawn O. Pearce]

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> Do not write to the reflog unless the refupdate logmessage is set

Why not?  What is the justification for this?  Isn't a reflog record
still useful, at least to point out that something happened at this
point in time, and here's the old/new values?

> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
> index a9ab73b..79d9f2d 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
> @@ -479,7 +479,8 @@ else if (status == Result.FAST_FORWARD)
>  			else if (status == Result.NEW)
>  				msg += ": created";
>  		}
> -		RefLogWriter.append(this, msg);
> +		if (msg != null)
> +			RefLogWriter.append(this, msg);
>  		if (!lock.commit())
>  			return Result.LOCK_FAILURE;
>  		db.stored(this.ref.getOrigName(),  ref.getName(), newValue, lock.getCommitLastModified());

-- 
Shawn.

Re: [EGIT PATCH 08/10] Add ref rename support to JGit

[Shawn O. Pearce]

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> Now refs can be renamed. The intent is that should be safe. Only the named
> refs and associated logs are updated. Any symbolic refs referring to the renames
> branches are unaffected, except HEAD, which is updated if the branch it refers
> to is being renamed.
...
> +//	public void testRenameBranchCannotLockAFileHEADisToLockHead() throws IOException {
> +//		an example following the rename failure pattern, but this one is ok!
> +//		tryRenameWhenLocked("HEAD", "refs/heads/b", "refs/heads/new/name",
> +//				"refs/heads/new/name");
> +//	}

Commented out test case?  Eh?

> +//	public void testRenameBranchCannotLockAFileHEADisOtherLockHead() throws IOException {
> +//		an example following the rename failure pattern, but this one is ok!
> +//		tryRenameWhenLocked("HEAD", "refs/heads/b", "refs/heads/new/name",
> +//				"refs/heads/a");
> +//	}

Ditto.

> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
...
> +	RefRename newRename(String fromRef, String toRef) throws IOException {
> +		refreshPackedRefs();
> +		Ref f = readRefBasic(fromRef, 0);
> +		Ref t = readRefBasic(toRef, 0);
> +		if (t != null)
> +			throw new IOException("Ref rename target exists: " + t.getName());

NAK, I'd prefer to have the RefRename return successfully,
but when we try to execute it, it has a result that fails with
RefUpdate.REJECTED.

> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java
...
> +	static void renameTo(final Repository db, final RefUpdate from,
> +			final RefUpdate to) throws IOException {
> +		final File logdir = new File(db.getDirectory(), Constants.LOGS);
> +		final File reflogFrom = new File(logdir, from.getName());
> +		if (!reflogFrom.exists())
> +			return;
> +		final File reflogTo = new File(logdir, to.getName());
> +		final File reflogToDir = reflogTo.getParentFile();
> +		File tmp = new File(logdir,"tmp-renamed-log");
> +		tmp.delete(); // if any exists

That's not thread safe.  Most of the rest of JGit is actually
thread safe.  I think we should be here too.  We should use
a temporary file name inside logdir, one that isn't a legal
ref name at all.  Maybe:

  File tmp = File.createTempFile("tmp..renamed", "-log", logdir);
  if (!reflogFrom.renameTo(tmp)) {
    tmp.delete();
    if (!reflogFrom.renameTo(tmp))
      throw new IOException....
  }

> +		if (!reflogToDir.exists() && !reflogToDir.mkdirs()) {
> +			throw new IOException("Cannot create directory " + reflogToDir);
> +		}
> +		if (!tmp.renameTo(reflogTo)) {
> +			throw new IOException("Cannot rename (" + tmp + ")" + reflogFrom
> +					+ " to " + reflogTo);
> +		}

If we fail here, should we attempt to put the tmp log file back
into the old name?  Or leave it orphaned under the tmp file?

> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefRename.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefRename.java
...
> +	public Result rename() throws IOException {
> +		Ref oldRef = oldFromDelete.db.readRef(Constants.HEAD);
> +		boolean renameHEADtoo = oldRef != null
> +				&& oldRef.getName().equals(oldFromDelete.getName());
> +		try {
> +			Repository db = oldFromDelete.getRepository();

Style nit: Hoist to top of method, so oldRef init can use it.

> +			RefLogWriter.renameTo(db, oldFromDelete,
> +					newToUpdate);

Style nit: This fits on one line.

> +			newToUpdate.setRefLogMessage(null, false);
> +			RefUpdate tmpUpdateRef = oldFromDelete.db.getRepository().updateRef(
> +					"RENAMED-REF");

Like the log, this isn't thread safe.

> +			if (renameHEADtoo) {
> +				try {
> +					oldFromDelete.db.link(Constants.HEAD, "RENAMED-REF");
> +				} catch (IOException e) {
> +					RefLogWriter.renameTo(db,
> +							newToUpdate, oldFromDelete);
> +					return renameResult = Result.LOCK_FAILURE;
> +				}
> +			}
> +			tmpUpdateRef.setNewObjectId(oldFromDelete.getOldObjectId());

We should also do:

  oldFromDelete.setExpectedOldObjectId(oldFromDelete.getOldObjectId())

so that if the old ref was modified between the time we read it
and the time we delete it, we fail on the delete, and don't whack
a ref that was modified by another thread.

> +			RefLogWriter.append(this, "jgit branch: renamed "

I think this should just be "renamed: $old to $new".

> +					+ db.shortenRefName(oldFromDelete.getName()) + " to "
> +					+ db.shortenRefName(newToUpdate.getName()));
> +			return renameResult = Result.RENAMED;
> +		} catch (RuntimeException e) {
> +			throw e;

What's the point of this catch block?

> +		} catch (IOException e) {
> +			System.err.println(e);

Please don't print to System.err from such a low level method.

> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
...
> +		/**
> +		 * The ref was renamed from another name
> +		 * <p>
> +		 */
> +		RENAMED

Is a new state really necessary?  Can't we use NEW on success?
After all, the dst is now created.

>  	}
>  
>  	/** Repository the ref is stored in. */
> -	private final RefDatabase db;
> +	final RefDatabase db;

I wonder if this shouldn't just be a public accessor method; we expose
getRepository() on other application visible objects like RevWalk.
  
>  	/** Location of the loose file holding the value of this ref. */
> -	private final File looseFile;
> +	final File looseFile;

I don't see this used outside of the class, is it still necessary?

>  	/** Result of the update operation. */
> -	private Result result = Result.NOT_ATTEMPTED;
> +	Result result = Result.NOT_ATTEMPTED;

Ditto.
  
> +	static void deleteEmptyDir(File dir, int depth) {
> +		for (; depth > 0 && dir != null; depth--) {
> +			if (dir.exists() && !dir.delete())

Why bother with a stat() call, just to do an rmdir()?

> -	private static int count(final String s, final char c) {
> +	static int count(final String s, final char c) {

Since all callers pass '/', maybe we should just change this to
use '/' as a constant in the loop and remove the char c parameter?
I think that would fix the nasty line wrapping in RefLogWriter too.

-- 
Shawn.

Re: [EGIT PATCH 05/10] Do not write to the reflog unless the refupdate logmessage is set

[Robin Rosenberg]

onsdag 03 juni 2009 17:41:29 skrev "Shawn O. Pearce" <spearce@spearce.org>:
> Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> > Do not write to the reflog unless the refupdate logmessage is set
> 
> Why not?  What is the justification for this?  Isn't a reflog record
> still useful, at least to point out that something happened at this
> point in time, and here's the old/new values?

I need to do an update without logging to avoid a "dummy" entry in the
reflog since I use the RefUpdate mechanism. Since this is an API and
not and end user function, I was thinking this might be ok after all.

Some other optione are:
1) set a special flag.
2) set a default message and allow the caller to set it to null and
not write if null. Today the default message is null leading to the
message "null" in the log, which is kind of ugly so we should
probably change this in any case.

-- robin

Re: [EGIT PATCH 05/10] Do not write to the reflog unless the refupdate logmessage is set

[Shawn O. Pearce]

Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
> onsdag 03 juni 2009 17:41:29 skrev "Shawn O. Pearce" <spearce@spearce.org>:
> > Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> > > Do not write to the reflog unless the refupdate logmessage is set
> > 
> > Why not?  What is the justification for this?  Isn't a reflog record
> > still useful, at least to point out that something happened at this
> > point in time, and here's the old/new values?
> 
> I need to do an update without logging to avoid a "dummy" entry in the
> reflog since I use the RefUpdate mechanism. Since this is an API and
> not and end user function, I was thinking this might be ok after all.

ACK, I agree.  How about this then?
 
--8<--
From: Robin Rosenberg <robin.rosenberg@dewire.com>
Subject: [PATCH] Do not write to the reflog if refLogMessage is null

This permits micro-update steps (or otherwise uninteresting states)
to be skipped in the reflog.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../tst/org/spearce/jgit/lib/RefUpdateTest.java    |    1 +
 .../src/org/spearce/jgit/lib/RefUpdate.java        |   53 +++++++++++++++----
 2 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
index 6b1975a..84653c8 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
@@ -126,6 +126,7 @@ public void testDeleteLooseAndItsDirectory() throws IOException {
 		RefUpdate updateRef = db.updateRef("refs/heads/z/c");
 		updateRef.setNewObjectId(pid);
 		updateRef.setForceUpdate(true);
+		updateRef.setRefLogMessage("new test ref", false);
 		Result update = updateRef.update();
 		assertEquals(Result.NEW, update); // internal
 		assertTrue(new File(db.getDirectory(), Constants.R_HEADS + "z")
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
index a9ab73b..17fe3be 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
@@ -165,6 +165,7 @@ RefUpdate(final RefDatabase r, final Ref ref, final File f) {
 		this.ref = ref;
 		oldValue = ref.getObjectId();
 		looseFile = f;
+		refLogMessage = "";
 	}
 
 	/** @return the repository the updated ref resides in */
@@ -264,7 +265,8 @@ public void setRefLogIdent(final PersonIdent pi) {
 	/**
 	 * Get the message to include in the reflog.
 	 * 
-	 * @return message the caller wants to include in the reflog.
+	 * @return message the caller wants to include in the reflog; null if the
+	 *         update should not be logged.
 	 */
 	public String getRefLogMessage() {
 		return refLogMessage;
@@ -281,8 +283,21 @@ public String getRefLogMessage() {
 	 *            message.
 	 */
 	public void setRefLogMessage(final String msg, final boolean appendStatus) {
-		refLogMessage = msg;
-		refLogIncludeResult = appendStatus;
+		if (msg == null && !appendStatus)
+			disableRefLog();
+		else if (msg == null && appendStatus) {
+			refLogMessage = "";
+			refLogIncludeResult = true;
+		} else {
+			refLogMessage = msg;
+			refLogIncludeResult = appendStatus;
+		}
+	}
+
+	/** Don't record this update in the ref's associated reflog. */
+	public void disableRefLog() {
+		refLogMessage = null;
+		refLogIncludeResult = false;
 	}
 
 	/**
@@ -471,21 +486,37 @@ private Result updateStore(final LockFile lock, final Result status)
 		lock.setNeedStatInformation(true);
 		lock.write(newValue);
 		String msg = getRefLogMessage();
-		if (msg != null && refLogIncludeResult) {
-			if (status == Result.FORCED)
-				msg += ": forced-update";
-			else if (status == Result.FAST_FORWARD)
-				msg += ": fast forward";
-			else if (status == Result.NEW)
-				msg += ": created";
+		if (msg != null) {
+			if (refLogIncludeResult) {
+				String strResult = toResultString(status);
+				if (strResult != null) {
+					if (msg.length() > 0)
+						msg = msg + ": " + strResult;
+					else
+						msg = strResult;
+				}
+			}
+			RefLogWriter.append(this, msg);
 		}
-		RefLogWriter.append(this, msg);
 		if (!lock.commit())
 			return Result.LOCK_FAILURE;
 		db.stored(this.ref.getOrigName(),  ref.getName(), newValue, lock.getCommitLastModified());
 		return status;
 	}
 
+	private static String toResultString(final Result status) {
+		switch (status) {
+		case FORCED:
+			return "forced-update";
+		case FAST_FORWARD:
+			return "fast forward";
+		case NEW:
+			return "created";
+		default:
+			return null;
+		}
+	}
+
 	/**
 	 * Handle the abstraction of storing a ref update. This is because both
 	 * updating and deleting of a ref have merge testing in common.
-- 
1.6.3.2.322.g117de

-- 
Shawn.

[EGIT PATCH v5 0/7] Ref rename support again

[Robin Rosenberg]

More tests, leading to fixes of minor bugs, plus corrections
according to Shawn's comments.

-- robin

Robin Rosenberg (6):
  Add methods to RawParseUtils for scanning backwards.
  Add a ref log reader class
  Add ref rename support to JGit
  Add ref rename support to the branch dialog
  Improve error reporting in the branch dialog
  Remove a TAB from the message Egit generates into the reflog on
    commit

Shawn O. Pearce (1):
  Do not write to the reflog unless the refupdate logmessage is set

 .../src/org/spearce/egit/ui/UIText.java            |   18 +
 .../egit/ui/internal/actions/CommitAction.java     |    2 +-
 .../ui/internal/dialogs/BranchSelectionDialog.java |  133 ++++++--
 .../src/org/spearce/egit/ui/uitext.properties      |    7 +
 .../org/spearce/jgit/test/resources/packed-refs    |    1 +
 .../tst/org/spearce/jgit/lib/RefUpdateTest.java    |  348 +++++++++++++++++++-
 .../tst/org/spearce/jgit/lib/ReflogReaderTest.java |  166 ++++++++++
 .../org/spearce/jgit/lib/RepositoryTestCase.java   |   18 +
 .../org/spearce/jgit/transport/TransportTest.java  |    2 +-
 .../src/org/spearce/jgit/lib/RefDatabase.java      |   31 ++
 .../src/org/spearce/jgit/lib/RefLogWriter.java     |   33 ++-
 .../src/org/spearce/jgit/lib/RefRename.java        |  163 +++++++++
 .../src/org/spearce/jgit/lib/RefUpdate.java        |  115 +++++--
 .../src/org/spearce/jgit/lib/ReflogReader.java     |  187 +++++++++++
 .../src/org/spearce/jgit/lib/Repository.java       |   30 ++
 .../src/org/spearce/jgit/util/RawParseUtils.java   |   84 +++++-
 16 files changed, 1269 insertions(+), 69 deletions(-)
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/ReflogReaderTest.java
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/RefRename.java
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/ReflogReader.java


Short log of my patch queue before rebase:

Mine
      Add methods to RawParseUtils for scanning backwards.
      Add a ref log reader class
      Do not write to the reflog unless the refupdate logmessage is set
      Revert "Do not write to the reflog unless the refupdate logmessage is set"
      Add ref rename support to JGit
      Add ref rename support to the branch dialog
      Improve error reporting in the branch dialog
      Shawns comments, except the no-reflog update
      Remove a TAB from the message Egit generated into the reflog on commit
      Follow C Git's format for renames in the reflog
      Improve refupdate comment
      ..ReflogReader
      Move reflog creation to testcase base class
      ..rename tests

Shawn:
      Do not write to the reflog unless the refupdate logmessage is set

[EGIT PATCH v5 1/7] Add methods to RawParseUtils for scanning backwards.

[Robin Rosenberg]

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../src/org/spearce/jgit/util/RawParseUtils.java   |   84 +++++++++++++++++++-
 1 files changed, 83 insertions(+), 1 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java b/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
index 79ebe41..bdd6a11 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
@@ -321,6 +321,67 @@ public static final int nextLF(final byte[] b, int ptr, final char chrA) {
 	}
 
 	/**
+	 * Locate the first position before a given character.
+	 * 
+	 * @param b
+	 *            buffer to scan.
+	 * @param ptr
+	 *            position within buffer to start looking for chrA at.
+	 * @param chrA
+	 *            character to find.
+	 * @return new position just before chrA, -1 for not found
+	 */
+	public static final int prev(final byte[] b, int ptr, final char chrA) {
+		if (ptr == b.length)
+			--ptr;
+		while (ptr >= 0) {
+			if (b[ptr--] == chrA)
+				return ptr;
+		}
+		return ptr;
+	}
+
+	/**
+	 * Locate the first position before the previous LF.
+	 * <p>
+	 * This method stops on the first '\n' it finds.
+	 * 
+	 * @param b
+	 *            buffer to scan.
+	 * @param ptr
+	 *            position within buffer to start looking for LF at.
+	 * @return new position just before the first LF found, -1 for not found
+	 */
+	public static final int prevLF(final byte[] b, int ptr) {
+		return prev(b, ptr, '\n');
+	}
+
+	/**
+	 * Locate the previous position before either the given character or LF.
+	 * <p>
+	 * This method stops on the first match it finds from either chrA or '\n'.
+	 * 
+	 * @param b
+	 *            buffer to scan.
+	 * @param ptr
+	 *            position within buffer to start looking for chrA or LF at.
+	 * @param chrA
+	 *            character to find.
+	 * @return new position just before the first chrA or LF to be found, -1 for
+	 *         not found
+	 */
+	public static final int prevLF(final byte[] b, int ptr, final char chrA) {
+		if (ptr == b.length)
+			--ptr;
+		while (ptr >= 0) {
+			final byte c = b[ptr--];
+			if (c == chrA || c == '\n')
+				return ptr;
+		}
+		return ptr;
+	}
+
+	/**
 	 * Index the region between <code>[ptr, end)</code> to find line starts.
 	 * <p>
 	 * The returned list is 1 indexed. Index 0 contains
@@ -519,7 +580,28 @@ public static PersonIdent parsePersonIdent(final byte[] raw, final int nameB) {
 	 *         after decoding the region through the specified character set.
 	 */
 	public static String decode(final byte[] buffer) {
-		return decode(Constants.CHARSET, buffer, 0, buffer.length);
+		return decode(buffer, 0, buffer.length);
+	}
+
+	/**
+	 * Decode a buffer under UTF-8, if possible.
+	 * 
+	 * If the byte stream cannot be decoded that way, the platform default is
+	 * tried and if that too fails, the fail-safe ISO-8859-1 encoding is tried.
+	 * 
+	 * @param buffer
+	 *            buffer to pull raw bytes from.
+	 * @param start
+	 *            start position in buffer
+	 * @param end
+	 *            one position past the last location within the buffer to take
+	 *            data from.
+	 * @return a string representation of the range <code>[start,end)</code>,
+	 *         after decoding the region through the specified character set.
+	 */
+	public static String decode(final byte[] buffer, final int start,
+			final int end) {
+		return decode(Constants.CHARSET, buffer, start, end);
 	}
 
 	/**
-- 
1.6.3.2.199.g7340d

[EGIT PATCH v5 2/7] Add a ref log reader class

[Robin Rosenberg]

This reader allows the caller to the reflog entries for a given ref.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../tst/org/spearce/jgit/lib/ReflogReaderTest.java |  166 +++++++++++++++++
 .../org/spearce/jgit/lib/RepositoryTestCase.java   |   18 ++
 .../src/org/spearce/jgit/lib/ReflogReader.java     |  187 ++++++++++++++++++++
 .../src/org/spearce/jgit/lib/Repository.java       |   14 ++
 4 files changed, 385 insertions(+), 0 deletions(-)
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/ReflogReaderTest.java
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/ReflogReader.java

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ReflogReaderTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ReflogReaderTest.java
new file mode 100644
index 0000000..2888da6
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ReflogReaderTest.java
@@ -0,0 +1,166 @@
+/*
+ * Copyright (C) 2009, Robin Rosenberg
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.spearce.jgit.lib;
+
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.List;
+
+import org.spearce.jgit.lib.ReflogReader.Entry;
+
+public class ReflogReaderTest extends RepositoryTestCase {
+
+	static byte[] oneLine = "da85355dfc525c9f6f3927b876f379f46ccf826e 3e7549db262d1e836d9bf0af7e22355468f1717c Robin Rosenberg <robin.rosenberg@dewire.com> 1243028200 +0200\tcommit: Add a toString for debugging to RemoteRefUpdate\n"
+			.getBytes();
+
+	static byte[] twoLine = ("0000000000000000000000000000000000000000 c6734895958052a9dbc396cff4459dc1a25029ab A U Thor <thor@committer.au> 1243028201 -0100\tbranch: Created from rr/renamebranchv4\n"
+			+ "c6734895958052a9dbc396cff4459dc1a25029ab 54794942a18a237c57a80719afed44bb78172b10 Same A U Thor <same.author@example.com> 1243028202 +0100\trebase finished: refs/heads/rr/renamebranch5 onto c6e3b9fe2da0293f11eae202ec35fb343191a82d\n")
+			.getBytes();
+
+	static byte[] twoLineWithAppendInProgress = ("0000000000000000000000000000000000000000 c6734895958052a9dbc396cff4459dc1a25029ab A U Thor <thor@committer.au> 1243028201 -0100\tbranch: Created from rr/renamebranchv4\n"
+			+ "c6734895958052a9dbc396cff4459dc1a25029ab 54794942a18a237c57a80719afed44bb78172b10 Same A U Thor <same.author@example.com> 1243028202 +0100\trebase finished: refs/heads/rr/renamebranch5 onto c6e3b9fe2da0293f11eae202ec35fb343191a82d\n"
+			+ "54794942a18a237c57a80719afed44bb78172b10 ")
+			.getBytes();
+
+	static byte[] aLine = "1111111111111111111111111111111111111111 3e7549db262d1e836d9bf0af7e22355468f1717c A U Thor <thor@committer.au> 1243028201 -0100\tbranch: change to a\n"
+			.getBytes();
+
+	static byte[] masterLine = "2222222222222222222222222222222222222222 3e7549db262d1e836d9bf0af7e22355468f1717c A U Thor <thor@committer.au> 1243028201 -0100\tbranch: change to master\n"
+			.getBytes();
+
+	static byte[] headLine = "3333333333333333333333333333333333333333 3e7549db262d1e836d9bf0af7e22355468f1717c A U Thor <thor@committer.au> 1243028201 -0100\tbranch: change to HEAD\n"
+			.getBytes();
+
+	public void testReadOneLine() throws Exception {
+		setupReflog("logs/refs/heads/master", oneLine);
+
+		ReflogReader reader = new ReflogReader(db, "refs/heads/master");
+		Entry e = reader.getLastEntry();
+		assertEquals(ObjectId
+				.fromString("da85355dfc525c9f6f3927b876f379f46ccf826e"), e
+				.getOldId());
+		assertEquals(ObjectId
+				.fromString("3e7549db262d1e836d9bf0af7e22355468f1717c"), e
+				.getNewId());
+		assertEquals("Robin Rosenberg", e.getWho().getName());
+		assertEquals("robin.rosenberg@dewire.com", e.getWho().getEmailAddress());
+		assertEquals(120, e.getWho().getTimeZoneOffset());
+		assertEquals("2009-05-22T23:36:40", iso(e.getWho().getWhen()));
+		assertEquals("commit: Add a toString for debugging to RemoteRefUpdate",
+				e.getComment());
+	}
+
+	private String iso(Date d) {
+		return new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss").format(d);
+	}
+
+	public void testReadTwoLine() throws Exception {
+		setupReflog("logs/refs/heads/master", twoLine);
+
+		ReflogReader reader = new ReflogReader(db, "refs/heads/master");
+		List<Entry> reverseEntries = reader.getReverseEntries();
+		assertEquals(2, reverseEntries.size());
+		Entry e = reverseEntries.get(0);
+		assertEquals(ObjectId
+				.fromString("c6734895958052a9dbc396cff4459dc1a25029ab"), e
+				.getOldId());
+		assertEquals(ObjectId
+				.fromString("54794942a18a237c57a80719afed44bb78172b10"), e
+				.getNewId());
+		assertEquals("Same A U Thor", e.getWho().getName());
+		assertEquals("same.author@example.com", e.getWho().getEmailAddress());
+		assertEquals(60, e.getWho().getTimeZoneOffset());
+		assertEquals("2009-05-22T23:36:42", iso(e.getWho().getWhen()));
+		assertEquals(
+				"rebase finished: refs/heads/rr/renamebranch5 onto c6e3b9fe2da0293f11eae202ec35fb343191a82d",
+				e.getComment());
+
+		e = reverseEntries.get(1);
+		assertEquals(ObjectId
+				.fromString("0000000000000000000000000000000000000000"), e
+				.getOldId());
+		assertEquals(ObjectId
+				.fromString("c6734895958052a9dbc396cff4459dc1a25029ab"), e
+				.getNewId());
+		assertEquals("A U Thor", e.getWho().getName());
+		assertEquals("thor@committer.au", e.getWho().getEmailAddress());
+		assertEquals(-60, e.getWho().getTimeZoneOffset());
+		assertEquals("2009-05-22T23:36:41", iso(e.getWho().getWhen()));
+		assertEquals("branch: Created from rr/renamebranchv4", e.getComment());
+	}
+
+	public void testReadWhileAppendIsInProgress() throws Exception {
+		setupReflog("logs/refs/heads/master", twoLineWithAppendInProgress);
+		ReflogReader reader = new ReflogReader(db, "refs/heads/master");
+		List<Entry> reverseEntries = reader.getReverseEntries();
+		assertEquals(2, reverseEntries.size());
+		Entry e = reverseEntries.get(0);
+		assertEquals(ObjectId
+				.fromString("c6734895958052a9dbc396cff4459dc1a25029ab"), e
+				.getOldId());
+		assertEquals(ObjectId
+				.fromString("54794942a18a237c57a80719afed44bb78172b10"), e
+				.getNewId());
+		assertEquals("Same A U Thor", e.getWho().getName());
+		assertEquals("same.author@example.com", e.getWho().getEmailAddress());
+		assertEquals(60, e.getWho().getTimeZoneOffset());
+		assertEquals("2009-05-22T23:36:42", iso(e.getWho().getWhen()));
+		assertEquals(
+				"rebase finished: refs/heads/rr/renamebranch5 onto c6e3b9fe2da0293f11eae202ec35fb343191a82d",
+				e.getComment());
+		// while similar to testReadTwoLine, we can assume that if we get the last entry
+		// right, everything else is too
+	}
+
+
+	public void testReadRightLog() throws Exception {
+		setupReflog("logs/refs/heads/a", aLine);
+		setupReflog("logs/refs/heads/master", masterLine);
+		setupReflog("logs/HEAD", headLine);
+		assertEquals("branch: change to master", db.getReflogReader("master")
+				.getLastEntry().getComment());
+		assertEquals("branch: change to a", db.getReflogReader("a")
+				.getLastEntry().getComment());
+		assertEquals("branch: change to HEAD", db.getReflogReader("HEAD")
+				.getLastEntry().getComment());
+	}
+
+	public void testNoLog() throws Exception {
+		assertEquals(0, db.getReflogReader("master").getReverseEntries().size());
+		assertNull(db.getReflogReader("master").getLastEntry());
+	}
+}
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
index 3b03ac1..3837ea9 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
@@ -40,6 +40,7 @@
 
 import java.io.File;
 import java.io.FileInputStream;
+import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStreamReader;
@@ -348,4 +349,21 @@ public void run() {
 		return newRepo;
 	}
 
+	protected void setupReflog(String logName, byte[] data)
+			throws FileNotFoundException, IOException {
+				File logfile = new File(db.getDirectory(), logName);
+				if (!logfile.getParentFile().mkdirs()
+						&& !logfile.getParentFile().isDirectory()) {
+					throw new IOException(
+							"oops, cannot create the directory for the test reflog file"
+									+ logfile);
+				}
+				FileOutputStream fileOutputStream = new FileOutputStream(logfile);
+				try {
+					fileOutputStream.write(data);
+				} finally {
+					fileOutputStream.close();
+				}
+			}
+
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ReflogReader.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ReflogReader.java
new file mode 100644
index 0000000..223347d
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ReflogReader.java
@@ -0,0 +1,187 @@
+/*
+ * Copyright (C) 2009, Robin Rosenberg
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.spearce.jgit.lib;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import org.spearce.jgit.util.NB;
+import org.spearce.jgit.util.RawParseUtils;
+
+/**
+ * Utility for reading reflog entries
+ */
+public class ReflogReader {
+	/**
+	 * Parsed reflog entry
+	 */
+	static public class Entry {
+		private ObjectId oldId;
+
+		private ObjectId newId;
+
+		private PersonIdent who;
+
+		private String comment;
+
+		Entry(byte[] raw, int pos) {
+			oldId = ObjectId.fromString(raw, pos);
+			pos += Constants.OBJECT_ID_LENGTH * 2;
+			if (raw[pos++] != ' ')
+				throw new IllegalArgumentException(
+						"Raw log message does not parse as log entry");
+			newId = ObjectId.fromString(raw, pos);
+			pos += Constants.OBJECT_ID_LENGTH * 2;
+			if (raw[pos++] != ' ') {
+				throw new IllegalArgumentException(
+						"Raw log message does not parse as log entry");
+			}
+			who = RawParseUtils.parsePersonIdent(raw, pos);
+			int p0 = RawParseUtils.next(raw, pos, '\t'); // personident has no
+															// \t
+			if (p0 == -1) {
+				throw new IllegalArgumentException(
+						"Raw log message does not parse as log entry");
+			}
+			int p1 = RawParseUtils.nextLF(raw, p0);
+			if (p1 == -1) {
+				throw new IllegalArgumentException(
+						"Raw log message does not parse as log entry");
+			}
+			comment = RawParseUtils.decode(raw, p0, p1 - 1);
+		}
+
+		/**
+		 * @return the commit id before the change
+		 */
+		public ObjectId getOldId() {
+			return oldId;
+		}
+
+		/**
+		 * @return the commit id after the change
+		 */
+		public ObjectId getNewId() {
+			return newId;
+		}
+
+		/**
+		 * @return user performin the change
+		 */
+		public PersonIdent getWho() {
+			return who;
+		}
+
+		/**
+		 * @return textual description of the change
+		 */
+		public String getComment() {
+			return comment;
+		}
+		
+		@Override
+		public String toString() {
+			return "Entry[" + oldId.name() + ", " + newId.name() + ", " + getWho() + ", "
+					+ getComment() + "]";
+		}
+	}
+
+	private File logName;
+
+	ReflogReader(Repository db, String refname) {
+		logName = new File(db.getDirectory(), "logs/" + refname);
+	}
+
+	/**
+	 * Get the last entry in the reflog
+	 * 
+	 * @return the latest reflog entry, or null if no log
+	 * @throws IOException
+	 */
+	public Entry getLastEntry() throws IOException {
+		List<Entry> entries = getReverseEntries(1);
+		return entries.size() > 0 ? entries.get(0) : null;
+	}
+
+	/**
+	 * @return all reflog entries in reverse order
+	 * @throws IOException
+	 */
+	public List<Entry> getReverseEntries() throws IOException {
+		return getReverseEntries(Integer.MAX_VALUE);
+	}
+
+	/**
+	 * @param max
+	 *            max numer of entries to read
+	 * @return all reflog entries in reverse order
+	 * @throws IOException
+	 */
+	public List<Entry> getReverseEntries(int max) throws IOException {
+		FileInputStream fileInputStream;
+		try {
+			fileInputStream = new FileInputStream(logName);
+		} catch (FileNotFoundException e) {
+			return Collections.emptyList();
+		}
+		try {
+			long logSize = fileInputStream.getChannel().size();
+			if (logSize > Integer.MAX_VALUE) {
+				// implementation limit, will suck with smaller files too
+				throw new IOException("Cannot handle reflog larger than "
+						+ Integer.MAX_VALUE + " bytes");
+			}
+			byte[] log = new byte[(int) logSize];
+			NB.readFully(fileInputStream, log, 0, log.length);
+			int rs = RawParseUtils.prevLF(log, log.length);
+			List<Entry> ret = new ArrayList<Entry>();
+			while (rs >= 0 && max-- > 0) {
+				rs = RawParseUtils.prevLF(log, rs);
+				Entry entry = new Entry(log, rs < 0 ? 0 : rs + 2);
+				ret.add(entry);
+			}
+			return ret;
+		} finally {
+			fileInputStream.close();
+		}
+	}
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
index 5def5d3..971215c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
@@ -1089,4 +1089,18 @@ public String shortenRefName(String refName) {
 			return refName.substring(Constants.R_REMOTES.length());
 		return refName;
 	}
+
+	/**
+	 * @param refName
+	 * @return a {@link ReflogReader} for the refname, or null if the names ref
+	 *         does not exist.
+	 * @throws IOException 
+	 */
+	public ReflogReader getReflogReader(String refName) throws IOException {
+		Ref ref = getRef(refName);
+		if (ref != null)
+			return new ReflogReader(this, ref.getOrigName());
+		else
+			return null;
+	}
 }
-- 
1.6.3.2.199.g7340d

[EGIT PATCH v5 3/7] Do not write to the reflog unless the refupdate logmessage is set

[Robin Rosenberg]

From: Shawn O. Pearce <spearce@spearce.org>

This permits micro-update steps (or otherwise uninteresting states)
to be skipped in the reflog.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../tst/org/spearce/jgit/lib/RefUpdateTest.java    |    1 +
 .../src/org/spearce/jgit/lib/RefUpdate.java        |   56 +++++++++++++++----
 2 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
index 6b1975a..84653c8 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
@@ -126,6 +126,7 @@ public void testDeleteLooseAndItsDirectory() throws IOException {
 		RefUpdate updateRef = db.updateRef("refs/heads/z/c");
 		updateRef.setNewObjectId(pid);
 		updateRef.setForceUpdate(true);
+		updateRef.setRefLogMessage("new test ref", false);
 		Result update = updateRef.update();
 		assertEquals(Result.NEW, update); // internal
 		assertTrue(new File(db.getDirectory(), Constants.R_HEADS + "z")
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
index a9ab73b..8044c6e 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
@@ -165,6 +165,7 @@ RefUpdate(final RefDatabase r, final Ref ref, final File f) {
 		this.ref = ref;
 		oldValue = ref.getObjectId();
 		looseFile = f;
+		refLogMessage = "";
 	}
 
 	/** @return the repository the updated ref resides in */
@@ -264,7 +265,8 @@ public void setRefLogIdent(final PersonIdent pi) {
 	/**
 	 * Get the message to include in the reflog.
 	 * 
-	 * @return message the caller wants to include in the reflog.
+	 * @return message the caller wants to include in the reflog; null if the
+	 *         update should not be logged.
 	 */
 	public String getRefLogMessage() {
 		return refLogMessage;
@@ -274,15 +276,29 @@ public String getRefLogMessage() {
 	 * Set the message to include in the reflog.
 	 * 
 	 * @param msg
-	 *            the message to describe this change.
+	 *            the message to describe this change. It may be null
+	 *            if appendStatus is null in order not to append to the reflog
 	 * @param appendStatus
 	 *            true if the status of the ref change (fast-forward or
 	 *            forced-update) should be appended to the user supplied
 	 *            message.
 	 */
 	public void setRefLogMessage(final String msg, final boolean appendStatus) {
-		refLogMessage = msg;
-		refLogIncludeResult = appendStatus;
+		if (msg == null && !appendStatus)
+			disableRefLog();
+		else if (msg == null && appendStatus) {
+			refLogMessage = "";
+			refLogIncludeResult = true;
+		} else {
+			refLogMessage = msg;
+			refLogIncludeResult = appendStatus;
+		}
+	}
+
+	/** Don't record this update in the ref's associated reflog. */
+	public void disableRefLog() {
+		refLogMessage = null;
+		refLogIncludeResult = false;
 	}
 
 	/**
@@ -471,21 +487,37 @@ private Result updateStore(final LockFile lock, final Result status)
 		lock.setNeedStatInformation(true);
 		lock.write(newValue);
 		String msg = getRefLogMessage();
-		if (msg != null && refLogIncludeResult) {
-			if (status == Result.FORCED)
-				msg += ": forced-update";
-			else if (status == Result.FAST_FORWARD)
-				msg += ": fast forward";
-			else if (status == Result.NEW)
-				msg += ": created";
+		if (msg != null) {
+			if (refLogIncludeResult) {
+				String strResult = toResultString(status);
+				if (strResult != null) {
+					if (msg.length() > 0)
+						msg = msg + ": " + strResult;
+					else
+						msg = strResult;
+				}
+			}
+			RefLogWriter.append(this, msg);
 		}
-		RefLogWriter.append(this, msg);
 		if (!lock.commit())
 			return Result.LOCK_FAILURE;
 		db.stored(this.ref.getOrigName(),  ref.getName(), newValue, lock.getCommitLastModified());
 		return status;
 	}
 
+	private static String toResultString(final Result status) {
+		switch (status) {
+		case FORCED:
+			return "forced-update";
+		case FAST_FORWARD:
+			return "fast forward";
+		case NEW:
+			return "created";
+		default:
+			return null;
+		}
+	}
+
 	/**
 	 * Handle the abstraction of storing a ref update. This is because both
 	 * updating and deleting of a ref have merge testing in common.
-- 
1.6.3.2.199.g7340d

[EGIT PATCH v5 4/7] Add ref rename support to JGit

[Robin Rosenberg]

Now refs can be renamed. The intent is that should be safe. Only the named
refs and associated logs are updated. Any symbolic refs referring to the renames
branches are unaffected, except HEAD, which is updated if the branch it refers
to is being renamed.

In order to perform operations like rename from a to a/b and vice verse, we
perform the rename similar to the way C git does it, i.e. one ref update at
a time using the same temporary intermediate names as the C version does.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../org/spearce/jgit/test/resources/packed-refs    |    1 +
 .../tst/org/spearce/jgit/lib/RefUpdateTest.java    |  347 +++++++++++++++++++-
 .../org/spearce/jgit/transport/TransportTest.java  |    2 +-
 .../src/org/spearce/jgit/lib/RefDatabase.java      |   31 ++
 .../src/org/spearce/jgit/lib/RefLogWriter.java     |   33 ++-
 .../src/org/spearce/jgit/lib/RefRename.java        |  163 +++++++++
 .../src/org/spearce/jgit/lib/RefUpdate.java        |   59 +++-
 .../src/org/spearce/jgit/lib/Repository.java       |   16 +
 8 files changed, 625 insertions(+), 27 deletions(-)
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/RefRename.java

diff --git a/org.spearce.jgit.test/tst-rsrc/org/spearce/jgit/test/resources/packed-refs b/org.spearce.jgit.test/tst-rsrc/org/spearce/jgit/test/resources/packed-refs
index 38a70ec..a6a14f2 100644
--- a/org.spearce.jgit.test/tst-rsrc/org/spearce/jgit/test/resources/packed-refs
+++ b/org.spearce.jgit.test/tst-rsrc/org/spearce/jgit/test/resources/packed-refs
@@ -6,6 +6,7 @@ f73b95671f326616d66b2afb3bdfcdbbce110b44 refs/heads/d
 d0114ab8ac326bab30e3a657a0397578c5a1af88 refs/heads/e
 47d3697c3747e8184e0dc479ccbd01e359023577 refs/heads/f
 175d5b80bd9768884d8fced02e9bd33488174396 refs/heads/g
+175d5b80bd9768884d8fced02e9bd33488174396 refs/heads/prefix/a
 49322bb17d3acc9146f98c97d078513228bbf3c0 refs/heads/master
 d86a2aada2f5e7ccf6f11880bfb9ab404e8a8864 refs/heads/pa
 6db9c2ebf75590eef973081736730a9ea169a0c4 refs/tags/A
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
index 84653c8..c505b79 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RefUpdateTest.java
@@ -39,6 +39,7 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 
@@ -83,9 +84,75 @@ public void testNoCacheObjectIdSubclass() throws IOException {
 		assertEquals(newid.copy(), r.getObjectId());
 	}
 
+	public void testNewNamespaceConflictWithLoosePrefixNameExists()
+			throws IOException {
+		final String newRef = "refs/heads/z";
+		final RefUpdate ru = updateRef(newRef);
+		final RevCommit newid = new RevCommit(ru.getNewObjectId()) {
+			// empty
+		};
+		ru.setNewObjectId(newid);
+		Result update = ru.update();
+		assertEquals(Result.NEW, update);
+		// end setup
+		final String newRef2 = "refs/heads/z/a";
+		final RefUpdate ru2 = updateRef(newRef2);
+		final RevCommit newid2 = new RevCommit(ru2.getNewObjectId()) {
+			// empty
+		};
+		ru.setNewObjectId(newid2);
+		Result update2 = ru2.update();
+		assertEquals(Result.LOCK_FAILURE, update2);
+	}
+
+	public void testNewNamespaceConflictWithPackedPrefixNameExists()
+			throws IOException {
+		final String newRef = "refs/heads/master/x";
+		final RefUpdate ru = updateRef(newRef);
+		final RevCommit newid = new RevCommit(ru.getNewObjectId()) {
+			// empty
+		};
+		ru.setNewObjectId(newid);
+		Result update = ru.update();
+		assertEquals(Result.LOCK_FAILURE, update);
+	}
+
+	public void testNewNamespaceConflictWithLoosePrefixOfExisting()
+			throws IOException {
+		final String newRef = "refs/heads/z/a";
+		final RefUpdate ru = updateRef(newRef);
+		final RevCommit newid = new RevCommit(ru.getNewObjectId()) {
+			// empty
+		};
+		ru.setNewObjectId(newid);
+		Result update = ru.update();
+		assertEquals(Result.NEW, update);
+		// end setup
+		final String newRef2 = "refs/heads/z";
+		final RefUpdate ru2 = updateRef(newRef2);
+		final RevCommit newid2 = new RevCommit(ru2.getNewObjectId()) {
+			// empty
+		};
+		ru.setNewObjectId(newid2);
+		Result update2 = ru2.update();
+		assertEquals(Result.LOCK_FAILURE, update2);
+	}
+
+	public void testNewNamespaceConflictWithPackedPrefixOfExisting()
+			throws IOException {
+		final String newRef = "refs/heads/prefix";
+		final RefUpdate ru = updateRef(newRef);
+		final RevCommit newid = new RevCommit(ru.getNewObjectId()) {
+			// empty
+		};
+		ru.setNewObjectId(newid);
+		Result update = ru.update();
+		assertEquals(Result.LOCK_FAILURE, update);
+	}
+
 	/**
 	 * Delete a ref that is pointed to by HEAD
-	 *
+	 * 
 	 * @throws IOException
 	 */
 	public void testDeleteHEADreferencedRef() throws IOException {
@@ -118,7 +185,7 @@ public void testDeleteHead() throws IOException {
 	/**
 	 * Delete a loose ref and make sure the directory in refs is deleted too,
 	 * and the reflog dir too
-	 *
+	 * 
 	 * @throws IOException
 	 */
 	public void testDeleteLooseAndItsDirectory() throws IOException {
@@ -172,7 +239,7 @@ public void testRefKeySameAsOrigName() {
 
 	/**
 	 * Try modify a ref forward, fast forward
-	 *
+	 * 
 	 * @throws IOException
 	 */
 	public void testUpdateRefForward() throws IOException {
@@ -197,7 +264,7 @@ public void testUpdateRefForward() throws IOException {
 	/**
 	 * Delete a ref that exists both as packed and loose. Make sure the ref
 	 * cannot be resolved after delete.
-	 *
+	 * 
 	 * @throws IOException
 	 */
 	public void testDeleteLoosePacked() throws IOException {
@@ -218,7 +285,7 @@ public void testDeleteLoosePacked() throws IOException {
 
 	/**
 	 * Try modify a ref to same
-	 *
+	 * 
 	 * @throws IOException
 	 */
 	public void testUpdateRefNoChange() throws IOException {
@@ -232,7 +299,7 @@ public void testUpdateRefNoChange() throws IOException {
 
 	/**
 	 * Try modify a ref, but get wrong expected old value
-	 *
+	 * 
 	 * @throws IOException
 	 */
 	public void testUpdateRefLockFailureWrongOldValue() throws IOException {
@@ -247,7 +314,7 @@ public void testUpdateRefLockFailureWrongOldValue() throws IOException {
 
 	/**
 	 * Try modify a ref that is locked
-	 *
+	 * 
 	 * @throws IOException
 	 */
 	public void testUpdateRefLockFailureLocked() throws IOException {
@@ -255,18 +322,20 @@ public void testUpdateRefLockFailureLocked() throws IOException {
 		ObjectId pid = db.resolve("refs/heads/master^");
 		RefUpdate updateRef = db.updateRef("refs/heads/master");
 		updateRef.setNewObjectId(pid);
-		LockFile lockFile1 = new LockFile(new File(db.getDirectory(),"refs/heads/master"));
+		LockFile lockFile1 = new LockFile(new File(db.getDirectory(),
+				"refs/heads/master"));
 		assertTrue(lockFile1.lock()); // precondition to test
 		Result update = updateRef.update();
 		assertEquals(Result.LOCK_FAILURE, update);
 		assertEquals(opid, db.resolve("refs/heads/master"));
-		LockFile lockFile2 = new LockFile(new File(db.getDirectory(),"refs/heads/master"));
+		LockFile lockFile2 = new LockFile(new File(db.getDirectory(),
+				"refs/heads/master"));
 		assertFalse(lockFile2.lock()); // was locked, still is
 	}
 
 	/**
 	 * Try to delete a ref. Delete requires force.
-	 *
+	 * 
 	 * @throws IOException
 	 */
 	public void testDeleteLoosePackedRejected() throws IOException {
@@ -279,4 +348,262 @@ public void testDeleteLoosePackedRejected() throws IOException {
 		assertEquals(oldpid, db.resolve("refs/heads/c"));
 	}
 
+	public void testRenameBranchNoPreviousLog() throws IOException {
+		assertFalse("precondition, no log on old branchg", new File(db
+				.getDirectory(), "logs/refs/heads/b").exists());
+		ObjectId rb = db.resolve("refs/heads/b");
+		ObjectId oldHead = db.resolve(Constants.HEAD);
+		assertFalse(rb.equals(oldHead)); // assumption for this test
+		RefRename renameRef = db.renameRef("refs/heads/b",
+				"refs/heads/new/name");
+		Result result = renameRef.rename();
+		assertEquals(Result.RENAMED, result);
+		assertEquals(rb, db.resolve("refs/heads/new/name"));
+		assertNull(db.resolve("refs/heads/b"));
+		assertEquals(1, db.getReflogReader("new/name").getReverseEntries().size());
+		assertEquals("Branch: renamed b to new/name", db.getReflogReader("new/name")
+				.getLastEntry().getComment());
+		assertFalse(new File(db.getDirectory(), "logs/refs/heads/b").exists());
+		assertEquals(oldHead, db.resolve(Constants.HEAD)); // unchanged
+	}
+
+	public void testRenameBranchHasPreviousLog() throws IOException {
+		ObjectId rb = db.resolve("refs/heads/b");
+		ObjectId oldHead = db.resolve(Constants.HEAD);
+		assertFalse("precondition for this test, branch b != HEAD", rb
+				.equals(oldHead));
+		RefLogWriter.writeReflog(db, rb, rb, "Just a message", "refs/heads/b");
+		assertTrue("no log on old branch", new File(db.getDirectory(),
+				"logs/refs/heads/b").exists());
+		RefRename renameRef = db.renameRef("refs/heads/b",
+				"refs/heads/new/name");
+		Result result = renameRef.rename();
+		assertEquals(Result.RENAMED, result);
+		assertEquals(rb, db.resolve("refs/heads/new/name"));
+		assertNull(db.resolve("refs/heads/b"));
+		assertEquals(2, db.getReflogReader("new/name").getReverseEntries().size());
+		assertEquals("Branch: renamed b to new/name", db.getReflogReader("new/name")
+				.getLastEntry().getComment());
+		assertEquals("Just a message", db.getReflogReader("new/name")
+				.getReverseEntries().get(1).getComment());
+		assertFalse(new File(db.getDirectory(), "logs/refs/heads/b").exists());
+		assertEquals(oldHead, db.resolve(Constants.HEAD)); // unchanged
+	}
+
+	public void testRenameCurrentBranch() throws IOException {
+		ObjectId rb = db.resolve("refs/heads/b");
+		db.writeSymref(Constants.HEAD, "refs/heads/b");
+		ObjectId oldHead = db.resolve(Constants.HEAD);
+		assertTrue("internal test condition, b == HEAD", rb.equals(oldHead));
+		RefLogWriter.writeReflog(db, rb, rb, "Just a message", "refs/heads/b");
+		assertTrue("no log on old branch", new File(db.getDirectory(),
+				"logs/refs/heads/b").exists());
+		RefRename renameRef = db.renameRef("refs/heads/b",
+				"refs/heads/new/name");
+		Result result = renameRef.rename();
+		assertEquals(Result.RENAMED, result);
+		assertEquals(rb, db.resolve("refs/heads/new/name"));
+		assertNull(db.resolve("refs/heads/b"));
+		assertEquals("Branch: renamed b to new/name", db.getReflogReader(
+				"new/name").getLastEntry().getComment());
+		assertFalse(new File(db.getDirectory(), "logs/refs/heads/b").exists());
+		assertEquals(rb, db.resolve(Constants.HEAD));
+		assertEquals(2, db.getReflogReader("new/name").getReverseEntries().size());
+		assertEquals("Branch: renamed b to new/name", db.getReflogReader("new/name").getReverseEntries().get(0).getComment());
+		assertEquals("Just a message", db.getReflogReader("new/name").getReverseEntries().get(1).getComment());
+	}
+
+	public void testRenameBranchAlsoInPack() throws IOException {
+		ObjectId rb = db.resolve("refs/heads/b");
+		ObjectId rb2 = db.resolve("refs/heads/b~1");
+		assertEquals(Ref.Storage.PACKED, db.getRef("refs/heads/b").getStorage());
+		RefUpdate updateRef = db.updateRef("refs/heads/b");
+		updateRef.setNewObjectId(rb2);
+		updateRef.setForceUpdate(true);
+		Result update = updateRef.update();
+		assertEquals("internal check new ref is loose", Result.FORCED, update);
+		assertEquals(Ref.Storage.LOOSE_PACKED, db.getRef("refs/heads/b")
+				.getStorage());
+		RefLogWriter.writeReflog(db, rb, rb, "Just a message", "refs/heads/b");
+		assertTrue("no log on old branch", new File(db.getDirectory(),
+				"logs/refs/heads/b").exists());
+		RefRename renameRef = db.renameRef("refs/heads/b",
+				"refs/heads/new/name");
+		Result result = renameRef.rename();
+		assertEquals(Result.RENAMED, result);
+		assertEquals(rb2, db.resolve("refs/heads/new/name"));
+		assertNull(db.resolve("refs/heads/b"));
+		assertEquals("Branch: renamed b to new/name", db.getReflogReader(
+				"new/name").getLastEntry().getComment());
+		assertFalse(new File(db.getDirectory(), "logs/refs/heads/b").exists());
+
+		// Create new Repository instance, to reread caches and make sure our
+		// assumptions are persistent.
+		Repository ndb = new Repository(db.getDirectory());
+		assertEquals(rb2, ndb.resolve("refs/heads/new/name"));
+		assertNull(ndb.resolve("refs/heads/b"));
+	}
+
+	public void tryRenameWhenLocked(String toLock, String fromName,
+			String toName, String headPointsTo) throws IOException {
+		// setup
+		db.writeSymref(Constants.HEAD, headPointsTo);
+		ObjectId oldfromId = db.resolve(fromName);
+		ObjectId oldHeadId = db.resolve(Constants.HEAD);
+		RefLogWriter.writeReflog(db, oldfromId, oldfromId, "Just a message",
+				fromName);
+		List<org.spearce.jgit.lib.ReflogReader.Entry> oldFromLog = db
+				.getReflogReader(fromName).getReverseEntries();
+		List<org.spearce.jgit.lib.ReflogReader.Entry> oldHeadLog = oldHeadId != null ? db
+				.getReflogReader(Constants.HEAD).getReverseEntries() : null;
+
+		assertTrue("internal check, we have a log", new File(db.getDirectory(),
+				"logs/" + fromName).exists());
+
+		// "someone" has branch X locked
+		assertTrue(new LockFile(new File(db.getDirectory(), toLock)).lock());
+
+		// Now this is our test
+		RefRename renameRef = db.renameRef(fromName, toName);
+		Result result = renameRef.rename();
+		assertEquals(Result.LOCK_FAILURE, result);
+
+		// Check that the involved refs are the same despite the failure
+		assertExists(false, toName);
+		if (!toLock.equals(toName))
+			assertExists(false, toName + ".lock");
+		assertExists(true, toLock + ".lock");
+		if (!toLock.equals(fromName))
+			assertExists(false, "logs/" + fromName + ".lock");
+		assertExists(false, "logs/" + toName + ".lock");
+		assertEquals(oldHeadId, db.resolve(Constants.HEAD));
+		assertEquals(oldfromId, db.resolve(fromName));
+		assertNull(db.resolve(toName));
+		assertEquals(oldFromLog.toString(), db.getReflogReader(fromName)
+				.getReverseEntries().toString());
+		if (oldHeadId != null)
+			assertEquals(oldHeadLog, db.getReflogReader(Constants.HEAD)
+					.getReverseEntries());
+	}
+
+	private void assertExists(boolean positive, String toName) {
+		assertEquals(toName + (positive ? " " : " does not ") + "exist",
+				positive, new File(db.getDirectory(), toName).exists());
+	}
+
+	public void testRenameBranchCannotLockAFileHEADisFromLockHEAD()
+			throws IOException {
+		tryRenameWhenLocked("HEAD", "refs/heads/b", "refs/heads/new/name",
+				"refs/heads/b");
+	}
+
+	public void testRenameBranchCannotLockAFileHEADisFromLockFrom()
+			throws IOException {
+		tryRenameWhenLocked("refs/heads/b", "refs/heads/b",
+				"refs/heads/new/name", "refs/heads/b");
+	}
+
+	public void testRenameBranchCannotLockAFileHEADisFromLockTo()
+			throws IOException {
+		tryRenameWhenLocked("refs/heads/new/name", "refs/heads/b",
+				"refs/heads/new/name", "refs/heads/b");
+	}
+
+	public void testRenameBranchCannotLockAFileHEADisToLockFrom()
+			throws IOException {
+		tryRenameWhenLocked("refs/heads/b", "refs/heads/b",
+				"refs/heads/new/name", "refs/heads/new/name");
+	}
+
+	public void testRenameBranchCannotLockAFileHEADisToLockTo()
+			throws IOException {
+		tryRenameWhenLocked("refs/heads/new/name", "refs/heads/b",
+				"refs/heads/new/name", "refs/heads/new/name");
+	}
+
+	public void testRenameBranchCannotLockAFileHEADisToLockTmp()
+			throws IOException {
+		tryRenameWhenLocked("RENAMED-REF.." + Thread.currentThread().getId(),
+				"refs/heads/b", "refs/heads/new/name", "refs/heads/new/name");
+	}
+
+	public void testRenameBranchCannotLockAFileHEADisOtherLockFrom()
+			throws IOException {
+		tryRenameWhenLocked("refs/heads/b", "refs/heads/b",
+				"refs/heads/new/name", "refs/heads/a");
+	}
+
+	public void testRenameBranchCannotLockAFileHEADisOtherLockTo()
+			throws IOException {
+		tryRenameWhenLocked("refs/heads/new/name", "refs/heads/b",
+				"refs/heads/new/name", "refs/heads/a");
+	}
+
+	public void testRenameBranchCannotLockAFileHEADisOtherLockTmp()
+			throws IOException {
+		tryRenameWhenLocked("RENAMED-REF.." + Thread.currentThread().getId(),
+				"refs/heads/b", "refs/heads/new/name", "refs/heads/a");
+	}
+
+	public void testRenameRefNameColission1avoided() throws IOException {
+		// setup
+		ObjectId rb = db.resolve("refs/heads/b");
+		db.writeSymref(Constants.HEAD, "refs/heads/a");
+		RefUpdate updateRef = db.updateRef("refs/heads/a");
+		updateRef.setNewObjectId(rb);
+		updateRef.setRefLogMessage("Setup", false);
+		assertEquals(Result.FAST_FORWARD, updateRef.update());
+		ObjectId oldHead = db.resolve(Constants.HEAD);
+		assertTrue(rb.equals(oldHead)); // assumption for this test
+		RefLogWriter.writeReflog(db, rb, rb, "Just a message", "refs/heads/a");
+		assertTrue("internal check, we have a log", new File(db.getDirectory(),
+				"logs/refs/heads/a").exists());
+
+		// Now this is our test
+		RefRename renameRef = db.renameRef("refs/heads/a", "refs/heads/a/b");
+		Result result = renameRef.rename();
+		assertEquals(Result.RENAMED, result);
+		assertNull(db.resolve("refs/heads/a"));
+		assertEquals(rb, db.resolve("refs/heads/a/b"));
+		assertEquals(3, db.getReflogReader("a/b").getReverseEntries().size());
+		assertEquals("Branch: renamed a to a/b", db.getReflogReader("a/b")
+				.getReverseEntries().get(0).getComment());
+		assertEquals("Just a message", db.getReflogReader("a/b")
+				.getReverseEntries().get(1).getComment());
+		assertEquals("Setup", db.getReflogReader("a/b").getReverseEntries()
+				.get(2).getComment());
+	}
+
+	public void testRenameRefNameColission2avoided() throws IOException {
+		// setup
+		ObjectId rb = db.resolve("refs/heads/b");
+		db.writeSymref(Constants.HEAD, "refs/heads/prefix/a");
+		RefUpdate updateRef = db.updateRef("refs/heads/prefix/a");
+		updateRef.setNewObjectId(rb);
+		updateRef.setRefLogMessage("Setup", false);
+		updateRef.setForceUpdate(true);
+		assertEquals(Result.FORCED, updateRef.update());
+		ObjectId oldHead = db.resolve(Constants.HEAD);
+		assertTrue(rb.equals(oldHead)); // assumption for this test
+		RefLogWriter.writeReflog(db, rb, rb, "Just a message",
+				"refs/heads/prefix/a");
+		assertTrue("internal check, we have a log", new File(db.getDirectory(),
+				"logs/refs/heads/prefix/a").exists());
+
+		// Now this is our test
+		RefRename renameRef = db.renameRef("refs/heads/prefix/a",
+				"refs/heads/prefix");
+		Result result = renameRef.rename();
+		assertEquals(Result.RENAMED, result);
+
+		assertNull(db.resolve("refs/heads/prefix/a"));
+		assertEquals(rb, db.resolve("refs/heads/prefix"));
+		assertEquals(3, db.getReflogReader("prefix").getReverseEntries().size());
+		assertEquals("Branch: renamed prefix/a to prefix", db.getReflogReader(
+				"prefix").getReverseEntries().get(0).getComment());
+		assertEquals("Just a message", db.getReflogReader("prefix")
+				.getReverseEntries().get(1).getComment());
+		assertEquals("Setup", db.getReflogReader("prefix").getReverseEntries()
+				.get(2).getComment());
+	}
 }
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/transport/TransportTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/transport/TransportTest.java
index c6e3335..fcf3f5c 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/transport/TransportTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/transport/TransportTest.java
@@ -123,7 +123,7 @@ public void testFindRemoteRefUpdatesWildcardNoTracking() throws IOException {
 				.findRemoteRefUpdatesFor(Collections.nCopies(1, new RefSpec(
 						"+refs/heads/*:refs/heads/test/*")));
 
-		assertEquals(9, result.size());
+		assertEquals(10, result.size());
 		boolean foundA = false;
 		boolean foundB = false;
 		for (final RemoteRefUpdate rru : result) {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
index 869d012..6d4f374 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
@@ -146,6 +146,23 @@ synchronized (this) {
 	}
 
 	/**
+	 * An set of update operations for renaming a ref
+	 *
+	 * @param fromRef Old ref name
+	 * @param toRef New ref name
+	 * @return a RefUpdate operation to rename a ref
+	 * @throws IOException
+	 */
+	RefRename newRename(String fromRef, String toRef) throws IOException {
+		refreshPackedRefs();
+		Ref f = readRefBasic(fromRef, 0);
+		Ref t = new Ref(Ref.Storage.NEW, toRef, null);
+		RefUpdate refUpdateFrom = new RefUpdate(this, f, fileForRef(f.getName()));
+		RefUpdate refUpdateTo = new RefUpdate(this, t, fileForRef(t.getName()));
+		return new RefRename(refUpdateTo, refUpdateFrom);
+	}
+
+	/**
 	 * Writes a symref (e.g. HEAD) to disk
 	 * 
 	 * @param name
@@ -158,11 +175,25 @@ void link(final String name, final String target) throws IOException {
 		final byte[] content = Constants.encode("ref: " + target + "\n");
 		lockAndWriteFile(fileForRef(name), content);
 		synchronized (this) {
+			looseSymRefs.remove(name);
 			setModified();
 		}
 		db.fireRefsMaybeChanged();
 	}
 
+	void uncacheSymRef(String name) {
+		synchronized(this) {
+			looseSymRefs.remove(name);
+			setModified();
+		}
+	}
+
+	void uncacheRef(String name) {
+		looseRefs.remove(name);
+		looseRefsMTime.remove(name);
+		packedRefs.remove(name);
+	}
+
 	private void setModified() {
 		lastRefModification = refModificationCounter++;
 	}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java
index a077051..0864209 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefLogWriter.java
@@ -44,7 +44,7 @@
 import java.io.IOException;
 
 /**
- * Utility class to add reflog entries
+ * Utility class to work with reflog files
  * 
  * @author Dave Watson
  */
@@ -58,6 +58,37 @@ static void append(final RefUpdate u, final String msg) throws IOException {
 		appendOneRecord(oldId, newId, ident, msg, db, u.getName());
 	}
 
+	static void append(RefRename refRename, String msg) throws IOException {
+		final ObjectId id = refRename.getObjectId();
+		final Repository db = refRename.getRepository();
+		final PersonIdent ident = refRename.getRefLogIdent();
+		appendOneRecord(id, id, ident, msg, db, refRename.getToName());
+	}
+
+	static void renameTo(final Repository db, final RefUpdate from,
+			final RefUpdate to) throws IOException {
+		final File logdir = new File(db.getDirectory(), Constants.LOGS);
+		final File reflogFrom = new File(logdir, from.getName());
+		if (!reflogFrom.exists())
+			return;
+		final File reflogTo = new File(logdir, to.getName());
+		final File reflogToDir = reflogTo.getParentFile();
+		File tmp = new File(logdir, "tmp-renamed-log.." + Thread.currentThread().getId());
+		if (!reflogFrom.renameTo(tmp)) {
+			throw new IOException("Cannot rename " + reflogFrom + " to (" + tmp
+					+ ")" + reflogTo);
+		}
+		RefUpdate.deleteEmptyDir(reflogFrom, RefUpdate.count(from.getName(),
+				'/'));
+		if (!reflogToDir.exists() && !reflogToDir.mkdirs()) {
+			throw new IOException("Cannot create directory " + reflogToDir);
+		}
+		if (!tmp.renameTo(reflogTo)) {
+			throw new IOException("Cannot rename (" + tmp + ")" + reflogFrom
+					+ " to " + reflogTo);
+		}
+	}
+
 	private static void appendOneRecord(final ObjectId oldId,
 			final ObjectId newId, PersonIdent ident, final String msg,
 			final Repository db, final String refName) throws IOException {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefRename.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefRename.java
new file mode 100644
index 0000000..c89459b
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefRename.java
@@ -0,0 +1,163 @@
+/*
+ * Copyright (C) 2009, Robin Rosenberg
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.spearce.jgit.lib;
+
+import java.io.IOException;
+
+import org.spearce.jgit.lib.RefUpdate.Result;
+
+/**
+ * A RefUpdate combination for renaming a ref
+ */
+public class RefRename {
+	private RefUpdate newToUpdate;
+
+	private RefUpdate oldFromDelete;
+
+	private Result renameResult = Result.NOT_ATTEMPTED;
+
+	RefRename(final RefUpdate toUpdate, final RefUpdate fromUpdate) {
+		newToUpdate = toUpdate;
+		oldFromDelete = fromUpdate;
+	}
+
+	/**
+	 * @return result of rename operation
+	 */
+	public Result getResult() {
+		return renameResult;
+	}
+
+	/**
+	 * @return the result of the new ref update
+	 * @throws IOException
+	 */
+	public Result rename() throws IOException {
+		Ref oldRef = oldFromDelete.db.readRef(Constants.HEAD);
+		boolean renameHEADtoo = oldRef != null
+				&& oldRef.getName().equals(oldFromDelete.getName());
+		Repository db = oldFromDelete.getRepository();
+		try {
+			RefLogWriter.renameTo(db, oldFromDelete,
+					newToUpdate);
+			newToUpdate.setRefLogMessage(null, false);
+			String tmpRefName = "RENAMED-REF.." + Thread.currentThread().getId();
+			RefUpdate tmpUpdateRef = db.updateRef(tmpRefName);
+			if (renameHEADtoo) {
+				try {
+					oldFromDelete.db.link(Constants.HEAD, tmpRefName);
+				} catch (IOException e) {
+					RefLogWriter.renameTo(db,
+							newToUpdate, oldFromDelete);
+					return renameResult = Result.LOCK_FAILURE;
+				}
+			}
+			tmpUpdateRef.setNewObjectId(oldFromDelete.getOldObjectId());
+			tmpUpdateRef.setForceUpdate(true);
+			Result update = tmpUpdateRef.update();
+			if (update != Result.FORCED && update != Result.NEW && update != Result.NO_CHANGE) {
+				RefLogWriter.renameTo(db,
+						newToUpdate, oldFromDelete);
+				if (renameHEADtoo) {
+					oldFromDelete.db.link(Constants.HEAD, oldFromDelete.getName());
+				}
+				return renameResult = update;
+			}
+
+			oldFromDelete.setExpectedOldObjectId(oldFromDelete.getOldObjectId());
+			oldFromDelete.setForceUpdate(true);
+			Result delete = oldFromDelete.delete();
+			if (delete != Result.FORCED) {
+				if (db.getRef(
+						oldFromDelete.getName()) != null) {
+					RefLogWriter.renameTo(db,
+							newToUpdate, oldFromDelete);
+					if (renameHEADtoo) {
+						oldFromDelete.db.link(Constants.HEAD, oldFromDelete
+								.getName());
+					}
+				}
+				return renameResult = delete;
+			}
+
+			newToUpdate.setNewObjectId(tmpUpdateRef.getNewObjectId());
+			Result updateResult = newToUpdate.update();
+			if (updateResult != Result.NEW) {
+				RefLogWriter.renameTo(db, newToUpdate, oldFromDelete);
+				if (renameHEADtoo) {
+					oldFromDelete.db.link(Constants.HEAD, oldFromDelete.getName());
+				}
+				oldFromDelete.setExpectedOldObjectId(null);
+				oldFromDelete.setNewObjectId(oldFromDelete.getOldObjectId());
+				oldFromDelete.setForceUpdate(true);
+				oldFromDelete.setRefLogMessage(null, false);
+				Result undelete = oldFromDelete.update();
+				if (undelete != Result.NEW && undelete != Result.LOCK_FAILURE)
+					return renameResult = Result.IO_FAILURE;
+				return renameResult = Result.LOCK_FAILURE;
+			}
+
+			if (renameHEADtoo) {
+				oldFromDelete.db.link(Constants.HEAD, newToUpdate.getName());
+			} else {
+				db.fireRefsMaybeChanged();
+			}
+			RefLogWriter.append(this, "Branch: renamed "
+					+ db.shortenRefName(oldFromDelete.getName()) + " to "
+					+ db.shortenRefName(newToUpdate.getName()));
+			return renameResult = Result.RENAMED;
+		} catch (RuntimeException e) {
+			throw e;
+		}
+	}
+
+	ObjectId getObjectId() {
+		return oldFromDelete.getOldObjectId();
+	}
+
+	Repository getRepository() {
+		return oldFromDelete.getRepository();
+	}
+
+	PersonIdent getRefLogIdent() {
+		return newToUpdate.getRefLogIdent();
+	}
+
+	String getToName() {
+		return newToUpdate.getName();
+	}
+}
\ No newline at end of file
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
index 8044c6e..f8ecc3c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
@@ -125,14 +125,20 @@
 		 * This kind of error doesn't include {@link #LOCK_FAILURE}, which is a
 		 * different case.
 		 */
-		IO_FAILURE
+		IO_FAILURE,
+
+		/**
+		 * The ref was renamed from another name
+		 * <p>
+		 */
+		RENAMED
 	}
 
 	/** Repository the ref is stored in. */
-	private final RefDatabase db;
+	final RefDatabase db;
 
 	/** Location of the loose file holding the value of this ref. */
-	private final File looseFile;
+	final File looseFile;
 
 	/** New value the caller wants this ref to have. */
 	private ObjectId newValue;
@@ -156,7 +162,7 @@
 	private ObjectId expValue;
 
 	/** Result of the update operation. */
-	private Result result = Result.NOT_ATTEMPTED;
+	Result result = Result.NOT_ATTEMPTED;
 
 	private final Ref ref;
 
@@ -434,6 +440,15 @@ private Result updateImpl(final RevWalk walk, final Store store)
 		RevObject newObj;
 		RevObject oldObj;
 
+		int lastSlash = getName().lastIndexOf('/');
+		if (lastSlash > 0)
+			if (db.getRepository().getRef(getName().substring(0, lastSlash)) != null)
+				return Result.LOCK_FAILURE;
+		String rName = getName() + "/";
+		for (Ref r : db.getAllRefs().values()) {
+			if (r.getName().startsWith(rName))
+				return Result.LOCK_FAILURE;
+		}
 		lock = new LockFile(looseFile);
 		if (!lock.lock())
 			return Result.LOCK_FAILURE;
@@ -527,7 +542,7 @@ abstract Result store(final LockFile lock, final Result status)
 				throws IOException;
 	}
 
-	private class UpdateStore extends Store {
+	class UpdateStore extends Store {
 
 		@Override
 		Result store(final LockFile lock, final Result status)
@@ -536,7 +551,7 @@ Result store(final LockFile lock, final Result status)
 		}
 	}
 
-	private class DeleteStore extends Store {
+	class DeleteStore extends Store {
 
 		@Override
 		Result store(LockFile lock, Result status) throws IOException {
@@ -557,28 +572,42 @@ Result store(LockFile lock, Result status) throws IOException {
 			lock.unlock();
 			if (storage.isLoose())
 				deleteFileAndEmptyDir(looseFile, levels);
+			db.uncacheRef(ref.getName());
 			return status;
 		}
 
 		private void deleteFileAndEmptyDir(final File file, final int depth)
 				throws IOException {
-			if (file.exists()) {
+			if (file.isFile()) {
 				if (!file.delete())
 					throw new IOException("File cannot be deleted: " + file);
-				deleteEmptyDir(file.getParentFile(), depth);
+				File dir = file.getParentFile();
+				for  (int i = 0; i < depth; ++i) {
+					if (!dir.delete())
+						break; // ignore problem here
+					dir = dir.getParentFile();
+				}
 			}
 		}
+	}
 
-		private void deleteEmptyDir(File dir, int depth) {
-			for (; depth > 0 && dir != null; depth--) {
-				if (!dir.delete())
-					break;
-				dir = dir.getParentFile();
-			}
+	UpdateStore newUpdateStore() {
+		return new UpdateStore();
+	}
+
+	DeleteStore newDeleteStore() {
+		return new DeleteStore();
+	}
+
+	static void deleteEmptyDir(File dir, int depth) {
+		for (; depth > 0 && dir != null; depth--) {
+			if (dir.exists() && !dir.delete())
+				break;
+			dir = dir.getParentFile();
 		}
 	}
 
-	private static int count(final String s, final char c) {
+	static int count(final String s, final char c) {
 		int count = 0;
 		for (int p = s.indexOf(c); p >= 0; p = s.indexOf(c, p + 1)) {
 			count++;
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
index 971215c..9b97fdc 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
@@ -468,6 +468,22 @@ public RefUpdate updateRef(final String ref) throws IOException {
 	}
 
 	/**
+	 * Create a command to rename a ref in this repository
+	 *
+	 * @param fromRef
+	 *            name of ref to rename from
+	 * @param toRef
+	 *            name of ref to rename to
+	 * @return an update command that knows how to rename a branch to another.
+	 * @throws IOException
+	 *             the rename could not be performed.
+	 *
+	 */
+	public RefRename renameRef(final String fromRef, final String toRef) throws IOException {
+		return refs.newRename(fromRef, toRef);
+	}
+
+	/**
 	 * Parse a git revision string and return an object id.
 	 *
 	 * Currently supported is combinations of these.
-- 
1.6.3.2.199.g7340d

[EGIT PATCH v5 5/7] Add ref rename support to the branch dialog

[Robin Rosenberg]

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../src/org/spearce/egit/ui/UIText.java            |   12 ++
 .../ui/internal/dialogs/BranchSelectionDialog.java |  106 ++++++++++++++++----
 .../src/org/spearce/egit/ui/uitext.properties      |    5 +
 3 files changed, 103 insertions(+), 20 deletions(-)

diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
index 654e155..aa31a93 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
@@ -893,6 +893,12 @@
 	public static String BranchSelectionDialog_ErrorCouldNotRefresh;
 
 	/** */
+	public static String BranchSelectionDialog_ErrorCouldNotRenameRef;
+
+	/** */
+	public static String BranchSelectionDialog_ErrorCouldNotRenameRef2;
+
+	/** */
 	public static String BranchSelectionDialog_BranchSuffix_Current;
 
 	/** */
@@ -929,6 +935,12 @@
 	public static String BranchSelectionDialog_QuestionNewBranchTitle;
 
 	/** */
+	public static String BranchSelectionDialog_QuestionNewBranchNameTitle;
+
+	/** */
+	public static String BranchSelectionDialog_QuestionNewBranchNameMessage;
+
+	/** */
 	public static String BranchSelectionDialog_QuestionNewBranchMessage;
 
 	/** */
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
index 9aad95b..28105af 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
@@ -48,8 +48,11 @@
 import org.spearce.egit.ui.UIText;
 import org.spearce.jgit.lib.Constants;
 import org.spearce.jgit.lib.ObjectId;
+import org.spearce.jgit.lib.Ref;
+import org.spearce.jgit.lib.RefRename;
 import org.spearce.jgit.lib.RefUpdate;
 import org.spearce.jgit.lib.Repository;
+import org.spearce.jgit.lib.RefUpdate.Result;
 
 /**
  * The branch and reset selection dialog
@@ -277,6 +280,30 @@ private void refNameFromDialog() {
 		}
 	}
 
+	private InputDialog getRefNameInputDialog(String prompt) {
+		InputDialog labelDialog = new InputDialog(
+				getShell(),
+				UIText.BranchSelectionDialog_QuestionNewBranchTitle,
+				prompt,
+				null, new IInputValidator() {
+					public String isValid(String newText) {
+						String testFor = Constants.R_HEADS + newText;
+						try {
+							if (repo.resolve(testFor) != null)
+								return UIText.BranchSelectionDialog_ErrorAlreadyExists;
+						} catch (IOException e1) {
+							Activator.logError(NLS.bind(
+									UIText.BranchSelectionDialog_ErrorCouldNotResolve, testFor), e1);
+						}
+						if (!Repository.isValidRefName(testFor))
+							return UIText.BranchSelectionDialog_ErrorInvalidRefName;
+						return null;
+					}
+				});
+		labelDialog.setBlockOnOpen(true);
+		return labelDialog;
+	}
+
 	@Override
 	protected void createButtonsForButtonBar(Composite parent) {
 		if (!showResetType) {
@@ -284,43 +311,81 @@ protected void createButtonsForButtonBar(Composite parent) {
 			newButton.setFont(JFaceResources.getDialogFont());
 			newButton.setText(UIText.BranchSelectionDialog_NewBranch);
 			((GridLayout)parent.getLayout()).numColumns++;
+			Button renameButton = new Button(parent, SWT.PUSH);
+			renameButton.setText("&Rename");
+			renameButton.addSelectionListener(new SelectionListener() {
+				public void widgetSelected(SelectionEvent e) {
+					// check what ref name the user selected, if any.
+					refNameFromDialog();
+
+					String branchName;
+					if (refName.startsWith(Constants.R_HEADS))
+						branchName = refName.substring(Constants.R_HEADS.length());
+					else
+						branchName = refName;	
+					InputDialog labelDialog = getRefNameInputDialog(NLS
+							.bind(
+									UIText.BranchSelectionDialog_QuestionNewBranchNameMessage,
+									branchName));
+					if (labelDialog.open() == Window.OK) {
+						String newRefName = Constants.R_HEADS + labelDialog.getValue();
+						try {
+							RefRename renameRef = repo.renameRef(refName, newRefName);
+							if (renameRef.rename() != Result.RENAMED) {
+								MessageDialog.openError(getShell(),
+										"Rename failed",
+										NLS.bind(UIText.BranchSelectionDialog_ErrorCouldNotRenameRef,
+												new Object[] { refName, newRefName, renameRef.getResult() }));
+								Activator.logError(NLS.bind(
+										UIText.BranchSelectionDialog_ErrorCouldNotRenameRef2,
+												new Object[] { refName, newRefName }), null);
+							}
+							// FIXME: Update HEAD
+						} catch (IOException e1) {
+							Activator.logError(NLS.bind(
+									UIText.BranchSelectionDialog_ErrorCouldNotRenameRef2,
+															newRefName), e1);
+						}
+						try {
+							branchTree.removeAll();
+							fillTreeWithBranches(newRefName);
+						} catch (IOException e1) {
+							Activator.logError(
+									UIText.BranchSelectionDialog_ErrorCouldNotRefreshBranchList,
+											e1);
+						}
+					}
+				}
+				public void widgetDefaultSelected(SelectionEvent e) {
+					widgetSelected(e);
+				}
+			});
 			newButton.addSelectionListener(new SelectionListener() {
 
 				public void widgetSelected(SelectionEvent e) {
 					// check what ref name the user selected, if any.
 					refNameFromDialog();
 
-					InputDialog labelDialog = new InputDialog(
-							getShell(),
-							UIText.BranchSelectionDialog_QuestionNewBranchTitle,
-							UIText.BranchSelectionDialog_QuestionNewBranchMessage,
-							null, new IInputValidator() {
-								public String isValid(String newText) {
-									String testFor = Constants.R_HEADS + newText;
-									try {
-										if (repo.resolve(testFor) != null)
-											return UIText.BranchSelectionDialog_ErrorAlreadyExists;
-									} catch (IOException e1) {
-										Activator.logError(NLS.bind(
-												UIText.BranchSelectionDialog_ErrorCouldNotResolve, testFor), e1);
-									}
-									if (!Repository.isValidRefName(testFor))
-										return UIText.BranchSelectionDialog_ErrorInvalidRefName;
-									return null;
-								}
-							});
-					labelDialog.setBlockOnOpen(true);
+					InputDialog labelDialog = getRefNameInputDialog(UIText.BranchSelectionDialog_QuestionNewBranchNameMessage);
 					if (labelDialog.open() == Window.OK) {
 						String newRefName = Constants.R_HEADS + labelDialog.getValue();
 						RefUpdate updateRef;
 						try {
 							updateRef = repo.updateRef(newRefName);
+							Ref startRef = repo.getRef(refName);
 							ObjectId startAt;
 							if (refName == null)
 								startAt = repo.resolve(Constants.HEAD);
 							else
 								startAt = repo.resolve(refName);
+							String startBranch;
+							if (startRef != null)
+								startBranch = refName;
+							else
+								startBranch = startAt.name();
+							startBranch = repo.shortenRefName(startBranch);
 							updateRef.setNewObjectId(startAt);
+							updateRef.setRefLogMessage("branch: Created from " + startBranch, false);
 							updateRef.update();
 						} catch (IOException e1) {
 							Activator.logError(NLS.bind(
@@ -338,6 +403,7 @@ public String isValid(String newText) {
 					}
 				}
 
+
 				public void widgetDefaultSelected(SelectionEvent e) {
 					widgetSelected(e);
 				}
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties b/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
index 1d21c81..f19db60 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
@@ -340,6 +340,9 @@ BranchSelectionDialog_ErrorCouldNotRefresh=Could not refresh list of branches
 BranchSelectionDialog_ErrorCouldNotRefreshBranchList=Could not refresh list of branches
 BranchSelectionDialog_ErrorCouldNotResolve=Could not attempt to resolve {0}
 BranchSelectionDialog_ErrorInvalidRefName=Invalid ref name
+BranchSelectionDialog_ErrorCouldNotRenameRef=Failed to rename branch {0} -> {1}, status={2}
+BranchSelectionDialog_ErrorCouldNotRenameRef2=Failed to rename branch {0} -> {1}
+
 BranchSelectionDialog_LocalBranches=Local Branches
 BranchSelectionDialog_NewBranch=&New branch
 BranchSelectionDialog_NoBranchSeletectMessage=You must select a valid ref.
@@ -347,7 +350,9 @@ BranchSelectionDialog_NoBranchSeletectTitle=No branch/tag selected
 BranchSelectionDialog_OkCheckout=&Checkout
 BranchSelectionDialog_OkReset=&Reset
 BranchSelectionDialog_QuestionNewBranchMessage=Enter name of new branch. It will branch from the selected branch. refs/heads/ will be prepended to the name you type
+BranchSelectionDialog_QuestionNewBranchNameMessage=Enter new name of the {0} branch. refs/heads/ will be prepended to the name you type
 BranchSelectionDialog_QuestionNewBranchTitle=New branch
+BranchSelectionDialog_QuestionNewBranchNameTitle=Rename branch
 BranchSelectionDialog_ReallyResetMessage=Resetting will overwrite any changes in your working directory.\n\nDo you wish to continue?
 BranchSelectionDialog_ReallyResetTitle=Really reset?
 BranchSelectionDialog_RemoteBranches=Remote Branches
-- 
1.6.3.2.199.g7340d

[EGIT PATCH v5 6/7] Improve error reporting in the branch dialog

[Robin Rosenberg]

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../src/org/spearce/egit/ui/UIText.java            |    6 ++
 .../ui/internal/dialogs/BranchSelectionDialog.java |   59 ++++++++++++--------
 .../src/org/spearce/egit/ui/uitext.properties      |    2 +
 3 files changed, 43 insertions(+), 24 deletions(-)

diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
index aa31a93..c120327 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
@@ -899,6 +899,12 @@
 	public static String BranchSelectionDialog_ErrorCouldNotRenameRef2;
 
 	/** */
+	public static String BranchSelectionDialog_BranchSelectionDialog_CreateFailedTitle;
+
+	/** */
+	public static String BranchSelectionDialog_BranchSelectionDialog_RenamedFailedTitle;
+
+	/** */
 	public static String BranchSelectionDialog_BranchSuffix_Current;
 
 	/** */
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
index 28105af..43588ae 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
@@ -104,7 +104,7 @@ getShell().setText(
 
 		try {
 			fillTreeWithBranches(null);
-		} catch (IOException e) {
+		} catch (Throwable e) {
 			Activator.logError(UIText.BranchSelectionDialog_ErrorCouldNotRefresh, e);
 		}
 
@@ -294,6 +294,7 @@ public String isValid(String newText) {
 						} catch (IOException e1) {
 							Activator.logError(NLS.bind(
 									UIText.BranchSelectionDialog_ErrorCouldNotResolve, testFor), e1);
+							return e1.getMessage();
 						}
 						if (!Repository.isValidRefName(testFor))
 							return UIText.BranchSelectionDialog_ErrorInvalidRefName;
@@ -332,27 +333,28 @@ public void widgetSelected(SelectionEvent e) {
 						try {
 							RefRename renameRef = repo.renameRef(refName, newRefName);
 							if (renameRef.rename() != Result.RENAMED) {
-								MessageDialog.openError(getShell(),
-										"Rename failed",
-										NLS.bind(UIText.BranchSelectionDialog_ErrorCouldNotRenameRef,
-												new Object[] { refName, newRefName, renameRef.getResult() }));
-								Activator.logError(NLS.bind(
-										UIText.BranchSelectionDialog_ErrorCouldNotRenameRef2,
-												new Object[] { refName, newRefName }), null);
+								reportError(
+										null,
+										UIText.BranchSelectionDialog_BranchSelectionDialog_RenamedFailedTitle,
+										UIText.BranchSelectionDialog_ErrorCouldNotRenameRef,
+										refName, newRefName, renameRef
+												.getResult());
 							}
-							// FIXME: Update HEAD
-						} catch (IOException e1) {
-							Activator.logError(NLS.bind(
-									UIText.BranchSelectionDialog_ErrorCouldNotRenameRef2,
-															newRefName), e1);
+						} catch (Throwable e1) {
+							reportError(
+									e1,
+									UIText.BranchSelectionDialog_BranchSelectionDialog_RenamedFailedTitle,
+									UIText.BranchSelectionDialog_ErrorCouldNotRenameRef,
+									refName, newRefName, e1.getMessage());
 						}
 						try {
 							branchTree.removeAll();
 							fillTreeWithBranches(newRefName);
-						} catch (IOException e1) {
-							Activator.logError(
-									UIText.BranchSelectionDialog_ErrorCouldNotRefreshBranchList,
-											e1);
+						} catch (Throwable e1) {
+							reportError(
+									e1,
+									UIText.BranchSelectionDialog_BranchSelectionDialog_RenamedFailedTitle,
+									UIText.BranchSelectionDialog_ErrorCouldNotRefreshBranchList);
 						}
 					}
 				}
@@ -387,18 +389,20 @@ public void widgetSelected(SelectionEvent e) {
 							updateRef.setNewObjectId(startAt);
 							updateRef.setRefLogMessage("branch: Created from " + startBranch, false);
 							updateRef.update();
-						} catch (IOException e1) {
-							Activator.logError(NLS.bind(
+						} catch (Throwable e1) {
+							reportError(
+									e1,
+									UIText.BranchSelectionDialog_BranchSelectionDialog_CreateFailedTitle,
 									UIText.BranchSelectionDialog_ErrorCouldNotCreateNewRef,
-															newRefName), e1);
+									newRefName);
 						}
 						try {
 							branchTree.removeAll();
 							fillTreeWithBranches(newRefName);
-						} catch (IOException e1) {
-							Activator.logError(
-									UIText.BranchSelectionDialog_ErrorCouldNotRefreshBranchList,
-											e1);
+						} catch (Throwable e1) {
+							reportError(e1,
+									UIText.BranchSelectionDialog_BranchSelectionDialog_CreateFailedTitle,
+									UIText.BranchSelectionDialog_ErrorCouldNotRefreshBranchList);
 						}
 					}
 				}
@@ -419,4 +423,11 @@ createButton(parent, IDialogConstants.OK_ID,
 	protected int getShellStyle() {
 		return super.getShellStyle() | SWT.RESIZE;
 	}
+
+	private void reportError(Throwable e, String title, String message,
+			Object... args) {
+		String msg = NLS.bind(message, args);
+		MessageDialog.openError(getShell(), title, msg);
+		Activator.logError(msg, e);
+	}
 }
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties b/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
index f19db60..93ce3c9 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
@@ -333,6 +333,8 @@ WindowCachePreferencePage_packedGitMMAP=Use virtual memory mapping
 
 BranchSelectionDialog_TitleCheckout=Checkout: {0}
 BranchSelectionDialog_TitleReset=Reset: {0}
+BranchSelectionDialog_BranchSelectionDialog_CreateFailedTitle=New branch creation failed
+BranchSelectionDialog_BranchSelectionDialog_RenamedFailedTitle=Rename failed
 BranchSelectionDialog_BranchSuffix_Current=\ (current)
 BranchSelectionDialog_ErrorAlreadyExists=Already exists
 BranchSelectionDialog_ErrorCouldNotCreateNewRef=Could not create new ref {0}
-- 
1.6.3.2.199.g7340d

[EGIT PATCH v5 7/7] Remove a TAB from the message Egit generates into the reflog on commit

[Robin Rosenberg]

RefUpdate already adds a TAB

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../egit/ui/internal/actions/CommitAction.java     |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/CommitAction.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/CommitAction.java
index 03649c6..8415002 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/CommitAction.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/CommitAction.java
@@ -295,7 +295,7 @@ private String buildReflogMessage(String commitMessage) {
 		if (newlineIndex > 0) {
 			firstLine = commitMessage.substring(0, newlineIndex);
 		}
-		String commitStr = amending ? "\tcommit (amend):" : "\tcommit: ";
+		String commitStr = amending ? "commit (amend):" : "commit: ";
 		String message = commitStr + firstLine;
 		return message;
 	}
-- 
1.6.3.2.199.g7340d

Re: [EGIT PATCH v5 5/7] Add ref rename support to the branch dialog

[Shawn O. Pearce]

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
> index 9aad95b..28105af 100644
> --- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
> +++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
> @@ -284,43 +311,81 @@ protected void createButtonsForButtonBar(Composite parent) {
>  			newButton.setFont(JFaceResources.getDialogFont());
>  			newButton.setText(UIText.BranchSelectionDialog_NewBranch);
>  			((GridLayout)parent.getLayout()).numColumns++;
> +			Button renameButton = new Button(parent, SWT.PUSH);
> +			renameButton.setText("&Rename");

Not localized.

> +							if (renameRef.rename() != Result.RENAMED) {
> +								MessageDialog.openError(getShell(),
> +										"Rename failed",

Not localized.

-- 
Shawn.

[EGIT PATCH v6 5a/7] Warn for unlocalized string literals in ui plugin.

[Robin Rosenberg]

Other changes are Eclipse 3.4 defaults.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../.settings/org.eclipse.jdt.core.prefs           |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/org.spearce.egit.ui/.settings/org.eclipse.jdt.core.prefs b/org.spearce.egit.ui/.settings/org.eclipse.jdt.core.prefs
index 6b39a92..b52927e 100644
--- a/org.spearce.egit.ui/.settings/org.eclipse.jdt.core.prefs
+++ b/org.spearce.egit.ui/.settings/org.eclipse.jdt.core.prefs
@@ -1,4 +1,4 @@
-#Sun Apr 06 22:05:18 CEST 2008
+#Sun Jun 14 18:21:07 CEST 2009
 eclipse.preferences.version=1
 org.eclipse.jdt.core.compiler.codegen.inlineJsrBytecode=enabled
 org.eclipse.jdt.core.compiler.codegen.targetPlatform=1.5
@@ -18,6 +18,7 @@ org.eclipse.jdt.core.compiler.problem.discouragedReference=warning
 org.eclipse.jdt.core.compiler.problem.emptyStatement=warning
 org.eclipse.jdt.core.compiler.problem.enumIdentifier=error
 org.eclipse.jdt.core.compiler.problem.fallthroughCase=warning
+org.eclipse.jdt.core.compiler.problem.fatalOptionalError=enabled
 org.eclipse.jdt.core.compiler.problem.fieldHiding=warning
 org.eclipse.jdt.core.compiler.problem.finalParameterBound=warning
 org.eclipse.jdt.core.compiler.problem.finallyBlockNotCompletingNormally=error
@@ -44,7 +45,7 @@ org.eclipse.jdt.core.compiler.problem.missingOverrideAnnotation=ignore
 org.eclipse.jdt.core.compiler.problem.missingSerialVersion=warning
 org.eclipse.jdt.core.compiler.problem.noEffectAssignment=error
 org.eclipse.jdt.core.compiler.problem.noImplicitStringConversion=error
-org.eclipse.jdt.core.compiler.problem.nonExternalizedStringLiteral=ignore
+org.eclipse.jdt.core.compiler.problem.nonExternalizedStringLiteral=warning
 org.eclipse.jdt.core.compiler.problem.nullReference=error
 org.eclipse.jdt.core.compiler.problem.overridingPackageDefaultMethod=warning
 org.eclipse.jdt.core.compiler.problem.parameterAssignment=warning
@@ -52,24 +53,31 @@ org.eclipse.jdt.core.compiler.problem.possibleAccidentalBooleanAssignment=error
 org.eclipse.jdt.core.compiler.problem.potentialNullReference=warning
 org.eclipse.jdt.core.compiler.problem.rawTypeReference=ignore
 org.eclipse.jdt.core.compiler.problem.redundantNullCheck=warning
+org.eclipse.jdt.core.compiler.problem.redundantSuperinterface=ignore
 org.eclipse.jdt.core.compiler.problem.specialParameterHidingField=disabled
 org.eclipse.jdt.core.compiler.problem.staticAccessReceiver=error
 org.eclipse.jdt.core.compiler.problem.suppressWarnings=enabled
+org.eclipse.jdt.core.compiler.problem.syntheticAccessEmulation=ignore
 org.eclipse.jdt.core.compiler.problem.typeParameterHiding=warning
 org.eclipse.jdt.core.compiler.problem.uncheckedTypeOperation=warning
 org.eclipse.jdt.core.compiler.problem.undocumentedEmptyBlock=warning
 org.eclipse.jdt.core.compiler.problem.unhandledWarningToken=warning
+org.eclipse.jdt.core.compiler.problem.unnecessaryElse=ignore
 org.eclipse.jdt.core.compiler.problem.unnecessaryTypeCheck=error
 org.eclipse.jdt.core.compiler.problem.unqualifiedFieldAccess=ignore
 org.eclipse.jdt.core.compiler.problem.unusedDeclaredThrownException=error
+org.eclipse.jdt.core.compiler.problem.unusedDeclaredThrownExceptionExemptExceptionAndThrowable=enabled
+org.eclipse.jdt.core.compiler.problem.unusedDeclaredThrownExceptionIncludeDocCommentReference=enabled
 org.eclipse.jdt.core.compiler.problem.unusedDeclaredThrownExceptionWhenOverriding=disabled
 org.eclipse.jdt.core.compiler.problem.unusedImport=error
 org.eclipse.jdt.core.compiler.problem.unusedLabel=error
 org.eclipse.jdt.core.compiler.problem.unusedLocal=error
 org.eclipse.jdt.core.compiler.problem.unusedParameter=warning
+org.eclipse.jdt.core.compiler.problem.unusedParameterIncludeDocCommentReference=enabled
 org.eclipse.jdt.core.compiler.problem.unusedParameterWhenImplementingAbstract=disabled
 org.eclipse.jdt.core.compiler.problem.unusedParameterWhenOverridingConcrete=disabled
 org.eclipse.jdt.core.compiler.problem.unusedPrivateMember=error
+org.eclipse.jdt.core.compiler.problem.unusedWarningToken=warning
 org.eclipse.jdt.core.compiler.problem.varargsArgumentNeedCast=error
 org.eclipse.jdt.core.compiler.source=1.5
 org.eclipse.jdt.core.formatter.align_type_members_on_columns=false
-- 
1.6.3.2.199.g7340d

[EGIT PATCH v6 5b/6] Add ref rename support to the branch dialog

[Robin Rosenberg]

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../src/org/spearce/egit/ui/UIText.java            |   16 +++
 .../ui/internal/dialogs/BranchSelectionDialog.java |  108 ++++++++++++++++----
 .../src/org/spearce/egit/ui/uitext.properties      |    7 ++
 3 files changed, 110 insertions(+), 21 deletions(-)

diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
index 654e155..76764ab 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
@@ -893,6 +893,12 @@
 	public static String BranchSelectionDialog_ErrorCouldNotRefresh;
 
 	/** */
+	public static String BranchSelectionDialog_ErrorCouldNotRenameRef;
+
+	/** */
+	public static String BranchSelectionDialog_ErrorCouldNotRenameRef2;
+
+	/** */
 	public static String BranchSelectionDialog_BranchSuffix_Current;
 
 	/** */
@@ -929,6 +935,12 @@
 	public static String BranchSelectionDialog_QuestionNewBranchTitle;
 
 	/** */
+	public static String BranchSelectionDialog_QuestionNewBranchNameTitle;
+
+	/** */
+	public static String BranchSelectionDialog_QuestionNewBranchNameMessage;
+
+	/** */
 	public static String BranchSelectionDialog_QuestionNewBranchMessage;
 
 	/** */
@@ -943,6 +955,8 @@
 	/** */
 	public static String BranchSelectionDialog_ErrorInvalidRefName;
 
+	public static String BranchSelectionDialog_ErrorRenameFailed;
+
 	/** */
 	public static String BranchSelectionDialog_OkCheckout;
 
@@ -952,6 +966,8 @@
 	/** */
 	public static String BranchSelectionDialog_Refs;
 
+	public static String BranchSelectionDialog_Rename;
+
 	/** */
 	public static String HistoryPage_ShowAllVersionsForProject;
 
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
index 9aad95b..f0c13d1 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
@@ -48,8 +48,11 @@
 import org.spearce.egit.ui.UIText;
 import org.spearce.jgit.lib.Constants;
 import org.spearce.jgit.lib.ObjectId;
+import org.spearce.jgit.lib.Ref;
+import org.spearce.jgit.lib.RefRename;
 import org.spearce.jgit.lib.RefUpdate;
 import org.spearce.jgit.lib.Repository;
+import org.spearce.jgit.lib.RefUpdate.Result;
 
 /**
  * The branch and reset selection dialog
@@ -173,7 +176,7 @@ private void fillTreeWithBranches(String select) throws IOException {
 					curSubPrefix = null;
 				}
 
-				int slashPos = shortName.indexOf("/");
+				int slashPos = shortName.indexOf("/"); //$NON-NLS-1$
 				if (slashPos > -1) {
 					String remoteName = shortName.substring(0, slashPos);
 					shortName = shortName.substring(slashPos+1);
@@ -277,6 +280,30 @@ private void refNameFromDialog() {
 		}
 	}
 
+	private InputDialog getRefNameInputDialog(String prompt) {
+		InputDialog labelDialog = new InputDialog(
+				getShell(),
+				UIText.BranchSelectionDialog_QuestionNewBranchTitle,
+				prompt,
+				null, new IInputValidator() {
+					public String isValid(String newText) {
+						String testFor = Constants.R_HEADS + newText;
+						try {
+							if (repo.resolve(testFor) != null)
+								return UIText.BranchSelectionDialog_ErrorAlreadyExists;
+						} catch (IOException e1) {
+							Activator.logError(NLS.bind(
+									UIText.BranchSelectionDialog_ErrorCouldNotResolve, testFor), e1);
+						}
+						if (!Repository.isValidRefName(testFor))
+							return UIText.BranchSelectionDialog_ErrorInvalidRefName;
+						return null;
+					}
+				});
+		labelDialog.setBlockOnOpen(true);
+		return labelDialog;
+	}
+
 	@Override
 	protected void createButtonsForButtonBar(Composite parent) {
 		if (!showResetType) {
@@ -284,43 +311,81 @@ protected void createButtonsForButtonBar(Composite parent) {
 			newButton.setFont(JFaceResources.getDialogFont());
 			newButton.setText(UIText.BranchSelectionDialog_NewBranch);
 			((GridLayout)parent.getLayout()).numColumns++;
+			Button renameButton = new Button(parent, SWT.PUSH);
+			renameButton.setText(UIText.BranchSelectionDialog_Rename);
+			renameButton.addSelectionListener(new SelectionListener() {
+				public void widgetSelected(SelectionEvent e) {
+					// check what ref name the user selected, if any.
+					refNameFromDialog();
+
+					String branchName;
+					if (refName.startsWith(Constants.R_HEADS))
+						branchName = refName.substring(Constants.R_HEADS.length());
+					else
+						branchName = refName;	
+					InputDialog labelDialog = getRefNameInputDialog(NLS
+							.bind(
+									UIText.BranchSelectionDialog_QuestionNewBranchNameMessage,
+									branchName));
+					if (labelDialog.open() == Window.OK) {
+						String newRefName = Constants.R_HEADS + labelDialog.getValue();
+						try {
+							RefRename renameRef = repo.renameRef(refName, newRefName);
+							if (renameRef.rename() != Result.RENAMED) {
+								MessageDialog.openError(getShell(),
+										UIText.BranchSelectionDialog_ErrorRenameFailed,
+										NLS.bind(UIText.BranchSelectionDialog_ErrorCouldNotRenameRef,
+												new Object[] { refName, newRefName, renameRef.getResult() }));
+								Activator.logError(NLS.bind(
+										UIText.BranchSelectionDialog_ErrorCouldNotRenameRef2,
+												new Object[] { refName, newRefName }), null);
+							}
+							// FIXME: Update HEAD
+						} catch (IOException e1) {
+							Activator.logError(NLS.bind(
+									UIText.BranchSelectionDialog_ErrorCouldNotRenameRef2,
+															newRefName), e1);
+						}
+						try {
+							branchTree.removeAll();
+							fillTreeWithBranches(newRefName);
+						} catch (IOException e1) {
+							Activator.logError(
+									UIText.BranchSelectionDialog_ErrorCouldNotRefreshBranchList,
+											e1);
+						}
+					}
+				}
+				public void widgetDefaultSelected(SelectionEvent e) {
+					widgetSelected(e);
+				}
+			});
 			newButton.addSelectionListener(new SelectionListener() {
 
 				public void widgetSelected(SelectionEvent e) {
 					// check what ref name the user selected, if any.
 					refNameFromDialog();
 
-					InputDialog labelDialog = new InputDialog(
-							getShell(),
-							UIText.BranchSelectionDialog_QuestionNewBranchTitle,
-							UIText.BranchSelectionDialog_QuestionNewBranchMessage,
-							null, new IInputValidator() {
-								public String isValid(String newText) {
-									String testFor = Constants.R_HEADS + newText;
-									try {
-										if (repo.resolve(testFor) != null)
-											return UIText.BranchSelectionDialog_ErrorAlreadyExists;
-									} catch (IOException e1) {
-										Activator.logError(NLS.bind(
-												UIText.BranchSelectionDialog_ErrorCouldNotResolve, testFor), e1);
-									}
-									if (!Repository.isValidRefName(testFor))
-										return UIText.BranchSelectionDialog_ErrorInvalidRefName;
-									return null;
-								}
-							});
-					labelDialog.setBlockOnOpen(true);
+					InputDialog labelDialog = getRefNameInputDialog(UIText.BranchSelectionDialog_QuestionNewBranchNameMessage);
 					if (labelDialog.open() == Window.OK) {
 						String newRefName = Constants.R_HEADS + labelDialog.getValue();
 						RefUpdate updateRef;
 						try {
 							updateRef = repo.updateRef(newRefName);
+							Ref startRef = repo.getRef(refName);
 							ObjectId startAt;
 							if (refName == null)
 								startAt = repo.resolve(Constants.HEAD);
 							else
 								startAt = repo.resolve(refName);
+							String startBranch;
+							if (startRef != null)
+								startBranch = refName;
+							else
+								startBranch = startAt.name();
+							startBranch = repo.shortenRefName(startBranch);
 							updateRef.setNewObjectId(startAt);
+							updateRef.setRefLogMessage("branch: Created from " + startBranch, false); //$NON-NLS-1$
 							updateRef.update();
 						} catch (IOException e1) {
 							Activator.logError(NLS.bind(
@@ -338,6 +403,7 @@ public String isValid(String newText) {
 					}
 				}
 
+
 				public void widgetDefaultSelected(SelectionEvent e) {
 					widgetSelected(e);
 				}
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties b/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
index 1d21c81..a2fd720 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
@@ -340,6 +340,10 @@ BranchSelectionDialog_ErrorCouldNotRefresh=Could not refresh list of branches
 BranchSelectionDialog_ErrorCouldNotRefreshBranchList=Could not refresh list of branches
 BranchSelectionDialog_ErrorCouldNotResolve=Could not attempt to resolve {0}
 BranchSelectionDialog_ErrorInvalidRefName=Invalid ref name
+BranchSelectionDialog_ErrorCouldNotRenameRef=Failed to rename branch {0} -> {1}, status={2}
+BranchSelectionDialog_ErrorCouldNotRenameRef2=Failed to rename branch {0} -> {1}
+BranchSelectionDialog_ErrorRenameFailed=Rename failed
+
 BranchSelectionDialog_LocalBranches=Local Branches
 BranchSelectionDialog_NewBranch=&New branch
 BranchSelectionDialog_NoBranchSeletectMessage=You must select a valid ref.
@@ -347,7 +351,9 @@ BranchSelectionDialog_NoBranchSeletectTitle=No branch/tag selected
 BranchSelectionDialog_OkCheckout=&Checkout
 BranchSelectionDialog_OkReset=&Reset
 BranchSelectionDialog_QuestionNewBranchMessage=Enter name of new branch. It will branch from the selected branch. refs/heads/ will be prepended to the name you type
+BranchSelectionDialog_QuestionNewBranchNameMessage=Enter new name of the {0} branch. refs/heads/ will be prepended to the name you type
 BranchSelectionDialog_QuestionNewBranchTitle=New branch
+BranchSelectionDialog_QuestionNewBranchNameTitle=Rename branch
 BranchSelectionDialog_ReallyResetMessage=Resetting will overwrite any changes in your working directory.\n\nDo you wish to continue?
 BranchSelectionDialog_ReallyResetTitle=Really reset?
 BranchSelectionDialog_RemoteBranches=Remote Branches
@@ -357,6 +363,7 @@ BranchSelectionDialog_ResetTypeMixed=&Mixed (working directory unmodified)
 BranchSelectionDialog_ResetTypeSoft=&Soft (Index and working directory unmodified)
 BranchSelectionDialog_Tags=Tags
 BranchSelectionDialog_Refs=&Refs
+BranchSelectionDialog_Rename=&Rename
 
 Decorator_exceptionMessage=Errors occurred while applying Git decorations to resources.
 
-- 
1.6.3.2.199.g7340d

[EGIT PATCH v6 6/7] Improve error reporting in the branch dialog

[Robin Rosenberg]

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../src/org/spearce/egit/ui/UIText.java            |    6 ++
 .../ui/internal/dialogs/BranchSelectionDialog.java |   59 ++++++++++++--------
 .../src/org/spearce/egit/ui/uitext.properties      |    2 +
 3 files changed, 43 insertions(+), 24 deletions(-)

diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
index 76764ab..b0e8c3e 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
@@ -899,6 +899,12 @@
 	public static String BranchSelectionDialog_ErrorCouldNotRenameRef2;
 
 	/** */
+	public static String BranchSelectionDialog_BranchSelectionDialog_CreateFailedTitle;
+
+	/** */
+	public static String BranchSelectionDialog_BranchSelectionDialog_RenamedFailedTitle;
+
+	/** */
 	public static String BranchSelectionDialog_BranchSuffix_Current;
 
 	/** */
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
index f0c13d1..09b6381 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
@@ -104,7 +104,7 @@ getShell().setText(
 
 		try {
 			fillTreeWithBranches(null);
-		} catch (IOException e) {
+		} catch (Throwable e) {
 			Activator.logError(UIText.BranchSelectionDialog_ErrorCouldNotRefresh, e);
 		}
 
@@ -294,6 +294,7 @@ public String isValid(String newText) {
 						} catch (IOException e1) {
 							Activator.logError(NLS.bind(
 									UIText.BranchSelectionDialog_ErrorCouldNotResolve, testFor), e1);
+							return e1.getMessage();
 						}
 						if (!Repository.isValidRefName(testFor))
 							return UIText.BranchSelectionDialog_ErrorInvalidRefName;
@@ -332,27 +333,28 @@ public void widgetSelected(SelectionEvent e) {
 						try {
 							RefRename renameRef = repo.renameRef(refName, newRefName);
 							if (renameRef.rename() != Result.RENAMED) {
-								MessageDialog.openError(getShell(),
-										UIText.BranchSelectionDialog_ErrorRenameFailed,
-										NLS.bind(UIText.BranchSelectionDialog_ErrorCouldNotRenameRef,
-												new Object[] { refName, newRefName, renameRef.getResult() }));
-								Activator.logError(NLS.bind(
-										UIText.BranchSelectionDialog_ErrorCouldNotRenameRef2,
-												new Object[] { refName, newRefName }), null);
+								reportError(
+										null,
+										UIText.BranchSelectionDialog_BranchSelectionDialog_RenamedFailedTitle,
+										UIText.BranchSelectionDialog_ErrorCouldNotRenameRef,
+										refName, newRefName, renameRef
+												.getResult());
 							}
-							// FIXME: Update HEAD
-						} catch (IOException e1) {
-							Activator.logError(NLS.bind(
-									UIText.BranchSelectionDialog_ErrorCouldNotRenameRef2,
-															newRefName), e1);
+						} catch (Throwable e1) {
+							reportError(
+									e1,
+									UIText.BranchSelectionDialog_BranchSelectionDialog_RenamedFailedTitle,
+									UIText.BranchSelectionDialog_ErrorCouldNotRenameRef,
+									refName, newRefName, e1.getMessage());
 						}
 						try {
 							branchTree.removeAll();
 							fillTreeWithBranches(newRefName);
-						} catch (IOException e1) {
-							Activator.logError(
-									UIText.BranchSelectionDialog_ErrorCouldNotRefreshBranchList,
-											e1);
+						} catch (Throwable e1) {
+							reportError(
+									e1,
+									UIText.BranchSelectionDialog_BranchSelectionDialog_RenamedFailedTitle,
+									UIText.BranchSelectionDialog_ErrorCouldNotRefreshBranchList);
 						}
 					}
 				}
@@ -387,18 +389,20 @@ public void widgetSelected(SelectionEvent e) {
 							updateRef.setNewObjectId(startAt);
 							updateRef.setRefLogMessage("branch: Created from " + startBranch, false); //$NON-NLS-1$
 							updateRef.update();
-						} catch (IOException e1) {
-							Activator.logError(NLS.bind(
+						} catch (Throwable e1) {
+							reportError(
+									e1,
+									UIText.BranchSelectionDialog_BranchSelectionDialog_CreateFailedTitle,
 									UIText.BranchSelectionDialog_ErrorCouldNotCreateNewRef,
-															newRefName), e1);
+									newRefName);
 						}
 						try {
 							branchTree.removeAll();
 							fillTreeWithBranches(newRefName);
-						} catch (IOException e1) {
-							Activator.logError(
-									UIText.BranchSelectionDialog_ErrorCouldNotRefreshBranchList,
-											e1);
+						} catch (Throwable e1) {
+							reportError(e1,
+									UIText.BranchSelectionDialog_BranchSelectionDialog_CreateFailedTitle,
+									UIText.BranchSelectionDialog_ErrorCouldNotRefreshBranchList);
 						}
 					}
 				}
@@ -419,4 +423,11 @@ createButton(parent, IDialogConstants.OK_ID,
 	protected int getShellStyle() {
 		return super.getShellStyle() | SWT.RESIZE;
 	}
+
+	private void reportError(Throwable e, String title, String message,
+			Object... args) {
+		String msg = NLS.bind(message, args);
+		MessageDialog.openError(getShell(), title, msg);
+		Activator.logError(msg, e);
+	}
 }
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties b/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
index a2fd720..a141c90 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
@@ -333,6 +333,8 @@ WindowCachePreferencePage_packedGitMMAP=Use virtual memory mapping
 
 BranchSelectionDialog_TitleCheckout=Checkout: {0}
 BranchSelectionDialog_TitleReset=Reset: {0}
+BranchSelectionDialog_BranchSelectionDialog_CreateFailedTitle=New branch creation failed
+BranchSelectionDialog_BranchSelectionDialog_RenamedFailedTitle=Rename failed
 BranchSelectionDialog_BranchSuffix_Current=\ (current)
 BranchSelectionDialog_ErrorAlreadyExists=Already exists
 BranchSelectionDialog_ErrorCouldNotCreateNewRef=Could not create new ref {0}
-- 
1.6.3.2.199.g7340d