Bazaar PQM Plugin

Merge lp:~spiv/bzr-pqm/non-local-submission into lp:bzr-pqm

  1. non-local-submission
  2. Merge into devel
Proposed by Andrew Bennetts
Status: Merged
Merged at revision: not available
Proposed branch: lp:~spiv/bzr-pqm/non-local-submission
Merge into: lp:bzr-pqm
Diff against target: 214 lines (+120/-11)
3 files modified
__init__.py (+10/-5)
pqm_submit.py (+45/-6)
test_pqm_submit.py (+65/-0)
To merge this branch: bzr merge lp:~spiv/bzr-pqm/non-local-submission
Reviewer Review Type Date Requested Status
Robert Collins Approve
Jonathan Lange (community) Needs Fixing
Review via email: mp+13333@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Andrew Bennetts (spiv) wrote :

This could be better, especially in some of the error handling, but it allows me to pqm-submit branches without downloading them.

It adds a --ignore-local option which will skip opening the local branch/tree, and associated checks. In this case it will try looking up the config options (pqm_email etc) from the locations.conf for the public location (which must be given on the command line), and failing that will try the global conf. So e.g. I now have a [http://bazaar.launchpad.net/~*/bzr/] section in my locations.conf to specify the pqm_email.

Probably the StackedConfig class belongs in some form in bzrlib proper (at the moment BranchConfig does something similar but in a less reusable way).

Anyway, this is functional, despite the rough corners.

Revision history for this message
Jonathan Lange (jml) wrote :

Wow, what an excellent idea.

It's not that hackish. Bazaar's config APIs don't really let you do much better than you are now, and I can tolerate the imperative linear search, because I'm a tolerant person.

I'm afraid though that this needs to at least have a good excuse for a lack of tests before it lands.

Thanks for JFDIng this.

jml

review: Needs Fixing
Revision history for this message
Robert Collins (lifeless) wrote :

I think this is:
 - non core
 - really really useful
 - going stale

So we should land it without tests, but file a bug saying that we're accruing some debt here.

review: Approve
lp:~spiv/bzr-pqm/non-local-submission updated
62. By Andrew Bennetts

Merge fix-tests.

63. By Andrew Bennetts

Add some tests.

Revision history for this message
Andrew Bennetts (spiv) wrote :

> Wow, what an excellent idea.

Thanks!

> I'm afraid though that this needs to at least have a good excuse for a lack of
> tests before it lands.

Some basic tests added. They are a bit too blackboxy, but they're a start. I had to fix the existing failures first, so hopefully that gives me some brownie points ;)

Revision history for this message
Andrew Bennetts (spiv) wrote :

Robert just rubberstamped the tests, and asked me to land it, so I am. Therefore further criticism is welcome in the form of bug reports :)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file '__init__.py'
--- __init__.py 2009-02-06 17:20:48 +0000
+++ __init__.py 2009-11-24 07:37:09 +0000
@@ -78,10 +78,11 @@
78 help='Use this url as the public location to the pqm.'),78 help='Use this url as the public location to the pqm.'),
79 Option('submit-branch', type=str,79 Option('submit-branch', type=str,
80 help='Use this url as the target submission branch.'),80 help='Use this url as the target submission branch.'),
81 Option('ignore-local', help='Do not check the local branch or tree.'),
81 ]82 ]
8283
83 def run(self, location=None, message=None, public_location=None,84 def run(self, location=None, message=None, public_location=None,
84 dry_run=False, submit_branch=None):85 dry_run=False, submit_branch=None, ignore_local=False):
85 from bzrlib import errors, trace, bzrdir86 from bzrlib import errors, trace, bzrdir
86 if __name__ != 'bzrlib.plugins.pqm':87 if __name__ != 'bzrlib.plugins.pqm':
87 trace.warning('The bzr-pqm plugin needs to be called'88 trace.warning('The bzr-pqm plugin needs to be called'
@@ -91,9 +92,13 @@
91 return 192 return 1
92 from bzrlib.plugins.pqm.pqm_submit import submit93 from bzrlib.plugins.pqm.pqm_submit import submit
9394
94 if location is None:95 if ignore_local:
95 location = '.'96 tree, b, relpath = None, None, None
96 tree, b, relpath = bzrdir.BzrDir.open_containing_tree_or_branch(location)97 else:
98 if location is None:
99 location = '.'
100 tree, b, relpath = bzrdir.BzrDir.open_containing_tree_or_branch(
101 location)
97 if relpath and not tree and location != '.':102 if relpath and not tree and location != '.':
98 raise errors.BzrCommandError(103 raise errors.BzrCommandError(
99 'No working tree was found, but we were not given the '104 'No working tree was found, but we were not given the '
@@ -105,7 +110,7 @@
105 submit(b, message=message, dry_run=dry_run,110 submit(b, message=message, dry_run=dry_run,
106 public_location=public_location,111 public_location=public_location,
107 submit_location=submit_branch,112 submit_location=submit_branch,
108 tree=tree)113 tree=tree, ignore_local=ignore_local)
109114
110115
111register_command(cmd_pqm_submit)116register_command(cmd_pqm_submit)
112117
=== modified file 'pqm_submit.py'
--- pqm_submit.py 2008-04-21 15:53:51 +0000
+++ pqm_submit.py 2009-11-24 07:37:09 +0000
@@ -75,7 +75,7 @@
75 If any of public_location, submit_location or message are75 If any of public_location, submit_location or message are
76 omitted, they will be calculated from source_branch.76 omitted, they will be calculated from source_branch.
77 """77 """
78 if source_branch is None:78 if source_branch is None and public_location is None:
79 raise errors.NoMergeSource()79 raise errors.NoMergeSource()
80 self.source_branch = source_branch80 self.source_branch = source_branch
81 self.tree = tree81 self.tree = tree
@@ -100,6 +100,9 @@
100 self.public_location = public_location100 self.public_location = public_location
101101
102 if submit_location is None:102 if submit_location is None:
103 if self.source_branch is None:
104 raise errors.BzrError(
105 "Cannot determine submit location to use.")
103 config = self.source_branch.get_config()106 config = self.source_branch.get_config()
104 # First check the deprecated pqm_branch config key:107 # First check the deprecated pqm_branch config key:
105 submit_location = config.get_user_option('pqm_branch')108 submit_location = config.get_user_option('pqm_branch')
@@ -166,7 +169,11 @@
166 unsigned_text = ''.join(self.to_lines())169 unsigned_text = ''.join(self.to_lines())
167 unsigned_text = unsigned_text.encode('ascii') #URLs should be ascii170 unsigned_text = unsigned_text.encode('ascii') #URLs should be ascii
168171
169 strategy = gpg.GPGStrategy(self.source_branch.get_config())172 if self.source_branch:
173 config = self.source_branch.get_config()
174 else:
175 config = _mod_config.GlobalConfig()
176 strategy = gpg.GPGStrategy(config)
170 return strategy.sign(unsigned_text)177 return strategy.sign(unsigned_text)
171178
172 def to_email(self, mail_from, mail_to, sign=True):179 def to_email(self, mail_from, mail_to, sign=True):
@@ -185,10 +192,41 @@
185 return message192 return message
186193
187194
195class StackedConfig(_mod_config.Config):
196
197 def __init__(self):
198 super(StackedConfig, self).__init__()
199 self._sources = []
200
201 def add_source(self, source):
202 self._sources.append(source)
203
204 def _get_user_option(self, option_name):
205 """See Config._get_user_option."""
206 for source in self._sources:
207 value = source._get_user_option(option_name)
208 if value is not None:
209 return value
210 return None
211
212 def _get_user_id(self):
213 for source in self._sources:
214 value = source._get_user_id()
215 if value is not None:
216 return value
217 return None
218
219
188def submit(branch, message, dry_run=False, public_location=None,220def submit(branch, message, dry_run=False, public_location=None,
189 submit_location=None, tree=None):221 submit_location=None, tree=None, ignore_local=False):
190 """Submit the given branch to the pqm."""222 """Submit the given branch to the pqm."""
191 config = branch.get_config()223 config = StackedConfig()
224 if branch:
225 config.add_source(branch.get_config())
226 else:
227 if public_location:
228 config.add_source(_mod_config.LocationConfig(public_location))
229 config.add_source(_mod_config.GlobalConfig())
192230
193 submission = PQMSubmission(231 submission = PQMSubmission(
194 source_branch=branch, public_location=public_location, message=message,232 source_branch=branch, public_location=public_location, message=message,
@@ -204,8 +242,9 @@
204 raise NoPQMSubmissionAddress(branch)242 raise NoPQMSubmissionAddress(branch)
205 mail_to = mail_to.encode('utf8') # same here243 mail_to = mail_to.encode('utf8') # same here
206244
207 submission.check_tree()245 if not ignore_local:
208 submission.check_public_branch()246 submission.check_tree()
247 submission.check_public_branch()
209248
210 message = submission.to_email(mail_from, mail_to)249 message = submission.to_email(mail_from, mail_to)
211250
212251
=== modified file 'test_pqm_submit.py'
--- test_pqm_submit.py 2009-11-24 01:35:37 +0000
+++ test_pqm_submit.py 2009-11-24 07:37:09 +0000
@@ -127,6 +127,16 @@
127 self.assertRaises(errors.NotBranchError,127 self.assertRaises(errors.NotBranchError,
128 submission.check_public_branch)128 submission.check_public_branch)
129129
130 def test_ignore_local(self):
131 submission = pqm_submit.PQMSubmission(
132 source_branch=None,
133 public_location='public-location',
134 submit_location='submit-branch',
135 message='merge message')
136 message = submission.to_email('from@address', 'to@address', sign=False)
137 self.assertContainsRe(
138 message.as_string(), 'star-merge public-location submit-branch')
139
130 def test_to_lines(self):140 def test_to_lines(self):
131 source_branch = self.make_branch('source')141 source_branch = self.make_branch('source')
132 submission = pqm_submit.PQMSubmission(142 submission = pqm_submit.PQMSubmission(
@@ -406,3 +416,58 @@
406 self.assertEqual(('jrandom@example.com', ['pqm@example.com']),416 self.assertEqual(('jrandom@example.com', ['pqm@example.com']),
407 call[1:3])417 call[1:3])
408 self.assertContainsRe(call[3], EMAIL)418 self.assertContainsRe(call[3], EMAIL)
419
420 def test_ignore_local(self):
421 """--ignore-local can submit a branch that isn't available locally.
422
423 It will use the location config of the public location to determine the
424 from and to email addresses.
425 """
426 config = _mod_config.LocationConfig('http://example.com/')
427 config.set_user_option('pqm_email', 'PQM <pqm@example.com>')
428 config.set_user_option(
429 'pqm_user_email', 'J. Random Hacker <jrandom@example.com>')
430 (out, err, connect_calls,
431 sendmail_calls) = self.run_bzr_fakemail(
432 ['pqm-submit', '-m', 'commit message',
433 '--submit-branch', 'http://example.com/submit/',
434 '--ignore-local', '--public-location',
435 'http://example.com/public/'
436 ])
437 self.assertEqual('', out)
438 self.assertEqual(1, len(connect_calls))
439 call = connect_calls[0]
440 self.assertEqual(('localhost', 0), call[1:3])
441 self.assertEqual(1, len(sendmail_calls))
442 call = sendmail_calls[0]
443 self.assertEqual(('jrandom@example.com', ['pqm@example.com']),
444 call[1:3])
445 self.assertContainsRe(call[3], EMAIL)
446
447 def test_ignore_local_global_config_fallback(self):
448 """--ignore-local can submit a branch that isn't available locally.
449
450 If there's no location config for the public location, it will
451 determine the from and to email addresses from the global config.
452 """
453 config = _mod_config.GlobalConfig()
454 config.set_user_option('pqm_email', 'PQM <pqm@example.com>')
455 config.set_user_option(
456 'pqm_user_email', 'J. Random Hacker <jrandom@example.com>')
457 (out, err, connect_calls,
458 sendmail_calls) = self.run_bzr_fakemail(
459 ['pqm-submit', '-m', 'commit message',
460 '--submit-branch', 'http://example.com/submit/',
461 '--ignore-local', '--public-location',
462 'http://example.com/public/'
463 ])
464 self.assertEqual('', out)
465 self.assertEqual(1, len(connect_calls))
466 call = connect_calls[0]
467 self.assertEqual(('localhost', 0), call[1:3])
468 self.assertEqual(1, len(sendmail_calls))
469 call = sendmail_calls[0]
470 self.assertEqual(('jrandom@example.com', ['pqm@example.com']),
471 call[1:3])
472 self.assertContainsRe(call[3], EMAIL)
473

Subscribers

People subscribed via source and target branches

to all changes:
to status/vote changes: