Merge lp:~jelmer/brz/brz-git-renames into lp:brz

Proposed by Jelmer Vernooij on 2018-09-14
Status: Merged
Approved by: Jelmer Vernooij on 2018-09-14
Approved revision: 7108
Merge reported by: The Breezy Bot
Merged at revision: not available
Proposed branch: lp:~jelmer/brz/brz-git-renames
Merge into: lp:brz
Diff against target: 503 lines (+218/-170)
8 files modified
breezy/builtins.py (+4/-4)
breezy/bzr/inventorytree.py (+1/-84)
breezy/bzr/workingtree.py (+23/-0)
breezy/tests/per_tree/test_inv.py (+0/-82)
breezy/tests/per_workingtree/__init__.py (+1/-0)
breezy/tests/per_workingtree/test_canonical_path.py (+109/-0)
breezy/tree.py (+44/-0)
breezy/workingtree.py (+36/-0)
To merge this branch: bzr merge lp:~jelmer/brz/brz-git-renames
Reviewer Review Type Date Requested Status
Martin Packman 2018-09-14 Approve on 2018-09-14
Review via email: mp+354952@code.launchpad.net

Description of the change

Some refactoring around get_canonical_inventory_paths.

* Drop the 'inventory' bit since it doesn't apply to non-inventory trees
* Move to working trees; canonicalization currently only matters there, where
  files can be stored on case-insensitive (VFAT) or normalizing filesystems
  (i.e. Mac OS X)
* Run tests against all working tree formats, including Git, not just InventoryTree ones

This fixes 'bzr mv' and 'bzr rename' for Git.

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

Okay, moves seem reasonable to me. The logic is not all correct, but we'll need to do another pass anyway to deal with filesystem mapping issues.

review: Approve
The Breezy Bot (the-breezy-bot) wrote :

Running landing tests failed
https://ci.breezy-vcs.org/job/land-brz/477/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'breezy/builtins.py'
2--- breezy/builtins.py 2018-08-12 23:18:19 +0000
3+++ breezy/builtins.py 2018-09-14 14:37:48 +0000
4@@ -1043,7 +1043,7 @@
5 into_existing = False
6 else:
7 # 'fix' the case of a potential 'from'
8- from_path = tree.get_canonical_inventory_path(rel_names[0])
9+ from_path = tree.get_canonical_path(rel_names[0])
10 if (not osutils.lexists(names_list[0]) and
11 tree.is_versioned(from_path) and
12 tree.stored_kind(from_path) == "directory"):
13@@ -1053,7 +1053,7 @@
14 # move into existing directory
15 # All entries reference existing inventory items, so fix them up
16 # for cicp file-systems.
17- rel_names = tree.get_canonical_inventory_paths(rel_names)
18+ rel_names = list(tree.get_canonical_paths(rel_names))
19 for src, dest in tree.move(rel_names[:-1], rel_names[-1], after=after):
20 if not is_quiet():
21 self.outf.write("%s => %s\n" % (src, dest))
22@@ -1065,13 +1065,13 @@
23
24 # for cicp file-systems: the src references an existing inventory
25 # item:
26- src = tree.get_canonical_inventory_path(rel_names[0])
27+ src = tree.get_canonical_path(rel_names[0])
28 # Find the canonical version of the destination: In all cases, the
29 # parent of the target must be in the inventory, so we fetch the
30 # canonical version from there (we do not always *use* the
31 # canonicalized tail portion - we may be attempting to rename the
32 # case of the tail)
33- canon_dest = tree.get_canonical_inventory_path(rel_names[1])
34+ canon_dest = tree.get_canonical_path(rel_names[1])
35 dest_parent = osutils.dirname(canon_dest)
36 spec_tail = osutils.basename(rel_names[1])
37 # For a CICP file-system, we need to avoid creating 2 inventory
38
39=== modified file 'breezy/bzr/inventorytree.py'
40--- breezy/bzr/inventorytree.py 2018-07-13 00:25:33 +0000
41+++ breezy/bzr/inventorytree.py 2018-09-14 14:37:48 +0000
42@@ -69,89 +69,6 @@
43 private to external API users.
44 """
45
46- def get_canonical_inventory_paths(self, paths):
47- """Like get_canonical_inventory_path() but works on multiple items.
48-
49- :param paths: A sequence of paths relative to the root of the tree.
50- :return: A list of paths, with each item the corresponding input path
51- adjusted to account for existing elements that match case
52- insensitively.
53- """
54- with self.lock_read():
55- return list(self._yield_canonical_inventory_paths(paths))
56-
57- def get_canonical_inventory_path(self, path):
58- """Returns the first inventory item that case-insensitively matches path.
59-
60- If a path matches exactly, it is returned. If no path matches exactly
61- but more than one path matches case-insensitively, it is implementation
62- defined which is returned.
63-
64- If no path matches case-insensitively, the input path is returned, but
65- with as many path entries that do exist changed to their canonical
66- form.
67-
68- If you need to resolve many names from the same tree, you should
69- use get_canonical_inventory_paths() to avoid O(N) behaviour.
70-
71- :param path: A paths relative to the root of the tree.
72- :return: The input path adjusted to account for existing elements
73- that match case insensitively.
74- """
75- with self.lock_read():
76- return next(self._yield_canonical_inventory_paths([path]))
77-
78- def _yield_canonical_inventory_paths(self, paths):
79- for path in paths:
80- # First, if the path as specified exists exactly, just use it.
81- if self.path2id(path) is not None:
82- yield path
83- continue
84- # go walkin...
85- cur_id = self.get_root_id()
86- cur_path = ''
87- bit_iter = iter(path.split("/"))
88- for elt in bit_iter:
89- lelt = elt.lower()
90- new_path = None
91- try:
92- for child in self.iter_child_entries(self.id2path(cur_id), cur_id):
93- try:
94- # XXX: it seem like if the child is known to be in the
95- # tree, we shouldn't need to go from its id back to
96- # its path -- mbp 2010-02-11
97- #
98- # XXX: it seems like we could be more efficient
99- # by just directly looking up the original name and
100- # only then searching all children; also by not
101- # chopping paths so much. -- mbp 2010-02-11
102- child_base = os.path.basename(self.id2path(child.file_id))
103- if (child_base == elt):
104- # if we found an exact match, we can stop now; if
105- # we found an approximate match we need to keep
106- # searching because there might be an exact match
107- # later.
108- cur_id = child.file_id
109- new_path = osutils.pathjoin(cur_path, child_base)
110- break
111- elif child_base.lower() == lelt:
112- cur_id = child.file_id
113- new_path = osutils.pathjoin(cur_path, child_base)
114- except errors.NoSuchId:
115- # before a change is committed we can see this error...
116- continue
117- except errors.NotADirectory:
118- pass
119- if new_path:
120- cur_path = new_path
121- else:
122- # got to the end of this directory and no entries matched.
123- # Return what matched so far, plus the rest as specified.
124- cur_path = osutils.pathjoin(cur_path, elt, *list(bit_iter))
125- break
126- yield cur_path
127- # all done.
128-
129 def _get_root_inventory(self):
130 return self._inventory
131
132@@ -451,7 +368,7 @@
133 def _fix_case_of_inventory_path(self, path):
134 """If our tree isn't case sensitive, return the canonical path"""
135 if not self.case_sensitive:
136- path = self.get_canonical_inventory_path(path)
137+ path = self.get_canonical_path(path)
138 return path
139
140 def smart_add(self, file_list, recurse=True, action=None, save=True):
141
142=== modified file 'breezy/bzr/workingtree.py'
143--- breezy/bzr/workingtree.py 2018-08-26 01:29:46 +0000
144+++ breezy/bzr/workingtree.py 2018-09-14 14:37:48 +0000
145@@ -78,6 +78,7 @@
146 )
147 from ..trace import mutter, note
148 from ..tree import (
149+ get_canonical_path,
150 FileTimestampUnavailable,
151 TreeDirectory,
152 TreeFile,
153@@ -1710,6 +1711,28 @@
154 else:
155 return self._check_for_tree_references(iterator)
156
157+ def get_canonical_paths(self, paths):
158+ """Look up canonical paths for multiple items.
159+
160+ :param paths: A sequence of paths relative to the root of the tree.
161+ :return: A iterator over paths, with each item the corresponding input path
162+ adjusted to account for existing elements that match case
163+ insensitively.
164+ """
165+ with self.lock_read():
166+ if not self.case_sensitive:
167+ normalize = lambda x: x.lower()
168+ elif sys.platform == 'darwin':
169+ import unicodedata
170+ normalize = lambda x: unicodedata.normalize('NFC', x)
171+ else:
172+ normalize = None
173+ for path in paths:
174+ if normalize is None or self.is_versioned(path):
175+ yield path
176+ else:
177+ yield get_canonical_path(self, path, normalize)
178+
179
180 class WorkingTreeFormatMetaDir(bzrdir.BzrFormat, WorkingTreeFormat):
181 """Base class for working trees that live in bzr meta directories."""
182
183=== modified file 'breezy/tests/per_tree/test_inv.py'
184--- breezy/tests/per_tree/test_inv.py 2018-07-03 02:40:57 +0000
185+++ breezy/tests/per_tree/test_inv.py 2018-09-14 14:37:48 +0000
186@@ -27,7 +27,6 @@
187 from breezy.mutabletree import MutableTree
188 from breezy.tests import TestSkipped
189 from breezy.transform import _PreviewTree
190-from breezy.uncommit import uncommit
191 from breezy.tests import (
192 features,
193 )
194@@ -98,84 +97,3 @@
195 self.addCleanup(tree.unlock)
196 self.assertEqual(set([]), tree.paths2ids(['file'],
197 require_versioned=False))
198-
199- def _make_canonical_test_tree(self, commit=True):
200- # make a tree used by all the 'canonical' tests below.
201- work_tree = self.make_branch_and_tree('tree')
202- self.build_tree(['tree/dir/', 'tree/dir/file'])
203- work_tree.add(['dir', 'dir/file'])
204- if commit:
205- work_tree.commit('commit 1')
206- # XXX: this isn't actually guaranteed to return the class we want to
207- # test -- mbp 2010-02-12
208- return work_tree
209-
210- def test_canonical_path(self):
211- work_tree = self._make_canonical_test_tree()
212- if not isinstance(work_tree, InventoryTree):
213- raise tests.TestNotApplicable(
214- "test not applicable on non-inventory tests")
215- self.assertEqual('dir/file',
216- work_tree.get_canonical_inventory_path('Dir/File'))
217-
218- def test_canonical_path_before_commit(self):
219- work_tree = self._make_canonical_test_tree(False)
220- if not isinstance(work_tree, InventoryTree):
221- # note: not committed.
222- raise tests.TestNotApplicable(
223- "test not applicable on non-inventory tests")
224- self.assertEqual('dir/file',
225- work_tree.get_canonical_inventory_path('Dir/File'))
226-
227- def test_canonical_path_dir(self):
228- # check it works when asked for just the directory portion.
229- work_tree = self._make_canonical_test_tree()
230- if not isinstance(work_tree, InventoryTree):
231- raise tests.TestNotApplicable(
232- "test not applicable on non-inventory tests")
233- self.assertEqual('dir', work_tree.get_canonical_inventory_path('Dir'))
234-
235- def test_canonical_path_root(self):
236- work_tree = self._make_canonical_test_tree()
237- if not isinstance(work_tree, InventoryTree):
238- raise tests.TestNotApplicable(
239- "test not applicable on non-inventory tests")
240- self.assertEqual('', work_tree.get_canonical_inventory_path(''))
241- self.assertEqual('/', work_tree.get_canonical_inventory_path('/'))
242-
243- def test_canonical_path_invalid_all(self):
244- work_tree = self._make_canonical_test_tree()
245- if not isinstance(work_tree, InventoryTree):
246- raise tests.TestNotApplicable(
247- "test not applicable on non-inventory tests")
248- self.assertEqual('foo/bar',
249- work_tree.get_canonical_inventory_path('foo/bar'))
250-
251- def test_canonical_invalid_child(self):
252- work_tree = self._make_canonical_test_tree()
253- if not isinstance(work_tree, InventoryTree):
254- raise tests.TestNotApplicable(
255- "test not applicable on non-inventory tests")
256- self.assertEqual('dir/None',
257- work_tree.get_canonical_inventory_path('Dir/None'))
258-
259- def test_canonical_tree_name_mismatch(self):
260- # see <https://bugs.launchpad.net/bzr/+bug/368931>
261- # some of the trees we want to use can only exist on a disk, not in
262- # memory - therefore we can only test this if the filesystem is
263- # case-sensitive.
264- self.requireFeature(features.case_sensitive_filesystem_feature)
265- work_tree = self.make_branch_and_tree('.')
266- self.build_tree(['test/', 'test/file', 'Test'])
267- work_tree.add(['test/', 'test/file', 'Test'])
268-
269- test_tree = self._convert_tree(work_tree)
270- if not isinstance(test_tree, InventoryTree):
271- raise tests.TestNotApplicable(
272- "test not applicable on non-inventory tests")
273- test_tree.lock_read()
274- self.addCleanup(test_tree.unlock)
275-
276- self.assertEqual(['test', 'test/file', 'Test', 'test/foo', 'Test/foo'],
277- test_tree.get_canonical_inventory_paths(
278- ['test', 'test/file', 'Test', 'test/foo', 'Test/foo']))
279
280=== modified file 'breezy/tests/per_workingtree/__init__.py'
281--- breezy/tests/per_workingtree/__init__.py 2017-09-10 20:57:03 +0000
282+++ breezy/tests/per_workingtree/__init__.py 2018-09-14 14:37:48 +0000
283@@ -123,6 +123,7 @@
284 'basis_inventory',
285 'basis_tree',
286 'break_lock',
287+ 'canonical_path',
288 'changes_from',
289 'check',
290 'check_state',
291
292=== added file 'breezy/tests/per_workingtree/test_canonical_path.py'
293--- breezy/tests/per_workingtree/test_canonical_path.py 1970-01-01 00:00:00 +0000
294+++ breezy/tests/per_workingtree/test_canonical_path.py 2018-09-14 14:37:48 +0000
295@@ -0,0 +1,109 @@
296+# Copyright (C) 2007-2010 Canonical Ltd
297+#
298+# This program is free software; you can redistribute it and/or modify
299+# it under the terms of the GNU General Public License as published by
300+# the Free Software Foundation; either version 2 of the License, or
301+# (at your option) any later version.
302+#
303+# This program is distributed in the hope that it will be useful,
304+# but WITHOUT ANY WARRANTY; without even the implied warranty of
305+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
306+# GNU General Public License for more details.
307+#
308+# You should have received a copy of the GNU General Public License
309+# along with this program; if not, write to the Free Software
310+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
311+
312+"""Tests for interface conformance of canonical paths of trees."""
313+
314+
315+from breezy import (
316+ tests,
317+ )
318+from breezy.tests.per_workingtree import (
319+ TestCaseWithWorkingTree,
320+ )
321+from breezy.tests import (
322+ features,
323+ )
324+
325+
326+class TestCanonicalPaths(TestCaseWithWorkingTree):
327+
328+ def _make_canonical_test_tree(self, commit=True):
329+ # make a tree used by all the 'canonical' tests below.
330+ work_tree = self.make_branch_and_tree('tree')
331+ self.build_tree(['tree/dir/', 'tree/dir/file'])
332+ work_tree.add(['dir', 'dir/file'])
333+ if commit:
334+ work_tree.commit('commit 1')
335+ # XXX: this isn't actually guaranteed to return the class we want to
336+ # test -- mbp 2010-02-12
337+ return work_tree
338+
339+ def test_canonical_path(self):
340+ work_tree = self._make_canonical_test_tree()
341+ if features.CaseInsensitiveFilesystemFeature.available():
342+ self.assertEqual('dir/file',
343+ work_tree.get_canonical_path('Dir/File'))
344+ else:
345+ self.assertEqual('Dir/File',
346+ work_tree.get_canonical_path('Dir/File'))
347+
348+ def test_canonical_path_before_commit(self):
349+ work_tree = self._make_canonical_test_tree(False)
350+ if features.CaseInsensitiveFilesystemFeature.available():
351+ self.assertEqual('dir/file',
352+ work_tree.get_canonical_path('Dir/File'))
353+ else:
354+ self.assertEqual('Dir/File',
355+ work_tree.get_canonical_path('Dir/File'))
356+
357+ def test_canonical_path_dir(self):
358+ # check it works when asked for just the directory portion.
359+ work_tree = self._make_canonical_test_tree()
360+ if features.CaseInsensitiveFilesystemFeature.available():
361+ self.assertEqual('dir', work_tree.get_canonical_path('Dir'))
362+ else:
363+ self.assertEqual('Dir', work_tree.get_canonical_path('Dir'))
364+
365+ def test_canonical_path_root(self):
366+ work_tree = self._make_canonical_test_tree()
367+ self.assertEqual('', work_tree.get_canonical_path(''))
368+ self.assertEqual('/', work_tree.get_canonical_path('/'))
369+
370+ def test_canonical_path_invalid_all(self):
371+ work_tree = self._make_canonical_test_tree()
372+ self.assertEqual('foo/bar',
373+ work_tree.get_canonical_path('foo/bar'))
374+
375+ def test_canonical_invalid_child(self):
376+ work_tree = self._make_canonical_test_tree()
377+ if features.CaseInsensitiveFilesystemFeature.available():
378+ self.assertEqual('dir/None',
379+ work_tree.get_canonical_path('Dir/None'))
380+ else:
381+ self.assertEqual('Dir/None',
382+ work_tree.get_canonical_path('Dir/None'))
383+
384+ def test_canonical_tree_name_mismatch(self):
385+ # see <https://bugs.launchpad.net/bzr/+bug/368931>
386+ # some of the trees we want to use can only exist on a disk, not in
387+ # memory - therefore we can only test this if the filesystem is
388+ # case-sensitive.
389+ self.requireFeature(features.case_sensitive_filesystem_feature)
390+ work_tree = self.make_branch_and_tree('.')
391+ self.build_tree(['test/', 'test/file', 'Test'])
392+ work_tree.add(['test/', 'test/file', 'Test'])
393+
394+ self.assertEqual(['test', 'Test', 'test/file', 'Test/file'],
395+ list(work_tree.get_canonical_paths(
396+ ['test', 'Test', 'test/file', 'Test/file'])))
397+
398+ test_revid = work_tree.commit('commit')
399+ test_tree = work_tree.branch.repository.revision_tree(test_revid)
400+ test_tree.lock_read()
401+ self.addCleanup(test_tree.unlock)
402+
403+ self.assertEqual(['', 'Test', 'test', 'test/file'],
404+ [p for p, e in test_tree.iter_entries_by_dir()])
405
406=== modified file 'breezy/tree.py'
407--- breezy/tree.py 2018-07-26 22:12:29 +0000
408+++ breezy/tree.py 2018-09-14 14:37:48 +0000
409@@ -1364,3 +1364,47 @@
410 return to_tree.id2path(file_id)
411 except errors.NoSuchId:
412 return None
413+
414+
415+def get_canonical_path(tree, path, normalize):
416+ """Find the canonical path of an item, ignoring case.
417+
418+ :param tree: Tree to traverse
419+ :param path: Case-insensitive path to look up
420+ :param normalize: Function to normalize a filename for comparison
421+ :return: The canonical path
422+ """
423+ # go walkin...
424+ cur_id = self.get_root_id()
425+ cur_path = ''
426+ bit_iter = iter(path.split("/"))
427+ for elt in bit_iter:
428+ lelt = normalize(elt)
429+ new_path = None
430+ try:
431+ for child in self.iter_child_entries(cur_path, cur_id):
432+ try:
433+ if child.name == elt:
434+ # if we found an exact match, we can stop now; if
435+ # we found an approximate match we need to keep
436+ # searching because there might be an exact match
437+ # later.
438+ cur_id = child.file_id
439+ new_path = osutils.pathjoin(cur_path, child.name)
440+ break
441+ elif normalize(child.name) == lelt:
442+ cur_id = child.file_id
443+ new_path = osutils.pathjoin(cur_path, child.name)
444+ except errors.NoSuchId:
445+ # before a change is committed we can see this error...
446+ continue
447+ except errors.NotADirectory:
448+ pass
449+ if new_path:
450+ cur_path = new_path
451+ else:
452+ # got to the end of this directory and no entries matched.
453+ # Return what matched so far, plus the rest as specified.
454+ cur_path = osutils.pathjoin(cur_path, elt, *list(bit_iter))
455+ break
456+ return cur_path
457
458=== modified file 'breezy/workingtree.py'
459--- breezy/workingtree.py 2018-08-07 19:23:46 +0000
460+++ breezy/workingtree.py 2018-09-14 14:37:48 +0000
461@@ -1337,6 +1337,42 @@
462 """Return the ShelfManager for this WorkingTree."""
463 raise NotImplementedError(self.get_shelf_manager)
464
465+ def get_canonical_paths(self, paths):
466+ """Like get_canonical_path() but works on multiple items.
467+
468+ :param paths: A sequence of paths relative to the root of the tree.
469+ :return: A list of paths, with each item the corresponding input path
470+ adjusted to account for existing elements that match case
471+ insensitively.
472+ """
473+ with self.lock_read():
474+ for path in paths:
475+ yield path
476+
477+ def get_canonical_path(self, path):
478+ """Returns the first item in the tree that matches a path.
479+
480+ This is meant to allow case-insensitive path lookups on e.g.
481+ FAT filesystems.
482+
483+ If a path matches exactly, it is returned. If no path matches exactly
484+ but more than one path matches according to the underlying file system,
485+ it is implementation defined which is returned.
486+
487+ If no path matches according to the file system, the input path is
488+ returned, but with as many path entries that do exist changed to their
489+ canonical form.
490+
491+ If you need to resolve many names from the same tree, you should
492+ use get_canonical_paths() to avoid O(N) behaviour.
493+
494+ :param path: A paths relative to the root of the tree.
495+ :return: The input path adjusted to account for existing elements
496+ that match case insensitively.
497+ """
498+ with self.lock_read():
499+ return next(self.get_canonical_paths([path]))
500+
501
502 class WorkingTreeFormatRegistry(controldir.ControlComponentFormatRegistry):
503 """Registry for working tree formats."""

Subscribers

People subscribed via source and target branches