Fix TreeWalk bug comparing DirCache and WorkingTree with ANY_DIFF

When comparing a DirCache and a WorkingTree using ANY_DIFF we
sometimes didn't recursive into a subtree of both sides gave us
zeroId() back for the identity of a subtree.  This happens when the
DirCache doesn't have a valid cache tree for the subtree, as then
it uses zeroId() for the ObjectId of the subtree, which then appears
to be equal to the zeroId() of the WorkingTreeIterator's subtree.

We work around this by adding a hasId() method that returns true
only if this iterator has a valid ObjectId.  The idEquals method
on TreeWalk than only performs a compare between two iterators if
both iterators have a valid id.

Change-Id: I695f7fafbeb452e8c0703a05c02921fae0822d3f
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheIterator.java b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheIterator.java
index aa8d9fb85d217779a6c434b39220df24ef6f8318..e685e0cad8e1877867a998e37e63ca5ab0ba069d 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheIterator.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheIterator.java
@@ -45,7 +45,6 @@
 package org.eclipse.jgit.dircache;
 
 import java.io.IOException;
-import java.util.Arrays;
 
 import org.eclipse.jgit.errors.IncorrectObjectTypeException;
 import org.eclipse.jgit.lib.Constants;
@@ -141,10 +140,17 @@ public class DirCacheIterator extends AbstractTreeIterator {
                return new EmptyTreeIterator(this, n, pathLen + 1);
        }
 
+       @Override
+       public boolean hasId() {
+               if (currentSubtree != null)
+                       return currentSubtree.isValid();
+               return currentEntry != null;
+       }
+
        @Override
        public byte[] idBuffer() {
                if (currentSubtree != null)
-                       return subtreeId;
+                       return currentSubtree.isValid() ? subtreeId : zeroid;
                if (currentEntry != null)
                        return currentEntry.idBuffer();
                return zeroid;
@@ -218,8 +224,6 @@ public class DirCacheIterator extends AbstractTreeIterator {
 
                                if (s.isValid())
                                        s.getObjectId().copyRawTo(subtreeId, 0);
-                               else
-                                       Arrays.fill(subtreeId, (byte) 0);
                                mode = FileMode.TREE.getBits();
                                path = cep;
                                pathLen = pathOffset + s.nameLength();

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/AbstractTreeIterator.java b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/AbstractTreeIterator.java
index eee62c63a5ab350080bf3e5c0da529884f981893..79b57d1eb63a6901ab6ba356060d020fb0f2ac38 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/AbstractTreeIterator.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/AbstractTreeIterator.java
@@ -369,6 +369,9 @@ public abstract class AbstractTreeIterator {
                                otherIterator.idBuffer(), otherIterator.idOffset());
        }
 
+       /** @return true if the entry has a valid ObjectId. */
+       public abstract boolean hasId();
+
        /**
         * Get the object id of the current entry.
         *

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/CanonicalTreeParser.java b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/CanonicalTreeParser.java
index 01b8274253bd8e13c78dcb001ceafcd6e64e57f4..1e49d380a80041efbd893fcf05995fdd0e3849d1 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/CanonicalTreeParser.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/CanonicalTreeParser.java
@@ -233,6 +233,11 @@ public class CanonicalTreeParser extends AbstractTreeIterator {
                return createSubtreeIterator(reader, new MutableObjectId());
        }
 
+       @Override
+       public boolean hasId() {
+               return true;
+       }
+
        @Override
        public byte[] idBuffer() {
                return raw;

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/EmptyTreeIterator.java b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/EmptyTreeIterator.java
index 49d75871e8438f48f24044c0588e8f2973e4ae3c..8dbf80e6a860e6ff68fee570831edc6619edce57 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/EmptyTreeIterator.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/EmptyTreeIterator.java
@@ -92,6 +92,11 @@ public class EmptyTreeIterator extends AbstractTreeIterator {
                return new EmptyTreeIterator(this);
        }
 
+       @Override
+       public boolean hasId() {
+               return false;
+       }
+
        @Override
        public ObjectId getEntryObjectId() {
                return ObjectId.zeroId();

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/TreeWalk.java b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/TreeWalk.java
index 16859646b626d6eb1f1e7324c4ad24e8a28d2878..992928bc4398936a0a8d7fc9e0c58f9a49632579 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/TreeWalk.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/TreeWalk.java
@@ -683,8 +683,6 @@ public class TreeWalk {
                final AbstractTreeIterator ch = currentHead;
                final AbstractTreeIterator a = trees[nthA];
                final AbstractTreeIterator b = trees[nthB];
-               if (a.matches == ch && b.matches == ch)
-                       return a.idEqual(b);
                if (a.matches != ch && b.matches != ch) {
                        // If neither tree matches the current path node then neither
                        // tree has this entry. In such case the ObjectId is zero(),
@@ -692,6 +690,10 @@ public class TreeWalk {
                        //
                        return true;
                }
+               if (!a.hasId() || !b.hasId())
+                       return false;
+               if (a.matches == ch && b.matches == ch)
+                       return a.idEqual(b);
                return false;
        }
 

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java
index 51c3483699a7709e85bc4138d74a50a0a1950200..5cc061bbb4d20f6d5cf2ea1fd903feb3cc6c6db7 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java
@@ -194,6 +194,13 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator {
                ignoreNode = new RootIgnoreNode(entry, repo);
        }
 
+       @Override
+       public boolean hasId() {
+               if (contentIdFromPtr == ptr)
+                       return true;
+               return (mode & FileMode.TYPE_MASK) == FileMode.TYPE_FILE;
+       }
+
        @Override
        public byte[] idBuffer() {
                if (contentIdFromPtr == ptr)