Merge lp:~ericmoritz/bzr/488519-walkdirs-encoding-bug into lp:bzr

Proposed by Eric Moritz
Status: Superseded
Proposed branch: lp:~ericmoritz/bzr/488519-walkdirs-encoding-bug
Merge into: lp:bzr
Diff against target: 187 lines (+72/-11)
4 files modified
bzrlib/errors.py (+1/-1)
bzrlib/osutils.py (+25/-9)
bzrlib/tests/__init__.py (+11/-0)
bzrlib/tests/test_osutils.py (+35/-1)
To merge this branch: bzr merge lp:~ericmoritz/bzr/488519-walkdirs-encoding-bug
Reviewer Review Type Date Requested Status
Martin Packman Pending
Eric Moritz Pending
Martin Pool Pending
Review via email: mp+27004@code.launchpad.net

This proposal has been superseded by a proposal from 2010-06-08.

Description of the change

A fix for bug #488519

To post a comment you must log in.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrlib/errors.py'
2--- bzrlib/errors.py 2010-05-28 06:04:57 +0000
3+++ bzrlib/errors.py 2010-06-08 01:47:22 +0000
4@@ -1914,7 +1914,7 @@
5
6 _fmt = "Parameter %(param)s is neither unicode nor utf8."
7
8-
9+
10 class ReusingTransform(BzrError):
11
12 _fmt = "Attempt to reuse a transform that has already been applied."
13
14=== modified file 'bzrlib/osutils.py'
15--- bzrlib/osutils.py 2010-05-27 17:59:18 +0000
16+++ bzrlib/osutils.py 2010-06-08 01:47:22 +0000
17@@ -931,7 +931,7 @@
18
19 def parent_directories(filename):
20 """Return the list of parent directories, deepest first.
21-
22+
23 For example, parent_directories("a/b/c") -> ["a/b", "a"].
24 """
25 parents = []
26@@ -961,7 +961,7 @@
27 # NB: This docstring is just an example, not a doctest, because doctest
28 # currently can't cope with the use of lazy imports in this namespace --
29 # mbp 20090729
30-
31+
32 # This currently doesn't report the failure at the time it occurs, because
33 # they tend to happen very early in startup when we can't check config
34 # files etc, and also we want to report all failures but not spam the user
35@@ -1037,8 +1037,8 @@
36
37
38 def delete_any(path):
39- """Delete a file, symlink or directory.
40-
41+ """Delete a file, symlink or directory.
42+
43 Will delete even if readonly.
44 """
45 try:
46@@ -1233,6 +1233,22 @@
47 # but for now, we haven't optimized...
48 return [canonical_relpath(base, p) for p in paths]
49
50+
51+def decode_filename(filename):
52+ """Decode the filename using the filesystem encoding
53+
54+ If it is unicode, it is returned.
55+ Otherwise it is decoded from the the filesystem's encoding. If decoding
56+ fails, a errors.BadFilenameEncoding exception is raised.
57+ """
58+ if isinstance(filename, unicode):
59+ return filename
60+ try:
61+ return filename.decode(_fs_enc)
62+ except UnicodeDecodeError:
63+ raise errors.BadFilenameEncoding(filename, _fs_enc)
64+
65+
66 def safe_unicode(unicode_or_utf8_string):
67 """Coerce unicode_or_utf8_string into unicode.
68
69@@ -1644,7 +1660,7 @@
70 dirblock = []
71 append = dirblock.append
72 try:
73- names = sorted(_listdir(top))
74+ names = sorted(map(decode_filename, _listdir(top)))
75 except OSError, e:
76 if not _is_error_enotdir(e):
77 raise
78@@ -2020,14 +2036,14 @@
79
80 def send_all(sock, bytes, report_activity=None):
81 """Send all bytes on a socket.
82-
83+
84 Breaks large blocks in smaller chunks to avoid buffering limitations on
85 some platforms, and catches EINTR which may be thrown if the send is
86 interrupted by a signal.
87
88 This is preferred to socket.sendall(), because it avoids portability bugs
89 and provides activity reporting.
90-
91+
92 :param report_activity: Call this as bytes are read, see
93 Transport._report_activity
94 """
95@@ -2121,7 +2137,7 @@
96
97 def until_no_eintr(f, *a, **kw):
98 """Run f(*a, **kw), retrying if an EINTR error occurs.
99-
100+
101 WARNING: you must be certain that it is safe to retry the call repeatedly
102 if EINTR does occur. This is typically only true for low-level operations
103 like os.read. If in any doubt, don't use this.
104@@ -2258,7 +2274,7 @@
105 if sys.platform == 'win32':
106 def open_file(filename, mode='r', bufsize=-1):
107 """This function is used to override the ``open`` builtin.
108-
109+
110 But it uses O_NOINHERIT flag so the file handle is not inherited by
111 child processes. Deleting or renaming a closed file opened with this
112 function is not blocking child processes.
113
114=== modified file 'bzrlib/tests/__init__.py'
115--- bzrlib/tests/__init__.py 2010-05-25 17:27:52 +0000
116+++ bzrlib/tests/__init__.py 2010-06-08 01:47:22 +0000
117@@ -4345,6 +4345,17 @@
118 UnicodeFilename = _UnicodeFilename()
119
120
121+class _ByteStringNamedFilesystem(Feature):
122+ """Is the filesystem based on bytes?"""
123+
124+ def _probe(self):
125+ if os.name == "posix":
126+ return True
127+ return False
128+
129+ByteStringNamedFilesystem = _ByteStringNamedFilesystem()
130+
131+
132 class _UTF8Filesystem(Feature):
133 """Is the filesystem UTF-8?"""
134
135
136=== modified file 'bzrlib/tests/test_osutils.py'
137--- bzrlib/tests/test_osutils.py 2010-05-27 17:59:18 +0000
138+++ bzrlib/tests/test_osutils.py 2010-06-08 01:47:22 +0000
139@@ -1083,6 +1083,40 @@
140 # Ensure the message contains the file name
141 self.assertContainsRe(str(e), "\./test-unreadable")
142
143+
144+ def test_walkdirs_encoding_error(self):
145+ # <https://bugs.launchpad.net/bzr/+bug/488519>
146+ # walkdirs didn't raise a useful message when the filenames
147+ # are not using the filesystem's encoding
148+
149+ # require a bytestring based filesystem
150+ self.requireFeature(tests.ByteStringNamedFilesystem)
151+
152+ tree = [
153+ '.bzr',
154+ '0file',
155+ '1dir/',
156+ '1dir/0file',
157+ '1dir/1dir/',
158+ '1file'
159+ ]
160+
161+ self.build_tree(tree)
162+
163+ # rename the 1file to a latin-1 filename
164+ os.rename("./1file", "\xe8file")
165+
166+ self._save_platform_info()
167+ win32utils.winver = None # Avoid the win32 detection code
168+ osutils._fs_enc = 'UTF-8'
169+
170+ # this should raise on error
171+ def attempt():
172+ for dirdetail, dirblock in osutils.walkdirs('.'):
173+ pass
174+
175+ self.assertRaises(errors.BadFilenameEncoding, attempt)
176+
177 def test__walkdirs_utf8(self):
178 tree = [
179 '.bzr',
180@@ -1921,7 +1955,7 @@
181 def restore_osutils_globals(self):
182 osutils._terminal_size_state = self._orig_terminal_size_state
183 osutils._first_terminal_size = self._orig_first_terminal_size
184-
185+
186 def replace_stdout(self, new):
187 self.overrideAttr(sys, 'stdout', new)
188