Merge lp:~gz/bzr-builddeb/nonascii_import_dir_508258 into lp:bzr-builddeb

[Martin Packman]

This branch should allow the package importer to work with non-ascii filenames from upstream sources. There are a few provisos as this depends on the fix from bug 488519. If the name can't be represented in the filesystem encoding, a (much nicer) error giving the filename and encoding is raised, except on bzr 2.1 which will fail in much the same way as currently.

The package importer is already making an effort to say it wants to use UTF-8 filenames, but is being foiled by bad filesystem walking code. Nearly every use of os.listdir is a bug, it's easy to use incorrectly even for code like this that doesn't care about running on non-posix systems. Swapping in osutils.walkdirs instead is not ideal because it's a rather baroque interface and still doesn't handle the edge case of undecodable bytestring names. However, it's in all the bzrlib versions that builddeb needs to support and gets the common case right.

It would be nice to get rid of the bits copied over from bzrtools entirely. The bzrtools_import module is built around the idea of importing from tar, and this bug is in the compat layer designed to make an actual directory look like a tarfile. But bzr-builddeb only wants the import_dir function, so that design is backwards. The only significant chunk is the TreeTransform construction in _import_archive which should really live somewhere else.

[Jelmer Vernooij (jelmer)]

+1

Also, I like the idea of getting rid of the bzrtools import code.

review: Approve (code)

=== modified file 'bzrtools_import.py'
--- bzrtools_import.py	2010-10-28 18:13:44 +0000
+++ bzrtools_import.py	2011-10-24 17:38:25 +0000
@@ -14,7 +14,7 @@
 from bzrlib.bzrdir import BzrDir
 from bzrlib.errors import NoSuchFile, BzrCommandError, NotBranchError
 from bzrlib.osutils import (pathjoin, isdir, file_iterator, basename,
-                            file_kind, splitpath, normpath)
+                            file_kind, splitpath, normpath, walkdirs)
 from bzrlib.trace import warning
 from bzrlib.transform import TreeTransform, resolve_conflicts, cook_conflicts
 from bzrlib.workingtree import WorkingTree
@@ -74,19 +74,10 @@
     def __repr__(self):
         return 'DirWrapper(%r)' % self.root
 
-    def getmembers(self, subdir=None):
-        if subdir is not None:
-            mydir = pathjoin(self.root, subdir)
-        else:
-            mydir = self.root
-        for child in os.listdir(mydir):
-            if subdir is not None:
-                child = pathjoin(subdir, child)
-            fi = FileInfo(self.root, child)
-            yield fi
-            if fi.isdir():
-                for v in self.getmembers(child):
-                    yield v
+    def getmembers(self):
+        for _, dir_block in walkdirs(self.root):
+            for relpath, _, _, stat_result, _ in dir_block:
+                yield FileInfo(self.root, relpath, stat_result)
 
     def extractfile(self, member):
         return open(member.fullpath)
@@ -94,7 +85,7 @@
 
 class FileInfo(object):
 
-    def __init__(self, root, filepath):
+    def __init__(self, root, filepath, stat):
         self.fullpath = pathjoin(root, filepath)
         self.root = root
         if filepath != '':
@@ -102,7 +93,6 @@
         else:
             self.name = basename(root)
         self.type = None
-        stat = os.lstat(self.fullpath)
         self.mode = stat.st_mode
         if self.isdir():
             self.name += '/'

=== modified file 'tests/__init__.py'
--- tests/__init__.py	2011-10-01 20:41:14 +0000
+++ tests/__init__.py	2011-10-24 17:38:25 +0000
@@ -36,9 +36,15 @@
 
 from bzrlib.tests import TestUtil, multiply_tests
 try:
-    from bzrlib.tests.features import ModuleAvailableFeature
+    from bzrlib.tests.features import (
+        ModuleAvailableFeature,
+        UnicodeFilenameFeature,
+        )
 except ImportError: # bzr < 2.5
-    from bzrlib.tests import ModuleAvailableFeature
+    from bzrlib.tests import (
+        ModuleAvailableFeature,
+        UnicodeFilenameFeature,
+        )
 
 
 def make_new_upstream_dir(source, dest):

=== modified file 'tests/test_bzrtools_import.py'
--- tests/test_bzrtools_import.py	2011-06-13 23:01:14 +0000
+++ tests/test_bzrtools_import.py	2011-10-24 17:38:25 +0000
@@ -19,10 +19,10 @@
 #
 
 from bzrlib.plugins.builddeb.bzrtools_import import import_dir
-from bzrlib.plugins.builddeb.tests import TestCaseWithTransport
-
-
-class ImportArchiveTests(TestCaseWithTransport):
+from bzrlib.plugins.builddeb import tests
+
+
+class ImportArchiveTests(tests.TestCaseWithTransport):
 
     def test_strips_common_prefix(self):
         tree = self.make_branch_and_tree(".")
@@ -172,3 +172,13 @@
         import_dir(tree, "source", target_tree=file_ids_tree)
         self.assertEqual("a-id", tree.path2id("b"))
         self.assertEqual("b-id", tree.path2id("b/b"))
+
+    def test_nonascii_filename(self):
+        self.requireFeature(tests.UnicodeFilenameFeature)
+        tree = self.make_branch_and_tree(".")
+        tree.lock_write()
+        self.addCleanup(tree.unlock)
+        self.build_tree(["source/", u"source/\xa7"])
+        import_dir(tree, "source")
+        self.assertEqual(["", u"\xa7"],
+            sorted([tree.id2path(i) for i in tree.all_file_ids()]))