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

Proposed by Jelmer Vernooij on 2018-08-26
Status: Merged
Approved by: Jelmer Vernooij on 2018-09-11
Approved revision: 7081
Merge reported by: The Breezy Bot
Merged at revision: not available
Proposed branch: lp:~jelmer/brz/bad-fs-python3
Merge into: lp:brz
Diff against target: 75 lines (+22/-4)
3 files modified
breezy/bzr/workingtree.py (+9/-1)
breezy/osutils.py (+7/-3)
python3.passing (+6/-0)
To merge this branch: bzr merge lp:~jelmer/brz/bad-fs-python3
Reviewer Review Type Date Requested Status
Martin Packman 2018-08-26 Approve on 2018-09-10
Review via email: mp+353749@code.launchpad.net

Commit message

Fix bad fs encoding tests on python 3.

Description of the change

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.

To post a comment you must log in.
Martin Packman (gz) wrote :

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) wrote :

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) wrote :

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

lp:~jelmer/brz/bad-fs-python3 updated on 2018-09-11
7081. By Jelmer Vernooij on 2018-09-11

Merge trunk

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
1480 continue1480 continue
14811481
1482 fl = []1482 fl = []
1483 for subf in os.listdir(dirabs):1483 for subf in os.listdir(dirabs.encode(osutils._fs_enc)):
1484 try:
1485 subf = subf.decode(osutils._fs_enc)
1486 except UnicodeDecodeError:
1487 path_os_enc = path.encode(osutils._fs_enc)
1488 relpath = path_os_enc + b'/' + subf
1489 raise errors.BadFilenameEncoding(relpath,
1490 osutils._fs_enc)
1491
1484 if self.controldir.is_control_filename(subf):1492 if self.controldir.is_control_filename(subf):
1485 continue1493 continue
1486 if subf not in dir_entry.children:1494 if subf not in dir_entry.children:
14871495
=== 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 @@
1898 See DirReader.read_dir for details.1898 See DirReader.read_dir for details.
1899 """1899 """
1900 _utf8_encode = self._utf8_encode1900 _utf8_encode = self._utf8_encode
1901 _fs_decode = lambda s: s.decode(_fs_enc)
1902 _fs_encode = lambda s: s.encode(_fs_enc)
1901 _lstat = os.lstat1903 _lstat = os.lstat
1902 _listdir = os.listdir1904 _listdir = os.listdir
1903 _kind_from_mode = file_kind_from_stat_mode1905 _kind_from_mode = file_kind_from_stat_mode
@@ -1910,12 +1912,13 @@
19101912
1911 dirblock = []1913 dirblock = []
1912 append = dirblock.append1914 append = dirblock.append
1913 for name in _listdir(top):1915 for name_native in _listdir(top.encode('utf-8')):
1914 try:1916 try:
1915 name_utf8 = _utf8_encode(name)[0]1917 name = _fs_decode(name_native)
1916 except UnicodeDecodeError:1918 except UnicodeDecodeError:
1917 raise errors.BadFilenameEncoding(1919 raise errors.BadFilenameEncoding(
1918 _utf8_encode(relprefix)[0] + name, _fs_enc)1920 relprefix + name_native, _fs_enc)
1921 name_utf8 = _utf8_encode(name)[0]
1919 abspath = top_slash + name1922 abspath = top_slash + name
1920 statvalue = _lstat(abspath)1923 statvalue = _lstat(abspath)
1921 kind = _kind_from_mode(statvalue.st_mode)1924 kind = _kind_from_mode(statvalue.st_mode)
@@ -2382,6 +2385,7 @@
2382 data, _ = self.encode(object, self.errors)2385 data, _ = self.encode(object, self.errors)
2383 self.stream.write(data)2386 self.stream.write(data)
23842387
2388
2385if sys.platform == 'win32':2389if sys.platform == 'win32':
2386 def open_file(filename, mode='r', bufsize=-1):2390 def open_file(filename, mode='r', bufsize=-1):
2387 """This function is used to override the ``open`` builtin.2391 """This function is used to override the ``open`` builtin.
23882392
=== 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 @@
22086breezy.tests.per_workingtree.test_workingtree.TestFormatAttributes.test_versioned_directories(WorkingTreeFormat6)22086breezy.tests.per_workingtree.test_workingtree.TestFormatAttributes.test_versioned_directories(WorkingTreeFormat6)
22087breezy.tests.per_workingtree.test_workingtree.TestFormatAttributes.test_versioned_directories(WorkingTreeFormat6,remote)22087breezy.tests.per_workingtree.test_workingtree.TestFormatAttributes.test_versioned_directories(WorkingTreeFormat6,remote)
22088breezy.tests.per_workingtree.test_workingtree.TestIllegalPaths.test_bad_fs_path(GitWorkingTreeFormat)22088breezy.tests.per_workingtree.test_workingtree.TestIllegalPaths.test_bad_fs_path(GitWorkingTreeFormat)
22089breezy.tests.per_workingtree.test_workingtree.TestIllegalPaths.test_bad_fs_path(WorkingTreeFormat2)
22090breezy.tests.per_workingtree.test_workingtree.TestIllegalPaths.test_bad_fs_path(WorkingTreeFormat3)
22091breezy.tests.per_workingtree.test_workingtree.TestIllegalPaths.test_bad_fs_path(WorkingTreeFormat4)
22092breezy.tests.per_workingtree.test_workingtree.TestIllegalPaths.test_bad_fs_path(WorkingTreeFormat5)
22093breezy.tests.per_workingtree.test_workingtree.TestIllegalPaths.test_bad_fs_path(WorkingTreeFormat6)
22094breezy.tests.per_workingtree.test_workingtree.TestIllegalPaths.test_bad_fs_path(WorkingTreeFormat6,remote)
22089breezy.tests.per_workingtree.test_workingtree.TestWorkingTree.test_add_conflicts(GitWorkingTreeFormat)22095breezy.tests.per_workingtree.test_workingtree.TestWorkingTree.test_add_conflicts(GitWorkingTreeFormat)
22090breezy.tests.per_workingtree.test_workingtree.TestWorkingTree.test_add_conflicts(WorkingTreeFormat2)22096breezy.tests.per_workingtree.test_workingtree.TestWorkingTree.test_add_conflicts(WorkingTreeFormat2)
22091breezy.tests.per_workingtree.test_workingtree.TestWorkingTree.test_add_conflicts(WorkingTreeFormat3)22097breezy.tests.per_workingtree.test_workingtree.TestWorkingTree.test_add_conflicts(WorkingTreeFormat3)

Subscribers

People subscribed via source and target branches