felipec / git

Fork of Junio Hamano's git for users
Other
204 stars 34 forks source link

fix utf-8 support for fetching #29

Closed KangOl closed 11 years ago

KangOl commented 11 years ago

the previous fix handled only the addition of utf-8 named files, not the deletion (and renaming)

KangOl commented 11 years ago

As a rename is just a deletion followed by a append of the same file, all cases are covered. But if you insist, I can make a more complete test.

felipec commented 11 years ago

In git it is, not in bazaar.

KangOl commented 11 years ago

git-remote-bzr only handle deletion and modifications. Anyway, I just commit a more complete test that cover all cases.

felipec commented 11 years ago

No:

    for oldpath, newpath, fid, kind, mod, _ in changes.renamed:
        removed[oldpath] = None
        modified[newpath] = fid

See? A bzr rename is translated to a git add and remove.

KangOl commented 11 years ago

That what I'm talking about. A rename in bzr is enough to test remove and add in git.

felipec commented 11 years ago

Suppose this:

--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -187,13 +187,13 @@ def get_filechanges(cur, prev):
         return s.encode('utf-8')

     for path, fid, kind in changes.added:
-        modified[u(path)] = fid
+        modified["foo"] = fid
     for path, fid, kind in changes.removed:
-        removed[u(path)] = None
+        removed["foo"] = None
     for path, fid, kind, mod, _ in changes.modified:
         modified[u(path)] = fid
     for oldpath, newpath, fid, kind, mod, _ in changes.renamed:
-        removed[u(oldpath)] = None
+        removed["foo"] = None
         if kind == 'directory':
             lst = cur.list_files(from_dir=newpath, recursive=True)
             for path, file_class, kind, fid, entry in lst:

The checks would succeed, even though something is totally wrong.

KangOl commented 11 years ago

Beside that this is not really test utf-8 filenames but the fact that filenames are respected, I completed the test.

felipec commented 11 years ago

It can be triggered also by UTF-8 issues. But anyway, the last one deals with everything, I've pushed it.

Thanks.