Merge lp:~wgrant/launchpad/bug-432832 into lp:launchpad

Proposed by William Grant
Status: Merged
Approved by: Paul Hummer
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~wgrant/launchpad/bug-432832
Merge into: lp:launchpad
Diff against target: None lines
To merge this branch: bzr merge lp:~wgrant/launchpad/bug-432832
Reviewer Review Type Date Requested Status
Paul Hummer (community) Approve
Review via email: mp+12109@code.launchpad.net
To post a comment you must log in.
Revision history for this message
William Grant (wgrant) wrote :

update-sourcecode is currently broken if the user doesn't have access to shipit or canonical-identity-provider branches.

A Branch.open() call will fail if the user cannot access the branch, but only the sprout and pull calls are wrapped in a try/except block. This branch moves the try/excepts to around the Branch.open() calls, allowing update-sourcecode to again complete for unprivileged users.

I believe removing the sprout/pull protection is fine, because the point of the 'optional' sourcedep flag is for update-sourcecode to not die if the branches are inaccessible due to a lack of privileges. If the Branch.open() succeeds, the correct privilege is clearly held, so any subsequent failure is real.

Revision history for this message
Paul Hummer (rockstar) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/devscripts/sourcecode.py'
2--- lib/devscripts/sourcecode.py 2009-09-18 08:33:35 +0000
3+++ lib/devscripts/sourcecode.py 2009-09-19 01:11:00 +0000
4@@ -95,8 +95,15 @@
5 """Get the new branches into sourcecode."""
6 for project, (branch_url, optional) in new_branches.iteritems():
7 destination = os.path.join(sourcecode_directory, project)
8- remote_branch = Branch.open(
9- branch_url, possible_transports=possible_transports)
10+ try:
11+ remote_branch = Branch.open(
12+ branch_url, possible_transports=possible_transports)
13+ except BzrError:
14+ if optional:
15+ report_exception(sys.exc_info(), sys.stderr)
16+ continue
17+ else:
18+ raise
19 possible_transports.append(
20 remote_branch.bzrdir.root_transport)
21 print 'Getting %s from %s' % (project, branch_url)
22@@ -105,16 +112,10 @@
23 # we should avoid sharing repositories to avoid format
24 # incompatibilities.
25 force_new_repo = not optional
26- try:
27- remote_branch.bzrdir.sprout(
28- destination, create_tree_if_local=True,
29- source_branch=remote_branch, force_new_repo=force_new_repo,
30- possible_transports=possible_transports)
31- except BzrError:
32- if optional:
33- report_exception(sys.exc_info(), sys.stderr)
34- else:
35- raise
36+ remote_branch.bzrdir.sprout(
37+ destination, create_tree_if_local=True,
38+ source_branch=remote_branch, force_new_repo=force_new_repo,
39+ possible_transports=possible_transports)
40
41
42 def update_branches(sourcecode_directory, update_branches,
43@@ -128,19 +129,20 @@
44 destination = os.path.join(sourcecode_directory, project)
45 print 'Updating %s' % (project,)
46 local_tree = WorkingTree.open(destination)
47- remote_branch = Branch.open(
48- branch_url, possible_transports=possible_transports)
49- possible_transports.append(
50- remote_branch.bzrdir.root_transport)
51 try:
52- local_tree.pull(
53- remote_branch, overwrite=True,
54- possible_transports=possible_transports)
55+ remote_branch = Branch.open(
56+ branch_url, possible_transports=possible_transports)
57 except BzrError:
58 if optional:
59 report_exception(sys.exc_info(), sys.stderr)
60+ continue
61 else:
62 raise
63+ possible_transports.append(
64+ remote_branch.bzrdir.root_transport)
65+ local_tree.pull(
66+ remote_branch, overwrite=True,
67+ possible_transports=possible_transports)
68
69
70 def remove_branches(sourcecode_directory, removed_branches):