]> source.dussan.org Git - jgit.git/commitdiff
Revert "Fix for core.autocrlf=input resulting in modified file..." 77/19977/1
authorMatthias Sohn <matthias.sohn@sap.com>
Wed, 18 Dec 2013 14:30:53 +0000 (15:30 +0100)
committerMatthias Sohn <matthias.sohn@sap.com>
Wed, 18 Dec 2013 14:36:02 +0000 (15:36 +0100)
This reverts commit 1def0a1257d32bce08e751242733cda3b2036cb8.

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>
org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/IndexDiffTest.java
org.eclipse.jgit.test/tst/org/eclipse/jgit/treewalk/FileTreeIteratorTest.java
org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java
org.eclipse.jgit/src/org/eclipse/jgit/lib/IndexDiff.java
org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java
org.eclipse.jgit/src/org/eclipse/jgit/treewalk/filter/IndexDiffFilter.java
org.eclipse.jgit/src/org/eclipse/jgit/util/io/EolCanonicalizingInputStream.java

index a1435aed4409fc0cc236bc397fd83520271155a7..51ba5f13ea1266d0aef965bf59d02a4f58b6fab1 100644 (file)
@@ -68,11 +68,9 @@ import org.eclipse.jgit.dircache.DirCacheEditor;
 import org.eclipse.jgit.dircache.DirCacheEditor.PathEdit;
 import org.eclipse.jgit.dircache.DirCacheEntry;
 import org.eclipse.jgit.junit.RepositoryTestCase;
-import org.eclipse.jgit.lib.CoreConfig.AutoCRLF;
 import org.eclipse.jgit.lib.IndexDiff.StageState;
 import org.eclipse.jgit.merge.MergeStrategy;
 import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.storage.file.FileBasedConfig;
 import org.eclipse.jgit.treewalk.FileTreeIterator;
 import org.eclipse.jgit.util.IO;
 import org.junit.Test;
@@ -531,35 +529,6 @@ public class IndexDiffTest extends RepositoryTestCase {
                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)
                        throws IOException {
                DirCacheBuilder builder = db.lockDirCache().builder();
index 4ce4c8d04f6b97e1642fd5fbe16cab504778a794..6014f3b60e2d3fd59adb141930a2bbdf49dbe398 100644 (file)
@@ -223,9 +223,7 @@ public class FileTreeIteratorTest extends RepositoryTestCase {
                ObjectId fromRaw = ObjectId.fromRaw(fti.idBuffer(), fti.idOffset());
                assertEquals("6b584e8ece562ebffc15d38808cd6b98fc3d97ea",
                                fromRaw.getName());
-               ObjectReader objectReader = db.newObjectReader();
-               assertFalse(fti.isModified(dce, false, objectReader));
-               objectReader.release();
+               assertFalse(fti.isModified(dce, false));
        }
 
        @Test
@@ -244,9 +242,7 @@ public class FileTreeIteratorTest extends RepositoryTestCase {
                                .getConfig().get(WorkingTreeOptions.KEY));
                while (!fti.getEntryPathString().equals("symlink"))
                        fti.next(1);
-               ObjectReader objectReader = db.newObjectReader();
-               assertFalse(fti.isModified(dce, false, objectReader));
-               objectReader.release();
+               assertFalse(fti.isModified(dce, false));
        }
 
        @Test
@@ -269,9 +265,7 @@ public class FileTreeIteratorTest extends RepositoryTestCase {
                // If the rounding trick does not work we could skip the compareMetaData
                // test and hope that we are usually testing the intended code path.
                assertEquals(MetadataDiff.SMUDGED, fti.compareMetadata(dce));
-               ObjectReader objectReader = db.newObjectReader();
-               assertTrue(fti.isModified(dce, false, objectReader));
-               objectReader.release();
+               assertTrue(fti.isModified(dce, false));
        }
 
        @Test
index 40efc95f88d95033c794823b8b6f934ab4b41767..f8c8442ff860c7d33dab7ce0b44751fae8a33335 100644 (file)
@@ -326,8 +326,7 @@ public class DirCacheCheckout {
                                                m.getEntryFileMode());
                        } else if (i.getDirCacheEntry() != null) {
                                // The index contains a file (and not a folder)
-                               if (f.isModified(i.getDirCacheEntry(), true,
-                                               this.walk.getObjectReader())
+                               if (f.isModified(i.getDirCacheEntry(), true)
                                                || i.getDirCacheEntry().getStage() != 0)
                                        // The working tree file is dirty or the index contains a
                                        // conflict
@@ -661,9 +660,7 @@ public class DirCacheCheckout {
                                break;
                        case 0xFFD: // 12 13 14
                                if (equalIdAndMode(hId, hMode, iId, iMode))
-                                       if (f == null
-                                                       || f.isModified(dce, true,
-                                                                       this.walk.getObjectReader()))
+                                       if (f == null || f.isModified(dce, true))
                                                conflict(name, dce, h, m);
                                        else
                                                remove(name);
@@ -777,8 +774,7 @@ public class DirCacheCheckout {
                                                // Nothing in Head
                                                // Something in Index
                                                if (dce != null
-                                                               && (f == null || f.isModified(dce, true,
-                                                                               this.walk.getObjectReader())))
+                                                               && (f == null || f.isModified(dce, true)))
                                                        // No file or file is dirty
                                                        // Nothing in Merge and current path is part of
                                                        // File/Folder conflict
@@ -845,9 +841,7 @@ public class DirCacheCheckout {
                                                // Something different from a submodule in Index
                                                // Nothing in Merge
                                                // Something in Head
-                                               if (f == null
-                                                               || f.isModified(dce, true,
-                                                                               this.walk.getObjectReader()))
+                                               if (f == null || f.isModified(dce, true))
                                                        // file is dirty
                                                        // Index contains the same as Head
                                                        // Something different from a submodule in Index
@@ -910,8 +904,7 @@ public class DirCacheCheckout {
                                                // file content
                                                update(name, mId, mMode);
                                        } else if (dce != null
-                                                       && (f == null || f.isModified(dce, true,
-                                                                       this.walk.getObjectReader()))) {
+                                                       && (f == null || f.isModified(dce, true))) {
                                                // File doesn't exist or is dirty
                                                // Head and Index don't contain a submodule
                                                // Head contains the same as Index. Merge differs
@@ -1048,8 +1041,7 @@ public class DirCacheCheckout {
                        wtIt = tw.getTree(1, WorkingTreeIterator.class);
                        if (dcIt == null || wtIt == null)
                                return true;
-                       if (wtIt.isModified(dcIt.getDirCacheEntry(), true,
-                                       this.walk.getObjectReader())) {
+                       if (wtIt.isModified(dcIt.getDirCacheEntry(), true)) {
                                return true;
                        }
                }
index 8eb033355016c1cd50f280472d9f802b74eda154..33654447ae849ad0d10c013d62ba242fc49903e7 100644 (file)
@@ -440,8 +440,7 @@ public class IndexDiff {
                                        missing.add(treeWalk.getPathString());
                                } else {
                                        if (workingTreeIterator.isModified(
-                                                       dirCacheIterator.getDirCacheEntry(), true,
-                                                       treeWalk.getObjectReader())) {
+                                                       dirCacheIterator.getDirCacheEntry(), true)) {
                                                // in index, in workdir, content differs => modified
                                                modified.add(treeWalk.getPathString());
                                        }
index f2274d5eaf35da8cc043fe2445c0265b6e53691f..07ba9d73a47e12ef16ccca6cc16bd1689ed93d4c 100644 (file)
@@ -73,12 +73,10 @@ import org.eclipse.jgit.ignore.IgnoreRule;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.CoreConfig;
-import org.eclipse.jgit.lib.CoreConfig.CheckStat;
 import org.eclipse.jgit.lib.FileMode;
 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.CoreConfig.CheckStat;
 import org.eclipse.jgit.submodule.SubmoduleWalk;
 import org.eclipse.jgit.util.FS;
 import org.eclipse.jgit.util.IO;
@@ -797,27 +795,23 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator {
         * @param forceContentCheck
         *            True if the actual file content should be checked if
         *            modification time differs.
-        * @param reader
-        *            access to repository objects if necessary.
         * @return true if content is most likely different.
-        * @since 3.2
         */
-       public boolean isModified(DirCacheEntry entry, boolean forceContentCheck,
-                       ObjectReader reader) {
+       public boolean isModified(DirCacheEntry entry, boolean forceContentCheck) {
                MetadataDiff diff = compareMetadata(entry);
                switch (diff) {
                case DIFFER_BY_TIMESTAMP:
                        if (forceContentCheck)
                                // But we are told to look at content even though timestamps
                                // tell us about modification
-                               return contentCheck(entry, reader);
+                               return contentCheck(entry);
                        else
                                // We are told to assume a modification if timestamps differs
                                return true;
                case SMUDGED:
                        // The file is clean by timestamps but the entry was smudged.
                        // Lets do a content check
-                       return contentCheck(entry, reader);
+                       return contentCheck(entry);
                case EQUAL:
                        return false;
                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
         * in the index.
@@ -880,12 +854,10 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator {
         *
         * @param entry
         *            the entry to be checked
-        * @param reader
-        *            acccess to repository data if necessary
-        * @return <code>true</code> if the content doesn't match,
-        *         <code>false</code> if it matches
+        * @return <code>true</code> if the content matches, <code>false</code>
+        *         otherwise
         */
-       private boolean contentCheck(DirCacheEntry entry, ObjectReader reader) {
+       private boolean contentCheck(DirCacheEntry entry) {
                if (getEntryObjectId().equals(entry.getObjectId())) {
                        // Content has not changed
 
@@ -901,68 +873,7 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator {
 
                        return false;
                } else {
-                       // Content differs: that's a real change, perhaps
-                       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;
-                       }
+                       // Content differs: that's a real change!
                        return true;
                }
        }
index 150d5c786a1e5ccad420b079b45909443be924be..1b231cce9dfbb8fb8087c1335b6190b8215894d8 100644 (file)
@@ -53,7 +53,6 @@ import org.eclipse.jgit.dircache.DirCacheIterator;
 import org.eclipse.jgit.errors.IncorrectObjectTypeException;
 import org.eclipse.jgit.errors.MissingObjectException;
 import org.eclipse.jgit.lib.FileMode;
-import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.treewalk.TreeWalk;
 import org.eclipse.jgit.treewalk.WorkingTreeIterator;
 
@@ -73,7 +72,7 @@ import org.eclipse.jgit.treewalk.WorkingTreeIterator;
  * <p>
  * If no difference is found then we have to compare index and working-tree as
  * 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.
  * <p>
  * 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
                // tree. Make use of the WorkingTreeIterator#isModified() method to
                // avoid computing SHA1 on filesystem content if not really needed.
-               return wi.isModified(di.getDirCacheEntry(), true, tw.getObjectReader());
+               return wi.isModified(di.getDirCacheEntry(), true);
        }
 
        /**
index d23e1c16094543d5e5726f1dd3c789a4faa9999e..f87ab689631355f9e2641cb34db4fe86fc06099b 100644 (file)
@@ -70,23 +70,6 @@ public class EolCanonicalizingInputStream extends InputStream {
 
        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
         *
@@ -97,25 +80,8 @@ public class EolCanonicalizingInputStream extends InputStream {
         * @since 2.0
         */
        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.detectBinary = detectBinary;
-               this.abortIfBinary = abortIfBinary;
        }
 
        @Override
@@ -162,14 +128,6 @@ public class EolCanonicalizingInputStream extends InputStream {
                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
        public void close() throws IOException {
                in.close();
@@ -182,8 +140,6 @@ public class EolCanonicalizingInputStream extends InputStream {
                if (detectBinary) {
                        isBinary = RawText.isBinary(buf, cnt);
                        detectBinary = false;
-                       if (isBinary && abortIfBinary)
-                               throw new IsBinaryException();
                }
                ptr = 0;
                return true;