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
=== modified file 'lib/devscripts/sourcecode.py'
--- lib/devscripts/sourcecode.py 2009-09-18 08:33:35 +0000
+++ lib/devscripts/sourcecode.py 2009-09-19 01:11:00 +0000
@@ -95,8 +95,15 @@
95 """Get the new branches into sourcecode."""95 """Get the new branches into sourcecode."""
96 for project, (branch_url, optional) in new_branches.iteritems():96 for project, (branch_url, optional) in new_branches.iteritems():
97 destination = os.path.join(sourcecode_directory, project)97 destination = os.path.join(sourcecode_directory, project)
98 remote_branch = Branch.open(98 try:
99 branch_url, possible_transports=possible_transports)99 remote_branch = Branch.open(
100 branch_url, possible_transports=possible_transports)
101 except BzrError:
102 if optional:
103 report_exception(sys.exc_info(), sys.stderr)
104 continue
105 else:
106 raise
100 possible_transports.append(107 possible_transports.append(
101 remote_branch.bzrdir.root_transport)108 remote_branch.bzrdir.root_transport)
102 print 'Getting %s from %s' % (project, branch_url)109 print 'Getting %s from %s' % (project, branch_url)
@@ -105,16 +112,10 @@
105 # we should avoid sharing repositories to avoid format112 # we should avoid sharing repositories to avoid format
106 # incompatibilities.113 # incompatibilities.
107 force_new_repo = not optional114 force_new_repo = not optional
108 try:115 remote_branch.bzrdir.sprout(
109 remote_branch.bzrdir.sprout(116 destination, create_tree_if_local=True,
110 destination, create_tree_if_local=True,117 source_branch=remote_branch, force_new_repo=force_new_repo,
111 source_branch=remote_branch, force_new_repo=force_new_repo,118 possible_transports=possible_transports)
112 possible_transports=possible_transports)
113 except BzrError:
114 if optional:
115 report_exception(sys.exc_info(), sys.stderr)
116 else:
117 raise
118119
119120
120def update_branches(sourcecode_directory, update_branches,121def update_branches(sourcecode_directory, update_branches,
@@ -128,19 +129,20 @@
128 destination = os.path.join(sourcecode_directory, project)129 destination = os.path.join(sourcecode_directory, project)
129 print 'Updating %s' % (project,)130 print 'Updating %s' % (project,)
130 local_tree = WorkingTree.open(destination)131 local_tree = WorkingTree.open(destination)
131 remote_branch = Branch.open(
132 branch_url, possible_transports=possible_transports)
133 possible_transports.append(
134 remote_branch.bzrdir.root_transport)
135 try:132 try:
136 local_tree.pull(133 remote_branch = Branch.open(
137 remote_branch, overwrite=True,134 branch_url, possible_transports=possible_transports)
138 possible_transports=possible_transports)
139 except BzrError:135 except BzrError:
140 if optional:136 if optional:
141 report_exception(sys.exc_info(), sys.stderr)137 report_exception(sys.exc_info(), sys.stderr)
138 continue
142 else:139 else:
143 raise140 raise
141 possible_transports.append(
142 remote_branch.bzrdir.root_transport)
143 local_tree.pull(
144 remote_branch, overwrite=True,
145 possible_transports=possible_transports)
144146
145147
146def remove_branches(sourcecode_directory, removed_branches):148def remove_branches(sourcecode_directory, removed_branches):