Fix "jgit checkout -f" to overwrite dirty worktree files

CheckoutCommand had a setForce() method. But this didn't correspond
to native git's 'git checkout -f' option. Deprecate the old setForce()
method and move its implementation to a new method setForceRefUpdate()
and use it to implement the -B option in the CLI class Checkout.

Add a setForced() method and use it to fix the associated '-f' option of
the CLI Checkout class to behave like native git's 'git checkout -f'
which overwrites dirty worktree files during checkout.

This is still not fully matching native git's behavior: updating
additionally dirty index entries is not done yet.

Bug: 530771
Change-Id: I776b78eb623b6ea0aca42f681788f2e4b1667f15
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

org.eclipse.jgit.ant/src/org/eclipse/jgit/ant/tasks/GitCheckoutTask.java

@@ -123,7 +123,7 @@ public class GitCheckoutTask extends Task {
 		}
 
 		try {
-			checkout.setCreateBranch(createBranch).setForce(force)
+			checkout.setCreateBranch(createBranch).setForceRefUpdate(force)
 					.setName(branch);
 			checkout.call();
 		} catch (Exception e) {

org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/CheckoutTest.java

@@ -58,6 +58,7 @@ import java.util.List;
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.api.errors.CheckoutConflictException;
 import org.eclipse.jgit.diff.DiffEntry;
+import org.eclipse.jgit.junit.LocalDiskRepositoryTestCase;
 import org.eclipse.jgit.lib.CLIRepositoryTestCase;
 import org.eclipse.jgit.lib.FileMode;
 import org.eclipse.jgit.lib.Ref;
@@ -684,4 +685,19 @@ public class CheckoutTest extends CLIRepositoryTestCase {
 			assertTrue(Files.isSymbolicLink(path));
 		}
 	}
+
+	@Test
+	public void testCheckoutForce_Bug530771() throws Exception {
+		try (Git git = new Git(db)) {
+			File f = writeTrashFile("a", "Hello world");
+			git.add().addFilepattern("a").call();
+			git.commit().setMessage("create a").call();
+			writeTrashFile("a", "Goodbye world");
+			assertEquals("[]",
+					Arrays.toString(execute("git checkout -f HEAD")));
+			assertEquals("Hello world", read(f));
+			assertEquals("[a, mode:100644, content:Hello world]",
+					indexState(db, LocalDiskRepositoryTestCase.CONTENT));
+		}
+	}
 }

org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/internal/CLIText.properties

@@ -341,7 +341,8 @@ usage_fetchThinPack=fetch thin pack
 usage_filesToAddContentFrom=Files to add content from
 usage_fixAThinPackToBeComplete=fix a thin pack to be complete
 usage_forEachRefOutput=for-each-ref output
-usage_forceCheckout=when switching branches, proceed even if the index or the working tree differs from HEAD
+usage_forcedSwitchBranch=when switching branches do it forcefully. Succeed even if resetting an existing branch would cause commits to become unreachable
+usage_forceCheckout=when checking out a commit succeed even if the working tree or the index is dirty. Overwrite the working tree or index in such cases
 usage_forceClean=required to delete files or directories
 usage_forceCreateBranchEvenExists=force create branch even exists
 usage_forcedFetch=force ref update fetch option

org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Checkout.java

@@ -69,8 +69,11 @@ class Checkout extends TextBuiltin {
 	@Option(name = "-b", usage = "usage_createBranchAndCheckout")
 	private boolean createBranch = false;
 
+	@Option(name = "-B", usage = "usage_forcedSwitchBranch")
+	private boolean forceSwitchBranch = false;
+
 	@Option(name = "--force", aliases = { "-f" }, usage = "usage_forceCheckout")
-	private boolean force = false;
+	private boolean forced = false;
 
 	@Option(name = "--orphan", usage = "usage_orphan")
 	private boolean orphan = false;
@@ -103,7 +106,8 @@ class Checkout extends TextBuiltin {
 			} else {
 				command.setCreateBranch(createBranch);
 				command.setName(name);
-				command.setForce(force);
+				command.setForceRefUpdate(forceSwitchBranch);
+				command.setForced(forced);
 				command.setOrphan(orphan);
 			}
 			try {

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

@@ -63,6 +63,7 @@ import java.net.URISyntaxException;
 
 import org.eclipse.jgit.api.CheckoutResult.Status;
 import org.eclipse.jgit.api.CreateBranchCommand.SetupUpstreamMode;
+import org.eclipse.jgit.api.errors.CheckoutConflictException;
 import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.api.errors.InvalidRefNameException;
 import org.eclipse.jgit.api.errors.InvalidRemoteException;
@@ -109,7 +110,7 @@ public class CheckoutCommandTest extends RepositoryTestCase {
 		git.add().addFilepattern("Test.txt").call();
 		initialCommit = git.commit().setMessage("Initial commit").call();
 
-		// create a master branch and switch to it
+		// create a test branch and switch to it
 		git.branchCreate().setName("test").call();
 		RefUpdate rup = db.updateRef(Constants.HEAD);
 		rup.link("refs/heads/test");
@@ -137,6 +138,18 @@ public class CheckoutCommandTest extends RepositoryTestCase {
 		assertEquals("refs/heads/master", git.getRepository().getFullBranch());
 	}
 
+	@Test
+	public void testCheckoutForced() throws Exception {
+		writeTrashFile("Test.txt", "Garbage");
+		try {
+			git.checkout().setName("master").call().getObjectId();
+			fail("Expected CheckoutConflictException didn't occur");
+		} catch (CheckoutConflictException e) {
+		}
+		assertEquals(initialCommit.getId(), git.checkout().setName("master")
+				.setForced(true).call().getObjectId());
+	}
+
 	@Test
 	public void testCreateBranchOnCheckout() throws Exception {
 		git.checkout().setCreateBranch(true).setName("test2").call();
@@ -165,7 +178,7 @@ public class CheckoutCommandTest extends RepositoryTestCase {
 			assertEquals(Status.CONFLICTS, co.getResult().getStatus());
 			assertTrue(co.getResult().getConflictList().contains("Test.txt"));
 		}
-		git.checkout().setName("master").setForce(true).call();
+		git.checkout().setName("master").setForced(true).call();
 		assertThat(read("Test.txt"), is("Hello world"));
 	}
 

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

@@ -162,7 +162,9 @@ public class CheckoutCommand extends GitCommand<Ref> {
 
 	private String name;
 
-	private boolean force = false;
+	private boolean forceRefUpdate = false;
+
+	private boolean forced = false;
 
 	private boolean createBranch = false;
 
@@ -269,7 +271,11 @@ public class CheckoutCommand extends GitCommand<Ref> {
 			try {
 				dco = new DirCacheCheckout(repo, headTree, dc,
 						newCommit.getTree());
-				dco.setFailOnConflict(!force);
+				dco.setFailOnConflict(true);
+				dco.setForce(forced);
+				if (forced) {
+					dco.setFailOnConflict(false);
+				}
 				dco.setProgressMonitor(monitor);
 				try {
 					dco.checkout();
@@ -286,7 +292,7 @@ public class CheckoutCommand extends GitCommand<Ref> {
 				ref = null;
 			String toName = Repository.shortenRefName(name);
 			RefUpdate refUpdate = repo.updateRef(Constants.HEAD, ref == null);
-			refUpdate.setForceUpdate(force);
+			refUpdate.setForceUpdate(forceRefUpdate);
 			refUpdate.setRefLogMessage(refLogMessage + " to " + toName, false); //$NON-NLS-1$
 			Result updateResult;
 			if (ref != null)
@@ -666,10 +672,54 @@ public class CheckoutCommand extends GitCommand<Ref> {
 	 *            set to a new start-point; if false, the existing branch will
 	 *            not be changed
 	 * @return this instance
+	 * @deprecated this method was badly named comparing its semantics to native
+	 *             git's checkout --force option, use
+	 *             {@link #setForceRefUpdate(boolean)} instead
 	 */
+	@Deprecated
 	public CheckoutCommand setForce(boolean force) {
+		return setForceRefUpdate(force);
+	}
+
+	/**
+	 * Specify to force the ref update in case of a branch switch.
+	 *
+	 * In releases prior to 5.2 this method was called setForce() but this name
+	 * was misunderstood to implement native git's --force option, which is not
+	 * true.
+	 *
+	 * @param forceRefUpdate
+	 *            if <code>true</code> and the branch with the given name
+	 *            already exists, the start-point of an existing branch will be
+	 *            set to a new start-point; if false, the existing branch will
+	 *            not be changed
+	 * @return this instance
+	 * @since 5.3
+	 */
+	public CheckoutCommand setForceRefUpdate(boolean forceRefUpdate) {
+		checkCallable();
+		this.forceRefUpdate = forceRefUpdate;
+		return this;
+	}
+
+	/**
+	 * Allow a checkout even if the workingtree or index differs from HEAD. This
+	 * matches native git's '--force' option.
+	 *
+	 * JGit releases before 5.2 had a method <code>setForce()</code> offering
+	 * semantics different from this new <code>setForced()</code>. This old
+	 * semantic can now be found in {@link #setForceRefUpdate(boolean)}
+	 *
+	 * @param forced
+	 *            if set to <code>true</code> then allow the checkout even if
+	 *            workingtree or index doesn't match HEAD. Overwrite workingtree
+	 *            files and index content with the new content in this case.
+	 * @return this instance
+	 * @since 5.3
+	 */
+	public CheckoutCommand setForced(boolean forced) {
 		checkCallable();
-		this.force = force;
+		this.forced = forced;
 		return this;
 	}
 

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

@@ -155,6 +155,8 @@ public class DirCacheCheckout {
 
 	private boolean failOnConflict = true;
 
+	private boolean force = false;
+
 	private ArrayList<String> toBeDeleted = new ArrayList<>();
 
 	private boolean initialCheckout;
@@ -427,11 +429,11 @@ public class DirCacheCheckout {
 					DirCacheEntry entry = i.getDirCacheEntry();
 					if (entry.getLastModified() == 0)
 						entry.setLastModified(f.getEntryLastModified());
-					keep(entry);
+					keep(entry, f);
 				}
 			} else
 				// The index contains a folder
-				keep(i.getDirCacheEntry());
+				keep(i.getDirCacheEntry(), f);
 		} else {
 			// There is no entry in the merge commit. Means: we want to delete
 			// what's currently in the index and working tree
@@ -821,14 +823,14 @@ public class DirCacheCheckout {
 
 				break;
 			case 0xDFD: // 3 4
-				keep(dce);
+				keep(dce, f);
 				break;
 			case 0xF0D: // 18
 				remove(name);
 				break;
 			case 0xDFF: // 5 5b 6 6b
 				if (equalIdAndMode(iId, iMode, mId, mMode))
-					keep(dce); // 5 6
+					keep(dce, f); // 5 6
 				else
 					conflict(name, dce, h, m); // 5b 6b
 				break;
@@ -858,7 +860,7 @@ public class DirCacheCheckout {
 					conflict(name, dce, h, m); // 9
 				break;
 			case 0xFD0: // keep without a rule
-				keep(dce);
+				keep(dce, f);
 				break;
 			case 0xFFD: // 12 13 14
 				if (equalIdAndMode(hId, hMode, iId, iMode))
@@ -878,7 +880,7 @@ public class DirCacheCheckout {
 					conflict(name, dce, h, m);
 				break;
 			default:
-				keep(dce);
+				keep(dce, f);
 			}
 			return;
 		}
@@ -964,7 +966,7 @@ public class DirCacheCheckout {
 					if (initialCheckout)
 						update(name, mId, mMode);
 					else
-						keep(dce);
+						keep(dce, f);
 				} else
 					conflict(name, dce, h, m);
 			}
@@ -1027,7 +1029,7 @@ public class DirCacheCheckout {
 						// Nothing in Head
 						// Something in Index
 						// -> Merge contains nothing new. Keep the index.
-						keep(dce);
+						keep(dce, f);
 				} else
 					// Merge contains something and it is not the same as Index
 					// Nothing in Head
@@ -1176,7 +1178,7 @@ public class DirCacheCheckout {
 					// to the other one.
 					// -> In all three cases we don't touch index and file.
 
-					keep(dce);
+					keep(dce, f);
 				}
 			}
 		}
@@ -1225,9 +1227,15 @@ public class DirCacheCheckout {
 		}
 	}
 
-	private void keep(DirCacheEntry e) {
+	private void keep(DirCacheEntry e, WorkingTreeIterator f)
+			throws IOException {
 		if (e != null && !FileMode.TREE.equals(e.getFileMode()))
 			builder.add(e);
+		if (force) {
+			if (f.isModified(e, true, this.walk.getObjectReader())) {
+				checkoutEntry(repo, e, this.walk.getObjectReader());
+			}
+		}
 	}
 
 	private void remove(String path) {
@@ -1261,6 +1269,20 @@ public class DirCacheCheckout {
 		this.failOnConflict = failOnConflict;
 	}
 
+	/**
+	 * If <code>true</code>, dirty worktree files may be overridden. If
+	 * <code>false</code> dirty worktree files will not be overridden in order
+	 * not to delete unsaved content. This corresponds to native git's 'git
+	 * checkout -f' option. By default this option is set to false.
+	 *
+	 * @param force
+	 *            a boolean.
+	 * @since 5.3
+	 */
+	public void setForce(boolean force) {
+		this.force = force;
+	}
+
 	/**
 	 * This method implements how to handle conflicts when
 	 * {@link #failOnConflict} is false