Merge lp:~jelmer/brz/bad-fs-python3 into lp:brz

[Jelmer Vernooij]

Fix bad fs encoding tests on python 3.

Fix bad fs encoding tests on python 3.

When calling os.listdir(), pass in a bytestring for the filename.

When passing in a unicode string, and if sys.getfilesytemencodeerror() == 'surrogatereplace', we're getting back surrogate characters in filenames - rather than UnicodeDecodeErrors being raised.

[Martin Packman (gz)]

Not sure about this, on Windows (and -ish on Macs) we're better off using unicode for filesystem operations, it's nix that gets into a mess with surrogateescape due to really being bytes underneath.

[Martin Packman (gz)]

Okay, I think this code is wrong in several circumstances, but:

1) There is a windows specific dirreader already at least
2) Fixing it should probably replace the interfaces with something better anyway

So, lets land, and follow up separately to work out some better cross-platform filesystem abstractions despite the mess that Python 3 made of this.

review: Approve

[The Breezy Bot (the-breezy-bot)]

Running landing tests failed
https://ci.breezy-vcs.org/job/land-brz/449/

[The Breezy Bot (the-breezy-bot)]

Merging failed
https://ci.breezy-vcs.org/job/land-brz/455/

[lp:~jelmer/brz/bad-fs-python3]

7081. By Jelmer Vernooij on 2018-09-11

Merge trunk

=== modified file 'breezy/bzr/workingtree.py'
--- breezy/bzr/workingtree.py	2018-07-26 22:33:54 +0000
+++ breezy/bzr/workingtree.py	2018-09-11 10:31:32 +0000
@@ -1480,7 +1480,15 @@
                 continue
 
             fl = []
-            for subf in os.listdir(dirabs):
+            for subf in os.listdir(dirabs.encode(osutils._fs_enc)):
+                try:
+                    subf = subf.decode(osutils._fs_enc)
+                except UnicodeDecodeError:
+                    path_os_enc = path.encode(osutils._fs_enc)
+                    relpath = path_os_enc + b'/' + subf
+                    raise errors.BadFilenameEncoding(relpath,
+                                                     osutils._fs_enc)
+
                 if self.controldir.is_control_filename(subf):
                     continue
                 if subf not in dir_entry.children:

=== modified file 'breezy/osutils.py'
--- breezy/osutils.py	2018-09-11 02:50:43 +0000
+++ breezy/osutils.py	2018-09-11 10:31:32 +0000
@@ -1898,6 +1898,8 @@
         See DirReader.read_dir for details.
         """
         _utf8_encode = self._utf8_encode
+        _fs_decode = lambda s: s.decode(_fs_enc)
+        _fs_encode = lambda s: s.encode(_fs_enc)
         _lstat = os.lstat
         _listdir = os.listdir
         _kind_from_mode = file_kind_from_stat_mode
@@ -1910,12 +1912,13 @@
 
         dirblock = []
         append = dirblock.append
-        for name in _listdir(top):
+        for name_native in _listdir(top.encode('utf-8')):
             try:
-                name_utf8 = _utf8_encode(name)[0]
+                name = _fs_decode(name_native)
             except UnicodeDecodeError:
                 raise errors.BadFilenameEncoding(
-                    _utf8_encode(relprefix)[0] + name, _fs_enc)
+                    relprefix + name_native, _fs_enc)
+            name_utf8 = _utf8_encode(name)[0]
             abspath = top_slash + name
             statvalue = _lstat(abspath)
             kind = _kind_from_mode(statvalue.st_mode)
@@ -2382,6 +2385,7 @@
             data, _ = self.encode(object, self.errors)
             self.stream.write(data)
 
+
 if sys.platform == 'win32':
     def open_file(filename, mode='r', bufsize=-1):
         """This function is used to override the ``open`` builtin.

=== modified file 'python3.passing'
--- python3.passing	2018-09-11 03:12:02 +0000
+++ python3.passing	2018-09-11 10:31:32 +0000
@@ -22086,6 +22086,12 @@
 breezy.tests.per_workingtree.test_workingtree.TestFormatAttributes.test_versioned_directories(WorkingTreeFormat6)
 breezy.tests.per_workingtree.test_workingtree.TestFormatAttributes.test_versioned_directories(WorkingTreeFormat6,remote)
 breezy.tests.per_workingtree.test_workingtree.TestIllegalPaths.test_bad_fs_path(GitWorkingTreeFormat)
+breezy.tests.per_workingtree.test_workingtree.TestIllegalPaths.test_bad_fs_path(WorkingTreeFormat2)
+breezy.tests.per_workingtree.test_workingtree.TestIllegalPaths.test_bad_fs_path(WorkingTreeFormat3)
+breezy.tests.per_workingtree.test_workingtree.TestIllegalPaths.test_bad_fs_path(WorkingTreeFormat4)
+breezy.tests.per_workingtree.test_workingtree.TestIllegalPaths.test_bad_fs_path(WorkingTreeFormat5)
+breezy.tests.per_workingtree.test_workingtree.TestIllegalPaths.test_bad_fs_path(WorkingTreeFormat6)
+breezy.tests.per_workingtree.test_workingtree.TestIllegalPaths.test_bad_fs_path(WorkingTreeFormat6,remote)
 breezy.tests.per_workingtree.test_workingtree.TestWorkingTree.test_add_conflicts(GitWorkingTreeFormat)
 breezy.tests.per_workingtree.test_workingtree.TestWorkingTree.test_add_conflicts(WorkingTreeFormat2)
 breezy.tests.per_workingtree.test_workingtree.TestWorkingTree.test_add_conflicts(WorkingTreeFormat3)