Merge lp:~vila/bzr-upload/499525-ignore into lp:bzr-upload
- 499525-ignore
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Martin Albisetti |
Approved revision: | not available |
Merged at revision: | not available |
Proposed branch: | lp:~vila/bzr-upload/499525-ignore |
Merge into: | lp:bzr-upload |
Diff against target: |
327 lines (+165/-38) 3 files modified
NEWS (+5/-0) __init__.py (+65/-30) tests/test_upload.py (+95/-8) |
To merge this branch: | bzr merge lp:~vila/bzr-upload/499525-ignore |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Martin Albisetti (community) | Approve | ||
Review via email: mp+16906@code.launchpad.net |
Commit message
Description of the change
Vincent Ladeuil (vila) wrote : | # |
Vincent Ladeuil (vila) wrote : | # |
I forgot to mention a nasty bug: if a file is uploaded, then ignored, then renamed (still ignored), then renamed (but not ignored anymore (think rename outside of a ignored dir)) then I think we'll crash :-/
I'm not sure it's worth the effort to handle that until people encounter the problem (and then, we can think about workarounds... In that context, an upload-force FILE (or a manual upload) may do the trick to resynchronize the remote site)
Martin Albisetti (beuno) wrote : | # |
Thanks for refactoring this into something more robust.
Mind filing the known bug so we can refer back to it?
Vincent Ladeuil (vila) wrote : | # |
>>>>> "beuno" == Martin Albisetti <email address hidden> writes:
beuno> Review: Approve
beuno> Thanks for refactoring this into something more robust.
beuno> Mind filing the known bug so we can refer back to it?
Preview Diff
1 | === modified file 'NEWS' |
2 | --- NEWS 2009-09-28 22:33:14 +0000 |
3 | +++ NEWS 2010-01-06 16:23:11 +0000 |
4 | @@ -16,6 +16,11 @@ |
5 | New Features |
6 | ************ |
7 | |
8 | +* ``.bzrignore-upload`` can be used to avoid uploading some files or |
9 | + directories. It uses the same syntax as ``.bzrignore`` including regular |
10 | + expressions. |
11 | + (Martin Albisetti, Vincent Ladeuil, #499525, #499941) |
12 | + |
13 | * Remote branches can be used to upload from. |
14 | (Gary van der Merwe, Vincent Ladeuil) |
15 | |
16 | |
17 | === modified file '__init__.py' |
18 | --- __init__.py 2009-12-02 10:02:05 +0000 |
19 | +++ __init__.py 2010-01-06 16:23:11 +0000 |
20 | @@ -164,6 +164,7 @@ |
21 | from bzrlib import ( |
22 | bzrdir, |
23 | errors, |
24 | + globbing, |
25 | ignores, |
26 | revisionspec, |
27 | transport, |
28 | @@ -262,6 +263,7 @@ |
29 | self._pending_deletions = [] |
30 | self._pending_renames = [] |
31 | self._uploaded_revid = None |
32 | + self._ignored = None |
33 | |
34 | def set_uploaded_revid(self, rev_id): |
35 | # XXX: Add tests for concurrent updates, etc. |
36 | @@ -279,28 +281,39 @@ |
37 | self._uploaded_revid = revision.NULL_REVISION |
38 | return self._uploaded_revid |
39 | |
40 | - def get_ignored(self): |
41 | - """Get upload-specific ignored files from the current branch""" |
42 | - try: |
43 | - ignore_file = self.tree.get_file_by_path('.bzrignore-upload') |
44 | - return ignores.parse_ignore_file(ignore_file) |
45 | - except errors.NoSuchId: |
46 | - return [] |
47 | + def _get_ignored(self): |
48 | + if self._ignored is None: |
49 | + try: |
50 | + ignore_file = self.tree.get_file_by_path('.bzrignore-upload') |
51 | + ignored_patterns = ignores.parse_ignore_file(ignore_file) |
52 | + except errors.NoSuchId: |
53 | + ignored_patterns = [] |
54 | + self._ignored = globbing.Globster(ignored_patterns) |
55 | + return self._ignored |
56 | + |
57 | + def is_ignored(self, relpath): |
58 | + glob = self._get_ignored() |
59 | + ignored = glob.match(relpath) |
60 | + import os |
61 | + if not ignored: |
62 | + # We still need to check that all parents are not ignored |
63 | + dir = os.path.dirname(relpath) |
64 | + while dir and not ignored: |
65 | + ignored = glob.match(dir) |
66 | + if not ignored: |
67 | + dir = os.path.dirname(dir) |
68 | + return ignored |
69 | |
70 | def upload_file(self, relpath, id, mode=None): |
71 | - ignored_files = self.get_ignored() |
72 | - if relpath not in ignored_files: |
73 | - if mode is None: |
74 | - if self.tree.is_executable(id): |
75 | - mode = 0775 |
76 | - else: |
77 | - mode = 0664 |
78 | - if not self.quiet: |
79 | - self.outf.write('Uploading %s\n' % relpath) |
80 | - self.to_transport.put_bytes(relpath, self.tree.get_file_text(id), mode) |
81 | - else: |
82 | - if not self.quiet: |
83 | - self.outf.write('Ignoring %s\n' % relpath) |
84 | + if mode is None: |
85 | + if self.tree.is_executable(id): |
86 | + mode = 0775 |
87 | + else: |
88 | + mode = 0664 |
89 | + if not self.quiet: |
90 | + self.outf.write('Uploading %s\n' % relpath) |
91 | + self.to_transport.put_bytes(relpath, self.tree.get_file_text(id), |
92 | + mode) |
93 | |
94 | def upload_file_robustly(self, relpath, id, mode=None): |
95 | """Upload a file, clearing the way on the remote side. |
96 | @@ -321,14 +334,9 @@ |
97 | self.upload_file(relpath, id, mode) |
98 | |
99 | def make_remote_dir(self, relpath, mode=None): |
100 | - ignored_files = self.get_ignored() |
101 | - if relpath not in ignored_files: |
102 | - if mode is None: |
103 | - mode = 0775 |
104 | - self.to_transport.mkdir(relpath, mode) |
105 | - else: |
106 | - if not self.quiet: |
107 | - self.outf.write('Ignoring %s\n' % relpath) |
108 | + if mode is None: |
109 | + mode = 0775 |
110 | + self.to_transport.mkdir(relpath, mode) |
111 | |
112 | def make_remote_dir_robustly(self, relpath, mode=None): |
113 | """Create a remote directory, clearing the way on the remote side. |
114 | @@ -359,6 +367,8 @@ |
115 | if not self.quiet: |
116 | self.outf.write('Deleting %s\n' % relpath) |
117 | self.to_transport.rmdir(relpath) |
118 | + # XXX: Add a test where a subdir is ignored but we still want to |
119 | + # delete the dir -- vila 100106 |
120 | |
121 | def delete_remote_dir_maybe(self, relpath): |
122 | """Try to delete relpath, keeping failures to retry later.""" |
123 | @@ -416,8 +426,12 @@ |
124 | for relpath, ie in self.tree.inventory.iter_entries(): |
125 | if relpath in ('', '.bzrignore', '.bzrignore-upload'): |
126 | # skip root ('') |
127 | - # .bzrignore has no meaning outside of a working tree |
128 | - # so do not upload it |
129 | + # .bzrignore and .bzrignore-upload have no meaning outside |
130 | + # a working tree so do not upload them |
131 | + continue |
132 | + if self.is_ignored(relpath): |
133 | + if not self.quiet: |
134 | + self.outf.write('Ignoring %s\n' % relpath) |
135 | continue |
136 | if ie.kind == 'file': |
137 | self.upload_file_robustly(relpath, ie.file_id) |
138 | @@ -456,6 +470,10 @@ |
139 | self.tree.lock_read() |
140 | try: |
141 | for (path, id, kind) in changes.removed: |
142 | + if self.is_ignored(path): |
143 | + if not self.quiet: |
144 | + self.outf.write('Ignoring %s\n' % path) |
145 | + continue |
146 | if kind is 'file': |
147 | self.delete_remote_file(path) |
148 | elif kind is 'directory': |
149 | @@ -465,6 +483,11 @@ |
150 | |
151 | for (old_path, new_path, id, kind, |
152 | content_change, exec_change) in changes.renamed: |
153 | + if self.is_ignored(old_path) and self.is_ignored(new_path): |
154 | + if not self.quiet: |
155 | + self.outf.write('Ignoring %s\n' % old_path) |
156 | + self.outf.write('Ignoring %s\n' % new_path) |
157 | + continue |
158 | if content_change: |
159 | # We update the old_path content because renames and |
160 | # deletions are differed. |
161 | @@ -474,6 +497,10 @@ |
162 | self.finish_deletions() |
163 | |
164 | for (path, id, old_kind, new_kind) in changes.kind_changed: |
165 | + if self.is_ignored(path): |
166 | + if not self.quiet: |
167 | + self.outf.write('Ignoring %s\n' % path) |
168 | + continue |
169 | if old_kind is 'file': |
170 | self.delete_remote_file(path) |
171 | elif old_kind is 'directory': |
172 | @@ -489,6 +516,10 @@ |
173 | raise NotImplementedError |
174 | |
175 | for (path, id, kind) in changes.added: |
176 | + if self.is_ignored(path): |
177 | + if not self.quiet: |
178 | + self.outf.write('Ignoring %s\n' % path) |
179 | + continue |
180 | if kind is 'file': |
181 | self.upload_file(path, id) |
182 | elif kind is 'directory': |
183 | @@ -499,6 +530,10 @@ |
184 | # XXX: Add a test for exec_change |
185 | for (path, id, kind, |
186 | content_change, exec_change) in changes.modified: |
187 | + if self.is_ignored(path): |
188 | + if not self.quiet: |
189 | + self.outf.write('Ignoring %s\n' % path) |
190 | + continue |
191 | if kind is 'file': |
192 | self.upload_file(path, id) |
193 | else: |
194 | |
195 | === modified file 'tests/test_upload.py' |
196 | --- tests/test_upload.py 2009-12-02 10:02:05 +0000 |
197 | +++ tests/test_upload.py 2010-01-06 16:23:11 +0000 |
198 | @@ -203,7 +203,9 @@ |
199 | # We don't want to use run_bzr here because redirected output are a |
200 | # pain to debug. But we need to provides a valid outf. |
201 | # XXX: Should a bug against bzr be filled about that ? |
202 | - cmd._setup_outf() |
203 | + |
204 | + # Short story: we don't expect any output so we may just use stdout |
205 | + cmd.outf = sys.stdout |
206 | return cmd |
207 | |
208 | def do_full_upload(self, *args, **kwargs): |
209 | @@ -445,7 +447,17 @@ |
210 | def test_ignore_file(self): |
211 | self.make_branch_and_working_tree() |
212 | self.do_full_upload() |
213 | - self.add_file('.bzrignore-upload','foo') |
214 | + self.add_file('.bzrignore-upload', 'foo') |
215 | + self.add_file('foo', 'bar') |
216 | + |
217 | + self.do_upload() |
218 | + |
219 | + self.failIfUpFileExists('foo') |
220 | + |
221 | + def test_ignore_regexp(self): |
222 | + self.make_branch_and_working_tree() |
223 | + self.do_full_upload() |
224 | + self.add_file('.bzrignore-upload', 'f*') |
225 | self.add_file('foo', 'bar') |
226 | |
227 | self.do_upload() |
228 | @@ -455,12 +467,74 @@ |
229 | def test_ignore_directory(self): |
230 | self.make_branch_and_working_tree() |
231 | self.do_full_upload() |
232 | - self.add_file('.bzrignore-upload','dir') |
233 | - self.add_dir('dir') |
234 | - |
235 | - self.do_upload() |
236 | - |
237 | - self.failIfUpFileExists('dir') |
238 | + self.add_file('.bzrignore-upload', 'dir') |
239 | + self.add_dir('dir') |
240 | + |
241 | + self.do_upload() |
242 | + |
243 | + self.failIfUpFileExists('dir') |
244 | + |
245 | + def test_ignore_nested_directory(self): |
246 | + self.make_branch_and_working_tree() |
247 | + self.do_full_upload() |
248 | + self.add_file('.bzrignore-upload', 'dir') |
249 | + self.add_dir('dir') |
250 | + self.add_dir('dir/foo') |
251 | + self.add_file('dir/foo/bar', 'bar contents') |
252 | + |
253 | + self.do_upload() |
254 | + |
255 | + self.failIfUpFileExists('dir') |
256 | + self.failIfUpFileExists('dir/foo/bar') |
257 | + |
258 | + def test_ignore_change_file_into_dir(self): |
259 | + self.make_branch_and_working_tree() |
260 | + self.add_file('hello', 'foo') |
261 | + self.do_full_upload() |
262 | + self.add_file('.bzrignore-upload', 'hello') |
263 | + self.transform_file_into_dir('hello') |
264 | + self.add_file('hello/file', 'bar') |
265 | + |
266 | + self.assertUpFileEqual('foo', 'hello') |
267 | + |
268 | + self.do_upload() |
269 | + |
270 | + self.assertUpFileEqual('foo', 'hello') |
271 | + |
272 | + def test_ignore_change_dir_into_file(self): |
273 | + self.make_branch_and_working_tree() |
274 | + self.add_dir('hello') |
275 | + self.add_file('hello/file', 'foo') |
276 | + self.do_full_upload() |
277 | + |
278 | + self.add_file('.bzrignore-upload', 'hello') |
279 | + self.delete_any('hello/file') |
280 | + self.transform_dir_into_file('hello', 'bar') |
281 | + |
282 | + self.assertUpFileEqual('foo', 'hello/file') |
283 | + |
284 | + self.do_upload() |
285 | + |
286 | + self.assertUpFileEqual('foo', 'hello/file') |
287 | + |
288 | + def test_ignore_delete_dir_in_subdir(self): |
289 | + self.make_branch_and_working_tree() |
290 | + self.add_dir('dir') |
291 | + self.add_dir('dir/subdir') |
292 | + self.add_file('dir/subdir/a', 'foo') |
293 | + self.do_full_upload() |
294 | + self.add_file('.bzrignore-upload', 'dir/subdir') |
295 | + self.rename_any('dir/subdir/a', 'dir/a') |
296 | + self.delete_any('dir/subdir') |
297 | + |
298 | + self.assertUpFileEqual('foo', 'dir/subdir/a') |
299 | + |
300 | + self.do_upload() |
301 | + |
302 | + # The file in the dir is not ignored. This a bit contrived but |
303 | + # indicates that we may encounter problems when ignored items appear |
304 | + # and disappear... -- vila 100106 |
305 | + self.assertUpFileEqual('foo', 'dir/a') |
306 | |
307 | |
308 | class TestFullUpload(tests.TestCaseWithTransport, TestUploadMixin): |
309 | @@ -576,6 +650,19 @@ |
310 | |
311 | self.assertUpFileEqual('bar', 'hello') |
312 | |
313 | + def test_ignore_delete_one_file(self): |
314 | + self.make_branch_and_working_tree() |
315 | + self.add_file('hello', 'foo') |
316 | + self.do_full_upload() |
317 | + self.add_file('.bzrignore-upload', 'hello') |
318 | + self.delete_any('hello') |
319 | + |
320 | + self.assertUpFileEqual('foo', 'hello') |
321 | + |
322 | + self.do_upload() |
323 | + |
324 | + self.assertUpFileEqual('foo', 'hello') |
325 | + |
326 | |
327 | class TestBranchUploadLocations(per_branch.TestCaseWithBranch): |
328 |
So, this should fixes both bug #499525 and bug #499941. /code.edge. launchpad. net/~beuno/ bzr-upload/ bug-499525/ +merge/ 16552 with an implementation that:
This also supersedes https:/
- reuse more code from bzrlib,
- also checks that no parents are ignored (there was a reason to use is_inside_any :)
Since it's more involved than the first approach, I'd like real-world tests as part of the review, thanks in advance.