Merge lp:~jelmer/bzr/2a-repo-supports-nested-trees into lp:bzr
- 2a-repo-supports-nested-trees
- Merge into bzr.dev
Status: | Rejected |
---|---|
Rejected by: | Jelmer Vernooij |
Proposed branch: | lp:~jelmer/bzr/2a-repo-supports-nested-trees |
Merge into: | lp:bzr |
Diff against target: |
322 lines (+91/-20) 10 files modified
bzrlib/bzrdir.py (+3/-3) bzrlib/commit.py (+0/-1) bzrlib/repofmt/groupcompress_repo.py (+1/-1) bzrlib/tests/per_pack_repository.py (+11/-2) bzrlib/tests/per_tree/test_path_content_summary.py (+1/-1) bzrlib/tests/test_merge.py (+2/-2) bzrlib/tests/test_smart.py (+1/-1) bzrlib/workingtree.py (+12/-1) bzrlib/workingtree_4.py (+55/-8) doc/en/release-notes/bzr-2.6.txt (+5/-0) |
To merge this branch: | bzr merge lp:~jelmer/bzr/2a-repo-supports-nested-trees |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Vincent Ladeuil | Needs Information | ||
Review via email: mp+89919@code.launchpad.net |
Commit message
Description of the change
Enable support for nested trees in the '2a' repository format.
Support for storing 'tree-reference' inventory entries has always
been present in the '2a' repository format, just never been officially
supported by the format. The actual change (a new inventory entry kind
'tree-reference') is small and non-controversial.
Instead, this marks working tree formats 4-6 as not actually supporting
nested trees. Previously it would claim to support nested trees
depending on whether the associated repository supported nested trees.
In practice the current wt implementation doesn't properly support nested trees:
* There is no cheap way to iterate over all nested trees, so every
working tree that supports nested trees (even if there are none) pays
the penalty of scanning the entire working tree for nested trees
every time it is accessed.
* There are various bugs around the current implementation that
make it impossible to use it with nested trees. Just checking out
a containing tree shows the kind of the nested trees of having changed
from nested tree to regular directory
This adds an experimental 'Working Tree Format 7' which does claim to support
nested trees and is used by development-
do whatever is necessary to properly handle tree references and then
potentially merge it back into the other working tree formats with a
feature flag.
This change is sufficient for bzr-git to allow importing
git repositories with submodules somewhere in their history (launchpad bug
#402814), although not yet sufficient to allow checking out trees with
submodules. We'd need working tree support for the latter, of course.
Jelmer Vernooij (jelmer) wrote : | # |
On Thu, Jan 26, 2012 at 09:27:24AM -0000, Vincent Ladeuil wrote:
> Review: Needs Information
> 280 + return "Working tree format 6"
>
> Really ?
I'm not sure what's so controversial about this?
> Apart from that I don't quite see how your cover letter relates to this
> proposal:
>
> > The actual change (a new inventory entry kind 'tree-reference') is small
> and non-controversial.
>
> There is nothing about that in your proposal, are you just referring to the
> initial introduction of the tree-reference entry kind bak in the days ?
Yes.
> Ot is the actual change really:
>
> 51 + supports_
>
> and
>
> 59 - supports_
> If that's the case, I'm surprised there is no fallouts from activating tree
> references support in 2a (line 51) !?! Not a single test failing ?
Basically, yeah.
> Also, what's the plan for RepositoryForma
> likely to get it forgotten, we at least need a bug to track its removal
> describing some plan (since it's an experimental one, I think the plan can
> be reduced to: delete it, users knew).
I think we should support upgrading from 2a-subtree to 2a and then get
rid of 2a.
> > Instead, this marks working tree formats 4-6 as not actually supporting
> nested trees.
>
> So *this* part is uncontroversial and can be landed IMHO.
I disagree, doing just that without adding a new working tree format
means we no longer test the current subtree support. So I think as
part of disabling nested tree support in wt 4-6 we should also add wt
7. Or do you perhaps mean that you would like to see the working tree
changes split out?
>
> 69 + if repo._format.
> 70 + matching_
> 71 + else:
> 72 + matching_
>
> 81 + if repo.supports_
> 82 + if repo._format.
> 83 + matching_
> 84 + else:
> 85 + matching_
> 86 + else:
> 87 + matching_
>
> Urgh, these tests were already confusing, now they are almost undecipherable
> :-(
>
> Can't you abstract the logic here so we get a clearer view of what we are
> tested ? Since these tests are about stacking, I'd really prefer them to be
> easier to grasp, especially regarding the compatibility between the various
> formats. There is also a hint that it's time to address:
>
> # TODO: Possibly this should be run per-repository-
> # TestNotApplicable on formats that don't support stacking. -- mbp
> # 20080729
I'll have a look.
>
> 100 + if not tree.supports_
> Why is that a method for working trees (or is it wt formats) and an
> attribute for repos ?
>
> Highly confusing :-/
I agree. That's unfortunately something we've had for ages. I think poolie
preferred methods so we've slowly been transitioning to that.
WorkingTreeFormat seems to prefer methods, and RepositoryFormat seems
to prefer attributes.
> > This change is sufficient for bzr-git to allow importing (launchpad bug
> #402814), although not yet sufficient to allow checking out trees with
> submodules. We'd need working tree su...
Vincent Ladeuil (vila) wrote : | # |
I'm sure I started replying shortly after your reply and I sadly discover my
answer is nowhere to be seen :-(
> On Thu, Jan 26, 2012 at 09:27:24AM -0000, Vincent Ladeuil wrote:
> > Review: Needs Information
> > 280 + return "Working tree format 6"
> >
> > Really ?
>
> I'm not sure what's so controversial about this?
*6* instead of 7 ? This line is part of WorkingTreeFormat7
>
> > Apart from that I don't quite see how your cover letter relates to this
> > proposal:
> >
> > > The actual change (a new inventory entry kind 'tree-reference') is small
> > and non-controversial.
> >
> > There is nothing about that in your proposal, are you just referring to the
> > initial introduction of the tree-reference entry kind bak in the days ?
> Yes.
Ok, with that in mind, I better understand the cover letter ('change' there
wasn't related to the proposal itself which confused me).
>
> > Ot is the actual change really:
> >
> > 51 + supports_
> >
> > and
> >
> > 59 - supports_
>
> > If that's the case, I'm surprised there is no fallouts from activating tree
> > references support in 2a (line 51) !?! Not a single test failing ?
> Basically, yeah.
;_; so be it.
>
> > Also, what's the plan for RepositoryForma
> > likely to get it forgotten, we at least need a bug to track its removal
> > describing some plan (since it's an experimental one, I think the plan can
> > be reduced to: delete it, users knew).
> I think we should support upgrading from 2a-subtree to 2a and then get
> rid of 2a.
I assume you meant 'get rid of 2a-subtree'. Ok.
>
> > > Instead, this marks working tree formats 4-6 as not actually supporting
> > nested trees.
> >
> > So *this* part is uncontroversial and can be landed IMHO.
> I disagree, doing just that without adding a new working tree format
> means we no longer test the current subtree support. So I think as
> part of disabling nested tree support in wt 4-6 we should also add wt
> 7. Or do you perhaps mean that you would like to see the working tree
> changes split out?
Yup.
>
> >
> > 69 + if repo._format.
> > 70 + matching_
> > 71 + else:
> > 72 + matching_
> >
> > 81 + if repo.supports_
> > 82 + if repo._format.
> > 83 + matching_
> > 84 + else:
> > 85 + matching_
> > 86 + else:
> > 87 + matching_
> >
> > Urgh, these tests were already confusing, now they are almost undecipherable
> > :-(
> >
> > Can't you abstract the logic here so we get a clearer view of what we are
> > tested ? Since these tests are about stacking, I'd really prefer them to be
> > easier to grasp, especially regarding the compatibility between the various
> > formats. There is also a hint that it's time to address:
> >
> > # TODO: Possibly this should be run per-repository-
> > # TestNotApplicable on formats that don't support stacking. -- mbp
> > # 20080729
> I'll have a look.
Cool.
>
> >
> > 100 + if not tree...
Preview Diff
1 | === modified file 'bzrlib/bzrdir.py' | |||
2 | --- bzrlib/bzrdir.py 2012-01-18 20:27:41 +0000 | |||
3 | +++ bzrlib/bzrdir.py 2012-01-24 15:41:25 +0000 | |||
4 | @@ -2193,7 +2193,7 @@ | |||
5 | 2193 | help='Variant of dirstate with support for nested trees. ' | 2193 | help='Variant of dirstate with support for nested trees. ' |
6 | 2194 | 'Introduced in 0.15.', | 2194 | 'Introduced in 0.15.', |
7 | 2195 | branch_format='bzrlib.branch.BzrBranchFormat6', | 2195 | branch_format='bzrlib.branch.BzrBranchFormat6', |
9 | 2196 | tree_format='bzrlib.workingtree_4.WorkingTreeFormat4', | 2196 | tree_format='bzrlib.workingtree_4.WorkingTreeFormat7', |
10 | 2197 | experimental=True, | 2197 | experimental=True, |
11 | 2198 | hidden=True, | 2198 | hidden=True, |
12 | 2199 | ) | 2199 | ) |
13 | @@ -2293,7 +2293,7 @@ | |||
14 | 2293 | 'http://doc.bazaar.canonical.com/latest/developers/development-repo.html ' | 2293 | 'http://doc.bazaar.canonical.com/latest/developers/development-repo.html ' |
15 | 2294 | 'before use.', | 2294 | 'before use.', |
16 | 2295 | branch_format='bzrlib.branch.BzrBranchFormat7', | 2295 | branch_format='bzrlib.branch.BzrBranchFormat7', |
18 | 2296 | tree_format='bzrlib.workingtree_4.WorkingTreeFormat6', | 2296 | tree_format='bzrlib.workingtree_4.WorkingTreeFormat7', |
19 | 2297 | experimental=True, | 2297 | experimental=True, |
20 | 2298 | hidden=True, | 2298 | hidden=True, |
21 | 2299 | alias=False, # Restore to being an alias when an actual development subtree format is added | 2299 | alias=False, # Restore to being an alias when an actual development subtree format is added |
22 | @@ -2309,7 +2309,7 @@ | |||
23 | 2309 | 'http://doc.bazaar.canonical.com/latest/developers/development-repo.html ' | 2309 | 'http://doc.bazaar.canonical.com/latest/developers/development-repo.html ' |
24 | 2310 | 'before use.', | 2310 | 'before use.', |
25 | 2311 | branch_format='bzrlib.branch.BzrBranchFormat7', | 2311 | branch_format='bzrlib.branch.BzrBranchFormat7', |
27 | 2312 | tree_format='bzrlib.workingtree_4.WorkingTreeFormat6', | 2312 | tree_format='bzrlib.workingtree_4.WorkingTreeFormat7', |
28 | 2313 | experimental=True, | 2313 | experimental=True, |
29 | 2314 | hidden=True, | 2314 | hidden=True, |
30 | 2315 | alias=False, | 2315 | alias=False, |
31 | 2316 | 2316 | ||
32 | === modified file 'bzrlib/commit.py' | |||
33 | --- bzrlib/commit.py 2011-12-18 12:46:49 +0000 | |||
34 | +++ bzrlib/commit.py 2012-01-24 15:41:25 +0000 | |||
35 | @@ -347,7 +347,6 @@ | |||
36 | 347 | # generation. See bug 347649. | 347 | # generation. See bug 347649. |
37 | 348 | self.use_record_iter_changes = ( | 348 | self.use_record_iter_changes = ( |
38 | 349 | not self.exclude and | 349 | not self.exclude and |
39 | 350 | not self.branch.repository._format.supports_tree_reference and | ||
40 | 351 | (self.branch.repository._format.fast_deltas or | 350 | (self.branch.repository._format.fast_deltas or |
41 | 352 | len(self.parents) < 2)) | 351 | len(self.parents) < 2)) |
42 | 353 | self.pb = ui.ui_factory.nested_progress_bar() | 352 | self.pb = ui.ui_factory.nested_progress_bar() |
43 | 354 | 353 | ||
44 | === modified file 'bzrlib/repofmt/groupcompress_repo.py' | |||
45 | --- bzrlib/repofmt/groupcompress_repo.py 2011-12-19 19:15:58 +0000 | |||
46 | +++ bzrlib/repofmt/groupcompress_repo.py 2012-01-24 15:41:25 +0000 | |||
47 | @@ -1362,6 +1362,7 @@ | |||
48 | 1362 | """A CHK repository that uses the bencode revision serializer.""" | 1362 | """A CHK repository that uses the bencode revision serializer.""" |
49 | 1363 | 1363 | ||
50 | 1364 | repository_class = CHKInventoryRepository | 1364 | repository_class = CHKInventoryRepository |
51 | 1365 | supports_tree_reference = True | ||
52 | 1365 | supports_external_lookups = True | 1366 | supports_external_lookups = True |
53 | 1366 | supports_chks = True | 1367 | supports_chks = True |
54 | 1367 | _commit_builder_class = PackRootCommitBuilder | 1368 | _commit_builder_class = PackRootCommitBuilder |
55 | @@ -1423,4 +1424,3 @@ | |||
56 | 1423 | "group compression and chk inventories") | 1424 | "group compression and chk inventories") |
57 | 1424 | 1425 | ||
58 | 1425 | experimental = True | 1426 | experimental = True |
59 | 1426 | supports_tree_reference = True | ||
60 | 1427 | 1427 | ||
61 | === modified file 'bzrlib/tests/per_pack_repository.py' | |||
62 | --- bzrlib/tests/per_pack_repository.py 2011-08-19 22:34:02 +0000 | |||
63 | +++ bzrlib/tests/per_pack_repository.py 2012-01-24 15:41:25 +0000 | |||
64 | @@ -884,7 +884,10 @@ | |||
65 | 884 | # can only stack on repositories that have compatible internal | 884 | # can only stack on repositories that have compatible internal |
66 | 885 | # metadata | 885 | # metadata |
67 | 886 | if getattr(repo._format, 'supports_tree_reference', False): | 886 | if getattr(repo._format, 'supports_tree_reference', False): |
69 | 887 | matching_format_name = 'pack-0.92-subtree' | 887 | if repo._format.supports_chks: |
70 | 888 | matching_format_name = '2a' | ||
71 | 889 | else: | ||
72 | 890 | matching_format_name = 'pack-0.92-subtree' | ||
73 | 888 | else: | 891 | else: |
74 | 889 | if repo._format.supports_chks: | 892 | if repo._format.supports_chks: |
75 | 890 | matching_format_name = '2a' | 893 | matching_format_name = '2a' |
76 | @@ -915,7 +918,13 @@ | |||
77 | 915 | if getattr(repo._format, 'supports_tree_reference', False): | 918 | if getattr(repo._format, 'supports_tree_reference', False): |
78 | 916 | # can only stack on repositories that have compatible internal | 919 | # can only stack on repositories that have compatible internal |
79 | 917 | # metadata | 920 | # metadata |
81 | 918 | matching_format_name = 'pack-0.92-subtree' | 921 | if repo.supports_rich_root(): |
82 | 922 | if repo._format.supports_chks: | ||
83 | 923 | matching_format_name = '2a' | ||
84 | 924 | else: | ||
85 | 925 | matching_format_name = 'pack-0.92-subtree' | ||
86 | 926 | else: | ||
87 | 927 | matching_format_name = 'dirstate-subtree' | ||
88 | 919 | mismatching_format_name = 'rich-root-pack' | 928 | mismatching_format_name = 'rich-root-pack' |
89 | 920 | else: | 929 | else: |
90 | 921 | if repo.supports_rich_root(): | 930 | if repo.supports_rich_root(): |
91 | 922 | 931 | ||
92 | === modified file 'bzrlib/tests/per_tree/test_path_content_summary.py' | |||
93 | --- bzrlib/tests/per_tree/test_path_content_summary.py 2011-12-16 17:34:46 +0000 | |||
94 | +++ bzrlib/tests/per_tree/test_path_content_summary.py 2012-01-24 15:41:25 +0000 | |||
95 | @@ -147,7 +147,7 @@ | |||
96 | 147 | 147 | ||
97 | 148 | def test_tree_content_summary(self): | 148 | def test_tree_content_summary(self): |
98 | 149 | tree = self.make_branch_and_tree('tree') | 149 | tree = self.make_branch_and_tree('tree') |
100 | 150 | if not tree.branch.repository._format.supports_tree_reference: | 150 | if not tree.supports_tree_reference(): |
101 | 151 | raise tests.TestNotApplicable("Tree references not supported.") | 151 | raise tests.TestNotApplicable("Tree references not supported.") |
102 | 152 | subtree = self.make_branch_and_tree('tree/path') | 152 | subtree = self.make_branch_and_tree('tree/path') |
103 | 153 | tree.add(['path']) | 153 | tree.add(['path']) |
104 | 154 | 154 | ||
105 | === modified file 'bzrlib/tests/test_merge.py' | |||
106 | --- bzrlib/tests/test_merge.py 2012-01-05 13:02:31 +0000 | |||
107 | +++ bzrlib/tests/test_merge.py 2012-01-24 15:41:25 +0000 | |||
108 | @@ -227,9 +227,9 @@ | |||
109 | 227 | 227 | ||
110 | 228 | def test_nested_merge(self): | 228 | def test_nested_merge(self): |
111 | 229 | tree = self.make_branch_and_tree('tree', | 229 | tree = self.make_branch_and_tree('tree', |
113 | 230 | format='dirstate-with-subtree') | 230 | format='development-subtree') |
114 | 231 | sub_tree = self.make_branch_and_tree('tree/sub-tree', | 231 | sub_tree = self.make_branch_and_tree('tree/sub-tree', |
116 | 232 | format='dirstate-with-subtree') | 232 | format='development-subtree') |
117 | 233 | sub_tree.set_root_id('sub-tree-root') | 233 | sub_tree.set_root_id('sub-tree-root') |
118 | 234 | self.build_tree_contents([('tree/sub-tree/file', 'text1')]) | 234 | self.build_tree_contents([('tree/sub-tree/file', 'text1')]) |
119 | 235 | sub_tree.add('file') | 235 | sub_tree.add('file') |
120 | 236 | 236 | ||
121 | === modified file 'bzrlib/tests/test_smart.py' | |||
122 | --- bzrlib/tests/test_smart.py 2012-01-19 19:18:47 +0000 | |||
123 | +++ bzrlib/tests/test_smart.py 2012-01-24 15:41:25 +0000 | |||
124 | @@ -2669,7 +2669,7 @@ | |||
125 | 2669 | base_inv = repository.revision_tree(base_revid).inventory | 2669 | base_inv = repository.revision_tree(base_revid).inventory |
126 | 2670 | inv = repository.revision_tree(revid).inventory | 2670 | inv = repository.revision_tree(revid).inventory |
127 | 2671 | inv_delta = inv._make_delta(base_inv) | 2671 | inv_delta = inv._make_delta(base_inv) |
129 | 2672 | serializer = inventory_delta.InventoryDeltaSerializer(True, False) | 2672 | serializer = inventory_delta.InventoryDeltaSerializer(True, True) |
130 | 2673 | return "".join(serializer.delta_to_lines(base_revid, revid, inv_delta)) | 2673 | return "".join(serializer.delta_to_lines(base_revid, revid, inv_delta)) |
131 | 2674 | 2674 | ||
132 | 2675 | def test_single(self): | 2675 | def test_single(self): |
133 | 2676 | 2676 | ||
134 | === modified file 'bzrlib/workingtree.py' | |||
135 | --- bzrlib/workingtree.py 2012-01-18 17:47:06 +0000 | |||
136 | +++ bzrlib/workingtree.py 2012-01-24 15:41:25 +0000 | |||
137 | @@ -840,7 +840,7 @@ | |||
138 | 840 | raise NotImplementedError(self.subsume) | 840 | raise NotImplementedError(self.subsume) |
139 | 841 | 841 | ||
140 | 842 | def _setup_directory_is_tree_reference(self): | 842 | def _setup_directory_is_tree_reference(self): |
142 | 843 | if self._branch.repository._format.supports_tree_reference: | 843 | if self.supports_tree_reference(): |
143 | 844 | self._directory_is_tree_reference = \ | 844 | self._directory_is_tree_reference = \ |
144 | 845 | self._directory_may_be_tree_reference | 845 | self._directory_may_be_tree_reference |
145 | 846 | else: | 846 | else: |
146 | @@ -3098,6 +3098,15 @@ | |||
147 | 3098 | """True if this format supports stored views.""" | 3098 | """True if this format supports stored views.""" |
148 | 3099 | return False | 3099 | return False |
149 | 3100 | 3100 | ||
150 | 3101 | def supports_tree_reference(self): | ||
151 | 3102 | """True if this format supports tree references. | ||
152 | 3103 | |||
153 | 3104 | Note that in order to be able to add tree references the | ||
154 | 3105 | working tree, branch and repository all have to support | ||
155 | 3106 | tree references. | ||
156 | 3107 | """ | ||
157 | 3108 | return False | ||
158 | 3109 | |||
159 | 3101 | @classmethod | 3110 | @classmethod |
160 | 3102 | @symbol_versioning.deprecated_method( | 3111 | @symbol_versioning.deprecated_method( |
161 | 3103 | symbol_versioning.deprecated_in((2, 4, 0))) | 3112 | symbol_versioning.deprecated_in((2, 4, 0))) |
162 | @@ -3181,6 +3190,8 @@ | |||
163 | 3181 | "bzrlib.workingtree_4", "WorkingTreeFormat5") | 3190 | "bzrlib.workingtree_4", "WorkingTreeFormat5") |
164 | 3182 | format_registry.register_lazy("Bazaar Working Tree Format 6 (bzr 1.14)\n", | 3191 | format_registry.register_lazy("Bazaar Working Tree Format 6 (bzr 1.14)\n", |
165 | 3183 | "bzrlib.workingtree_4", "WorkingTreeFormat6") | 3192 | "bzrlib.workingtree_4", "WorkingTreeFormat6") |
166 | 3193 | format_registry.register_lazy("Bazaar Working Tree Format 7 (bzr 2.6)\n", | ||
167 | 3194 | "bzrlib.workingtree_4", "WorkingTreeFormat7") | ||
168 | 3184 | format_registry.register_lazy("Bazaar-NG Working Tree format 3", | 3195 | format_registry.register_lazy("Bazaar-NG Working Tree format 3", |
169 | 3185 | "bzrlib.workingtree_3", "WorkingTreeFormat3") | 3196 | "bzrlib.workingtree_3", "WorkingTreeFormat3") |
170 | 3186 | format_registry.set_default_key("Bazaar Working Tree Format 6 (bzr 1.14)\n") | 3197 | format_registry.set_default_key("Bazaar Working Tree Format 6 (bzr 1.14)\n") |
171 | 3187 | 3198 | ||
172 | === modified file 'bzrlib/workingtree_4.py' | |||
173 | --- bzrlib/workingtree_4.py 2012-01-06 14:09:04 +0000 | |||
174 | +++ bzrlib/workingtree_4.py 2012-01-24 15:41:25 +0000 | |||
175 | @@ -201,7 +201,7 @@ | |||
176 | 201 | WorkingTree._comparison_data(self, entry, path) | 201 | WorkingTree._comparison_data(self, entry, path) |
177 | 202 | # it looks like a plain directory, but it's really a reference -- see | 202 | # it looks like a plain directory, but it's really a reference -- see |
178 | 203 | # also kind() | 203 | # also kind() |
180 | 204 | if (self._repo_supports_tree_reference and kind == 'directory' | 204 | if (self.supports_tree_reference() and kind == 'directory' |
181 | 205 | and entry is not None and entry.kind == 'tree-reference'): | 205 | and entry is not None and entry.kind == 'tree-reference'): |
182 | 206 | kind = 'tree-reference' | 206 | kind = 'tree-reference' |
183 | 207 | return kind, executable, stat_value | 207 | return kind, executable, stat_value |
184 | @@ -346,7 +346,7 @@ | |||
185 | 346 | # add this entry to the parent map. | 346 | # add this entry to the parent map. |
186 | 347 | parent_ies[(dirname + '/' + name).strip('/')] = inv_entry | 347 | parent_ies[(dirname + '/' + name).strip('/')] = inv_entry |
187 | 348 | elif kind == 'tree-reference': | 348 | elif kind == 'tree-reference': |
189 | 349 | if not self._repo_supports_tree_reference: | 349 | if not self.supports_tree_reference(): |
190 | 350 | raise errors.UnsupportedOperation( | 350 | raise errors.UnsupportedOperation( |
191 | 351 | self._generate_inventory, | 351 | self._generate_inventory, |
192 | 352 | self.branch.repository) | 352 | self.branch.repository) |
193 | @@ -526,7 +526,7 @@ | |||
194 | 526 | return iter(result) | 526 | return iter(result) |
195 | 527 | 527 | ||
196 | 528 | def iter_references(self): | 528 | def iter_references(self): |
198 | 529 | if not self._repo_supports_tree_reference: | 529 | if not self.supports_tree_reference(): |
199 | 530 | # When the repo doesn't support references, we will have nothing to | 530 | # When the repo doesn't support references, we will have nothing to |
200 | 531 | # return | 531 | # return |
201 | 532 | return | 532 | return |
202 | @@ -551,6 +551,7 @@ | |||
203 | 551 | entry = self._get_entry(file_id=file_id, path=path) | 551 | entry = self._get_entry(file_id=file_id, path=path) |
204 | 552 | state._observed_sha1(entry, sha1, statvalue) | 552 | state._observed_sha1(entry, sha1, statvalue) |
205 | 553 | 553 | ||
206 | 554 | @needs_read_lock | ||
207 | 554 | def kind(self, file_id): | 555 | def kind(self, file_id): |
208 | 555 | """Return the kind of a file. | 556 | """Return the kind of a file. |
209 | 556 | 557 | ||
210 | @@ -568,7 +569,7 @@ | |||
211 | 568 | def _kind(self, relpath): | 569 | def _kind(self, relpath): |
212 | 569 | abspath = self.abspath(relpath) | 570 | abspath = self.abspath(relpath) |
213 | 570 | kind = file_kind(abspath) | 571 | kind = file_kind(abspath) |
215 | 571 | if (self._repo_supports_tree_reference and kind == 'directory'): | 572 | if (self.supports_tree_reference() and kind == 'directory'): |
216 | 572 | entry = self._get_entry(path=relpath) | 573 | entry = self._get_entry(path=relpath) |
217 | 573 | if entry[1] is not None: | 574 | if entry[1] is not None: |
218 | 574 | if entry[1][0][0] == 't': | 575 | if entry[1][0][0] == 't': |
219 | @@ -1176,7 +1177,8 @@ | |||
220 | 1176 | 1177 | ||
221 | 1177 | @needs_read_lock | 1178 | @needs_read_lock |
222 | 1178 | def supports_tree_reference(self): | 1179 | def supports_tree_reference(self): |
224 | 1179 | return self._repo_supports_tree_reference | 1180 | return (self._format.supports_tree_reference() and |
225 | 1181 | self._repo_supports_tree_reference) | ||
226 | 1180 | 1182 | ||
227 | 1181 | def unlock(self): | 1183 | def unlock(self): |
228 | 1182 | """Unlock in format 4 trees needs to write the entire dirstate.""" | 1184 | """Unlock in format 4 trees needs to write the entire dirstate.""" |
229 | @@ -1445,6 +1447,19 @@ | |||
230 | 1445 | return views.PathBasedViews(self) | 1447 | return views.PathBasedViews(self) |
231 | 1446 | 1448 | ||
232 | 1447 | 1449 | ||
233 | 1450 | class WorkingTree7(ContentFilteringDirStateWorkingTree): | ||
234 | 1451 | """This is the Format 7 working tree. | ||
235 | 1452 | |||
236 | 1453 | This differs from WorkingTree6 by: | ||
237 | 1454 | - Supporting nested trees. | ||
238 | 1455 | |||
239 | 1456 | This is new in bzr 2.6. | ||
240 | 1457 | """ | ||
241 | 1458 | |||
242 | 1459 | def _make_views(self): | ||
243 | 1460 | return views.PathBasedViews(self) | ||
244 | 1461 | |||
245 | 1462 | |||
246 | 1448 | class DirStateWorkingTreeFormat(WorkingTreeFormatMetaDir): | 1463 | class DirStateWorkingTreeFormat(WorkingTreeFormatMetaDir): |
247 | 1449 | 1464 | ||
248 | 1450 | missing_parent_conflicts = True | 1465 | missing_parent_conflicts = True |
249 | @@ -1597,8 +1612,7 @@ | |||
250 | 1597 | def _get_matchingbzrdir(self): | 1612 | def _get_matchingbzrdir(self): |
251 | 1598 | """Overrideable method to get a bzrdir for testing.""" | 1613 | """Overrideable method to get a bzrdir for testing.""" |
252 | 1599 | # please test against something that will let us do tree references | 1614 | # please test against something that will let us do tree references |
255 | 1600 | return bzrdir.format_registry.make_bzrdir( | 1615 | return bzrdir.format_registry.make_bzrdir('2a') |
254 | 1601 | 'dirstate-with-subtree') | ||
256 | 1602 | 1616 | ||
257 | 1603 | _matchingbzrdir = property(__get_matchingbzrdir) | 1617 | _matchingbzrdir = property(__get_matchingbzrdir) |
258 | 1604 | 1618 | ||
259 | @@ -1678,6 +1692,39 @@ | |||
260 | 1678 | return True | 1692 | return True |
261 | 1679 | 1693 | ||
262 | 1680 | 1694 | ||
263 | 1695 | class WorkingTreeFormat7(DirStateWorkingTreeFormat): | ||
264 | 1696 | """WorkingTree format supporting nested trees. | ||
265 | 1697 | |||
266 | 1698 | This format is still experimental and can change. | ||
267 | 1699 | """ | ||
268 | 1700 | |||
269 | 1701 | upgrade_recommended = False | ||
270 | 1702 | |||
271 | 1703 | _tree_class = WorkingTree7 | ||
272 | 1704 | |||
273 | 1705 | @classmethod | ||
274 | 1706 | def get_format_string(cls): | ||
275 | 1707 | """See WorkingTreeFormat.get_format_string().""" | ||
276 | 1708 | return "Bazaar Working Tree Format 7 (bzr 2.6)\n" | ||
277 | 1709 | |||
278 | 1710 | def get_format_description(self): | ||
279 | 1711 | """See WorkingTreeFormat.get_format_description().""" | ||
280 | 1712 | return "Working tree format 6" | ||
281 | 1713 | |||
282 | 1714 | def _init_custom_control_files(self, wt): | ||
283 | 1715 | """Subclasses with custom control files should override this method.""" | ||
284 | 1716 | wt._transport.put_bytes('views', '', mode=wt.bzrdir._get_file_mode()) | ||
285 | 1717 | |||
286 | 1718 | def supports_content_filtering(self): | ||
287 | 1719 | return True | ||
288 | 1720 | |||
289 | 1721 | def supports_views(self): | ||
290 | 1722 | return True | ||
291 | 1723 | |||
292 | 1724 | def supports_tree_reference(self): | ||
293 | 1725 | return True | ||
294 | 1726 | |||
295 | 1727 | |||
296 | 1681 | class DirStateRevisionTree(InventoryTree): | 1728 | class DirStateRevisionTree(InventoryTree): |
297 | 1682 | """A revision tree pulling the inventory from a dirstate. | 1729 | """A revision tree pulling the inventory from a dirstate. |
298 | 1683 | 1730 | ||
299 | @@ -1738,7 +1785,7 @@ | |||
300 | 1738 | return path_utf8.decode('utf8') | 1785 | return path_utf8.decode('utf8') |
301 | 1739 | 1786 | ||
302 | 1740 | def iter_references(self): | 1787 | def iter_references(self): |
304 | 1741 | if not self._repo_supports_tree_reference: | 1788 | if not self.supports_tree_reference(): |
305 | 1742 | # When the repo doesn't support references, we will have nothing to | 1789 | # When the repo doesn't support references, we will have nothing to |
306 | 1743 | # return | 1790 | # return |
307 | 1744 | return iter([]) | 1791 | return iter([]) |
308 | 1745 | 1792 | ||
309 | === modified file 'doc/en/release-notes/bzr-2.6.txt' | |||
310 | --- doc/en/release-notes/bzr-2.6.txt 2012-01-23 15:07:14 +0000 | |||
311 | +++ doc/en/release-notes/bzr-2.6.txt 2012-01-24 15:41:25 +0000 | |||
312 | @@ -31,6 +31,11 @@ | |||
313 | 31 | the URL scheme when accessing a HTTPS location. | 31 | the URL scheme when accessing a HTTPS location. |
314 | 32 | (Jelmer Vernooij, #125055) | 32 | (Jelmer Vernooij, #125055) |
315 | 33 | 33 | ||
316 | 34 | * The ``2a`` repository format now supports tree references, making | ||
317 | 35 | it possible to import Git repositories with submodules in their | ||
318 | 36 | history. It is not yet possible to check out revisions | ||
319 | 37 | that contain nested trees. (Jelmer Vernooij, #402814) | ||
320 | 38 | |||
321 | 34 | Bug Fixes | 39 | Bug Fixes |
322 | 35 | ********* | 40 | ********* |
323 | 36 | 41 |
280 + return "Working tree format 6"
Really ?
Apart from that I don't quite see how your cover letter relates to this
proposal:
> The actual change (a new inventory entry kind 'tree-reference') is small
and non-controversial.
There is nothing about that in your proposal, are you just referring to the
initial introduction of the tree-reference entry kind bak in the days ?
Ot is the actual change really:
51 + supports_ tree_reference = True
and
59 - supports_ tree_reference = True
If that's the case, I'm surprised there is no fallouts from activating tree
references support in 2a (line 51) !?! Not a single test failing ?
Also, what's the plan for RepositoryForma t2aSubtree ? Keeping it this way is
likely to get it forgotten, we at least need a bug to track its removal
describing some plan (since it's an experimental one, I think the plan can
be reduced to: delete it, users knew).
> Instead, this marks working tree formats 4-6 as not actually supporting
nested trees.
So *this* part is uncontroversial and can be landed IMHO.
69 + if repo._format. supports_ chks: format_ name = '2a' format_ name = 'pack-0.92-subtree'
70 + matching_
71 + else:
72 + matching_
81 + if repo.supports_ rich_root( ): supports_ chks: format_ name = '2a' format_ name = 'pack-0.92-subtree' format_ name = 'dirstate-subtree'
82 + if repo._format.
83 + matching_
84 + else:
85 + matching_
86 + else:
87 + matching_
Urgh, these tests were already confusing, now they are almost undecipherable
:-(
Can't you abstract the logic here so we get a clearer view of what we are
tested ? Since these tests are about stacking, I'd really prefer them to be
easier to grasp, especially regarding the compatibility between the various
formats. There is also a hint that it's time to address:
# TODO: Possibly this should be run per-repository- format and raise
# TestNotApplicable on formats that don't support stacking. -- mbp
# 20080729
100 + if not tree.supports_ tree_reference( ):
Why is that a method for working trees (or is it wt formats) and an
attribute for repos ?
Highly confusing :-/
> This change is sufficient for bzr-git to allow importing (launchpad bug
#402814), although not yet sufficient to allow checking out trees with
submodules. We'd need working tree support for the latter, of course.
Meh, can you elaborate on that ? How does bzr-git uses this change ? By
using wt format 7 ? And how do you allow checking out such trees ? Or is
this proposal incomplete in this respect ?
We add a discussion in Budapest where you mentioned a list of limitations
(which I agreed with) to allow experimenting with subtrees. It would be nice
to formalize them so we a get some understanding on where this proposal
stands on the roadmap ;)
Failing that, I don't know how to review this proposal or if it needs to
be split into several parts that would be easier to understand.