Merge lp:~jelmer/brz/memorytree-symlinks into lp:brz/3.0

Proposed by Jelmer Vernooij on 2019-02-27
Status: Merged
Approved by: Jelmer Vernooij on 2019-03-04
Approved revision: 7064
Merge reported by: The Breezy Bot
Merged at revision: not available
Proposed branch: lp:~jelmer/brz/memorytree-symlinks
Merge into: lp:brz/3.0
Diff against target: 502 lines (+137/-106)
4 files modified
breezy/git/memorytree.py (+4/-0)
breezy/memorytree.py (+18/-4)
breezy/tests/test_memorytree.py (+65/-70)
breezy/transport/memory.py (+50/-32)
To merge this branch: bzr merge lp:~jelmer/brz/memorytree-symlinks
Reviewer Review Type Date Requested Status
Martin Packman 2019-02-27 Approve on 2019-03-03
Review via email: mp+363704@code.launchpad.net

Commit message

Implement MemoryTree.get_symlink_target.

Description of the change

Implement MemoryTree.get_symlink_target.

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

Thanks! See one inline comment about a duplicated statement.

review: Approve
Jelmer Vernooij (jelmer) :
lp:~jelmer/brz/memorytree-symlinks updated on 2019-03-04
7064. By Jelmer Vernooij on 2019-03-04

Fix flake8ness.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'breezy/git/memorytree.py'
2--- breezy/git/memorytree.py 2018-11-18 00:25:19 +0000
3+++ breezy/git/memorytree.py 2019-03-04 01:49:51 +0000
4@@ -258,3 +258,7 @@
5 def kind(self, p):
6 stat_value = self._file_transport.stat(p)
7 return osutils.file_kind_from_stat_mode(stat_value.st_mode)
8+
9+ def get_symlink_target(self, path):
10+ with self.lock_read():
11+ return self._file_transport.readlink(path)
12
13=== modified file 'breezy/memorytree.py'
14--- breezy/memorytree.py 2018-11-18 00:25:19 +0000
15+++ breezy/memorytree.py 2019-03-04 01:49:51 +0000
16@@ -22,6 +22,7 @@
17 from __future__ import absolute_import
18
19 import os
20+import stat
21
22 from . import (
23 errors,
24@@ -62,7 +63,15 @@
25 with self.lock_tree_write():
26 for f, file_id, kind in zip(files, ids, kinds):
27 if kind is None:
28- kind = 'file'
29+ st_mode = self._file_transport.stat(f).st_mode
30+ if stat.S_ISREG(st_mode):
31+ kind = 'file'
32+ elif stat.S_ISLNK(st_mode):
33+ kind = 'symlink'
34+ elif stat.S_ISDIR(st_mode):
35+ kind = 'directory'
36+ else:
37+ raise AssertionError('Unknown file kind')
38 if file_id is None:
39 self._inventory.add_path(f, kind=kind)
40 else:
41@@ -127,7 +136,7 @@
42 # memory tree does not support nested trees yet.
43 return kind, None, None, None
44 elif kind == 'symlink':
45- raise NotImplementedError('symlink support')
46+ return kind, None, None, self._inventory[id].symlink_target
47 else:
48 raise NotImplementedError('unknown kind')
49
50@@ -148,8 +157,7 @@
51 return self._inventory.get_entry_by_path(path).executable
52
53 def kind(self, path):
54- file_id = self.path2id(path)
55- return self._inventory[file_id].kind
56+ return self._inventory.get_entry_by_path(path).kind
57
58 def mkdir(self, path, file_id=None):
59 """See MutableTree.mkdir()."""
60@@ -227,6 +235,8 @@
61 continue
62 if entry.kind == 'directory':
63 self._file_transport.mkdir(path)
64+ elif entry.kind == 'symlink':
65+ self._file_transport.symlink(entry.symlink_target, path)
66 elif entry.kind == 'file':
67 self._file_transport.put_file(
68 path, self._basis_tree.get_file(path))
69@@ -302,6 +312,10 @@
70 else:
71 raise
72
73+ def get_symlink_target(self, path):
74+ with self.lock_read():
75+ return self._file_transport.readlink(path)
76+
77 def set_parent_trees(self, parents_list, allow_leftmost_as_ghost=False):
78 """See MutableTree.set_parent_trees()."""
79 if len(parents_list) == 0:
80
81=== modified file 'breezy/tests/test_memorytree.py'
82--- breezy/tests/test_memorytree.py 2018-11-11 04:08:32 +0000
83+++ breezy/tests/test_memorytree.py 2019-03-04 01:49:51 +0000
84@@ -46,21 +46,17 @@
85 rev_id = tree.commit('first post')
86 tree.unlock()
87 tree = MemoryTree.create_on_branch(branch)
88- tree.lock_read()
89- self.assertEqual([rev_id], tree.get_parent_ids())
90- with tree.get_file('foo') as f:
91- self.assertEqual(b'contents of foo\n', f.read())
92- tree.unlock()
93+ with tree.lock_read():
94+ self.assertEqual([rev_id], tree.get_parent_ids())
95+ with tree.get_file('foo') as f:
96+ self.assertEqual(b'contents of foo\n', f.read())
97
98 def test_get_root_id(self):
99 branch = self.make_branch('branch')
100 tree = MemoryTree.create_on_branch(branch)
101- tree.lock_write()
102- try:
103+ with tree.lock_write():
104 tree.add([''])
105 self.assertIsNot(None, tree.get_root_id())
106- finally:
107- tree.unlock()
108
109 def test_lock_tree_write(self):
110 """Check we can lock_tree_write and unlock MemoryTrees."""
111@@ -73,9 +69,8 @@
112 """Check that we error when trying to upgrade a read lock to write."""
113 branch = self.make_branch('branch')
114 tree = MemoryTree.create_on_branch(branch)
115- tree.lock_read()
116- self.assertRaises(errors.ReadOnlyError, tree.lock_tree_write)
117- tree.unlock()
118+ with tree.lock_read():
119+ self.assertRaises(errors.ReadOnlyError, tree.lock_tree_write)
120
121 def test_lock_write(self):
122 """Check we can lock_write and unlock MemoryTrees."""
123@@ -88,58 +83,63 @@
124 """Check that we error when trying to upgrade a read lock to write."""
125 branch = self.make_branch('branch')
126 tree = MemoryTree.create_on_branch(branch)
127- tree.lock_read()
128- self.assertRaises(errors.ReadOnlyError, tree.lock_write)
129- tree.unlock()
130+ with tree.lock_read():
131+ self.assertRaises(errors.ReadOnlyError, tree.lock_write)
132
133 def test_add_with_kind(self):
134 branch = self.make_branch('branch')
135 tree = MemoryTree.create_on_branch(branch)
136- tree.lock_write()
137- tree.add(['', 'afile', 'adir'], None,
138- ['directory', 'file', 'directory'])
139- self.assertEqual('afile', tree.id2path(tree.path2id('afile')))
140- self.assertEqual('adir', tree.id2path(tree.path2id('adir')))
141- self.assertFalse(tree.has_filename('afile'))
142- self.assertFalse(tree.has_filename('adir'))
143- tree.unlock()
144+ with tree.lock_write():
145+ tree.add(['', 'afile', 'adir'], None,
146+ ['directory', 'file', 'directory'])
147+ self.assertEqual('afile', tree.id2path(tree.path2id('afile')))
148+ self.assertEqual('adir', tree.id2path(tree.path2id('adir')))
149+ self.assertFalse(tree.has_filename('afile'))
150+ self.assertFalse(tree.has_filename('adir'))
151
152 def test_put_new_file(self):
153 branch = self.make_branch('branch')
154 tree = MemoryTree.create_on_branch(branch)
155- tree.lock_write()
156- tree.add(['', 'foo'], ids=[b'root-id', b'foo-id'],
157- kinds=['directory', 'file'])
158- tree.put_file_bytes_non_atomic('foo', b'barshoom')
159- self.assertEqual(b'barshoom', tree.get_file('foo').read())
160- tree.unlock()
161+ with tree.lock_write():
162+ tree.add(['', 'foo'], ids=[b'root-id', b'foo-id'],
163+ kinds=['directory', 'file'])
164+ tree.put_file_bytes_non_atomic('foo', b'barshoom')
165+ with tree.get_file('foo') as f:
166+ self.assertEqual(b'barshoom', f.read())
167
168 def test_put_existing_file(self):
169 branch = self.make_branch('branch')
170 tree = MemoryTree.create_on_branch(branch)
171- tree.lock_write()
172- tree.add(['', 'foo'], ids=[b'root-id', b'foo-id'],
173- kinds=['directory', 'file'])
174- tree.put_file_bytes_non_atomic('foo', b'first-content')
175- tree.put_file_bytes_non_atomic('foo', b'barshoom')
176- self.assertEqual(b'barshoom', tree.get_file('foo').read())
177- tree.unlock()
178+ with tree.lock_write():
179+ tree.add(['', 'foo'], ids=[b'root-id', b'foo-id'],
180+ kinds=['directory', 'file'])
181+ tree.put_file_bytes_non_atomic('foo', b'first-content')
182+ tree.put_file_bytes_non_atomic('foo', b'barshoom')
183+ self.assertEqual(b'barshoom', tree.get_file('foo').read())
184
185 def test_add_in_subdir(self):
186 branch = self.make_branch('branch')
187 tree = MemoryTree.create_on_branch(branch)
188- tree.lock_write()
189- self.addCleanup(tree.unlock)
190- tree.add([''], [b'root-id'], ['directory'])
191- # Unfortunately, the only way to 'mkdir' is to call 'tree.mkdir', but
192- # that *always* adds the directory as well. So if you want to create a
193- # file in a subdirectory, you have to split out the 'mkdir()' calls
194- # from the add and put_file_bytes_non_atomic calls. :(
195- tree.mkdir('adir', b'dir-id')
196- tree.add(['adir/afile'], [b'file-id'], ['file'])
197- self.assertEqual('adir/afile', tree.id2path(b'file-id'))
198- self.assertEqual('adir', tree.id2path(b'dir-id'))
199- tree.put_file_bytes_non_atomic('adir/afile', b'barshoom')
200+ with tree.lock_write():
201+ tree.add([''], [b'root-id'], ['directory'])
202+ # Unfortunately, the only way to 'mkdir' is to call 'tree.mkdir', but
203+ # that *always* adds the directory as well. So if you want to create a
204+ # file in a subdirectory, you have to split out the 'mkdir()' calls
205+ # from the add and put_file_bytes_non_atomic calls. :(
206+ tree.mkdir('adir', b'dir-id')
207+ tree.add(['adir/afile'], [b'file-id'], ['file'])
208+ self.assertEqual('adir/afile', tree.id2path(b'file-id'))
209+ self.assertEqual('adir', tree.id2path(b'dir-id'))
210+ tree.put_file_bytes_non_atomic('adir/afile', b'barshoom')
211+
212+ def test_add_symlink(self):
213+ branch = self.make_branch('branch')
214+ tree = MemoryTree.create_on_branch(branch)
215+ with tree.lock_write():
216+ tree._file_transport.symlink('bar', 'foo')
217+ tree.add(['', 'foo'])
218+ self.assertEqual('symlink', tree.kind('foo'))
219+ self.assertEqual('bar', tree.get_symlink_target('foo'))
220
221 def test_commit_trivial(self):
222 """Smoke test for commit on a MemoryTree.
223@@ -149,40 +149,35 @@
224 """
225 branch = self.make_branch('branch')
226 tree = MemoryTree.create_on_branch(branch)
227- tree.lock_write()
228- tree.add(['', 'foo'], ids=[b'root-id', b'foo-id'],
229- kinds=['directory', 'file'])
230- tree.put_file_bytes_non_atomic('foo', b'barshoom')
231- revision_id = tree.commit('message baby')
232- # the parents list for the tree should have changed.
233- self.assertEqual([revision_id], tree.get_parent_ids())
234- tree.unlock()
235+ with tree.lock_write():
236+ tree.add(['', 'foo'], ids=[b'root-id', b'foo-id'],
237+ kinds=['directory', 'file'])
238+ tree.put_file_bytes_non_atomic('foo', b'barshoom')
239+ revision_id = tree.commit('message baby')
240+ # the parents list for the tree should have changed.
241+ self.assertEqual([revision_id], tree.get_parent_ids())
242 # and we should have a revision that is accessible outside the tree lock
243 revtree = tree.branch.repository.revision_tree(revision_id)
244- revtree.lock_read()
245- self.addCleanup(revtree.unlock)
246- with revtree.get_file('foo') as f:
247+ with revtree.lock_read(), revtree.get_file('foo') as f:
248 self.assertEqual(b'barshoom', f.read())
249
250 def test_unversion(self):
251 """Some test for unversion of a memory tree."""
252 branch = self.make_branch('branch')
253 tree = MemoryTree.create_on_branch(branch)
254- tree.lock_write()
255- tree.add(['', 'foo'], ids=[b'root-id', b'foo-id'],
256- kinds=['directory', 'file'])
257- tree.unversion(['foo'])
258- self.assertFalse(tree.is_versioned('foo'))
259- self.assertFalse(tree.has_id(b'foo-id'))
260- tree.unlock()
261+ with tree.lock_write():
262+ tree.add(['', 'foo'], ids=[b'root-id', b'foo-id'],
263+ kinds=['directory', 'file'])
264+ tree.unversion(['foo'])
265+ self.assertFalse(tree.is_versioned('foo'))
266+ self.assertFalse(tree.has_id(b'foo-id'))
267
268 def test_last_revision(self):
269 """There should be a last revision method we can call."""
270 tree = self.make_branch_and_memory_tree('branch')
271- tree.lock_write()
272- tree.add('')
273- rev_id = tree.commit('first post')
274- tree.unlock()
275+ with tree.lock_write():
276+ tree.add('')
277+ rev_id = tree.commit('first post')
278 self.assertEqual(rev_id, tree.last_revision())
279
280 def test_rename_file(self):
281
282=== modified file 'breezy/transport/memory.py'
283--- breezy/transport/memory.py 2018-11-17 16:53:10 +0000
284+++ breezy/transport/memory.py 2019-03-04 01:49:51 +0000
285@@ -26,9 +26,10 @@
286 from io import (
287 BytesIO,
288 )
289+import itertools
290 import os
291 import errno
292-from stat import S_IFREG, S_IFDIR, S_IFLNK
293+from stat import S_IFREG, S_IFDIR, S_IFLNK, S_ISDIR
294
295 from .. import (
296 transport,
297@@ -50,20 +51,16 @@
298
299 class MemoryStat(object):
300
301- def __init__(self, size, kind, perms):
302+ def __init__(self, size, kind, perms=None):
303 self.st_size = size
304- if kind == 'file':
305+ if not S_ISDIR(kind):
306 if perms is None:
307 perms = 0o644
308- self.st_mode = S_IFREG | perms
309- elif kind == 'directory':
310+ self.st_mode = kind | perms
311+ else:
312 if perms is None:
313 perms = 0o755
314- self.st_mode = S_IFDIR | perms
315- elif kind == 'symlink':
316- self.st_mode = S_IFLNK | 0o644
317- else:
318- raise AssertionError('unknown kind %r' % kind)
319+ self.st_mode = kind | perms
320
321
322 class MemoryTransport(transport.Transport):
323@@ -111,7 +108,7 @@
324
325 def append_file(self, relpath, f, mode=None):
326 """See Transport.append_file()."""
327- _abspath = self._abspath(relpath)
328+ _abspath = self._resolve_symlinks(relpath)
329 self._check_parent(_abspath)
330 orig_content, orig_mode = self._files.get(_abspath, (b"", None))
331 if mode is None:
332@@ -128,16 +125,20 @@
333 def has(self, relpath):
334 """See Transport.has()."""
335 _abspath = self._abspath(relpath)
336- return ((_abspath in self._files)
337- or (_abspath in self._dirs)
338- or (_abspath in self._symlinks))
339+ for container in (self._files, self._dirs, self._symlinks):
340+ if _abspath in container.keys():
341+ return True
342+ return False
343
344 def delete(self, relpath):
345 """See Transport.delete()."""
346 _abspath = self._abspath(relpath)
347- if _abspath not in self._files:
348+ if _abspath in self._files:
349+ del self._files[_abspath]
350+ elif _abspath in self._symlinks:
351+ del self._symlinks[_abspath]
352+ else:
353 raise NoSuchFile(relpath)
354- del self._files[_abspath]
355
356 def external_url(self):
357 """See breezy.transport.Transport.external_url."""
358@@ -147,7 +148,7 @@
359
360 def get(self, relpath):
361 """See Transport.get()."""
362- _abspath = self._abspath(relpath)
363+ _abspath = self._resolve_symlinks(relpath)
364 if _abspath not in self._files:
365 if _abspath in self._dirs:
366 return LateReadError(relpath)
367@@ -157,15 +158,20 @@
368
369 def put_file(self, relpath, f, mode=None):
370 """See Transport.put_file()."""
371- _abspath = self._abspath(relpath)
372+ _abspath = self._resolve_symlinks(relpath)
373 self._check_parent(_abspath)
374 raw_bytes = f.read()
375 self._files[_abspath] = (raw_bytes, mode)
376 return len(raw_bytes)
377
378+ def symlink(self, source, target):
379+ _abspath = self._resolve_symlinks(target)
380+ self._check_parent(_abspath)
381+ self._symlinks[_abspath] = self._abspath(source)
382+
383 def mkdir(self, relpath, mode=None):
384 """See Transport.mkdir()."""
385- _abspath = self._abspath(relpath)
386+ _abspath = self._resolve_symlinks(relpath)
387 self._check_parent(_abspath)
388 if _abspath in self._dirs:
389 raise FileExists(relpath)
390@@ -183,13 +189,13 @@
391 return True
392
393 def iter_files_recursive(self):
394- for file in self._files:
395+ for file in itertools.chain(self._files, self._symlinks):
396 if file.startswith(self._cwd):
397 yield urlutils.escape(file[len(self._cwd):])
398
399 def list_dir(self, relpath):
400 """See Transport.list_dir()."""
401- _abspath = self._abspath(relpath)
402+ _abspath = self._resolve_symlinks(relpath)
403 if _abspath != '/' and _abspath not in self._dirs:
404 raise NoSuchFile(relpath)
405 result = []
406@@ -197,7 +203,7 @@
407 if not _abspath.endswith('/'):
408 _abspath += '/'
409
410- for path_group in self._files, self._dirs:
411+ for path_group in self._files, self._dirs, self._symlinks:
412 for path in path_group:
413 if path.startswith(_abspath):
414 trailing = path[len(_abspath):]
415@@ -207,8 +213,8 @@
416
417 def rename(self, rel_from, rel_to):
418 """Rename a file or directory; fail if the destination exists"""
419- abs_from = self._abspath(rel_from)
420- abs_to = self._abspath(rel_to)
421+ abs_from = self._resolve_symlinks(rel_from)
422+ abs_to = self._resolve_symlinks(rel_to)
423
424 def replace(x):
425 if x == abs_from:
426@@ -233,21 +239,25 @@
427 # fail differently depending on dict order. So work on copy, fail on
428 # error on only replace dicts if all goes well.
429 renamed_files = self._files.copy()
430+ renamed_symlinks = self._symlinks.copy()
431 renamed_dirs = self._dirs.copy()
432 do_renames(renamed_files)
433+ do_renames(renamed_symlinks)
434 do_renames(renamed_dirs)
435 # We may have been cloned so modify in place
436 self._files.clear()
437 self._files.update(renamed_files)
438+ self._symlinks.clear()
439+ self._symlinks.update(renamed_symlinks)
440 self._dirs.clear()
441 self._dirs.update(renamed_dirs)
442
443 def rmdir(self, relpath):
444 """See Transport.rmdir."""
445- _abspath = self._abspath(relpath)
446+ _abspath = self._resolve_symlinks(relpath)
447 if _abspath in self._files:
448 self._translate_error(IOError(errno.ENOTDIR, relpath), relpath)
449- for path in self._files:
450+ for path in itertools.chain(self._files, self._symlinks):
451 if path.startswith(_abspath + '/'):
452 self._translate_error(IOError(errno.ENOTEMPTY, relpath),
453 relpath)
454@@ -262,13 +272,13 @@
455 def stat(self, relpath):
456 """See Transport.stat()."""
457 _abspath = self._abspath(relpath)
458- if _abspath in self._files:
459- return MemoryStat(len(self._files[_abspath][0]), 'file',
460+ if _abspath in self._files.keys():
461+ return MemoryStat(len(self._files[_abspath][0]), S_IFREG,
462 self._files[_abspath][1])
463- elif _abspath in self._dirs:
464- return MemoryStat(0, 'directory', self._dirs[_abspath])
465- elif _abspath in self._symlinks:
466- return MemoryStat(0, 'symlink', 0)
467+ elif _abspath in self._dirs.keys():
468+ return MemoryStat(0, S_IFDIR, self._dirs[_abspath])
469+ elif _abspath in self._symlinks.keys():
470+ return MemoryStat(0, S_IFLNK)
471 else:
472 raise NoSuchFile(_abspath)
473
474@@ -280,6 +290,12 @@
475 """See Transport.lock_write()."""
476 return _MemoryLock(self._abspath(relpath), self)
477
478+ def _resolve_symlinks(self, relpath):
479+ path = self._abspath(relpath)
480+ while path in self._symlinks.keys():
481+ path = self._symlinks[path]
482+ return path
483+
484 def _abspath(self, relpath):
485 """Generate an internal absolute path."""
486 relpath = urlutils.unescape(relpath)
487@@ -336,6 +352,7 @@
488 def start_server(self):
489 self._dirs = {'/': None}
490 self._files = {}
491+ self._symlinks = {}
492 self._locks = {}
493 self._scheme = "memory+%s:///" % id(self)
494
495@@ -344,6 +361,7 @@
496 result = memory.MemoryTransport(url)
497 result._dirs = self._dirs
498 result._files = self._files
499+ result._symlinks = self._symlinks
500 result._locks = self._locks
501 return result
502 self._memory_factory = memory_factory

Subscribers

People subscribed via source and target branches