ResetCommand: Allow reset on unborn branch when ref not specified

In C Git 1.8.2, "git reset" now also works on an unborn branch (no HEAD
yet) if no explicit ref was specified. In that case, it is treated as a
reset to an empty tree.

This can be useful for callers because "unborn branch" no longer has to
be special-cased to "git rm --cached".

Bug: 414870
Change-Id: Ied750116f767518ae4d48823cf00752b049a8477
Signed-off-by: Robin Stocker <robin@nibor.org>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

org.eclipse.jgit.test/tst/org/eclipse/jgit/api/ResetCommandTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2011-2012, Chris Aniszczyk <caniszczyk@gmail.com>
+ * Copyright (C) 2011-2013, Chris Aniszczyk <caniszczyk@gmail.com>
  * and other copyright owners as documented in the project's IP log.
  *
  * This program and the accompanying materials are made available
@@ -403,6 +403,27 @@ public class ResetCommandTest extends RepositoryTestCase {
 				indexState(0));
 	}
 
+	@Test
+	public void testPathsResetOnUnbornBranch() throws Exception {
+		git = new Git(db);
+		writeTrashFile("a.txt", "content");
+		git.add().addFilepattern("a.txt").call();
+		// Should assume an empty tree, like in C Git 1.8.2
+		git.reset().addPath("a.txt").call();
+
+		DirCache cache = db.readDirCache();
+		DirCacheEntry aEntry = cache.getEntry("a.txt");
+		assertNull(aEntry);
+	}
+
+	@Test(expected = JGitInternalException.class)
+	public void testPathsResetToNonexistingRef() throws Exception {
+		git = new Git(db);
+		writeTrashFile("a.txt", "content");
+		git.add().addFilepattern("a.txt").call();
+		git.reset().setRef("doesnotexist").addPath("a.txt").call();
+	}
+
 	@Test
 	public void testHardResetOnTag() throws Exception {
 		setupRepository();
@@ -453,6 +474,21 @@ public class ResetCommandTest extends RepositoryTestCase {
 		assertNull(db.readSquashCommitMsg());
 	}
 
+	@Test
+	public void testHardResetOnUnbornBranch() throws Exception {
+		git = new Git(db);
+		File fileA = writeTrashFile("a.txt", "content");
+		git.add().addFilepattern("a.txt").call();
+		// Should assume an empty tree, like in C Git 1.8.2
+		git.reset().setMode(ResetType.HARD).call();
+
+		DirCache cache = db.readDirCache();
+		DirCacheEntry aEntry = cache.getEntry("a.txt");
+		assertNull(aEntry);
+		assertFalse(fileA.exists());
+		assertNull(db.resolve(Constants.HEAD));
+	}
+
 	private void assertReflog(ObjectId prevHead, ObjectId head)
 			throws IOException {
 		// Check the reflog for HEAD

org.eclipse.jgit/src/org/eclipse/jgit/api/ResetCommand.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2011-2012, Chris Aniszczyk <caniszczyk@gmail.com>
+ * Copyright (C) 2011-2013, Chris Aniszczyk <caniszczyk@gmail.com>
  * and other copyright owners as documented in the project's IP log.
  *
  * This program and the accompanying materials are made available
@@ -67,6 +67,7 @@ import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.treewalk.AbstractTreeIterator;
 import org.eclipse.jgit.treewalk.CanonicalTreeParser;
+import org.eclipse.jgit.treewalk.EmptyTreeIterator;
 import org.eclipse.jgit.treewalk.TreeWalk;
 import org.eclipse.jgit.treewalk.filter.PathFilterGroup;
 
@@ -114,7 +115,9 @@ public class ResetCommand extends GitCommand<Ref> {
 		KEEP // TODO not implemented yet
 	}
 
-	private String ref = Constants.HEAD;
+	// We need to be able to distinguish whether the caller set the ref
+	// explicitly or not, so we apply the default (HEAD) only later.
+	private String ref = null;
 
 	private ResetType mode;
 
@@ -139,9 +142,6 @@ public class ResetCommand extends GitCommand<Ref> {
 	public Ref call() throws GitAPIException, CheckoutConflictException {
 		checkCallable();
 
-		Ref r;
-		RevCommit commit;
-
 		try {
 			RepositoryState state = repo.getRepositoryState();
 			final boolean merging = state.equals(RepositoryState.MERGING)
@@ -152,61 +152,55 @@ public class ResetCommand extends GitCommand<Ref> {
 			final boolean reverting = state.equals(RepositoryState.REVERTING)
 					|| state.equals(RepositoryState.REVERTING_RESOLVED);
 
-			// resolve the ref to a commit
+			final ObjectId commitId = resolveRefToCommitId();
-			final ObjectId commitId;
+			// When ref is explicitly specified, it has to resolve
-			try {
+			if (ref != null && commitId == null) {
-				commitId = repo.resolve(ref + "^{commit}"); //$NON-NLS-1$
+				// @TODO throw an InvalidRefNameException. We can't do that
-				if (commitId == null) {
+				// now because this would break the API
-					// @TODO throw an InvalidRefNameException. We can't do that
+				throw new JGitInternalException("Invalid ref " + ref
-					// now because this would break the API
+						+ " specified");
-					throw new JGitInternalException("Invalid ref " + ref
-							+ " specified");
-				}
-			} catch (IOException e) {
-				throw new JGitInternalException(
-						MessageFormat.format(JGitText.get().cannotRead, ref),
-						e);
-			}
-			RevWalk rw = new RevWalk(repo);
-			try {
-				commit = rw.parseCommit(commitId);
-			} catch (IOException e) {
-				throw new JGitInternalException(
-						MessageFormat.format(
-						JGitText.get().cannotReadCommit, commitId.toString()),
-						e);
-			} finally {
-				rw.release();
 			}
 
+			final ObjectId commitTree;
+			if (commitId != null)
+				commitTree = parseCommit(commitId).getTree();
+			else
+				commitTree = null;
+
 			if (!filepaths.isEmpty()) {
 				// reset [commit] -- paths
-				resetIndexForPaths(commit);
+				resetIndexForPaths(commitTree);
 				setCallable(false);
 				return repo.getRef(Constants.HEAD);
 			}
 
-			// write the ref
+			final Ref result;
-			final RefUpdate ru = repo.updateRef(Constants.HEAD);
+			if (commitId != null) {
-			ru.setNewObjectId(commitId);
+				// write the ref
-
+				final RefUpdate ru = repo.updateRef(Constants.HEAD);
-			String refName = Repository.shortenRefName(ref);
+				ru.setNewObjectId(commitId);
-			String message = refName + ": updating " + Constants.HEAD; //$NON-NLS-1$
+
-			ru.setRefLogMessage(message, false);
+				String refName = Repository.shortenRefName(getRefOrHEAD());
-			if (ru.forceUpdate() == RefUpdate.Result.LOCK_FAILURE)
+				String message = refName + ": updating " + Constants.HEAD; //$NON-NLS-1$
-				throw new JGitInternalException(MessageFormat.format(
+				ru.setRefLogMessage(message, false);
-						JGitText.get().cannotLock, ru.getName()));
+				if (ru.forceUpdate() == RefUpdate.Result.LOCK_FAILURE)
-
+					throw new JGitInternalException(MessageFormat.format(
-			ObjectId origHead = ru.getOldObjectId();
+							JGitText.get().cannotLock, ru.getName()));
-			if (origHead != null)
+
-				repo.writeOrigHead(origHead);
+				ObjectId origHead = ru.getOldObjectId();
+				if (origHead != null)
+					repo.writeOrigHead(origHead);
+				result = ru.getRef();
+			} else {
+				result = repo.getRef(Constants.HEAD);
+			}
 
 			switch (mode) {
 				case HARD:
-					checkoutIndex(commit);
+					checkoutIndex(commitTree);
 					break;
 				case MIXED:
-					resetIndex(commit);
+					resetIndex(commitTree);
 					break;
 				case SOFT: // do nothing, only the ref was changed
 					break;
@@ -228,19 +222,41 @@ public class ResetCommand extends GitCommand<Ref> {
 			}
 
 			setCallable(false);
-			r = ru.getRef();
+			return result;
 		} catch (IOException e) {
 			throw new JGitInternalException(
 					JGitText.get().exceptionCaughtDuringExecutionOfResetCommand,
 					e);
 		}
+	}
+
+	private RevCommit parseCommit(final ObjectId commitId) {
+		RevCommit commit;
+		RevWalk rw = new RevWalk(repo);
+		try {
+			commit = rw.parseCommit(commitId);
+		} catch (IOException e) {
+			throw new JGitInternalException(MessageFormat.format(
+					JGitText.get().cannotReadCommit, commitId.toString()), e);
+		} finally {
+			rw.release();
+		}
+		return commit;
+	}
 
-		return r;
+	private ObjectId resolveRefToCommitId() {
+		try {
+			return repo.resolve(getRefOrHEAD() + "^{commit}"); //$NON-NLS-1$
+		} catch (IOException e) {
+			throw new JGitInternalException(
+					MessageFormat.format(JGitText.get().cannotRead, getRefOrHEAD()),
+					e);
+		}
 	}
 
 	/**
 	 * @param ref
-	 *            the ref to reset to
+	 *            the ref to reset to, defaults to HEAD if not specified
 	 * @return this instance
 	 */
 	public ResetCommand setRef(String ref) {
@@ -276,7 +292,14 @@ public class ResetCommand extends GitCommand<Ref> {
 		return this;
 	}
 
-	private void resetIndexForPaths(RevCommit commit) {
+	private String getRefOrHEAD() {
+		if (ref != null)
+			return ref;
+		else
+			return Constants.HEAD;
+	}
+
+	private void resetIndexForPaths(ObjectId commitTree) {
 		DirCache dc = null;
 		try {
 			dc = repo.lockDirCache();
@@ -284,7 +307,10 @@ public class ResetCommand extends GitCommand<Ref> {
 
 			final TreeWalk tw = new TreeWalk(repo);
 			tw.addTree(new DirCacheBuildIterator(builder));
-			tw.addTree(commit.getTree());
+			if (commitTree != null)
+				tw.addTree(commitTree);
+			else
+				tw.addTree(new EmptyTreeIterator());
 			tw.setFilter(PathFilterGroup.createFromStrings(filepaths));
 			tw.setRecursive(true);
 
@@ -310,14 +336,17 @@ public class ResetCommand extends GitCommand<Ref> {
 		}
 	}
 
-	private void resetIndex(RevCommit commit) throws IOException {
+	private void resetIndex(ObjectId commitTree) throws IOException {
 		DirCache dc = repo.lockDirCache();
 		TreeWalk walk = null;
 		try {
 			DirCacheBuilder builder = dc.builder();
 
 			walk = new TreeWalk(repo);
-			walk.addTree(commit.getTree());
+			if (commitTree != null)
+				walk.addTree(commitTree);
+			else
+				walk.addTree(new EmptyTreeIterator());
 			walk.addTree(new DirCacheIterator(dc));
 			walk.setRecursive(true);
 
@@ -352,12 +381,12 @@ public class ResetCommand extends GitCommand<Ref> {
 		}
 	}
 
-	private void checkoutIndex(RevCommit commit) throws IOException,
+	private void checkoutIndex(ObjectId commitTree) throws IOException,
 			GitAPIException {
 		DirCache dc = repo.lockDirCache();
 		try {
 			DirCacheCheckout checkout = new DirCacheCheckout(repo, dc,
-					commit.getTree());
+					commitTree);
 			checkout.setFailOnConflict(false);
 			try {
 				checkout.checkout();

org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java

@@ -280,7 +280,7 @@ public class DirCacheCheckout {
 		builder = dc.builder();
 
 		walk = new NameConflictTreeWalk(repo);
-		walk.addTree(mergeCommitTree);
+		addTree(walk, mergeCommitTree);
 		walk.addTree(new DirCacheBuildIterator(builder));
 		walk.addTree(workingTree);