Skip to content
This repository was archived by the owner on Sep 11, 2020. It is now read-only.

Merkletrie.Difftree return wrong results for directories named after files without extension. #275

Closed
alcortesm opened this issue Feb 20, 2017 · 0 comments · Fixed by #276
Milestone

Comments

@alcortesm
Copy link
Contributor

alcortesm commented Feb 20, 2017

On http://github.com/git/git.git:

; git diff-tree -r 249894 c1f7a56
:000000 100644 0000000000000000000000000000000000000000 1239c70de997dd93a59bf6d3d9cc126bffbc5392 A	src/runtime/cgo/tmpdir_darwin.go

But the current merkletrie.difftree implementation returns something different:

; go-git-diff-tree-sprint-demo . 249894ab6c4d8439e7fd47ca55ba618d636ab239 c1f7a56fc05584b931c979779dd086c417b50dcb
:000644 000000 169a31d4c6ed57a4fb6ba44b28cb52c97d9b007a 0000000000000000000000000000000000000000 D	src/runtime/cgo.go
:000000 000644 0000000000000000000000000000000000000000 1239c70de997dd93a59bf6d3d9cc126bffbc5392 A	src/runtime/cgo/tmpdir_darwin.go
:000000 000644 0000000000000000000000000000000000000000 169a31d4c6ed57a4fb6ba44b28cb52c97d9b007a A	src/runtime/cgo.go

This is because we have a bug in how merkletrie.Difftree tells insertions from deletions. The way to do it is, while iterating over the tree, checking which one of the current noders would come first when iterating over the tree:

  • If from would come after to, it means the to has been inserted (otherwise we will be having it in the from too).

  • If from would come before to, it means from was deleted (otherwise we will be having it in the to too).

The current way to tell which one would come first is by comparing their path lexicographically, but this is wrong, as it returns incorrect results with some nested directories, example:

                       Insert 
                       a/b/d.go

             From                      To
               a                        a
              /  \                    /  \
            b    b.go               b     b.go
           /                      /  \
         /                      /      \
       c.go                   c.go      d.go

When from -> a/b.go and to -> a/b/d.go, from will be lexicographically smaller than to, which will result in believing from has been deleted, which is not.

To fix this, we must compare paths level by level, instead of over the full path.

@alcortesm alcortesm changed the title Merkletrie.Difftree return wrong resutls for nested directories Merkletrie.Difftree return wrong resutls for directories named after files without extension. Feb 20, 2017
@smola smola modified the milestone: v4.0.0-rc10 Feb 21, 2017
@ajnavarro ajnavarro changed the title Merkletrie.Difftree return wrong resutls for directories named after files without extension. Merkletrie.Difftree return wrong results for directories named after files without extension. Feb 21, 2017
alcortesm added a commit that referenced this issue Feb 22, 2017
Fix #275 .

It was not possible to write a test for this issue as the original fsnoder didn't support filenames with length > 1. Therefore this patch has 3 commits:

add support for long filenames in fsnoder.
add a test case for the issue using the new long filenames from step 1.
fix the issue by comparing paths level by level instead of lexigographically over the whole path.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants