Browse Source

Revert "Fix for core.autocrlf=input resulting in modified file..."

This reverts commit 1def0a1257.

We found this fix uncovers problems with unsmudged DirCacheEntry's. This
surfaced because egit's ui test CreatePatchActionTest failed since jgit
computes a wrong status. JGit doesn't detect modified content in now
unsmudged entries. Hence revert this change until these problems are
fixed.

Change-Id: Ia04277ce316d35fc5b0d82c93d2078b856af24bb
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>
stable-3.2
Matthias Sohn 11 years ago
parent
commit
0ff691cdb5
  1. 31
      org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/IndexDiffTest.java
  2. 12
      org.eclipse.jgit.test/tst/org/eclipse/jgit/treewalk/FileTreeIteratorTest.java
  3. 20
      org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java
  4. 3
      org.eclipse.jgit/src/org/eclipse/jgit/lib/IndexDiff.java
  5. 105
      org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java
  6. 5
      org.eclipse.jgit/src/org/eclipse/jgit/treewalk/filter/IndexDiffFilter.java
  7. 44
      org.eclipse.jgit/src/org/eclipse/jgit/util/io/EolCanonicalizingInputStream.java

31
org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/IndexDiffTest.java

@ -68,11 +68,9 @@ import org.eclipse.jgit.dircache.DirCacheEditor;
import org.eclipse.jgit.dircache.DirCacheEditor.PathEdit; import org.eclipse.jgit.dircache.DirCacheEditor.PathEdit;
import org.eclipse.jgit.dircache.DirCacheEntry; import org.eclipse.jgit.dircache.DirCacheEntry;
import org.eclipse.jgit.junit.RepositoryTestCase; import org.eclipse.jgit.junit.RepositoryTestCase;
import org.eclipse.jgit.lib.CoreConfig.AutoCRLF;
import org.eclipse.jgit.lib.IndexDiff.StageState; import org.eclipse.jgit.lib.IndexDiff.StageState;
import org.eclipse.jgit.merge.MergeStrategy; import org.eclipse.jgit.merge.MergeStrategy;
import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.storage.file.FileBasedConfig;
import org.eclipse.jgit.treewalk.FileTreeIterator; import org.eclipse.jgit.treewalk.FileTreeIterator;
import org.eclipse.jgit.util.IO; import org.eclipse.jgit.util.IO;
import org.junit.Test; import org.junit.Test;
@ -531,35 +529,6 @@ public class IndexDiffTest extends RepositoryTestCase {
assertTrue(StageState.BOTH_ADDED.hasTheirs()); assertTrue(StageState.BOTH_ADDED.hasTheirs());
} }
@Test
public void testAutoCRLFInput() throws Exception {
Git git = new Git(db);
FileBasedConfig config = db.getConfig();
// Make sure core.autocrlf is false before adding
config.setEnum(ConfigConstants.CONFIG_CORE_SECTION, null,
ConfigConstants.CONFIG_KEY_AUTOCRLF, AutoCRLF.FALSE);
config.save();
// File is already in repository with CRLF
writeTrashFile("crlf.txt", "this\r\ncontains\r\ncrlf\r\n");
git.add().addFilepattern("crlf.txt").call();
git.commit().setMessage("Add crlf.txt").call();
// Now set core.autocrlf to input
config.setEnum(ConfigConstants.CONFIG_CORE_SECTION, null,
ConfigConstants.CONFIG_KEY_AUTOCRLF, AutoCRLF.INPUT);
config.save();
FileTreeIterator iterator = new FileTreeIterator(db);
IndexDiff diff = new IndexDiff(db, Constants.HEAD, iterator);
diff.diff();
assertTrue(
"Expected no modified files, but there were: "
+ diff.getModified(), diff.getModified().isEmpty());
}
private void verifyStageState(StageState expected, int... stages) private void verifyStageState(StageState expected, int... stages)
throws IOException { throws IOException {
DirCacheBuilder builder = db.lockDirCache().builder(); DirCacheBuilder builder = db.lockDirCache().builder();

12
org.eclipse.jgit.test/tst/org/eclipse/jgit/treewalk/FileTreeIteratorTest.java

@ -223,9 +223,7 @@ public class FileTreeIteratorTest extends RepositoryTestCase {
ObjectId fromRaw = ObjectId.fromRaw(fti.idBuffer(), fti.idOffset()); ObjectId fromRaw = ObjectId.fromRaw(fti.idBuffer(), fti.idOffset());
assertEquals("6b584e8ece562ebffc15d38808cd6b98fc3d97ea", assertEquals("6b584e8ece562ebffc15d38808cd6b98fc3d97ea",
fromRaw.getName()); fromRaw.getName());
ObjectReader objectReader = db.newObjectReader(); assertFalse(fti.isModified(dce, false));
assertFalse(fti.isModified(dce, false, objectReader));
objectReader.release();
} }
@Test @Test
@ -244,9 +242,7 @@ public class FileTreeIteratorTest extends RepositoryTestCase {
.getConfig().get(WorkingTreeOptions.KEY)); .getConfig().get(WorkingTreeOptions.KEY));
while (!fti.getEntryPathString().equals("symlink")) while (!fti.getEntryPathString().equals("symlink"))
fti.next(1); fti.next(1);
ObjectReader objectReader = db.newObjectReader(); assertFalse(fti.isModified(dce, false));
assertFalse(fti.isModified(dce, false, objectReader));
objectReader.release();
} }
@Test @Test
@ -269,9 +265,7 @@ public class FileTreeIteratorTest extends RepositoryTestCase {
// If the rounding trick does not work we could skip the compareMetaData // If the rounding trick does not work we could skip the compareMetaData
// test and hope that we are usually testing the intended code path. // test and hope that we are usually testing the intended code path.
assertEquals(MetadataDiff.SMUDGED, fti.compareMetadata(dce)); assertEquals(MetadataDiff.SMUDGED, fti.compareMetadata(dce));
ObjectReader objectReader = db.newObjectReader(); assertTrue(fti.isModified(dce, false));
assertTrue(fti.isModified(dce, false, objectReader));
objectReader.release();
} }
@Test @Test

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

@ -326,8 +326,7 @@ public class DirCacheCheckout {
m.getEntryFileMode()); m.getEntryFileMode());
} else if (i.getDirCacheEntry() != null) { } else if (i.getDirCacheEntry() != null) {
// The index contains a file (and not a folder) // The index contains a file (and not a folder)
if (f.isModified(i.getDirCacheEntry(), true, if (f.isModified(i.getDirCacheEntry(), true)
this.walk.getObjectReader())
|| i.getDirCacheEntry().getStage() != 0) || i.getDirCacheEntry().getStage() != 0)
// The working tree file is dirty or the index contains a // The working tree file is dirty or the index contains a
// conflict // conflict
@ -661,9 +660,7 @@ public class DirCacheCheckout {
break; break;
case 0xFFD: // 12 13 14 case 0xFFD: // 12 13 14
if (equalIdAndMode(hId, hMode, iId, iMode)) if (equalIdAndMode(hId, hMode, iId, iMode))
if (f == null if (f == null || f.isModified(dce, true))
|| f.isModified(dce, true,
this.walk.getObjectReader()))
conflict(name, dce, h, m); conflict(name, dce, h, m);
else else
remove(name); remove(name);
@ -777,8 +774,7 @@ public class DirCacheCheckout {
// Nothing in Head // Nothing in Head
// Something in Index // Something in Index
if (dce != null if (dce != null
&& (f == null || f.isModified(dce, true, && (f == null || f.isModified(dce, true)))
this.walk.getObjectReader())))
// No file or file is dirty // No file or file is dirty
// Nothing in Merge and current path is part of // Nothing in Merge and current path is part of
// File/Folder conflict // File/Folder conflict
@ -845,9 +841,7 @@ public class DirCacheCheckout {
// Something different from a submodule in Index // Something different from a submodule in Index
// Nothing in Merge // Nothing in Merge
// Something in Head // Something in Head
if (f == null if (f == null || f.isModified(dce, true))
|| f.isModified(dce, true,
this.walk.getObjectReader()))
// file is dirty // file is dirty
// Index contains the same as Head // Index contains the same as Head
// Something different from a submodule in Index // Something different from a submodule in Index
@ -910,8 +904,7 @@ public class DirCacheCheckout {
// file content // file content
update(name, mId, mMode); update(name, mId, mMode);
} else if (dce != null } else if (dce != null
&& (f == null || f.isModified(dce, true, && (f == null || f.isModified(dce, true))) {
this.walk.getObjectReader()))) {
// File doesn't exist or is dirty // File doesn't exist or is dirty
// Head and Index don't contain a submodule // Head and Index don't contain a submodule
// Head contains the same as Index. Merge differs // Head contains the same as Index. Merge differs
@ -1048,8 +1041,7 @@ public class DirCacheCheckout {
wtIt = tw.getTree(1, WorkingTreeIterator.class); wtIt = tw.getTree(1, WorkingTreeIterator.class);
if (dcIt == null || wtIt == null) if (dcIt == null || wtIt == null)
return true; return true;
if (wtIt.isModified(dcIt.getDirCacheEntry(), true, if (wtIt.isModified(dcIt.getDirCacheEntry(), true)) {
this.walk.getObjectReader())) {
return true; return true;
} }
} }

3
org.eclipse.jgit/src/org/eclipse/jgit/lib/IndexDiff.java

@ -440,8 +440,7 @@ public class IndexDiff {
missing.add(treeWalk.getPathString()); missing.add(treeWalk.getPathString());
} else { } else {
if (workingTreeIterator.isModified( if (workingTreeIterator.isModified(
dirCacheIterator.getDirCacheEntry(), true, dirCacheIterator.getDirCacheEntry(), true)) {
treeWalk.getObjectReader())) {
// in index, in workdir, content differs => modified // in index, in workdir, content differs => modified
modified.add(treeWalk.getPathString()); modified.add(treeWalk.getPathString());
} }

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

@ -73,12 +73,10 @@ import org.eclipse.jgit.ignore.IgnoreRule;
import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.internal.JGitText;
import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.CoreConfig; import org.eclipse.jgit.lib.CoreConfig;
import org.eclipse.jgit.lib.CoreConfig.CheckStat;
import org.eclipse.jgit.lib.FileMode; import org.eclipse.jgit.lib.FileMode;
import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectLoader;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.lib.CoreConfig.CheckStat;
import org.eclipse.jgit.submodule.SubmoduleWalk; import org.eclipse.jgit.submodule.SubmoduleWalk;
import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.FS;
import org.eclipse.jgit.util.IO; import org.eclipse.jgit.util.IO;
@ -797,27 +795,23 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator {
* @param forceContentCheck * @param forceContentCheck
* True if the actual file content should be checked if * True if the actual file content should be checked if
* modification time differs. * modification time differs.
* @param reader
* access to repository objects if necessary.
* @return true if content is most likely different. * @return true if content is most likely different.
* @since 3.2
*/ */
public boolean isModified(DirCacheEntry entry, boolean forceContentCheck, public boolean isModified(DirCacheEntry entry, boolean forceContentCheck) {
ObjectReader reader) {
MetadataDiff diff = compareMetadata(entry); MetadataDiff diff = compareMetadata(entry);
switch (diff) { switch (diff) {
case DIFFER_BY_TIMESTAMP: case DIFFER_BY_TIMESTAMP:
if (forceContentCheck) if (forceContentCheck)
// But we are told to look at content even though timestamps // But we are told to look at content even though timestamps
// tell us about modification // tell us about modification
return contentCheck(entry, reader); return contentCheck(entry);
else else
// We are told to assume a modification if timestamps differs // We are told to assume a modification if timestamps differs
return true; return true;
case SMUDGED: case SMUDGED:
// The file is clean by timestamps but the entry was smudged. // The file is clean by timestamps but the entry was smudged.
// Lets do a content check // Lets do a content check
return contentCheck(entry, reader); return contentCheck(entry);
case EQUAL: case EQUAL:
return false; return false;
case DIFFER_BY_METADATA: case DIFFER_BY_METADATA:
@ -828,26 +822,6 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator {
} }
} }
/**
* Checks whether this entry differs from a given entry from the
* {@link DirCache}.
*
* File status information is used and if status is same we consider the
* file identical to the state in the working directory. Native git uses
* more stat fields than we have accessible in Java.
*
* @param entry
* the entry from the dircache we want to compare against
* @param forceContentCheck
* True if the actual file content should be checked if
* modification time differs.
* @return true if content is most likely different.
* @deprecated Use {@link #isModified(DirCacheEntry, boolean, ObjectReader)}
*/
public boolean isModified(DirCacheEntry entry, boolean forceContentCheck) {
return isModified(entry, false, null);
}
/** /**
* Get the file mode to use for the current entry when it is to be updated * Get the file mode to use for the current entry when it is to be updated
* in the index. * in the index.
@ -880,12 +854,10 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator {
* *
* @param entry * @param entry
* the entry to be checked * the entry to be checked
* @param reader * @return <code>true</code> if the content matches, <code>false</code>
* acccess to repository data if necessary * otherwise
* @return <code>true</code> if the content doesn't match,
* <code>false</code> if it matches
*/ */
private boolean contentCheck(DirCacheEntry entry, ObjectReader reader) { private boolean contentCheck(DirCacheEntry entry) {
if (getEntryObjectId().equals(entry.getObjectId())) { if (getEntryObjectId().equals(entry.getObjectId())) {
// Content has not changed // Content has not changed
@ -901,68 +873,7 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator {
return false; return false;
} else { } else {
// Content differs: that's a real change, perhaps // Content differs: that's a real change!
if (reader == null) // deprecated use, do no further checks
return true;
switch (getOptions().getAutoCRLF()) {
case INPUT:
case TRUE:
InputStream dcIn = null;
try {
ObjectLoader loader = reader.open(entry.getObjectId());
if (loader == null)
return true;
// We need to compute the length, but only if it is not
// a binary stream.
dcIn = new EolCanonicalizingInputStream(
loader.openStream(), true, true /* abort if binary */);
long dcInLen;
try {
dcInLen = computeLength(dcIn);
} catch (EolCanonicalizingInputStream.IsBinaryException e) {
// ok, we know it's different so unsmudge the entry
entry.setLength(entry.getLength());
return true;
} finally {
dcIn.close();
}
dcIn = new EolCanonicalizingInputStream(
loader.openStream(), true);
byte[] autoCrLfHash = computeHash(dcIn, dcInLen);
boolean changed = getEntryObjectId().compareTo(
autoCrLfHash, 0) != 0;
if (!changed) {
// Update the index with the eol'ed hash, so we can
// detect the no-change faster next time
entry.setObjectIdFromRaw(autoCrLfHash, 0);
}
// Ok, we know whether it has changed, so unsmudge the
// dirache entry
entry.setLength(loader.getSize());
return changed;
} catch (IOException e) {
return true;
} finally {
if (dcIn != null)
try {
dcIn.close();
} catch (IOException e) {
// empty
}
}
case FALSE:
// Ok, we know it's different so unsmudge the dircache entry
try {
ObjectLoader loader = reader.open(entry.getObjectId());
if (loader != null)
entry.setLength((int) loader.getSize());
} catch (IOException e) {
// panic, no, but don't unsmudge
}
break;
}
return true; return true;
} }
} }

5
org.eclipse.jgit/src/org/eclipse/jgit/treewalk/filter/IndexDiffFilter.java

@ -53,7 +53,6 @@ import org.eclipse.jgit.dircache.DirCacheIterator;
import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.lib.FileMode; import org.eclipse.jgit.lib.FileMode;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.treewalk.TreeWalk; import org.eclipse.jgit.treewalk.TreeWalk;
import org.eclipse.jgit.treewalk.WorkingTreeIterator; import org.eclipse.jgit.treewalk.WorkingTreeIterator;
@ -73,7 +72,7 @@ import org.eclipse.jgit.treewalk.WorkingTreeIterator;
* <p> * <p>
* If no difference is found then we have to compare index and working-tree as * If no difference is found then we have to compare index and working-tree as
* the last step. By making use of * the last step. By making use of
* {@link WorkingTreeIterator#isModified(org.eclipse.jgit.dircache.DirCacheEntry, boolean, ObjectReader)} * {@link WorkingTreeIterator#isModified(org.eclipse.jgit.dircache.DirCacheEntry, boolean)}
* we can avoid the computation of the content id if the file is not dirty. * we can avoid the computation of the content id if the file is not dirty.
* <p> * <p>
* Instances of this filter should not be used for multiple {@link TreeWalk}s. * Instances of this filter should not be used for multiple {@link TreeWalk}s.
@ -219,7 +218,7 @@ public class IndexDiffFilter extends TreeFilter {
// Only one chance left to detect a diff: between index and working // Only one chance left to detect a diff: between index and working
// tree. Make use of the WorkingTreeIterator#isModified() method to // tree. Make use of the WorkingTreeIterator#isModified() method to
// avoid computing SHA1 on filesystem content if not really needed. // avoid computing SHA1 on filesystem content if not really needed.
return wi.isModified(di.getDirCacheEntry(), true, tw.getObjectReader()); return wi.isModified(di.getDirCacheEntry(), true);
} }
/** /**

44
org.eclipse.jgit/src/org/eclipse/jgit/util/io/EolCanonicalizingInputStream.java

@ -70,23 +70,6 @@ public class EolCanonicalizingInputStream extends InputStream {
private boolean detectBinary; private boolean detectBinary;
private boolean abortIfBinary;
/**
* A special exception thrown when {@link EolCanonicalizingInputStream} is
* told to throw an exception when attempting to read a binary file. The
* exception may be thrown at any stage during reading.
*
* @since 3.2
*/
public static class IsBinaryException extends IOException {
private static final long serialVersionUID = 1L;
IsBinaryException() {
super();
}
}
/** /**
* Creates a new InputStream, wrapping the specified stream * Creates a new InputStream, wrapping the specified stream
* *
@ -97,25 +80,8 @@ public class EolCanonicalizingInputStream extends InputStream {
* @since 2.0 * @since 2.0
*/ */
public EolCanonicalizingInputStream(InputStream in, boolean detectBinary) { public EolCanonicalizingInputStream(InputStream in, boolean detectBinary) {
this(in, detectBinary, false);
}
/**
* Creates a new InputStream, wrapping the specified stream
*
* @param in
* raw input stream
* @param detectBinary
* whether binaries should be detected
* @param abortIfBinary
* throw an IOException if the file is binary
* @since 3.2
*/
public EolCanonicalizingInputStream(InputStream in, boolean detectBinary,
boolean abortIfBinary) {
this.in = in; this.in = in;
this.detectBinary = detectBinary; this.detectBinary = detectBinary;
this.abortIfBinary = abortIfBinary;
} }
@Override @Override
@ -162,14 +128,6 @@ public class EolCanonicalizingInputStream extends InputStream {
return i == off ? -1 : i - off; return i == off ? -1 : i - off;
} }
/**
* @return true if the stream has detected as a binary so far
* @since 3.2
*/
public boolean isBinary() {
return isBinary;
}
@Override @Override
public void close() throws IOException { public void close() throws IOException {
in.close(); in.close();
@ -182,8 +140,6 @@ public class EolCanonicalizingInputStream extends InputStream {
if (detectBinary) { if (detectBinary) {
isBinary = RawText.isBinary(buf, cnt); isBinary = RawText.isBinary(buf, cnt);
detectBinary = false; detectBinary = false;
if (isBinary && abortIfBinary)
throw new IsBinaryException();
} }
ptr = 0; ptr = 0;
return true; return true;

Loading…
Cancel
Save