Merge lp:~jml/launchpad/ubuntu-package-bug-345737 into lp:launchpad
- ubuntu-package-bug-345737
- Merge into devel
Status: | Merged | ||||
---|---|---|---|---|---|
Merged at revision: | not available | ||||
Proposed branch: | lp:~jml/launchpad/ubuntu-package-bug-345737 | ||||
Merge into: | lp:launchpad | ||||
Diff against target: | None lines | ||||
To merge this branch: | bzr merge lp:~jml/launchpad/ubuntu-package-bug-345737 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Tim Penhey (community) | Approve | ||
Review via email:
|
Commit message
Description of the change
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Jonathan Lange (jml) wrote : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Tim Penhey (thumper) wrote : | # |
> bazaar_identity has been changed to return 'lp:ubuntu/package' for source
> package branches associated to the development focus package.
My first thought on reading this was "for just the release pocket?", but I
guess we'll see.
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -1131,6 +1130,10 @@
> 'series': use_series.name}
>
> if branch.
> + distro_package = branch.
> + linked_branch = ICanHasLinkedBr
> + if linked_
> + return lp_prefix + linked_
What object traversal is really going on here? And how will it impact getting
bazaar_identities for branch listings?
branch.
.distribution_
Adaption -> other queries?
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -30,6 +33,16 @@
> """See `ICanHasLinkedB
> return self.product_
>
> + @property
> + def bzr_identity(self):
> + """See `ICanHasLinkedB
> + return '/'.join(
> + [self.product_
I think that the bzr_identity here (and in following adapters) should really
have the lp: prefix too (from the config variable). Otherwise bzr_identity
functions in different places return different types of strings.
Also, it shouldn't really return anything if there is not a branch set.
> @@ -45,6 +58,15 @@
> """See `ICanHasLinkedB
> return ICanHasLinkedBr
>
> + @property
> + def bzr_identity(self):
> + """See `ICanHasLinkedB
> + return self.product.name
> +
> + def setBranch(self, branch, registrant=None):
> + """See `ICanHasLinkedB
> +
ICanHasLinkedBr
I think you should also pass the registrant through. If we start to record
it, it'll save changing this later.
> +
>
> class PackageLinkedBr
> """Implement a linked branch for a source package pocket."""
> @@ -61,3 +83,50 @@
> package = self.suite_
> pocket = self.suite_
> return package.
> +
> + @property
> + def bzr_identity(self):
> + """See `ICanHasLinkedB
> + return self.suite_
> +
> + def setBranch(self, branch, registrant):
> + """See `ICanHasLinkedB
> + package = self.suite_
> + pocket = self.suite_
> + package.
> +
> +
> +class DistributionPac
> + """Implement a linked branch for an `IDistributionS
> +
...
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Jonathan Lange (jml) wrote : | # |
On Fri, Jul 17, 2009 at 11:00 AM, Tim Penhey<email address hidden> wrote:
> Review: Needs Information
>> bazaar_identity has been changed to return 'lp:ubuntu/package' for source
>> package branches associated to the development focus package.
>
> My first thought on reading this was "for just the release pocket?", but I
> guess we'll see.
>
Yeah, just for the release pocket.
>> === modified file 'lib/lp/
>> --- lib/lp/
>> +++ lib/lp/
>> @@ -1131,6 +1130,10 @@
>> 'series': use_series.name}
>>
>> if branch.
>> + distro_package = branch.
>> + linked_branch = ICanHasLinkedBr
>> + if linked_
>> + return lp_prefix + linked_
>
> What object traversal is really going on here? And how will it impact getting
> bazaar_identities for branch listings?
>
> branch.
> .distribution_
No queries, it's a virtual object.
> Adaption -> other queries?
>
No queries in adaptation either.
However, getting the bzr_identity from the adapted object requires
loading the currentseries from the DistroSeries table. You probably
want to have a look at Distribution.
some of your other questions.
If possible, I'd rather land this branch now with some performance
issues than in 2 months time.
>> === modified file 'lib/lp/
>> --- lib/lp/
>> +++ lib/lp/
>> @@ -30,6 +33,16 @@
>> """See `ICanHasLinkedB
>> return self.product_
>>
>> + @property
>> + def bzr_identity(self):
>> + """See `ICanHasLinkedB
>> + return '/'.join(
>> + [self.product_
>
> I think that the bzr_identity here (and in following adapters) should really
> have the lp: prefix too (from the config variable). Otherwise bzr_identity
> functions in different places return different types of strings.
>
Fair enough, although in some ways I'd prefer keeping the current
behaviour and picking a different name. It seems wrong to imbue this
code with knowledge of the name of the server.
Oh, I see later on you suggest the names 'bzr_path' and
'bzr_identity_path' for a property with the current behaviour. I like
that.
Changed.
> Also, it shouldn't really return anything if there is not a branch set.
>
Why do you say that?
>> @@ -45,6 +58,15 @@
>> """See `ICanHasLinkedB
>> return ICanHasLinkedBr
>>
>> + @property
>> + def bzr_identity(self):
>> + """See `ICanHasLinkedB
>> + return self.product.name
>> +
>> + def setBranch(self, branch, registrant=None):
>> + """See `ICanHasLinkedB
>> +
> ICanHasLinkedBr
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Tim Penhey (thumper) wrote : | # |
On Fri, 17 Jul 2009 20:03:07 Jonathan Lange wrote:
> If possible, I'd rather land this branch now with some performance
> issues than in 2 months time.
I agree.
> > Also, it shouldn't really return anything if there is not a branch set.
>
> Why do you say that?
OK, I guess not. As long as the documentation says that the bzr_path is the
path of the branch if there was a branch set.
> > ICanHasLinkedBr
>
> Your wrapping here is very weird.
I blame my mail client.
> > This one has always bothered me. We should not allow junk branches to be
> > linked rather than just saying that their links don't show it. What do
> > you think?
>
> As a general principle, I think we should do as little special-casing
> of +junk as possible. It always comes back to bite us.
>
> I personally think that we can do without this special case at all.
> However, if pressed, I'd favour simply preventing junk branches from
> being linked. I think it's for another branch though.
Definitely another branch.
> > To save with the login, logout dance, how about we just remove the
> > security proxy as with the other tests, but still pass the registrant
> > through.
>
> Doesn't work. Internally, the link is made by code that looks like:
> getUtility(
> distroseries, sourcepackagename)
>
> getUtility guarantees that the object is secured, and 'new' requires
> lp.Edit permissions.
Aah, ok. I wish we had a nicer way to test this without the logins, but not
for this branch.
> > The main questions I have resolve around the currentseries for a
> > distribution, and the actual content of ICanHasLinkedBr
>
> Cool. I hope they're addressed here.
Yes. I wish that distribution always had one series by default like a
Product, but I think you've done as well as you can given that "some"
distributions may well not have set any series.
review approve
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Jonathan Lange (jml) wrote : | # |
On Mon, Jul 20, 2009 at 8:48 AM, Tim Penhey<email address hidden> wrote:
> Review: Approve
> On Fri, 17 Jul 2009 20:03:07 Jonathan Lange wrote:
>> If possible, I'd rather land this branch now with some performance
>> issues than in 2 months time.
>
> I agree.
>
>> > Also, it shouldn't really return anything if there is not a branch set.
>>
>> Why do you say that?
>
> OK, I guess not. As long as the documentation says that the bzr_path is the
> path of the branch if there was a branch set.
>
Done.
Landing now.
jml
Preview Diff
1 | === modified file 'lib/lp/code/configure.zcml' |
2 | --- lib/lp/code/configure.zcml 2009-07-15 01:45:35 +0000 |
3 | +++ lib/lp/code/configure.zcml 2009-07-16 04:47:24 +0000 |
4 | @@ -801,6 +801,7 @@ |
5 | <adapter factory="lp.code.model.linkedbranch.ProductSeriesLinkedBranch" /> |
6 | <adapter factory="lp.code.model.linkedbranch.ProductLinkedBranch" /> |
7 | <adapter factory="lp.code.model.linkedbranch.PackageLinkedBranch" /> |
8 | + <adapter factory="lp.code.model.linkedbranch.DistributionPackageLinkedBranch" /> |
9 | |
10 | <lp:help-folder |
11 | folder="help" type="canonical.launchpad.layers.CodeLayer" /> |
12 | |
13 | === modified file 'lib/lp/code/interfaces/branch.py' |
14 | --- lib/lp/code/interfaces/branch.py 2009-07-15 01:45:35 +0000 |
15 | +++ lib/lp/code/interfaces/branch.py 2009-07-16 04:47:24 +0000 |
16 | @@ -64,6 +64,7 @@ |
17 | ) |
18 | from lp.code.interfaces.branchlookup import IBranchLookup |
19 | from lp.code.interfaces.branchtarget import IHasBranchTarget |
20 | +from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch |
21 | from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities |
22 | from lp.registry.interfaces.role import IHasOwner |
23 | from lp.registry.interfaces.person import IPerson |
24 | @@ -81,6 +82,7 @@ |
25 | class BranchCreationException(Exception): |
26 | """Base class for branch creation exceptions.""" |
27 | |
28 | + |
29 | class BranchExists(BranchCreationException): |
30 | """Raised when creating a branch that already exists.""" |
31 | |
32 | @@ -738,8 +740,7 @@ |
33 | 'development focus, then the result should be lp:product. ' |
34 | 'If the branch is related to a series, then ' |
35 | 'lp:product/series. Otherwise the result is ' |
36 | - 'lp:~user/product/branch-name.' |
37 | - ))) |
38 | + 'lp:~user/product/branch-name.'))) |
39 | |
40 | def addToLaunchBag(launchbag): |
41 | """Add information about this branch to `launchbag'. |
42 | @@ -1083,8 +1084,6 @@ |
43 | last_scanned_id = Attribute("The revision id of the tip revision.") |
44 | |
45 | |
46 | - |
47 | - |
48 | class IBranchCloud(Interface): |
49 | """A utility to generate data for branch clouds. |
50 | |
51 | @@ -1131,6 +1130,10 @@ |
52 | 'series': use_series.name} |
53 | |
54 | if branch.sourcepackage is not None: |
55 | + distro_package = branch.sourcepackage.distribution_sourcepackage |
56 | + linked_branch = ICanHasLinkedBranch(distro_package) |
57 | + if linked_branch.branch == branch: |
58 | + return lp_prefix + linked_branch.bzr_identity |
59 | suite_sourcepackages = branch.associatedSuiteSourcePackages() |
60 | # Take the first link if there is one. |
61 | if len(suite_sourcepackages) > 0: |
62 | |
63 | === modified file 'lib/lp/code/interfaces/branchlookup.py' |
64 | --- lib/lp/code/interfaces/branchlookup.py 2009-05-15 03:16:18 +0000 |
65 | +++ lib/lp/code/interfaces/branchlookup.py 2009-07-16 05:00:18 +0000 |
66 | @@ -16,8 +16,14 @@ |
67 | class ILinkedBranchTraversable(Interface): |
68 | """A thing that can be traversed to find a thing linked to a branch.""" |
69 | |
70 | - def traverse(self, name): |
71 | - """Return the object beneath this one that matches 'name'.""" |
72 | + def traverse(self, name, segments): |
73 | + """Return the object beneath this one that matches 'name'. |
74 | + |
75 | + :param name: The name of the object being traversed to. |
76 | + :param segments: Remaining path segments. |
77 | + :return: An `ILinkedBranchTraversable` object if traversing should |
78 | + continue, an `ICanHasLinkedBranch` object otherwise. |
79 | + """ |
80 | |
81 | |
82 | class ILinkedBranchTraverser(Interface): |
83 | @@ -40,7 +46,7 @@ |
84 | :return: One of |
85 | * `IProduct` |
86 | * `IProductSeries` |
87 | - * (ISourcePackage, PackagePublishingPocket) |
88 | + * `ISuiteSourcePackage` |
89 | * `IDistributionSourcePackage` |
90 | """ |
91 | |
92 | |
93 | === modified file 'lib/lp/code/interfaces/linkedbranch.py' |
94 | --- lib/lp/code/interfaces/linkedbranch.py 2009-05-24 18:09:51 +0000 |
95 | +++ lib/lp/code/interfaces/linkedbranch.py 2009-07-16 01:58:59 +0000 |
96 | @@ -23,6 +23,16 @@ |
97 | """Something that has a linked branch.""" |
98 | |
99 | branch = Attribute("The linked branch.") |
100 | + bzr_identity = Attribute('The Bazaar branch path for the linked branch.') |
101 | + |
102 | + def setBranch(branch, registrant=None): |
103 | + """Set the linked branch. |
104 | + |
105 | + :param branch: An `IBranch`. After calling this, |
106 | + `ICanHasLinkedBranch.branch` will be 'branch'. |
107 | + :param registrant: The `IPerson` linking the branch. Not used by all |
108 | + implementations. |
109 | + """ |
110 | |
111 | |
112 | class CannotHaveLinkedBranch(Exception): |
113 | |
114 | === modified file 'lib/lp/code/model/branchlookup.py' |
115 | --- lib/lp/code/model/branchlookup.py 2009-05-15 03:16:18 +0000 |
116 | +++ lib/lp/code/model/branchlookup.py 2009-07-16 05:39:51 +0000 |
117 | @@ -29,7 +29,7 @@ |
118 | from lp.code.interfaces.linkedbranch import get_linked_branch, NoLinkedBranch |
119 | from lp.registry.interfaces.distribution import IDistribution |
120 | from lp.registry.interfaces.distroseries import ( |
121 | - IDistroSeries, IDistroSeriesSet) |
122 | + IDistroSeries, IDistroSeriesSet, NoSuchDistroSeries) |
123 | from lp.registry.interfaces.pillar import IPillarNameSet |
124 | from lp.registry.interfaces.product import ( |
125 | InvalidProductName, IProduct, NoSuchProduct) |
126 | @@ -65,7 +65,7 @@ |
127 | |
128 | implements(ILinkedBranchTraversable) |
129 | |
130 | - def traverse(self, name): |
131 | + def traverse(self, name, segments): |
132 | """See `ITraversable`. |
133 | |
134 | :raise NoSuchProduct: If 'name' doesn't match an existing pillar. |
135 | @@ -102,7 +102,7 @@ |
136 | adapts(IProduct) |
137 | implements(ILinkedBranchTraversable) |
138 | |
139 | - def traverse(self, name): |
140 | + def traverse(self, name, segments): |
141 | """See `ITraversable`. |
142 | |
143 | :raises NoSuchProductSeries: if 'name' doesn't match an existing |
144 | @@ -124,12 +124,18 @@ |
145 | adapts(IDistribution) |
146 | implements(ILinkedBranchTraversable) |
147 | |
148 | - def traverse(self, name): |
149 | + def traverse(self, name, segments): |
150 | """See `ITraversable`.""" |
151 | - # XXX: JonathanLange 2009-03-20 spec=package-branches bug=345737: This |
152 | - # could also try to find a package and then return a reference to its |
153 | - # development focus. |
154 | - return getUtility(IDistroSeriesSet).fromSuite(self.context, name) |
155 | + try: |
156 | + return getUtility(IDistroSeriesSet).fromSuite(self.context, name) |
157 | + except NoSuchDistroSeries: |
158 | + sourcepackage = self.context.getSourcePackage(name) |
159 | + if sourcepackage is None: |
160 | + if segments: |
161 | + raise |
162 | + else: |
163 | + raise NoSuchSourcePackageName(name) |
164 | + return sourcepackage |
165 | |
166 | |
167 | class DistroSeriesTraversable: |
168 | @@ -145,7 +151,7 @@ |
169 | self.distroseries = distroseries |
170 | self.pocket = pocket |
171 | |
172 | - def traverse(self, name): |
173 | + def traverse(self, name, segments): |
174 | """See `ITraversable`.""" |
175 | sourcepackage = self.distroseries.getSourcePackage(name) |
176 | if sourcepackage is None: |
177 | @@ -170,7 +176,7 @@ |
178 | traversable = RootTraversable() |
179 | while segments: |
180 | name = segments.pop(0) |
181 | - context = traversable.traverse(name) |
182 | + context = traversable.traverse(name, segments) |
183 | traversable = adapt(context, ILinkedBranchTraversable) |
184 | if traversable is None: |
185 | break |
186 | @@ -332,7 +338,7 @@ |
187 | namespace_set = getUtility(IBranchNamespaceSet) |
188 | segments = iter(path.lstrip('~').split('/')) |
189 | branch = namespace_set.traverse(segments) |
190 | - suffix = '/'.join(segments) |
191 | + suffix = '/'.join(segments) |
192 | if not check_permission('launchpad.View', branch): |
193 | raise NoSuchBranch(path) |
194 | if suffix == '': |
195 | |
196 | === modified file 'lib/lp/code/model/linkedbranch.py' |
197 | --- lib/lp/code/model/linkedbranch.py 2009-05-15 03:16:18 +0000 |
198 | +++ lib/lp/code/model/linkedbranch.py 2009-07-16 05:38:41 +0000 |
199 | @@ -11,9 +11,12 @@ |
200 | from zope.interface import implements |
201 | |
202 | from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch |
203 | +from lp.registry.interfaces.distributionsourcepackage import ( |
204 | + IDistributionSourcePackage) |
205 | from lp.registry.interfaces.product import IProduct |
206 | from lp.registry.interfaces.productseries import IProductSeries |
207 | from lp.registry.interfaces.suitesourcepackage import ISuiteSourcePackage |
208 | +from lp.soyuz.interfaces.publishing import PackagePublishingPocket |
209 | |
210 | |
211 | class ProductSeriesLinkedBranch: |
212 | @@ -30,6 +33,16 @@ |
213 | """See `ICanHasLinkedBranch`.""" |
214 | return self.product_series.branch |
215 | |
216 | + @property |
217 | + def bzr_identity(self): |
218 | + """See `ICanHasLinkedBranch`.""" |
219 | + return '/'.join( |
220 | + [self.product_series.product.name, self.product_series.name]) |
221 | + |
222 | + def setBranch(self, branch, registrant=None): |
223 | + """See `ICanHasLinkedBranch`.""" |
224 | + self.product_series.branch = branch |
225 | + |
226 | |
227 | class ProductLinkedBranch: |
228 | """Implement a linked branch for a product.""" |
229 | @@ -45,6 +58,15 @@ |
230 | """See `ICanHasLinkedBranch`.""" |
231 | return ICanHasLinkedBranch(self.product.development_focus).branch |
232 | |
233 | + @property |
234 | + def bzr_identity(self): |
235 | + """See `ICanHasLinkedBranch`.""" |
236 | + return self.product.name |
237 | + |
238 | + def setBranch(self, branch, registrant=None): |
239 | + """See `ICanHasLinkedBranch`.""" |
240 | + ICanHasLinkedBranch(self.product.development_focus).setBranch(branch) |
241 | + |
242 | |
243 | class PackageLinkedBranch: |
244 | """Implement a linked branch for a source package pocket.""" |
245 | @@ -61,3 +83,50 @@ |
246 | package = self.suite_sourcepackage.sourcepackage |
247 | pocket = self.suite_sourcepackage.pocket |
248 | return package.getBranch(pocket) |
249 | + |
250 | + @property |
251 | + def bzr_identity(self): |
252 | + """See `ICanHasLinkedBranch`.""" |
253 | + return self.suite_sourcepackage.path |
254 | + |
255 | + def setBranch(self, branch, registrant): |
256 | + """See `ICanHasLinkedBranch`.""" |
257 | + package = self.suite_sourcepackage.sourcepackage |
258 | + pocket = self.suite_sourcepackage.pocket |
259 | + package.setBranch(pocket, branch, registrant) |
260 | + |
261 | + |
262 | +class DistributionPackageLinkedBranch: |
263 | + """Implement a linked branch for an `IDistributionSourcePackage`.""" |
264 | + |
265 | + adapts(IDistributionSourcePackage) |
266 | + implements(ICanHasLinkedBranch) |
267 | + |
268 | + def __init__(self, distribution_sourcepackage): |
269 | + self._distribution_sourcepackage = distribution_sourcepackage |
270 | + |
271 | + @property |
272 | + def branch(self): |
273 | + """See `ICanHasLinkedBranch`.""" |
274 | + development_package = ( |
275 | + self._distribution_sourcepackage.development_version) |
276 | + if development_package is None: |
277 | + return None |
278 | + suite_sourcepackage = development_package.getSuiteSourcePackage( |
279 | + PackagePublishingPocket.RELEASE) |
280 | + return ICanHasLinkedBranch(suite_sourcepackage).branch |
281 | + |
282 | + @property |
283 | + def bzr_identity(self): |
284 | + """See `ICanHasLinkedBranch`.""" |
285 | + return '/'.join( |
286 | + [self._distribution_sourcepackage.distribution.name, |
287 | + self._distribution_sourcepackage.sourcepackagename.name]) |
288 | + |
289 | + def setBranch(self, branch, registrant): |
290 | + """See `ICanHasLinkedBranch`.""" |
291 | + development_package = ( |
292 | + self._distribution_sourcepackage.development_version) |
293 | + suite_sourcepackage = development_package.getSuiteSourcePackage( |
294 | + PackagePublishingPocket.RELEASE) |
295 | + ICanHasLinkedBranch(suite_sourcepackage).setBranch(branch, registrant) |
296 | |
297 | === modified file 'lib/lp/code/model/tests/test_branch.py' |
298 | --- lib/lp/code/model/tests/test_branch.py 2009-07-08 04:57:51 +0000 |
299 | +++ lib/lp/code/model/tests/test_branch.py 2009-07-16 03:22:24 +0000 |
300 | @@ -20,49 +20,49 @@ |
301 | from canonical.config import config |
302 | from canonical.database.constants import UTC_NOW |
303 | from canonical.launchpad import _ |
304 | -from lp.code.model.branch import ( |
305 | - ClearDependentBranch, ClearOfficialPackageBranch, ClearSeriesBranch, |
306 | - DeleteCodeImport, DeletionCallable, DeletionOperation, |
307 | - update_trigger_modified_fields) |
308 | -from lp.code.model.branchjob import ( |
309 | - BranchDiffJob, BranchJob, BranchJobType, ReclaimBranchSpaceJob) |
310 | -from lp.code.model.branchmergeproposal import ( |
311 | - BranchMergeProposal) |
312 | -from lp.bugs.model.bugbranch import BugBranch |
313 | -from lp.code.model.codeimport import CodeImport, CodeImportSet |
314 | -from lp.code.model.codereviewcomment import CodeReviewComment |
315 | -from lp.registry.model.product import ProductSet |
316 | -from lp.blueprints.model.specificationbranch import ( |
317 | - SpecificationBranch) |
318 | -from lp.registry.model.sourcepackage import SourcePackage |
319 | from canonical.launchpad.ftests import ( |
320 | ANONYMOUS, login, login_person, logout, syncUpdate) |
321 | -from lp.bugs.interfaces.bug import CreateBugParams, IBugSet |
322 | +from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities |
323 | +from canonical.launchpad.webapp.interfaces import IOpenLaunchBag |
324 | +from canonical.testing import DatabaseFunctionalLayer, LaunchpadZopelessLayer |
325 | + |
326 | from lp.blueprints.interfaces.specification import ( |
327 | ISpecificationSet, SpecificationDefinitionStatus) |
328 | +from lp.blueprints.model.specificationbranch import ( |
329 | + SpecificationBranch) |
330 | +from lp.bugs.interfaces.bug import CreateBugParams, IBugSet |
331 | +from lp.bugs.model.bugbranch import BugBranch |
332 | from lp.code.bzr import BranchFormat, RepositoryFormat |
333 | from lp.code.enums import ( |
334 | BranchLifecycleStatus, BranchSubscriptionNotificationLevel, BranchType, |
335 | BranchVisibilityRule, CodeReviewNotificationLevel) |
336 | from lp.code.interfaces.branch import ( |
337 | BranchCannotBePrivate, BranchCannotBePublic, |
338 | - CannotDeleteBranch) |
339 | + CannotDeleteBranch, DEFAULT_BRANCH_STATUS_IN_LISTING) |
340 | +from lp.code.interfaces.branchlookup import IBranchLookup |
341 | +from lp.code.interfaces.branchnamespace import IBranchNamespaceSet |
342 | from lp.code.interfaces.branchmergeproposal import InvalidBranchMergeProposal |
343 | +from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch |
344 | from lp.code.interfaces.seriessourcepackagebranch import ( |
345 | IFindOfficialBranchLinks) |
346 | +from lp.code.model.branch import ( |
347 | + ClearDependentBranch, ClearOfficialPackageBranch, ClearSeriesBranch, |
348 | + DeleteCodeImport, DeletionCallable, DeletionOperation, |
349 | + update_trigger_modified_fields) |
350 | +from lp.code.model.branchjob import ( |
351 | + BranchDiffJob, BranchJob, BranchJobType, ReclaimBranchSpaceJob) |
352 | +from lp.code.model.branchmergeproposal import ( |
353 | + BranchMergeProposal) |
354 | +from lp.code.model.codeimport import CodeImport, CodeImportSet |
355 | +from lp.code.model.codereviewcomment import CodeReviewComment |
356 | from lp.registry.interfaces.person import IPersonSet |
357 | from lp.registry.interfaces.product import IProductSet |
358 | -from lp.code.interfaces.branch import DEFAULT_BRANCH_STATUS_IN_LISTING |
359 | -from lp.code.interfaces.branchlookup import IBranchLookup |
360 | -from lp.code.interfaces.branchnamespace import IBranchNamespaceSet |
361 | -from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities |
362 | +from lp.registry.model.product import ProductSet |
363 | +from lp.registry.model.sourcepackage import SourcePackage |
364 | from lp.soyuz.interfaces.publishing import PackagePublishingPocket |
365 | from lp.testing import ( |
366 | run_with_login, TestCase, TestCaseWithFactory, time_counter) |
367 | from lp.testing.factory import LaunchpadObjectFactory |
368 | -from canonical.launchpad.webapp.interfaces import IOpenLaunchBag |
369 | - |
370 | -from canonical.testing import DatabaseFunctionalLayer, LaunchpadZopelessLayer |
371 | |
372 | |
373 | class TestCodeImport(TestCase): |
374 | @@ -94,7 +94,7 @@ |
375 | def _makeRevision(self, revno): |
376 | # Make a revision and add it to the branch. |
377 | rev = self.factory.makeRevision() |
378 | - br = self.branch.createBranchRevision(revno, rev) |
379 | + self.branch.createBranchRevision(revno, rev) |
380 | return rev |
381 | |
382 | def testGetBySequenceNumber(self): |
383 | @@ -117,7 +117,7 @@ |
384 | self.assertEqual(1, branch_revision.sequence) |
385 | |
386 | def testNonExistant(self): |
387 | - rev1 = self._makeRevision(1) |
388 | + self._makeRevision(1) |
389 | self.assertTrue(self.branch.getBranchRevision(sequence=2) is None) |
390 | rev2 = self.factory.makeRevision() |
391 | self.assertTrue(self.branch.getBranchRevision(revision=rev2) is None) |
392 | @@ -418,9 +418,10 @@ |
393 | # If a branch is the development focus branch for a product, then it's |
394 | # bzr identity is lp:product. |
395 | branch = self.factory.makeProductBranch() |
396 | - product = branch.product |
397 | - removeSecurityProxy(product).development_focus.branch = branch |
398 | - self.assertBzrIdentity(branch, product.name) |
399 | + product = removeSecurityProxy(branch.product) |
400 | + linked_branch = ICanHasLinkedBranch(product) |
401 | + linked_branch.setBranch(branch) |
402 | + self.assertBzrIdentity(branch, linked_branch.bzr_identity) |
403 | |
404 | def test_linked_to_product_series(self): |
405 | # If a branch is the development focus branch for a product series, |
406 | @@ -428,8 +429,9 @@ |
407 | branch = self.factory.makeProductBranch() |
408 | product = branch.product |
409 | series = self.factory.makeProductSeries(product=product) |
410 | - series.branch = branch |
411 | - self.assertBzrIdentity(branch, '%s/%s' % (product.name, series.name)) |
412 | + linked_branch = ICanHasLinkedBranch(series) |
413 | + linked_branch.setBranch(branch) |
414 | + self.assertBzrIdentity(branch, linked_branch.bzr_identity) |
415 | |
416 | def test_private_linked_to_product(self): |
417 | # If a branch is private, then the bzr identity is the unique name, |
418 | @@ -439,8 +441,8 @@ |
419 | owner = removeSecurityProxy(branch).owner |
420 | login_person(owner) |
421 | self.addCleanup(logout) |
422 | - product = branch.product |
423 | - removeSecurityProxy(product).development_focus.branch = branch |
424 | + product = removeSecurityProxy(branch.product) |
425 | + ICanHasLinkedBranch(product).setBranch(branch) |
426 | self.assertBzrIdentity(branch, branch.unique_name) |
427 | |
428 | def test_linked_to_series_and_dev_focus(self): |
429 | @@ -448,33 +450,54 @@ |
430 | # branch for a series, the bzr identity will be the storter of the two |
431 | # URLs. |
432 | branch = self.factory.makeProductBranch() |
433 | - product = branch.product |
434 | - removeSecurityProxy(product).development_focus.branch = branch |
435 | - series = self.factory.makeProductSeries(product=product) |
436 | - series.branch = branch |
437 | - self.assertBzrIdentity(branch, product.name) |
438 | + series = self.factory.makeProductSeries(product=branch.product) |
439 | + product_link = ICanHasLinkedBranch( |
440 | + removeSecurityProxy(branch.product)) |
441 | + series_link = ICanHasLinkedBranch(series) |
442 | + product_link.setBranch(branch) |
443 | + series_link.setBranch(branch) |
444 | + self.assertBzrIdentity(branch, product_link.bzr_identity) |
445 | |
446 | def test_junk_branch_always_unique_name(self): |
447 | # For junk branches, the bzr identity is always based on the unique |
448 | # name of the branch, even if it's linked to a product, product series |
449 | # or whatever. |
450 | branch = self.factory.makePersonalBranch() |
451 | - product = self.factory.makeProduct() |
452 | - removeSecurityProxy(product).development_focus.branch = branch |
453 | + product = removeSecurityProxy(self.factory.makeProduct()) |
454 | + ICanHasLinkedBranch(product).setBranch(branch) |
455 | self.assertBzrIdentity(branch, branch.unique_name) |
456 | |
457 | - def test_linked_to_package_release(self): |
458 | - # If a branch is linked to the release pocket of a package, then the |
459 | + def test_linked_to_package(self): |
460 | + # If a branch is linked to a pocket of a package, then the |
461 | # bzr identity is the path to that package. |
462 | branch = self.factory.makePackageBranch() |
463 | + # Have to pick something that's not RELEASE in order to guarantee that |
464 | + # it's not the dev focus source package. |
465 | + pocket = PackagePublishingPocket.BACKPORTS |
466 | + linked_branch = ICanHasLinkedBranch( |
467 | + branch.sourcepackage.getSuiteSourcePackage(pocket)) |
468 | registrant = getUtility( |
469 | ILaunchpadCelebrities).ubuntu_branches.teamowner |
470 | login_person(registrant) |
471 | - branch.sourcepackage.setBranch( |
472 | - PackagePublishingPocket.RELEASE, branch, registrant) |
473 | + linked_branch.setBranch(branch, registrant) |
474 | logout() |
475 | login(ANONYMOUS) |
476 | - self.assertBzrIdentity(branch, branch.sourcepackage.path) |
477 | + self.assertBzrIdentity(branch, linked_branch.bzr_identity) |
478 | + |
479 | + def test_linked_to_dev_package(self): |
480 | + # If a branch is linked to the development focus version of a package |
481 | + # then the bzr identity is distro/package. |
482 | + sourcepackage = self.factory.makeSourcePackage() |
483 | + distro_package = sourcepackage.distribution_sourcepackage |
484 | + branch = self.factory.makePackageBranch( |
485 | + sourcepackage=distro_package.development_version) |
486 | + linked_branch = ICanHasLinkedBranch(distro_package) |
487 | + registrant = getUtility( |
488 | + ILaunchpadCelebrities).ubuntu_branches.teamowner |
489 | + run_with_login( |
490 | + registrant, |
491 | + linked_branch.setBranch, branch, registrant) |
492 | + self.assertBzrIdentity(branch, linked_branch.bzr_identity) |
493 | |
494 | |
495 | class TestBranchDeletion(TestCaseWithFactory): |
496 | @@ -506,7 +529,7 @@ |
497 | |
498 | def test_stackedBranchDisablesDeletion(self): |
499 | # A branch that is stacked upon cannot be deleted. |
500 | - branch = self.factory.makeAnyBranch(stacked_on=self.branch) |
501 | + self.factory.makeAnyBranch(stacked_on=self.branch) |
502 | self.assertFalse(self.branch.canBeDeleted()) |
503 | |
504 | def test_subscriptionDoesntDisableDeletion(self): |
505 | @@ -679,8 +702,7 @@ |
506 | ' proposal.')), |
507 | merge_proposal2: |
508 | ('delete', _('This branch is the source branch of this merge' |
509 | - ' proposal.')) |
510 | - }, |
511 | + ' proposal.'))}, |
512 | self.branch.deletionRequirements()) |
513 | self.assertEqual({ |
514 | merge_proposal1: |
515 | @@ -688,8 +710,7 @@ |
516 | ' proposal.')), |
517 | merge_proposal2: |
518 | ('delete', _('This branch is the target branch of this merge' |
519 | - ' proposal.')) |
520 | - }, |
521 | + ' proposal.'))}, |
522 | merge_proposal1.target_branch.deletionRequirements()) |
523 | self.assertEqual({ |
524 | merge_proposal1: |
525 | @@ -697,8 +718,7 @@ |
526 | ' proposal.')), |
527 | merge_proposal2: |
528 | ('alter', _('This branch is the dependent branch of this merge' |
529 | - ' proposal.')) |
530 | - }, |
531 | + ' proposal.'))}, |
532 | merge_proposal1.dependent_branch.deletionRequirements()) |
533 | |
534 | def test_deleteMergeProposalSource(self): |
535 | @@ -722,7 +742,6 @@ |
536 | def test_deleteMergeProposalDependent(self): |
537 | """break_links enables deleting merge proposal dependant branches.""" |
538 | merge_proposal1, merge_proposal2 = self.makeMergeProposals() |
539 | - merge_proposal1_id = merge_proposal1.id |
540 | merge_proposal1.dependent_branch.destroySelf(break_references=True) |
541 | self.assertEqual(None, merge_proposal1.dependent_branch) |
542 | |
543 | @@ -755,7 +774,6 @@ |
544 | def test_branchWithBugDeletion(self): |
545 | """break_links allows deleting a branch with a bug.""" |
546 | bug1 = self.factory.makeBug() |
547 | - bug2 = self.factory.makeBug() |
548 | bug1.linkBranch(self.branch, self.branch.owner) |
549 | bug_branch1 = bug1.linked_branches[0] |
550 | bug_branch1_id = bug_branch1.id |
551 | @@ -951,7 +969,7 @@ |
552 | # some_branch.getStackedBranchesWithIncompleteMirrors does not include |
553 | # stacked branches that haven't been mirrored at all. |
554 | branch = self.factory.makeAnyBranch() |
555 | - stacked_a = self.factory.makeAnyBranch(stacked_on=branch) |
556 | + self.factory.makeAnyBranch(stacked_on=branch) |
557 | self.assertEqual( |
558 | set(), set(branch.getStackedBranchesWithIncompleteMirrors())) |
559 | |
560 | @@ -1097,8 +1115,7 @@ |
561 | branch pair, then another landing target specifying the same pair |
562 | raises. |
563 | """ |
564 | - proposal = self.source.addLandingTarget( |
565 | - self.user, self.target, self.dependent) |
566 | + self.source.addLandingTarget(self.user, self.target, self.dependent) |
567 | |
568 | self.assertRaises( |
569 | InvalidBranchMergeProposal, self.source.addLandingTarget, |
570 | @@ -1113,8 +1130,7 @@ |
571 | self.user, self.target, self.dependent) |
572 | proposal.rejectBranch(self.user, 'some_revision') |
573 | syncUpdate(proposal) |
574 | - new_proposal = self.source.addLandingTarget( |
575 | - self.user, self.target, self.dependent) |
576 | + self.source.addLandingTarget(self.user, self.target, self.dependent) |
577 | |
578 | def test_attributeAssignment(self): |
579 | """Smoke test to make sure the assignments are there.""" |
580 | @@ -1614,7 +1630,6 @@ |
581 | |
582 | def test_spec_unlink(self): |
583 | # Branches can be unlinked from the spec as well. |
584 | - user = getUtility(IPersonSet).getByEmail('test@canonical.com') |
585 | branch = self.factory.makeAnyBranch() |
586 | spec = self.factory.makeSpecification() |
587 | branch.linkSpecification(spec, self.user) |
588 | |
589 | === modified file 'lib/lp/code/model/tests/test_branchlookup.py' |
590 | --- lib/lp/code/model/tests/test_branchlookup.py 2009-06-02 13:52:48 +0000 |
591 | +++ lib/lp/code/model/tests/test_branchlookup.py 2009-07-16 05:52:44 +0000 |
592 | @@ -18,7 +18,7 @@ |
593 | from lp.code.interfaces.branchnamespace import ( |
594 | get_branch_namespace, InvalidNamespace) |
595 | from lp.code.interfaces.linkedbranch import ( |
596 | - CannotHaveLinkedBranch, NoLinkedBranch) |
597 | + CannotHaveLinkedBranch, ICanHasLinkedBranch, NoLinkedBranch) |
598 | from lp.registry.interfaces.distroseries import NoSuchDistroSeries |
599 | from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities |
600 | from lp.registry.interfaces.person import NoSuchPerson |
601 | @@ -230,7 +230,7 @@ |
602 | |
603 | This is because Launchpad doesn't currently support ftp. |
604 | """ |
605 | - branch = self.makeProductBranch() |
606 | + self.makeProductBranch() |
607 | branch_set = getUtility(IBranchLookup) |
608 | branch2 = branch_set.getByUrl('ftp://bazaar.launchpad.dev/~aa/b/c') |
609 | self.assertIs(None, branch2) |
610 | @@ -311,7 +311,7 @@ |
611 | # a non-existent series. |
612 | self.assertRaises( |
613 | NoSuchProduct, self.traverser.traverse, 'bb/dd') |
614 | - product = self.factory.makeProduct(name='bb') |
615 | + self.factory.makeProduct(name='bb') |
616 | self.assertRaises( |
617 | NoSuchProductSeries, self.traverser.traverse, 'bb/dd') |
618 | |
619 | @@ -345,6 +345,13 @@ |
620 | ssp = package.getSuiteSourcePackage(PackagePublishingPocket.RELEASE) |
621 | self.assertTraverses(package.path, ssp) |
622 | |
623 | + def test_distribution_source_package(self): |
624 | + # `traverse` resolves 'distro/package' to the distribution source |
625 | + # package. |
626 | + dsp = self.factory.makeDistributionSourcePackage() |
627 | + path = '%s/%s' % (dsp.distribution.name, dsp.sourcepackagename.name) |
628 | + self.assertTraverses(path, dsp) |
629 | + |
630 | def test_traverse_source_package_pocket(self): |
631 | # `traverse` resolves 'distro/series-pocket/package' to the official |
632 | # branch for 'pocket' on that package. |
633 | @@ -366,7 +373,7 @@ |
634 | def test_no_such_distro_series(self): |
635 | # `traverse` raises `NoSuchDistroSeries` if the distro series doesn't |
636 | # exist. |
637 | - distro = self.factory.makeDistribution(name='distro') |
638 | + self.factory.makeDistribution(name='distro') |
639 | self.assertRaises( |
640 | NoSuchDistroSeries, self.traverser.traverse, |
641 | 'distro/series/package') |
642 | @@ -380,6 +387,14 @@ |
643 | self.assertRaises( |
644 | NoSuchSourcePackageName, self.traverser.traverse, path) |
645 | |
646 | + def test_no_such_distribution_sourcepackage(self): |
647 | + # `traverse` raises `NoSuchSourcePackageName` if the package in |
648 | + # distro/package doesn't exist. |
649 | + distribution = self.factory.makeDistribution() |
650 | + path = '%s/doesntexist' % distribution.name |
651 | + self.assertRaises( |
652 | + NoSuchSourcePackageName, self.traverser.traverse, path) |
653 | + |
654 | |
655 | class TestGetByLPPath(TestCaseWithFactory): |
656 | """Ensure URLs are correctly expanded.""" |
657 | @@ -397,10 +412,10 @@ |
658 | # components are found. |
659 | self.assertRaises( |
660 | NoSuchPerson, self.branch_lookup.getByLPPath, '~aa/bb/c') |
661 | - owner = self.factory.makePerson(name='aa') |
662 | + self.factory.makePerson(name='aa') |
663 | self.assertRaises( |
664 | NoSuchProduct, self.branch_lookup.getByLPPath, '~aa/bb/c') |
665 | - product = self.factory.makeProduct(name='bb') |
666 | + self.factory.makeProduct(name='bb') |
667 | self.assertRaises( |
668 | NoSuchBranch, self.branch_lookup.getByLPPath, '~aa/bb/c') |
669 | |
670 | @@ -434,7 +449,7 @@ |
671 | # doesn't match an existing branch. |
672 | self.assertRaises( |
673 | NoSuchPerson, self.branch_lookup.getByLPPath, '~aa/+junk/c') |
674 | - owner = self.factory.makePerson(name='aa') |
675 | + self.factory.makePerson(name='aa') |
676 | self.assertRaises( |
677 | NoSuchBranch, self.branch_lookup.getByLPPath, '~aa/+junk/c') |
678 | |
679 | @@ -455,6 +470,24 @@ |
680 | (branch, 'foo/bar/baz'), |
681 | self.branch_lookup.getByLPPath(path)) |
682 | |
683 | + def test_resolve_distro_package_branch(self): |
684 | + # getByLPPath returns the branch associated with the distribution |
685 | + # source package referred to by the path. |
686 | + sourcepackage = self.factory.makeSourcePackage() |
687 | + branch = self.factory.makePackageBranch(sourcepackage=sourcepackage) |
688 | + distro_package = sourcepackage.distribution_sourcepackage |
689 | + ubuntu_branches = getUtility(ILaunchpadCelebrities).ubuntu_branches |
690 | + registrant = ubuntu_branches.teamowner |
691 | + run_with_login( |
692 | + registrant, |
693 | + ICanHasLinkedBranch(distro_package).setBranch, branch, registrant) |
694 | + self.assertEqual( |
695 | + (branch, None), |
696 | + self.branch_lookup.getByLPPath( |
697 | + '%s/%s' % ( |
698 | + distro_package.distribution.name, |
699 | + distro_package.sourcepackagename.name))) |
700 | + |
701 | def test_no_product_series_branch(self): |
702 | # getByLPPath raises `NoLinkedBranch` if there's no branch registered |
703 | # linked to the requested series. |
704 | @@ -501,6 +534,12 @@ |
705 | self.branch_lookup.getByLPPath, distribution.name) |
706 | self.assertEqual(distribution, exception.component) |
707 | |
708 | + def test_distribution_with_no_series(self): |
709 | + distro_package = self.factory.makeDistributionSourcePackage() |
710 | + path = ICanHasLinkedBranch(distro_package).bzr_identity |
711 | + self.assertRaises( |
712 | + NoLinkedBranch, self.branch_lookup.getByLPPath, path) |
713 | + |
714 | def test_project_linked_branch(self): |
715 | # Projects cannot have linked branches, so `getByLPPath` raises a |
716 | # `CannotHaveLinkedBranch` error if we try to get the linked branch |
717 | |
718 | === modified file 'lib/lp/code/model/tests/test_linkedbranch.py' |
719 | --- lib/lp/code/model/tests/test_linkedbranch.py 2009-05-24 18:09:51 +0000 |
720 | +++ lib/lp/code/model/tests/test_linkedbranch.py 2009-07-16 05:38:41 +0000 |
721 | @@ -18,11 +18,11 @@ |
722 | from lp.testing import run_with_login, TestCaseWithFactory |
723 | |
724 | |
725 | -class TestLinkedBranch(TestCaseWithFactory): |
726 | +class TestProductSeriesLinkedBranch(TestCaseWithFactory): |
727 | |
728 | layer = DatabaseFunctionalLayer |
729 | |
730 | - def test_product_series(self): |
731 | + def test_branch(self): |
732 | # The linked branch of a product series is its branch attribute. |
733 | product_series = self.factory.makeProductSeries() |
734 | product_series.branch = self.factory.makeProductBranch( |
735 | @@ -30,7 +30,29 @@ |
736 | self.assertEqual( |
737 | product_series.branch, ICanHasLinkedBranch(product_series).branch) |
738 | |
739 | - def test_product(self): |
740 | + def test_setBranch(self): |
741 | + # setBranch sets the linked branch of the product series. |
742 | + product_series = self.factory.makeProductSeries() |
743 | + branch = self.factory.makeProductBranch( |
744 | + product=product_series.product) |
745 | + ICanHasLinkedBranch(product_series).setBranch(branch) |
746 | + self.assertEqual(branch, product_series.branch) |
747 | + |
748 | + def test_bzr_identity(self): |
749 | + # The bzr_identity of a product series linked branch is |
750 | + # product/product_series. |
751 | + product_series = self.factory.makeProductSeries() |
752 | + bzr_identity = '%s/%s' % ( |
753 | + product_series.product.name, product_series.name) |
754 | + self.assertEqual( |
755 | + bzr_identity, ICanHasLinkedBranch(product_series).bzr_identity) |
756 | + |
757 | + |
758 | +class TestProductLinkedBranch(TestCaseWithFactory): |
759 | + |
760 | + layer = DatabaseFunctionalLayer |
761 | + |
762 | + def test_branch(self): |
763 | # The linked branch of a product is the linked branch of its |
764 | # development focus product series. |
765 | branch = self.factory.makeProductBranch() |
766 | @@ -38,22 +60,130 @@ |
767 | removeSecurityProxy(product).development_focus.branch = branch |
768 | self.assertEqual(branch, ICanHasLinkedBranch(product).branch) |
769 | |
770 | - def test_suitesourcepackage(self): |
771 | + def test_setBranch(self): |
772 | + # setBranch sets the linked branch of the development focus product |
773 | + # series. |
774 | + branch = self.factory.makeProductBranch() |
775 | + product = removeSecurityProxy(branch.product) |
776 | + ICanHasLinkedBranch(product).setBranch(branch) |
777 | + self.assertEqual(branch, product.development_focus.branch) |
778 | + |
779 | + def test_bzr_identity(self): |
780 | + # The bzr_identity of a product linked branch is the product name. |
781 | + product = self.factory.makeProduct() |
782 | + self.assertEqual( |
783 | + product.name, ICanHasLinkedBranch(product).bzr_identity) |
784 | + |
785 | + |
786 | +class TestSuiteSourcePackageLinkedBranch(TestCaseWithFactory): |
787 | + |
788 | + layer = DatabaseFunctionalLayer |
789 | + |
790 | + def test_branch(self): |
791 | # The linked branch of a suite source package is the official branch |
792 | # for the pocket of that source package. |
793 | branch = self.factory.makeAnyBranch() |
794 | + suite_sourcepackage = self.factory.makeSuiteSourcePackage() |
795 | + ubuntu_branches = getUtility(ILaunchpadCelebrities).ubuntu_branches |
796 | + registrant = ubuntu_branches.teamowner |
797 | + run_with_login( |
798 | + registrant, |
799 | + suite_sourcepackage.sourcepackage.setBranch, |
800 | + suite_sourcepackage.pocket, branch, registrant) |
801 | + self.assertEqual( |
802 | + branch, ICanHasLinkedBranch(suite_sourcepackage).branch) |
803 | + |
804 | + def test_setBranch(self): |
805 | + # setBranch sets the official branch for the appropriate pocket of the |
806 | + # source package. |
807 | + branch = self.factory.makeAnyBranch() |
808 | + suite_sourcepackage = self.factory.makeSuiteSourcePackage() |
809 | + ubuntu_branches = getUtility(ILaunchpadCelebrities).ubuntu_branches |
810 | + registrant = ubuntu_branches.teamowner |
811 | + run_with_login( |
812 | + registrant, |
813 | + ICanHasLinkedBranch(suite_sourcepackage).setBranch, |
814 | + branch, registrant) |
815 | + self.assertEqual( |
816 | + branch, |
817 | + suite_sourcepackage.sourcepackage.getBranch( |
818 | + suite_sourcepackage.pocket)) |
819 | + |
820 | + def test_bzr_identity(self): |
821 | + # The bzr_identity of a suite source package linked branch is the path |
822 | + # of that suite source package. |
823 | + suite_sourcepackage = self.factory.makeSuiteSourcePackage() |
824 | + self.assertEqual( |
825 | + suite_sourcepackage.path, |
826 | + ICanHasLinkedBranch(suite_sourcepackage).bzr_identity) |
827 | + |
828 | + |
829 | +class TestDistributionSourcePackageLinkedBranch(TestCaseWithFactory): |
830 | + |
831 | + layer = DatabaseFunctionalLayer |
832 | + |
833 | + def test_branch(self): |
834 | + # The linked branch of a distribution source package is the official |
835 | + # branch for the release pocket of the development focus series for |
836 | + # that package. Phew. |
837 | + branch = self.factory.makeAnyBranch() |
838 | sourcepackage = self.factory.makeSourcePackage() |
839 | + dev_sourcepackage = sourcepackage.development_version |
840 | pocket = PackagePublishingPocket.RELEASE |
841 | + |
842 | ubuntu_branches = getUtility(ILaunchpadCelebrities).ubuntu_branches |
843 | registrant = ubuntu_branches.teamowner |
844 | run_with_login( |
845 | ubuntu_branches.teamowner, |
846 | - sourcepackage.setBranch, pocket, branch, registrant) |
847 | - suite_sourcepackage = sourcepackage.getSuiteSourcePackage(pocket) |
848 | - self.assertEqual( |
849 | - branch, ICanHasLinkedBranch(suite_sourcepackage).branch) |
850 | - |
851 | - def test_project(self): |
852 | + dev_sourcepackage.setBranch, pocket, branch, registrant) |
853 | + |
854 | + distribution_sourcepackage = sourcepackage.distribution_sourcepackage |
855 | + self.assertEqual( |
856 | + branch, ICanHasLinkedBranch(distribution_sourcepackage).branch) |
857 | + |
858 | + def test_branch_when_no_series(self): |
859 | + # Our data model allows distributions that have no series. The linked |
860 | + # branch for a package in such a distribution is always None. |
861 | + distro_package = self.factory.makeDistributionSourcePackage() |
862 | + self.assertIs(None, ICanHasLinkedBranch(distro_package).branch) |
863 | + |
864 | + def test_setBranch(self): |
865 | + # Setting the linked branch for a distribution source package links |
866 | + # the branch to the release pocket of the development focus series for |
867 | + # that package. |
868 | + branch = self.factory.makeAnyBranch() |
869 | + sourcepackage = self.factory.makeSourcePackage() |
870 | + distribution_sourcepackage = sourcepackage.distribution_sourcepackage |
871 | + |
872 | + ubuntu_branches = getUtility(ILaunchpadCelebrities).ubuntu_branches |
873 | + registrant = ubuntu_branches.teamowner |
874 | + run_with_login( |
875 | + registrant, |
876 | + ICanHasLinkedBranch(distribution_sourcepackage).setBranch, |
877 | + branch, registrant) |
878 | + |
879 | + dev_sourcepackage = sourcepackage.development_version |
880 | + pocket = PackagePublishingPocket.RELEASE |
881 | + self.assertEqual(branch, dev_sourcepackage.getBranch(pocket)) |
882 | + |
883 | + def test_bzr_identity(self): |
884 | + # The bzr_identity of a distribution source package linked branch is |
885 | + # distro/package. |
886 | + distribution_sourcepackage = ( |
887 | + self.factory.makeDistributionSourcePackage()) |
888 | + self.assertEqual( |
889 | + '%s/%s' % ( |
890 | + distribution_sourcepackage.distribution.name, |
891 | + distribution_sourcepackage.sourcepackagename.name), |
892 | + ICanHasLinkedBranch(distribution_sourcepackage).bzr_identity) |
893 | + |
894 | + |
895 | +class TestProjectLinkedBranch(TestCaseWithFactory): |
896 | + |
897 | + layer = DatabaseFunctionalLayer |
898 | + |
899 | + def test_cannot_have_linked_branch(self): |
900 | + # Projects cannot have linked branches. |
901 | project = self.factory.makeProject() |
902 | self.assertRaises( |
903 | CannotHaveLinkedBranch, get_linked_branch, project) |
904 | |
905 | === modified file 'lib/lp/registry/browser/distributionsourcepackage.py' |
906 | --- lib/lp/registry/browser/distributionsourcepackage.py 2009-07-10 12:34:49 +0000 |
907 | +++ lib/lp/registry/browser/distributionsourcepackage.py 2009-07-16 03:53:20 +0000 |
908 | @@ -31,9 +31,8 @@ |
909 | from lp.answers.browser.questiontarget import ( |
910 | QuestionTargetFacetMixin, QuestionTargetTraversalMixin) |
911 | from canonical.launchpad.webapp import ( |
912 | - ApplicationMenu, GetitemNavigation, LaunchpadEditFormView, |
913 | - LaunchpadFormView, Link, StandardLaunchpadFacets, action, canonical_url, |
914 | - redirection) |
915 | + ApplicationMenu, LaunchpadEditFormView, LaunchpadFormView, Link, |
916 | + Navigation, StandardLaunchpadFacets, action, canonical_url, redirection) |
917 | from canonical.launchpad.webapp.menu import enabled_with_permission |
918 | from canonical.launchpad.webapp.breadcrumb import BreadcrumbBuilder |
919 | |
920 | @@ -88,13 +87,16 @@ |
921 | return Link('+filebug', text, icon='bug') |
922 | |
923 | |
924 | -class DistributionSourcePackageNavigation(GetitemNavigation, |
925 | +class DistributionSourcePackageNavigation(Navigation, |
926 | BugTargetTraversalMixin, QuestionTargetTraversalMixin): |
927 | |
928 | usedfor = IDistributionSourcePackage |
929 | |
930 | redirection("+editbugcontact", "+subscribe") |
931 | |
932 | + def traverse(self, name): |
933 | + return self.context.getVersion(name) |
934 | + |
935 | |
936 | class DecoratedDistributionSourcePackageRelease: |
937 | """A decorated DistributionSourcePackageRelease. |
938 | |
939 | === modified file 'lib/lp/registry/configure.zcml' |
940 | --- lib/lp/registry/configure.zcml 2009-07-13 14:13:07 +0000 |
941 | +++ lib/lp/registry/configure.zcml 2009-07-16 04:47:24 +0000 |
942 | @@ -357,6 +357,7 @@ |
943 | <allow |
944 | attributes=" |
945 | distribution |
946 | + development_version |
947 | sourcepackagename |
948 | name |
949 | displayname |
950 | |
951 | === modified file 'lib/lp/registry/interfaces/distributionsourcepackage.py' |
952 | --- lib/lp/registry/interfaces/distributionsourcepackage.py 2009-07-10 15:38:46 +0000 |
953 | +++ lib/lp/registry/interfaces/distributionsourcepackage.py 2009-07-16 04:48:10 +0000 |
954 | @@ -68,6 +68,10 @@ |
955 | "The list of all releases of this source package " |
956 | "in this distribution.") |
957 | |
958 | + development_version = Attribute( |
959 | + 'The development version of this source package. None if there is no ' |
960 | + 'such package.') |
961 | + |
962 | def getReleasesAndPublishingHistory(): |
963 | """Return a list of all releases of this source package in this |
964 | distribution and their correspodning publishing history. |
965 | @@ -85,9 +89,6 @@ |
966 | "Return a list of CURRENT publishing records for this source " |
967 | "package in this distribution.") |
968 | |
969 | - def __getitem__(version): |
970 | - """Should map to getVersion.""" |
971 | - |
972 | def getVersion(version): |
973 | """Return the a DistributionSourcePackageRelease with the given |
974 | version, or None if there has never been a release with that |
975 | |
976 | === modified file 'lib/lp/registry/interfaces/sourcepackage.py' |
977 | --- lib/lp/registry/interfaces/sourcepackage.py 2009-06-12 16:36:02 +0000 |
978 | +++ lib/lp/registry/interfaces/sourcepackage.py 2009-07-15 07:25:04 +0000 |
979 | @@ -125,6 +125,9 @@ |
980 | development_version = Attribute( |
981 | "This package on the distro's current series.") |
982 | |
983 | + distribution_sourcepackage = Attribute( |
984 | + "The IDistributionSourcePackage for this source package.") |
985 | + |
986 | def __getitem__(version): |
987 | """Return the source package release with the given version in this |
988 | distro series, or None.""" |
989 | |
990 | === modified file 'lib/lp/registry/model/distributionsourcepackage.py' |
991 | --- lib/lp/registry/model/distributionsourcepackage.py 2009-07-10 15:38:46 +0000 |
992 | +++ lib/lp/registry/model/distributionsourcepackage.py 2009-07-16 04:48:10 +0000 |
993 | @@ -89,6 +89,14 @@ |
994 | self.sourcepackagename.name, self.distribution.displayname) |
995 | |
996 | @property |
997 | + def development_version(self): |
998 | + """See `IDistributionSourcePackage`.""" |
999 | + series = self.distribution.currentseries |
1000 | + if series is None: |
1001 | + return None |
1002 | + return series.getSourcePackage(self.sourcepackagename) |
1003 | + |
1004 | + @property |
1005 | def _self_in_database(self): |
1006 | """Return the equivalent database-backed record of self.""" |
1007 | # XXX: allenap 2008-11-13 bug=297736: This is a temporary |
1008 | @@ -124,9 +132,6 @@ |
1009 | _get_bug_reporting_guidelines, |
1010 | _set_bug_reporting_guidelines) |
1011 | |
1012 | - def __getitem__(self, version): |
1013 | - return self.getVersion(version) |
1014 | - |
1015 | @property |
1016 | def latest_overall_publication(self): |
1017 | """See `IDistributionSourcePackage`.""" |
1018 | |
1019 | === modified file 'lib/lp/registry/model/sourcepackage.py' |
1020 | --- lib/lp/registry/model/sourcepackage.py 2009-07-11 09:33:12 +0000 |
1021 | +++ lib/lp/registry/model/sourcepackage.py 2009-07-15 07:25:04 +0000 |
1022 | @@ -424,6 +424,11 @@ |
1023 | self.sourcepackagename, self.distribution.currentseries) |
1024 | |
1025 | @property |
1026 | + def distribution_sourcepackage(self): |
1027 | + """See `ISourcePackage`.""" |
1028 | + return self.distribution.getSourcePackage(self.sourcepackagename) |
1029 | + |
1030 | + @property |
1031 | def bug_reporting_guidelines(self): |
1032 | """See `IBugTarget`.""" |
1033 | return self.distribution.bug_reporting_guidelines |
1034 | |
1035 | === modified file 'lib/lp/registry/tests/test_distributionsourcepackage.py' |
1036 | --- lib/lp/registry/tests/test_distributionsourcepackage.py 2009-07-10 15:38:46 +0000 |
1037 | +++ lib/lp/registry/tests/test_distributionsourcepackage.py 2009-07-15 07:55:56 +0000 |
1038 | @@ -109,5 +109,25 @@ |
1039 | |
1040 | self.assertEqual(related_archive_names, ['gedit-nightly']) |
1041 | |
1042 | + def test_development_version(self): |
1043 | + # IDistributionSourcePackage.development_version is the ISourcePackage |
1044 | + # for the current series of the distribution. |
1045 | + dsp = self.factory.makeDistributionSourcePackage() |
1046 | + series = self.factory.makeDistroRelease(distribution=dsp.distribution) |
1047 | + self.assertEqual(series, dsp.distribution.currentseries) |
1048 | + development_version = dsp.distribution.currentseries.getSourcePackage( |
1049 | + dsp.sourcepackagename) |
1050 | + self.assertEqual(development_version, dsp.development_version) |
1051 | + |
1052 | + def test_development_version_no_current_series(self): |
1053 | + # IDistributionSourcePackage.development_version is the ISourcePackage |
1054 | + # for the current series of the distribution. |
1055 | + dsp = self.factory.makeDistributionSourcePackage() |
1056 | + currentseries = dsp.distribution.currentseries |
1057 | + # The current series is None by default. |
1058 | + self.assertIs(None, currentseries) |
1059 | + self.assertEqual(None, dsp.development_version) |
1060 | + |
1061 | + |
1062 | def test_suite(): |
1063 | return unittest.TestLoader().loadTestsFromName(__name__) |
1064 | |
1065 | === modified file 'lib/lp/registry/tests/test_sourcepackage.py' |
1066 | --- lib/lp/registry/tests/test_sourcepackage.py 2009-06-02 13:52:48 +0000 |
1067 | +++ lib/lp/registry/tests/test_sourcepackage.py 2009-07-15 07:25:04 +0000 |
1068 | @@ -154,6 +154,17 @@ |
1069 | self.assertEqual( |
1070 | dev_sourcepackage, dev_sourcepackage.development_version) |
1071 | |
1072 | + def test_distribution_sourcepackage(self): |
1073 | + # ISourcePackage.distribution_sourcepackage is the distribution source |
1074 | + # package for the ISourcePackage. |
1075 | + sourcepackage = self.factory.makeSourcePackage() |
1076 | + distribution = sourcepackage.distribution |
1077 | + distribution_sourcepackage = distribution.getSourcePackage( |
1078 | + sourcepackage.sourcepackagename) |
1079 | + self.assertEqual( |
1080 | + distribution_sourcepackage, |
1081 | + sourcepackage.distribution_sourcepackage) |
1082 | + |
1083 | |
1084 | class TestSourcePackageSecurity(TestCaseWithFactory): |
1085 | """Tests for source package branch linking security.""" |
1086 | |
1087 | === modified file 'lib/lp/soyuz/doc/buildd-queuebuilder-lookup.txt' |
1088 | --- lib/lp/soyuz/doc/buildd-queuebuilder-lookup.txt 2009-04-29 20:27:44 +0000 |
1089 | +++ lib/lp/soyuz/doc/buildd-queuebuilder-lookup.txt 2009-07-16 03:31:45 +0000 |
1090 | @@ -31,7 +31,7 @@ |
1091 | >>> partner_archive = getUtility(IArchiveSet).getByDistroPurpose( |
1092 | ... distribution=ubuntu, purpose=ArchivePurpose.PARTNER) |
1093 | |
1094 | - >>> dspr = ubuntu.getSourcePackage('mozilla-firefox')['0.9'] |
1095 | + >>> dspr = ubuntu.getSourcePackage('mozilla-firefox').getVersion('0.9') |
1096 | >>> the_spr = dspr.sourcepackagerelease |
1097 | |
1098 | >>> initial_candidates = [] |
1099 | |
1100 | === modified file 'lib/lp/soyuz/doc/distroseriesqueue.txt' |
1101 | --- lib/lp/soyuz/doc/distroseriesqueue.txt 2009-07-03 09:49:13 +0000 |
1102 | +++ lib/lp/soyuz/doc/distroseriesqueue.txt 2009-07-16 03:31:45 +0000 |
1103 | @@ -712,7 +712,8 @@ |
1104 | have in the sampledata. |
1105 | |
1106 | >>> [binary_queue] = queue_items |
1107 | - >>> non_matching_pmount = ubuntu.getSourcePackage('pmount')['0.1-2'] |
1108 | + >>> pmount = ubuntu.getSourcePackage('pmount') |
1109 | + >>> non_matching_pmount = pmount.getVersion('0.1-2') |
1110 | >>> unused = binary_queue.addSource( |
1111 | ... non_matching_pmount.sourcepackagerelease) |
1112 | |
1113 | @@ -732,7 +733,7 @@ |
1114 | ... PackagePublishingPocket.RELEASE, |
1115 | ... 'pmount_0.1-1_source.changes', 'some content', |
1116 | ... breezy_autotest.main_archive) |
1117 | - >>> matching_pmount = ubuntu.getSourcePackage('pmount')['0.1-1'] |
1118 | + >>> matching_pmount = pmount.getVersion('0.1-1') |
1119 | >>> unused = candidate_queue.addSource( |
1120 | ... matching_pmount.sourcepackagerelease) |
1121 | |
1122 | @@ -1087,7 +1088,7 @@ |
1123 | are. Contents can be attached to it. |
1124 | |
1125 | # Retrieve a SourcePackageRelease from the sampledata. |
1126 | - >>> a_source_package = ubuntu.getSourcePackage('pmount')['0.1-2'] |
1127 | + >>> a_source_package = pmount.getVersion('0.1-2') |
1128 | >>> a_source_release = a_source_package.sourcepackagerelease |
1129 | |
1130 | >>> unused = delayed_copy.addSource(a_source_release) |
1131 | |
1132 | === modified file 'lib/lp/soyuz/doc/package-diff.txt' |
1133 | --- lib/lp/soyuz/doc/package-diff.txt 2009-05-06 20:53:05 +0000 |
1134 | +++ lib/lp/soyuz/doc/package-diff.txt 2009-07-16 03:31:45 +0000 |
1135 | @@ -18,8 +18,8 @@ |
1136 | >>> ubuntu = getUtility(IDistributionSet).getByName('ubuntu') |
1137 | >>> pmount = ubuntu.getSourcePackage('pmount') |
1138 | |
1139 | - >>> pmount_from = pmount['0.1-1'].sourcepackagerelease |
1140 | - >>> pmount_to = pmount['0.1-2'].sourcepackagerelease |
1141 | + >>> pmount_from = pmount.getVersion('0.1-1').sourcepackagerelease |
1142 | + >>> pmount_to = pmount.getVersion('0.1-2').sourcepackagerelease |
1143 | |
1144 | A packageDiff can be created from the two packages by calling |
1145 | requestDiffTo(). It takes two arguments: the user requesting the |
This branch makes lp:ubuntu/foo resolve to the branch associated with the development version of the 'foo' package on Ubuntu, defined as the branch linked to the Release pocket of that package of the current series of that distribution.
To do this, I've registered a new adapter from DistributionSou rcePackage to ICanHasLinkedBr anch. While I was there, I also added a bzr_identity attribute to ICanHasLinkedBr anch, as well as a setBranch method. Neither of these are deeply associated with the change, but they are both things that are related & I've wanted to do for a while.
bazaar_identity has been changed to return 'lp:ubuntu/package' for source package branches associated to the development focus package.
I had to change the linked branch traversal code to pass segments into the traversable adapters. This was so I could raise context-aware errors in the case of 'ubuntu/ doesntexist' and 'ubuntu/ doesntexist/ openssh' .
I've also increased the interlinking of distro-related registry objects. In particular, SourcePackage now links to DistributionSou rcePackage (the latter being conceptually a parent of the former), and DistributionSou rcePackage links to the development source package.
There are a lot of pyflakes & pep8 cleanups along the way, inflating the size of the diff.
I've also based this branch on lp:~jml/launchpad/remove-getitem. This was necessary in order to get the adapt() method behaving correctly with DistributionSou rcePackage.