From c40fcefddec1d07fb5a06c2173b409f37f7409d1 Mon Sep 17 00:00:00 2001 From: Vadim Markovtsev Date: Mon, 11 Feb 2019 21:13:44 +0100 Subject: [PATCH] Remove Unicode normalization in difftree Fixes #1057 Signed-off-by: Vadim Markovtsev --- utils/merkletrie/difftree_test.go | 23 ++++++++++++++++++- utils/merkletrie/noder/path.go | 10 +++------ utils/merkletrie/noder/path_test.go | 8 +++++-- worktree_test.go | 35 +++++++++++++++++++++++++---- 4 files changed, 62 insertions(+), 14 deletions(-) diff --git a/utils/merkletrie/difftree_test.go b/utils/merkletrie/difftree_test.go index ab0eb5772..ac861454b 100644 --- a/utils/merkletrie/difftree_test.go +++ b/utils/merkletrie/difftree_test.go @@ -475,8 +475,29 @@ func (s *DiffTreeSuite) TestIssue275(c *C) { }) } +func (s *DiffTreeSuite) TestIssue1057(c *C) { + p1 := "TestAppWithUnicodéPath" + p2 := "TestAppWithUnicodéPath" + c.Assert(p1 == p2, Equals, false) + do(c, []diffTreeTest{ + { + fmt.Sprintf("(%s(x.go<1>))", p1), + fmt.Sprintf("(%s(x.go<1>) %s(x.go<1>))", p1, p2), + fmt.Sprintf("+%s/x.go", p2), + }, + }) + // swap p1 with p2 + do(c, []diffTreeTest{ + { + fmt.Sprintf("(%s(x.go<1>))", p2), + fmt.Sprintf("(%s(x.go<1>) %s(x.go<1>))", p1, p2), + fmt.Sprintf("+%s/x.go", p1), + }, + }) +} + func (s *DiffTreeSuite) TestCancel(c *C) { - t := diffTreeTest{"()", "(a<> b<1> c() d<> e<2> f())", "+a +b +d +e"} + t := diffTreeTest{"()", "(a<> b<1> c() d<> e<2> f())", "+a +b +d +e"} comment := Commentf("\n%s", "test cancel:") a, err := fsnoder.New(t.from) diff --git a/utils/merkletrie/noder/path.go b/utils/merkletrie/noder/path.go index e9c905c96..1c7ef54ee 100644 --- a/utils/merkletrie/noder/path.go +++ b/utils/merkletrie/noder/path.go @@ -3,8 +3,6 @@ package noder import ( "bytes" "strings" - - "golang.org/x/text/unicode/norm" ) // Path values represent a noder and its ancestors. The root goes first @@ -80,11 +78,9 @@ func (p Path) Compare(other Path) int { case i == len(p): return -1 default: - form := norm.Form(norm.NFC) - this := form.String(p[i].Name()) - that := form.String(other[i].Name()) - - cmp := strings.Compare(this, that) + // We do *not* normalize Unicode here. CGit doesn't. + // https://github.com/src-d/go-git/issues/1057 + cmp := strings.Compare(p[i].Name(), other[i].Name()) if cmp != 0 { return cmp } diff --git a/utils/merkletrie/noder/path_test.go b/utils/merkletrie/noder/path_test.go index be2544454..f49f028d3 100644 --- a/utils/merkletrie/noder/path_test.go +++ b/utils/merkletrie/noder/path_test.go @@ -156,6 +156,10 @@ func (s *PathSuite) TestCompareMixedDepths(c *C) { func (s *PathSuite) TestCompareNormalization(c *C) { p1 := Path([]Noder{&noderMock{name: norm.Form(norm.NFKC).String("페")}}) p2 := Path([]Noder{&noderMock{name: norm.Form(norm.NFKD).String("페")}}) - c.Assert(p1.Compare(p2), Equals, 0) - c.Assert(p2.Compare(p1), Equals, 0) + c.Assert(p1.Compare(p2), Equals, 1) + c.Assert(p2.Compare(p1), Equals, -1) + p1 = Path([]Noder{&noderMock{name: "TestAppWithUnicodéPath"}}) + p2 = Path([]Noder{&noderMock{name: "TestAppWithUnicodéPath"}}) + c.Assert(p1.Compare(p2), Equals, -1) + c.Assert(p2.Compare(p1), Equals, 1) } diff --git a/worktree_test.go b/worktree_test.go index c714011c0..5c5aef919 100644 --- a/worktree_test.go +++ b/worktree_test.go @@ -365,8 +365,14 @@ func (s *WorktreeSuite) TestFilenameNormalization(c *C) { w, err := server.Worktree() c.Assert(err, IsNil) - util.WriteFile(w.Filesystem, filename, []byte("foo"), 0755) - _, err = w.Add(filename) + + writeFile := func(path string) { + err := util.WriteFile(w.Filesystem, path, []byte("foo"), 0755) + c.Assert(err, IsNil) + } + + writeFile(filename) + origHash, err := w.Add(filename) c.Assert(err, IsNil) _, err = w.Commit("foo", &CommitOptions{Author: defaultSignature()}) c.Assert(err, IsNil) @@ -387,11 +393,32 @@ func (s *WorktreeSuite) TestFilenameNormalization(c *C) { c.Assert(err, IsNil) modFilename := norm.Form(norm.NFKD).String(filename) - util.WriteFile(w.Filesystem, modFilename, []byte("foo"), 0755) + writeFile(modFilename) + + _, err = w.Add(filename) + c.Assert(err, IsNil) + modHash, err := w.Add(modFilename) + c.Assert(err, IsNil) + // At this point we've got two files with the same content. + // Hence their hashes must be the same. + c.Assert(origHash == modHash, Equals, true) + status, err = w.Status() + c.Assert(err, IsNil) + // However, their names are different and the work tree is still dirty. + c.Assert(status.IsClean(), Equals, false) + + // Revert back the deletion of the first file. + writeFile(filename) _, err = w.Add(filename) c.Assert(err, IsNil) - _, err = w.Add(modFilename) + + status, err = w.Status() + c.Assert(err, IsNil) + // Still dirty - the second file is added. + c.Assert(status.IsClean(), Equals, false) + + _, err = w.Remove(modFilename) c.Assert(err, IsNil) status, err = w.Status()