From 02bd26e5a636fe2d9f128860530eda27ac35b334 Mon Sep 17 00:00:00 2001 From: Robin Stocker Date: Mon, 12 Aug 2013 15:13:03 +0200 Subject: [PATCH] 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 Signed-off-by: Matthias Sohn --- .../eclipse/jgit/api/ResetCommandTest.java | 38 ++++- .../org/eclipse/jgit/api/ResetCommand.java | 141 +++++++++++------- .../jgit/dircache/DirCacheCheckout.java | 2 +- 3 files changed, 123 insertions(+), 58 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/ResetCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/ResetCommandTest.java index da57cb324..82249766a 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/ResetCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/ResetCommandTest.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2011-2012, Chris Aniszczyk + * Copyright (C) 2011-2013, Chris Aniszczyk * 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 diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/ResetCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/ResetCommand.java index 6827026ba..3a1c209a2 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/ResetCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/ResetCommand.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2011-2012, Chris Aniszczyk + * Copyright (C) 2011-2013, Chris Aniszczyk * 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 { 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 { 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 { final boolean reverting = state.equals(RepositoryState.REVERTING) || state.equals(RepositoryState.REVERTING_RESOLVED); - // resolve the ref to a commit - final ObjectId commitId; - try { - commitId = repo.resolve(ref + "^{commit}"); //$NON-NLS-1$ - if (commitId == null) { - // @TODO throw an InvalidRefNameException. We can't do that - // now because this would break the API - 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 commitId = resolveRefToCommitId(); + // When ref is explicitly specified, it has to resolve + if (ref != null && commitId == null) { + // @TODO throw an InvalidRefNameException. We can't do that + // now because this would break the API + throw new JGitInternalException("Invalid ref " + ref + + " specified"); } + 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 RefUpdate ru = repo.updateRef(Constants.HEAD); - ru.setNewObjectId(commitId); - - String refName = Repository.shortenRefName(ref); - String message = refName + ": updating " + Constants.HEAD; //$NON-NLS-1$ - ru.setRefLogMessage(message, false); - if (ru.forceUpdate() == RefUpdate.Result.LOCK_FAILURE) - throw new JGitInternalException(MessageFormat.format( - JGitText.get().cannotLock, ru.getName())); - - ObjectId origHead = ru.getOldObjectId(); - if (origHead != null) - repo.writeOrigHead(origHead); + final Ref result; + if (commitId != null) { + // write the ref + final RefUpdate ru = repo.updateRef(Constants.HEAD); + ru.setNewObjectId(commitId); + + String refName = Repository.shortenRefName(getRefOrHEAD()); + String message = refName + ": updating " + Constants.HEAD; //$NON-NLS-1$ + ru.setRefLogMessage(message, false); + if (ru.forceUpdate() == RefUpdate.Result.LOCK_FAILURE) + throw new JGitInternalException(MessageFormat.format( + JGitText.get().cannotLock, ru.getName())); + + 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 { } 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 { 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 { 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 { } } - 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 { } } - 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(); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java index 5458426b1..dd9ee10bf 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java +++ b/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);