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
1=== modified file 'breezy/bzr/workingtree.py'
2--- breezy/bzr/workingtree.py 2018-07-26 22:33:54 +0000
3+++ breezy/bzr/workingtree.py 2018-09-11 10:31:32 +0000
4@@ -1480,7 +1480,15 @@
5 continue
6
7 fl = []
8- for subf in os.listdir(dirabs):
9+ for subf in os.listdir(dirabs.encode(osutils._fs_enc)):
10+ try:
11+ subf = subf.decode(osutils._fs_enc)
12+ except UnicodeDecodeError:
13+ path_os_enc = path.encode(osutils._fs_enc)
14+ relpath = path_os_enc + b'/' + subf
15+ raise errors.BadFilenameEncoding(relpath,
16+ osutils._fs_enc)
17+
18 if self.controldir.is_control_filename(subf):
19 continue
20 if subf not in dir_entry.children:
21
22=== modified file 'breezy/osutils.py'
23--- breezy/osutils.py 2018-09-11 02:50:43 +0000
24+++ breezy/osutils.py 2018-09-11 10:31:32 +0000
25@@ -1898,6 +1898,8 @@
26 See DirReader.read_dir for details.
27 """
28 _utf8_encode = self._utf8_encode
29+ _fs_decode = lambda s: s.decode(_fs_enc)
30+ _fs_encode = lambda s: s.encode(_fs_enc)
31 _lstat = os.lstat
32 _listdir = os.listdir
33 _kind_from_mode = file_kind_from_stat_mode
34@@ -1910,12 +1912,13 @@
35
36 dirblock = []
37 append = dirblock.append
38- for name in _listdir(top):
39+ for name_native in _listdir(top.encode('utf-8')):
40 try:
41- name_utf8 = _utf8_encode(name)[0]
42+ name = _fs_decode(name_native)
43 except UnicodeDecodeError:
44 raise errors.BadFilenameEncoding(
45- _utf8_encode(relprefix)[0] + name, _fs_enc)
46+ relprefix + name_native, _fs_enc)
47+ name_utf8 = _utf8_encode(name)[0]
48 abspath = top_slash + name
49 statvalue = _lstat(abspath)
50 kind = _kind_from_mode(statvalue.st_mode)
51@@ -2382,6 +2385,7 @@
52 data, _ = self.encode(object, self.errors)
53 self.stream.write(data)
54
55+
56 if sys.platform == 'win32':
57 def open_file(filename, mode='r', bufsize=-1):
58 """This function is used to override the ``open`` builtin.
59
60=== modified file 'python3.passing'
61--- python3.passing 2018-09-11 03:12:02 +0000
62+++ python3.passing 2018-09-11 10:31:32 +0000
63@@ -22086,6 +22086,12 @@
64 breezy.tests.per_workingtree.test_workingtree.TestFormatAttributes.test_versioned_directories(WorkingTreeFormat6)
65 breezy.tests.per_workingtree.test_workingtree.TestFormatAttributes.test_versioned_directories(WorkingTreeFormat6,remote)
66 breezy.tests.per_workingtree.test_workingtree.TestIllegalPaths.test_bad_fs_path(GitWorkingTreeFormat)
67+breezy.tests.per_workingtree.test_workingtree.TestIllegalPaths.test_bad_fs_path(WorkingTreeFormat2)
68+breezy.tests.per_workingtree.test_workingtree.TestIllegalPaths.test_bad_fs_path(WorkingTreeFormat3)
69+breezy.tests.per_workingtree.test_workingtree.TestIllegalPaths.test_bad_fs_path(WorkingTreeFormat4)
70+breezy.tests.per_workingtree.test_workingtree.TestIllegalPaths.test_bad_fs_path(WorkingTreeFormat5)
71+breezy.tests.per_workingtree.test_workingtree.TestIllegalPaths.test_bad_fs_path(WorkingTreeFormat6)
72+breezy.tests.per_workingtree.test_workingtree.TestIllegalPaths.test_bad_fs_path(WorkingTreeFormat6,remote)
73 breezy.tests.per_workingtree.test_workingtree.TestWorkingTree.test_add_conflicts(GitWorkingTreeFormat)
74 breezy.tests.per_workingtree.test_workingtree.TestWorkingTree.test_add_conflicts(WorkingTreeFormat2)
75 breezy.tests.per_workingtree.test_workingtree.TestWorkingTree.test_add_conflicts(WorkingTreeFormat3)

Subscribers

People subscribed via source and target branches