Use working tree iterator to compare file modes

Add isModeDifferent method to WorkingTreeIterator
that compares mode with consideration of the
core.filemode setting in the config.

Bug: 379004
Change-Id: I07335300d787a69c3d1608242238991d5b5214ac
Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>

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

@@ -44,6 +44,7 @@ package org.eclipse.jgit.api;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
 import java.io.File;
@@ -54,7 +55,10 @@ import org.eclipse.jgit.api.CherryPickResult.CherryPickStatus;
 import org.eclipse.jgit.api.ResetCommand.ResetType;
 import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.api.errors.JGitInternalException;
+import org.eclipse.jgit.dircache.DirCache;
+import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.FileMode;
 import org.eclipse.jgit.lib.RepositoryState;
 import org.eclipse.jgit.lib.RepositoryTestCase;
 import org.eclipse.jgit.merge.ResolveMerger.MergeFailureReason;
@@ -183,6 +187,42 @@ public class CherryPickCommandTest extends RepositoryTestCase {
 				.exists());
 	}
 
+	@Test
+	public void testCherryPickOverExecutableChangeOnNonExectuableFileSystem()
+			throws Exception {
+		Git git = new Git(db);
+		File file = writeTrashFile("test.txt", "a");
+		assertNotNull(git.add().addFilepattern("test.txt").call());
+		assertNotNull(git.commit().setMessage("commit1").call());
+
+		assertNotNull(git.checkout().setCreateBranch(true).setName("a").call());
+
+		writeTrashFile("test.txt", "b");
+		assertNotNull(git.add().addFilepattern("test.txt").call());
+		RevCommit commit2 = git.commit().setMessage("commit2").call();
+		assertNotNull(commit2);
+
+		assertNotNull(git.checkout().setName(Constants.MASTER).call());
+
+		DirCache cache = db.lockDirCache();
+		cache.getEntry("test.txt").setFileMode(FileMode.EXECUTABLE_FILE);
+		cache.write();
+		assertTrue(cache.commit());
+		cache.unlock();
+
+		assertNotNull(git.commit().setMessage("commit3").call());
+
+		db.getFS().setExecute(file, false);
+		git.getRepository()
+				.getConfig()
+				.setBoolean(ConfigConstants.CONFIG_CORE_SECTION, null,
+						ConfigConstants.CONFIG_KEY_FILEMODE, false);
+
+		CherryPickResult result = git.cherryPick().include(commit2).call();
+		assertNotNull(result);
+		assertEquals(CherryPickStatus.OK, result.getStatus());
+	}
+
 	private RevCommit prepareCherryPick(final Git git) throws Exception {
 		// create, add and commit file a
 		writeTrashFile("a", "a");

org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java

@@ -399,7 +399,7 @@ public class ResolveMerger extends ThreeWayMerger {
 					else {
 						// the preferred version THEIRS has a different mode
 						// than ours. Check it out!
-						if (isWorktreeDirty())
+						if (isWorktreeDirty(work))
 							return false;
 						DirCacheEntry e = add(tw.getRawPath(), theirs,
 								DirCacheEntry.STAGE_0);
@@ -434,7 +434,7 @@ public class ResolveMerger extends ThreeWayMerger {
 			// THEIRS. THEIRS is chosen.
 
 			// Check worktree before checking out THEIRS
-			if (isWorktreeDirty())
+			if (isWorktreeDirty(work))
 				return false;
 			if (nonTree(modeT)) {
 				DirCacheEntry e = add(tw.getRawPath(), theirs,
@@ -485,7 +485,7 @@ public class ResolveMerger extends ThreeWayMerger {
 
 		if (nonTree(modeO) && nonTree(modeT)) {
 			// Check worktree before modifying files
-			if (isWorktreeDirty())
+			if (isWorktreeDirty(work))
 				return false;
 
 			MergeResult<RawText> result = contentMerge(base, ours, theirs);
@@ -507,7 +507,7 @@ public class ResolveMerger extends ThreeWayMerger {
 				// OURS was deleted checkout THEIRS
 				if (modeO == 0) {
 					// Check worktree before checking out THEIRS
-					if (isWorktreeDirty())
+					if (isWorktreeDirty(work))
 						return false;
 					if (nonTree(modeT)) {
 						if (e != null)
@@ -563,7 +563,7 @@ public class ResolveMerger extends ThreeWayMerger {
 		return isDirty;
 	}
 
-	private boolean isWorktreeDirty() {
+	private boolean isWorktreeDirty(WorkingTreeIterator work) {
 		if (inCore)
 			return false;
 
@@ -571,8 +571,13 @@ public class ResolveMerger extends ThreeWayMerger {
 		final int modeO = tw.getRawMode(T_OURS);
 
 		// Worktree entry has to match ours to be considered clean
-		final boolean isDirty = nonTree(modeF)
+		final boolean isDirty;
-				&& !(modeO == modeF && tw.idEqual(T_FILE, T_OURS));
+		if (nonTree(modeF))
+			isDirty = work.isModeDifferent(modeO)
+					|| !tw.idEqual(T_FILE, T_OURS);
+		else
+			isDirty = false;
+
 		if (isDirty)
 			failingPaths.put(tw.getPathString(),
 					MergeFailureReason.DIRTY_WORKTREE);

org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java

@@ -695,6 +695,33 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator {
 		DIFFER_BY_TIMESTAMP
 	}
 
+	/**
+	 * Is the file mode of the current entry different than the given raw mode?
+	 *
+	 * @param rawMode
+	 * @return true if different, false otherwise
+	 */
+	public boolean isModeDifferent(final int rawMode) {
+		// Determine difference in mode-bits of file and index-entry. In the
+		// bitwise presentation of modeDiff we'll have a '1' when the two modes
+		// differ at this position.
+		int modeDiff = getEntryRawMode() ^ rawMode;
+
+		if (modeDiff == 0)
+			return false;
+
+		// Do not rely on filemode differences in case of symbolic links
+		if (FileMode.SYMLINK.equals(rawMode))
+			return false;
+
+		// Ignore the executable file bits if WorkingTreeOptions tell me to
+		// do so. Ignoring is done by setting the bits representing a
+		// EXECUTABLE_FILE to '0' in modeDiff
+		if (!state.options.isFileMode())
+			modeDiff &= ~FileMode.EXECUTABLE_FILE.getBits();
+		return modeDiff != 0;
+	}
+
 	/**
 	 * Compare the metadata (mode, length, modification-timestamp) of the
 	 * current entry and a {@link DirCacheEntry}
@@ -714,23 +741,8 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator {
 		if (!entry.isSmudged() && entry.getLength() != (int) getEntryLength())
 			return MetadataDiff.DIFFER_BY_METADATA;
 
-		// Determine difference in mode-bits of file and index-entry. In the
+		if (isModeDifferent(entry.getRawMode()))
-		// bitwise presentation of modeDiff we'll have a '1' when the two modes
-		// differ at this position.
-		int modeDiff = getEntryRawMode() ^ entry.getRawMode();
-
-		// Do not rely on filemode differences in case of symbolic links
-		if (modeDiff != 0 && !FileMode.SYMLINK.equals(entry.getRawMode())) {
-			// Ignore the executable file bits if WorkingTreeOptions tell me to
-			// do so. Ignoring is done by setting the bits representing a
-			// EXECUTABLE_FILE to '0' in modeDiff
-			if (!state.options.isFileMode())
-				modeDiff &= ~FileMode.EXECUTABLE_FILE.getBits();
-			if (modeDiff != 0)
-				// Report a modification if the modes still (after potentially
-				// ignoring EXECUTABLE_FILE bits) differ
 			return MetadataDiff.DIFFER_BY_METADATA;
-		}
 
 		// Git under windows only stores seconds so we round the timestamp
 		// Java gives us if it looks like the timestamp in index is seconds