Merge lp:~vila/bzr-upload/499525-ignore into lp:bzr-upload

Proposed by Vincent Ladeuil
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
Reviewer Review Type Date Requested Status
Martin Albisetti (community) Approve
Review via email: mp+16906@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Vincent Ladeuil (vila) wrote :

So, this should fixes both bug #499525 and bug #499941.
This also supersedes https://code.edge.launchpad.net/~beuno/bzr-upload/bug-499525/+merge/16552 with an implementation that:
- 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.

Revision history for this message
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)

Revision history for this message
Martin Albisetti (beuno) wrote :

Thanks for refactoring this into something more robust.
Mind filing the known bug so we can refer back to it?

review: Approve
Revision history for this message
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?

https://bugs.launchpad.net/bzr-upload/+bug/504686

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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

Subscribers

People subscribed via source and target branches

to status/vote changes: