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
1=== modified file '__init__.py'
2--- __init__.py 2009-02-06 17:20:48 +0000
3+++ __init__.py 2009-11-24 07:37:09 +0000
4@@ -78,10 +78,11 @@
5 help='Use this url as the public location to the pqm.'),
6 Option('submit-branch', type=str,
7 help='Use this url as the target submission branch.'),
8+ Option('ignore-local', help='Do not check the local branch or tree.'),
9 ]
10
11 def run(self, location=None, message=None, public_location=None,
12- dry_run=False, submit_branch=None):
13+ dry_run=False, submit_branch=None, ignore_local=False):
14 from bzrlib import errors, trace, bzrdir
15 if __name__ != 'bzrlib.plugins.pqm':
16 trace.warning('The bzr-pqm plugin needs to be called'
17@@ -91,9 +92,13 @@
18 return 1
19 from bzrlib.plugins.pqm.pqm_submit import submit
20
21- if location is None:
22- location = '.'
23- tree, b, relpath = bzrdir.BzrDir.open_containing_tree_or_branch(location)
24+ if ignore_local:
25+ tree, b, relpath = None, None, None
26+ else:
27+ if location is None:
28+ location = '.'
29+ tree, b, relpath = bzrdir.BzrDir.open_containing_tree_or_branch(
30+ location)
31 if relpath and not tree and location != '.':
32 raise errors.BzrCommandError(
33 'No working tree was found, but we were not given the '
34@@ -105,7 +110,7 @@
35 submit(b, message=message, dry_run=dry_run,
36 public_location=public_location,
37 submit_location=submit_branch,
38- tree=tree)
39+ tree=tree, ignore_local=ignore_local)
40
41
42 register_command(cmd_pqm_submit)
43
44=== modified file 'pqm_submit.py'
45--- pqm_submit.py 2008-04-21 15:53:51 +0000
46+++ pqm_submit.py 2009-11-24 07:37:09 +0000
47@@ -75,7 +75,7 @@
48 If any of public_location, submit_location or message are
49 omitted, they will be calculated from source_branch.
50 """
51- if source_branch is None:
52+ if source_branch is None and public_location is None:
53 raise errors.NoMergeSource()
54 self.source_branch = source_branch
55 self.tree = tree
56@@ -100,6 +100,9 @@
57 self.public_location = public_location
58
59 if submit_location is None:
60+ if self.source_branch is None:
61+ raise errors.BzrError(
62+ "Cannot determine submit location to use.")
63 config = self.source_branch.get_config()
64 # First check the deprecated pqm_branch config key:
65 submit_location = config.get_user_option('pqm_branch')
66@@ -166,7 +169,11 @@
67 unsigned_text = ''.join(self.to_lines())
68 unsigned_text = unsigned_text.encode('ascii') #URLs should be ascii
69
70- strategy = gpg.GPGStrategy(self.source_branch.get_config())
71+ if self.source_branch:
72+ config = self.source_branch.get_config()
73+ else:
74+ config = _mod_config.GlobalConfig()
75+ strategy = gpg.GPGStrategy(config)
76 return strategy.sign(unsigned_text)
77
78 def to_email(self, mail_from, mail_to, sign=True):
79@@ -185,10 +192,41 @@
80 return message
81
82
83+class StackedConfig(_mod_config.Config):
84+
85+ def __init__(self):
86+ super(StackedConfig, self).__init__()
87+ self._sources = []
88+
89+ def add_source(self, source):
90+ self._sources.append(source)
91+
92+ def _get_user_option(self, option_name):
93+ """See Config._get_user_option."""
94+ for source in self._sources:
95+ value = source._get_user_option(option_name)
96+ if value is not None:
97+ return value
98+ return None
99+
100+ def _get_user_id(self):
101+ for source in self._sources:
102+ value = source._get_user_id()
103+ if value is not None:
104+ return value
105+ return None
106+
107+
108 def submit(branch, message, dry_run=False, public_location=None,
109- submit_location=None, tree=None):
110+ submit_location=None, tree=None, ignore_local=False):
111 """Submit the given branch to the pqm."""
112- config = branch.get_config()
113+ config = StackedConfig()
114+ if branch:
115+ config.add_source(branch.get_config())
116+ else:
117+ if public_location:
118+ config.add_source(_mod_config.LocationConfig(public_location))
119+ config.add_source(_mod_config.GlobalConfig())
120
121 submission = PQMSubmission(
122 source_branch=branch, public_location=public_location, message=message,
123@@ -204,8 +242,9 @@
124 raise NoPQMSubmissionAddress(branch)
125 mail_to = mail_to.encode('utf8') # same here
126
127- submission.check_tree()
128- submission.check_public_branch()
129+ if not ignore_local:
130+ submission.check_tree()
131+ submission.check_public_branch()
132
133 message = submission.to_email(mail_from, mail_to)
134
135
136=== modified file 'test_pqm_submit.py'
137--- test_pqm_submit.py 2009-11-24 01:35:37 +0000
138+++ test_pqm_submit.py 2009-11-24 07:37:09 +0000
139@@ -127,6 +127,16 @@
140 self.assertRaises(errors.NotBranchError,
141 submission.check_public_branch)
142
143+ def test_ignore_local(self):
144+ submission = pqm_submit.PQMSubmission(
145+ source_branch=None,
146+ public_location='public-location',
147+ submit_location='submit-branch',
148+ message='merge message')
149+ message = submission.to_email('from@address', 'to@address', sign=False)
150+ self.assertContainsRe(
151+ message.as_string(), 'star-merge public-location submit-branch')
152+
153 def test_to_lines(self):
154 source_branch = self.make_branch('source')
155 submission = pqm_submit.PQMSubmission(
156@@ -406,3 +416,58 @@
157 self.assertEqual(('jrandom@example.com', ['pqm@example.com']),
158 call[1:3])
159 self.assertContainsRe(call[3], EMAIL)
160+
161+ def test_ignore_local(self):
162+ """--ignore-local can submit a branch that isn't available locally.
163+
164+ It will use the location config of the public location to determine the
165+ from and to email addresses.
166+ """
167+ config = _mod_config.LocationConfig('http://example.com/')
168+ config.set_user_option('pqm_email', 'PQM <pqm@example.com>')
169+ config.set_user_option(
170+ 'pqm_user_email', 'J. Random Hacker <jrandom@example.com>')
171+ (out, err, connect_calls,
172+ sendmail_calls) = self.run_bzr_fakemail(
173+ ['pqm-submit', '-m', 'commit message',
174+ '--submit-branch', 'http://example.com/submit/',
175+ '--ignore-local', '--public-location',
176+ 'http://example.com/public/'
177+ ])
178+ self.assertEqual('', out)
179+ self.assertEqual(1, len(connect_calls))
180+ call = connect_calls[0]
181+ self.assertEqual(('localhost', 0), call[1:3])
182+ self.assertEqual(1, len(sendmail_calls))
183+ call = sendmail_calls[0]
184+ self.assertEqual(('jrandom@example.com', ['pqm@example.com']),
185+ call[1:3])
186+ self.assertContainsRe(call[3], EMAIL)
187+
188+ def test_ignore_local_global_config_fallback(self):
189+ """--ignore-local can submit a branch that isn't available locally.
190+
191+ If there's no location config for the public location, it will
192+ determine the from and to email addresses from the global config.
193+ """
194+ config = _mod_config.GlobalConfig()
195+ config.set_user_option('pqm_email', 'PQM <pqm@example.com>')
196+ config.set_user_option(
197+ 'pqm_user_email', 'J. Random Hacker <jrandom@example.com>')
198+ (out, err, connect_calls,
199+ sendmail_calls) = self.run_bzr_fakemail(
200+ ['pqm-submit', '-m', 'commit message',
201+ '--submit-branch', 'http://example.com/submit/',
202+ '--ignore-local', '--public-location',
203+ 'http://example.com/public/'
204+ ])
205+ self.assertEqual('', out)
206+ self.assertEqual(1, len(connect_calls))
207+ call = connect_calls[0]
208+ self.assertEqual(('localhost', 0), call[1:3])
209+ self.assertEqual(1, len(sendmail_calls))
210+ call = sendmail_calls[0]
211+ self.assertEqual(('jrandom@example.com', ['pqm@example.com']),
212+ call[1:3])
213+ self.assertContainsRe(call[3], EMAIL)
214+

Subscribers

People subscribed via source and target branches

to all changes:
to status/vote changes: