Merge lp:~jelmer/brz/objects-2c into lp:brz

Proposed by Jelmer Vernooij on 2019-06-17
Status: Merged
Approved by: Jelmer Vernooij on 2019-06-22
Approved revision: 7360
Merge reported by: The Breezy Bot
Merged at revision: not available
Proposed branch: lp:~jelmer/brz/objects-2c
Merge into: lp:brz
Diff against target: 500 lines (+62/-73)
8 files modified
breezy/bzr/inventorytree.py (+7/-6)
breezy/merge.py (+31/-41)
breezy/plugins/changelog_merge/tests/test_changelog_merge.py (+3/-4)
breezy/tests/per_merger.py (+15/-14)
breezy/tests/per_tree/test_tree.py (+1/-2)
breezy/tests/per_workingtree/test_merge_from_branch.py (+1/-1)
breezy/tests/test_transform.py (+2/-2)
breezy/transform.py (+2/-3)
To merge this branch: bzr merge lp:~jelmer/brz/objects-2c
Reviewer Review Type Date Requested Status
Martin Packman 2019-06-17 Approve on 2019-06-21
Review via email: mp+368932@code.launchpad.net

Commit message

Drop file_id attribute from MergeHookParams and Tree.plan_file_merge.

Description of the change

Drop file_id attribute from MergeHookParams and Tree.plan_file_merge.

(This depends on objects-2a and objects-2b, but I can't set two prerequisite branches)

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

New changes here all seem fine. Some interface changes, which may be worth calling out in NEWS?

review: Approve
lp:~jelmer/brz/objects-2c updated on 2019-06-22
7359. By Jelmer Vernooij on 2019-06-22

Fix formatting.

7360. By Jelmer Vernooij on 2019-06-22

Fix conflict tests.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'breezy/bzr/inventorytree.py'
2--- breezy/bzr/inventorytree.py 2019-06-21 16:18:13 +0000
3+++ breezy/bzr/inventorytree.py 2019-06-22 13:51:41 +0000
4@@ -215,11 +215,12 @@
5 raise errors.NotADirectory(path)
6 return iter(viewvalues(ie.children))
7
8- def _get_plan_merge_data(self, file_id, other, base):
9+ def _get_plan_merge_data(self, path, other, base):
10 from . import versionedfile
11+ file_id = self.path2id(path)
12 vf = versionedfile._PlanMergeVersionedFile(file_id)
13 last_revision_a = self._get_file_revision(
14- self.id2path(file_id), file_id, vf, b'this:')
15+ path, file_id, vf, b'this:')
16 last_revision_b = other._get_file_revision(
17 other.id2path(file_id), file_id, vf, b'other:')
18 if base is None:
19@@ -229,7 +230,7 @@
20 base.id2path(file_id), file_id, vf, b'base:')
21 return vf, last_revision_a, last_revision_b, last_revision_base
22
23- def plan_file_merge(self, file_id, other, base=None):
24+ def plan_file_merge(self, path, other, base=None):
25 """Generate a merge plan based on annotations.
26
27 If the file contains uncommitted changes in this tree, they will be
28@@ -237,12 +238,12 @@
29 uncommitted changes in the other tree, they will be assigned to the
30 'other:' pseudo-revision.
31 """
32- data = self._get_plan_merge_data(file_id, other, base)
33+ data = self._get_plan_merge_data(path, other, base)
34 vf, last_revision_a, last_revision_b, last_revision_base = data
35 return vf.plan_merge(last_revision_a, last_revision_b,
36 last_revision_base)
37
38- def plan_file_lca_merge(self, file_id, other, base=None):
39+ def plan_file_lca_merge(self, path, other, base=None):
40 """Generate a merge plan based lca-newness.
41
42 If the file contains uncommitted changes in this tree, they will be
43@@ -250,7 +251,7 @@
44 uncommitted changes in the other tree, they will be assigned to the
45 'other:' pseudo-revision.
46 """
47- data = self._get_plan_merge_data(file_id, other, base)
48+ data = self._get_plan_merge_data(path, other, base)
49 vf, last_revision_a, last_revision_b, last_revision_base = data
50 return vf.plan_lca_merge(last_revision_a, last_revision_b,
51 last_revision_base)
52
53=== modified file 'breezy/merge.py'
54--- breezy/merge.py 2019-06-22 11:16:17 +0000
55+++ breezy/merge.py 2019-06-22 13:51:41 +0000
56@@ -223,20 +223,18 @@
57
58 There are some fields hooks can access:
59
60- :ivar file_id: the file ID of the file being merged
61 :ivar base_path: Path in base tree
62 :ivar other_path: Path in other tree
63 :ivar this_path: Path in this tree
64 :ivar trans_id: the transform ID for the merge of this file
65- :ivar this_kind: kind of file_id in 'this' tree
66- :ivar other_kind: kind of file_id in 'other' tree
67+ :ivar this_kind: kind of file in 'this' tree
68+ :ivar other_kind: kind of file in 'other' tree
69 :ivar winner: one of 'this', 'other', 'conflict'
70 """
71
72- def __init__(self, merger, file_id, paths, trans_id, this_kind, other_kind,
73+ def __init__(self, merger, paths, trans_id, this_kind, other_kind,
74 winner):
75 self._merger = merger
76- self.file_id = file_id
77 self.paths = paths
78 self.base_path, self.other_path, self.this_path = paths
79 self.trans_id = trans_id
80@@ -804,16 +802,18 @@
81 with ui.ui_factory.nested_progress_bar() as child_pb:
82 for num, (file_id, changed, paths3, parents3, names3,
83 executable3) in enumerate(entries):
84+ trans_id = self.tt.trans_id_file_id(file_id)
85+
86 # Try merging each entry
87 child_pb.update(gettext('Preparing file merge'),
88 num, len(entries))
89- self._merge_names(file_id, paths3, parents3,
90+ self._merge_names(trans_id, file_id, paths3, parents3,
91 names3, resolver=resolver)
92 if changed:
93- file_status = self._do_merge_contents(paths3, file_id)
94+ file_status = self._do_merge_contents(paths3, trans_id, file_id)
95 else:
96 file_status = 'unmodified'
97- self._merge_executable(paths3, file_id, executable3,
98+ self._merge_executable(paths3, trans_id, executable3,
99 file_status, resolver=resolver)
100 self.tt.fixup_new_roots()
101 self._finish_computing_transform()
102@@ -1182,8 +1182,8 @@
103 # At this point, the lcas disagree, and the tip disagree
104 return 'conflict'
105
106- def _merge_names(self, file_id, paths, parents, names, resolver):
107- """Perform a merge on file_id names and parents"""
108+ def _merge_names(self, trans_id, file_id, paths, parents, names, resolver):
109+ """Perform a merge on file names and parents"""
110 base_name, other_name, this_name = names
111 base_parent, other_parent, this_parent = parents
112 unused_base_path, other_path, this_path = paths
113@@ -1202,7 +1202,6 @@
114 # Creating helpers (.OTHER or .THIS) here cause problems down the
115 # road if a ContentConflict needs to be created so we should not do
116 # that
117- trans_id = self.tt.trans_id_file_id(file_id)
118 self._raw_conflicts.append(('path conflict', trans_id, file_id,
119 this_parent, this_name,
120 other_parent, other_name))
121@@ -1225,10 +1224,9 @@
122 parent_trans_id = transform.ROOT_PARENT
123 else:
124 parent_trans_id = self.tt.trans_id_file_id(parent_id)
125- self.tt.adjust_path(name, parent_trans_id,
126- self.tt.trans_id_file_id(file_id))
127+ self.tt.adjust_path(name, parent_trans_id, trans_id)
128
129- def _do_merge_contents(self, paths, file_id):
130+ def _do_merge_contents(self, paths, trans_id, file_id):
131 """Performs a merge on file_id contents."""
132 def contents_pair(tree, path):
133 if path is None:
134@@ -1272,10 +1270,8 @@
135 return "unmodified"
136 # We have a hypothetical conflict, but if we have files, then we
137 # can try to merge the content
138- trans_id = self.tt.trans_id_file_id(file_id)
139 params = MergeFileHookParams(
140- self, file_id, (base_path, other_path,
141- this_path), trans_id, this_pair[0],
142+ self, (base_path, other_path, this_path), trans_id, this_pair[0],
143 other_pair[0], winner)
144 hooks = self.active_hooks
145 hook_status = 'not_applicable'
146@@ -1372,8 +1368,8 @@
147 return 'delete', None
148 else:
149 raise AssertionError(
150- 'winner is OTHER, but file_id %r not in THIS or OTHER tree'
151- % (file_id,))
152+ 'winner is OTHER, but file %r not in THIS or OTHER tree'
153+ % (merge_hook_params.base_path,))
154
155 def merge_contents(self, merge_hook_params):
156 """Fallback merge logic after user installed hooks."""
157@@ -1389,7 +1385,7 @@
158 # have agreement that output should be a file.
159 try:
160 self.text_merge(merge_hook_params.trans_id,
161- merge_hook_params.paths, merge_hook_params.file_id)
162+ merge_hook_params.paths)
163 except errors.BinaryFile:
164 return 'not_applicable', None
165 return 'done', None
166@@ -1409,8 +1405,8 @@
167 return []
168 return tree.get_file_lines(path)
169
170- def text_merge(self, trans_id, paths, file_id):
171- """Perform a three-way text merge on a file_id"""
172+ def text_merge(self, trans_id, paths):
173+ """Perform a three-way text merge on a file"""
174 # it's possible that we got here with base as a different type.
175 # if so, we just want two-way text conflicts.
176 base_path, other_path, this_path = paths
177@@ -1445,6 +1441,7 @@
178 self._raw_conflicts.append(('text conflict', trans_id))
179 name = self.tt.final_name(trans_id)
180 parent_id = self.tt.final_parent(trans_id)
181+ file_id = self.tt.final_file_id(trans_id)
182 file_group = self._dump_conflicts(name, paths, parent_id, file_id,
183 this_lines, base_lines,
184 other_lines)
185@@ -1515,14 +1512,7 @@
186 filter_tree_path=filter_tree_path)
187 return trans_id
188
189- def merge_executable(self, paths, file_id, file_status):
190- """Perform a merge on the execute bit."""
191- executable = [self.executable(t, p, file_id)
192- for t, p in zip([self.base_tree, self.other_tree, self.this_tree], paths)]
193- self._merge_executable(paths, file_id, executable, file_status,
194- resolver=self._three_way)
195-
196- def _merge_executable(self, paths, file_id, executable, file_status,
197+ def _merge_executable(self, paths, trans_id, executable, file_status,
198 resolver):
199 """Perform a merge on the execute bit."""
200 base_executable, other_executable, this_executable = executable
201@@ -1539,7 +1529,6 @@
202 winner = "other"
203 if winner == 'this' and file_status != "modified":
204 return
205- trans_id = self.tt.trans_id_file_id(file_id)
206 if self.tt.final_kind(trans_id) != "file":
207 return
208 if winner == "this":
209@@ -1552,7 +1541,6 @@
210 elif base_path is not None:
211 executability = base_executable
212 if executability is not None:
213- trans_id = self.tt.trans_id_file_id(file_id)
214 self.tt.set_executability(executability, trans_id)
215
216 def cook_conflicts(self, fs_conflicts):
217@@ -1634,11 +1622,11 @@
218 history_based = True
219 requires_file_merge_plan = True
220
221- def _generate_merge_plan(self, file_id, base):
222- return self.this_tree.plan_file_merge(file_id, self.other_tree,
223+ def _generate_merge_plan(self, this_path, base):
224+ return self.this_tree.plan_file_merge(this_path, self.other_tree,
225 base=base)
226
227- def _merged_lines(self, file_id):
228+ def _merged_lines(self, this_path):
229 """Generate the merged lines.
230 There is no distinction between lines that are meant to contain <<<<<<<
231 and conflicts.
232@@ -1647,7 +1635,7 @@
233 base = self.base_tree
234 else:
235 base = None
236- plan = self._generate_merge_plan(file_id, base)
237+ plan = self._generate_merge_plan(this_path, base)
238 if 'merge' in debug.debug_flags:
239 plan = list(plan)
240 trans_id = self.tt.trans_id_file_id(file_id)
241@@ -1663,13 +1651,13 @@
242 base_lines = None
243 return lines, base_lines
244
245- def text_merge(self, trans_id, paths, file_id):
246+ def text_merge(self, trans_id, paths):
247 """Perform a (weave) text merge for a given file and file-id.
248 If conflicts are encountered, .THIS and .OTHER files will be emitted,
249 and a conflict will be noted.
250 """
251 base_path, other_path, this_path = paths
252- lines, base_lines = self._merged_lines(file_id)
253+ lines, base_lines = self._merged_lines(this_path)
254 lines = list(lines)
255 # Note we're checking whether the OUTPUT is binary in this case,
256 # because we don't want to get into weave merge guts.
257@@ -1680,6 +1668,7 @@
258 self._raw_conflicts.append(('text conflict', trans_id))
259 name = self.tt.final_name(trans_id)
260 parent_id = self.tt.final_parent(trans_id)
261+ file_id = self.tt.final_file_id(trans_id)
262 file_group = self._dump_conflicts(name, paths, parent_id, file_id,
263 no_base=False,
264 base_lines=base_lines)
265@@ -1690,8 +1679,8 @@
266
267 requires_file_merge_plan = True
268
269- def _generate_merge_plan(self, file_id, base):
270- return self.this_tree.plan_file_lca_merge(file_id, self.other_tree,
271+ def _generate_merge_plan(self, this_path, base):
272+ return self.this_tree.plan_file_lca_merge(this_path, self.other_tree,
273 base=base)
274
275
276@@ -1708,7 +1697,7 @@
277 out_file.write(line)
278 return out_path
279
280- def text_merge(self, trans_id, paths, file_id):
281+ def text_merge(self, trans_id, paths):
282 """Perform a diff3 merge using a specified file-id and trans-id.
283 If conflicts are encountered, .BASE, .THIS. and .OTHER conflict files
284 will be dumped, and a will be conflict noted.
285@@ -1732,6 +1721,7 @@
286 if status == 1:
287 name = self.tt.final_name(trans_id)
288 parent_id = self.tt.final_parent(trans_id)
289+ file_id = self.tt.final_file_id(trans_id)
290 self._dump_conflicts(name, paths, parent_id, file_id)
291 self._raw_conflicts.append(('text conflict', trans_id))
292 finally:
293
294=== modified file 'breezy/plugins/changelog_merge/tests/test_changelog_merge.py'
295--- breezy/plugins/changelog_merge/tests/test_changelog_merge.py 2018-11-11 04:08:32 +0000
296+++ breezy/plugins/changelog_merge/tests/test_changelog_merge.py 2019-06-22 13:51:41 +0000
297@@ -193,10 +193,9 @@
298 # won't write the new value to disk where get_user_option can get it).
299 merger.this_branch.get_config().set_user_option(
300 'changelog_merge_files', 'ChangeLog')
301- merge_hook_params = merge.MergeFileHookParams(merger, b'clog-id',
302- ['ChangeLog', 'ChangeLog',
303- 'ChangeLog'], None,
304- 'file', 'file', 'conflict')
305+ merge_hook_params = merge.MergeFileHookParams(
306+ merger, ['ChangeLog', 'ChangeLog', 'ChangeLog'], None,
307+ 'file', 'file', 'conflict')
308 changelog_merger = changelog_merge.ChangeLogMerger(merger)
309 return changelog_merger, merge_hook_params
310
311
312=== modified file 'breezy/tests/per_merger.py'
313--- breezy/tests/per_merger.py 2019-06-17 23:01:58 +0000
314+++ breezy/tests/per_merger.py 2019-06-22 13:51:41 +0000
315@@ -242,7 +242,7 @@
316 class HookSuccess(_mod_merge.AbstractPerFileMerger):
317 def merge_contents(self, merge_params):
318 test.hook_log.append(('success',))
319- if merge_params.file_id == b'1':
320+ if merge_params.this_path == 'name1':
321 return 'success', [b'text-merged-by-hook']
322 return 'not_applicable', None
323
324@@ -257,7 +257,7 @@
325 class HookConflict(_mod_merge.AbstractPerFileMerger):
326 def merge_contents(self, merge_params):
327 test.hook_log.append(('conflict',))
328- if merge_params.file_id == b'1':
329+ if merge_params.this_path == 'name1':
330 return ('conflicted',
331 [b'text-with-conflict-markers-from-hook'])
332 return 'not_applicable', None
333@@ -273,7 +273,7 @@
334 class HookDelete(_mod_merge.AbstractPerFileMerger):
335 def merge_contents(self, merge_params):
336 test.hook_log.append(('delete',))
337- if merge_params.file_id == b'1':
338+ if merge_params.this_path == 'name1':
339 return 'delete', None
340 return 'not_applicable', None
341
342@@ -309,8 +309,9 @@
343 self.addCleanup(builder.cleanup)
344 return builder
345
346- def create_file_needing_contents_merge(self, builder, file_id):
347- builder.add_file(file_id, builder.tree_root, "name1", b"text1", True)
348+ def create_file_needing_contents_merge(self, builder, name):
349+ file_id = name.encode('ascii') + b'-id'
350+ builder.add_file(file_id, builder.tree_root, name, b"text1", True)
351 builder.change_contents(file_id, other=b"text4", this=b"text3")
352
353 def test_change_vs_change(self):
354@@ -340,19 +341,19 @@
355 """A hook's result can be the deletion of a file."""
356 self.install_hook_delete()
357 builder = self.make_merge_builder()
358- self.create_file_needing_contents_merge(builder, b"1")
359+ self.create_file_needing_contents_merge(builder, "name1")
360 conflicts = builder.merge(self.merge_type)
361 self.assertEqual(conflicts, [])
362- self.assertRaises(errors.NoSuchId, builder.this.id2path, b'1')
363+ self.assertFalse(builder.this.is_versioned('name1'))
364 self.assertEqual([], list(builder.this.list_files()))
365
366 def test_result_can_be_conflict(self):
367 """A hook's result can be a conflict."""
368 self.install_hook_conflict()
369 builder = self.make_merge_builder()
370- self.create_file_needing_contents_merge(builder, b"1")
371+ self.create_file_needing_contents_merge(builder, "name1")
372 conflicts = builder.merge(self.merge_type)
373- self.assertEqual(conflicts, [TextConflict('name1', file_id=b'1')])
374+ self.assertEqual(conflicts, [TextConflict('name1', file_id=b'name1-id')])
375 # The hook still gets to set the file contents in this case, so that it
376 # can insert custom conflict markers.
377 with builder.this.get_file('name1') as f:
378@@ -375,7 +376,7 @@
379 self.install_hook_inactive()
380 self.install_hook_success()
381 builder = self.make_merge_builder()
382- self.create_file_needing_contents_merge(builder, b"1")
383+ self.create_file_needing_contents_merge(builder, "name1")
384 conflicts = builder.merge(self.merge_type)
385 self.assertEqual(conflicts, [])
386 with builder.this.get_file('name1') as f:
387@@ -389,7 +390,7 @@
388 self.install_hook_noop()
389 self.install_hook_success()
390 builder = self.make_merge_builder()
391- self.create_file_needing_contents_merge(builder, b"1")
392+ self.create_file_needing_contents_merge(builder, "name1")
393 conflicts = builder.merge(self.merge_type)
394 self.assertEqual(conflicts, [])
395 with builder.this.get_file('name1') as f:
396@@ -402,7 +403,7 @@
397 self.install_hook_success()
398 self.install_hook_noop()
399 builder = self.make_merge_builder()
400- self.create_file_needing_contents_merge(builder, b"1")
401+ self.create_file_needing_contents_merge(builder, "name1")
402 conflicts = builder.merge(self.merge_type)
403 self.assertEqual([('success',)], self.hook_log)
404
405@@ -412,7 +413,7 @@
406 self.install_hook_conflict()
407 self.install_hook_noop()
408 builder = self.make_merge_builder()
409- self.create_file_needing_contents_merge(builder, b"1")
410+ self.create_file_needing_contents_merge(builder, "name1")
411 conflicts = builder.merge(self.merge_type)
412 self.assertEqual([('conflict',)], self.hook_log)
413
414@@ -422,6 +423,6 @@
415 self.install_hook_delete()
416 self.install_hook_noop()
417 builder = self.make_merge_builder()
418- self.create_file_needing_contents_merge(builder, b"1")
419+ self.create_file_needing_contents_merge(builder, "name1")
420 conflicts = builder.merge(self.merge_type)
421 self.assertEqual([('delete',)], self.hook_log)
422
423=== modified file 'breezy/tests/per_tree/test_tree.py'
424--- breezy/tests/per_tree/test_tree.py 2019-06-18 13:48:45 +0000
425+++ breezy/tests/per_tree/test_tree.py 2019-06-22 13:51:41 +0000
426@@ -51,7 +51,6 @@
427 work_a = self.make_branch_and_tree('wta')
428 self.build_tree_contents([('wta/file', b'a\nb\nc\nd\n')])
429 work_a.add('file')
430- file_id = work_a.path2id('file')
431 work_a.commit('base version')
432 work_b = work_a.controldir.sprout('wtb').open_workingtree()
433 self.build_tree_contents([('wta/file', b'b\nc\nd\ne\n')])
434@@ -72,7 +71,7 @@
435 ('unchanged', b'd\n'),
436 ('new-a', b'e\n'),
437 ('new-b', b'f\n'),
438- ], list(tree_a.plan_file_merge(file_id, tree_b)))
439+ ], list(tree_a.plan_file_merge('file', tree_b)))
440
441
442 class TestReference(TestCaseWithTree):
443
444=== modified file 'breezy/tests/per_workingtree/test_merge_from_branch.py'
445--- breezy/tests/per_workingtree/test_merge_from_branch.py 2018-11-11 04:08:32 +0000
446+++ breezy/tests/per_workingtree/test_merge_from_branch.py 2019-06-22 13:51:41 +0000
447@@ -115,7 +115,7 @@
448 this.commit('content -> baz')
449
450 class QuxMerge(merge.Merge3Merger):
451- def text_merge(self, trans_id, paths, file_id):
452+ def text_merge(self, trans_id, paths):
453 self.tt.create_file([b'qux'], trans_id)
454 this.merge_from_branch(other.branch, merge_type=QuxMerge)
455 self.assertEqual(b'qux', this.get_file_text('foo'))
456
457=== modified file 'breezy/tests/test_transform.py'
458--- breezy/tests/test_transform.py 2019-06-22 11:16:17 +0000
459+++ breezy/tests/test_transform.py 2019-06-22 13:51:41 +0000
460@@ -3264,7 +3264,7 @@
461 ('unchanged', b'd\n'),
462 ('new-a', b'e\n'),
463 ('new-b', b'f\n'),
464- ], list(tree_a.plan_file_merge(b'file-id', tree_b)))
465+ ], list(tree_a.plan_file_merge('file', tree_b)))
466
467 def test_plan_file_merge_revision_tree(self):
468 work_a = self.make_branch_and_tree('wta')
469@@ -3287,7 +3287,7 @@
470 ('unchanged', b'd\n'),
471 ('new-a', b'e\n'),
472 ('new-b', b'f\n'),
473- ], list(tree_a.plan_file_merge(b'file-id', tree_b)))
474+ ], list(tree_a.plan_file_merge('file', tree_b)))
475
476 def test_walkdirs(self):
477 preview = self.get_empty_preview()
478
479=== modified file 'breezy/transform.py'
480--- breezy/transform.py 2019-06-22 11:16:17 +0000
481+++ breezy/transform.py 2019-06-22 13:51:41 +0000
482@@ -2675,8 +2675,7 @@
483 divert.add(file_id)
484 if (file_id not in divert
485 and _content_match(
486- tree, entry, tree_path, file_id, kind,
487- target_path)):
488+ tree, entry, tree_path, kind, target_path)):
489 tt.delete_contents(tt.trans_id_tree_path(tree_path))
490 if kind == 'directory':
491 reparent = True
492@@ -2785,7 +2784,7 @@
493 return by_parent[old_parent]
494
495
496-def _content_match(tree, entry, tree_path, file_id, kind, target_path):
497+def _content_match(tree, entry, tree_path, kind, target_path):
498 if entry.kind != kind:
499 return False
500 if entry.kind == "directory":

Subscribers

People subscribed via source and target branches