Bazaar

Merge lp:~jameinel/bzr/2.1.0b2-471193-st-and-chk-map into lp:bzr

  1. 2.1.0b2-471193-st-and-chk-map
  2. Merge into bzr.dev
Proposed by John A Meinel
Status: Merged
Merged at revision: not available
Proposed branch: lp:~jameinel/bzr/2.1.0b2-471193-st-and-chk-map
Merge into: lp:bzr
Diff against target: 565 lines
10 files modified
NEWS (+12/-3)
bzrlib/chk_map.py (+3/-3)
bzrlib/help_topics/en/debug-flags.txt (+1/-0)
bzrlib/plugins/launchpad/__init__.py (+12/-23)
bzrlib/plugins/launchpad/lp_registration.py (+25/-4)
bzrlib/plugins/launchpad/test_lp_directory.py (+144/-1)
bzrlib/static_tuple.py (+17/-0)
bzrlib/tests/test__static_tuple.py (+26/-0)
bzrlib/tests/test_http.py (+55/-1)
bzrlib/transport/http/_urllib2_wrappers.py (+19/-7)
To merge this branch: bzr merge lp:~jameinel/bzr/2.1.0b2-471193-st-and-chk-map
Reviewer Review Type Date Requested Status
Vincent Ladeuil Approve
Review via email: mp+14309@code.launchpad.net
To post a comment you must log in.
Revision history for this message
John A Meinel (jameinel) wrote :

This fixes a critical bug for the 2.1.0b2 series.

Basically, there are some code paths by-which regular tuples can end up where we may not want them. We *could* change all of our apis to auto-cast, but that will end up negating the performance advantage. (As then you have 2 objects a tuple *and* a StaticTuple, referring to the same things.)

However, we need a migration strategy. So for now, I'm adding a debug flag that I can set, similar to '-Dhpss'. When set, I chose to cause an exception rather than just printing a traceback. We can change our minds later.

Revision history for this message
Vincent Ladeuil (vila) wrote :

Don't forget to land in 2.1.0b2 and not bzr.dev !

The long term intent being to get rid of expect_static_tuple,
please say so in the doc string.

Also, you may want to mail bzr.dev users about setting the flag in their bazaar.conf
debug_flags defaults.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2009-11-01 05:41:57 +0000
3+++ NEWS 2009-11-02 18:26:13 +0000
4@@ -101,9 +101,10 @@
5
6 Key highlights in this release are: improved handling of
7 failures-during-cleanup for commit, fixing a long-standing bug with
8-``bzr+http`` and shared repositories, and a new StaticTuple datatype,
9-allowing us to reduce memory consumption (50%) and garbage collector
10-overhead (40% faster) for many operations.
11+``bzr+http`` and shared repositories, all ``lp:`` urls to be resolved
12+behind proxies, and a new StaticTuple datatype, allowing us to reduce
13+memory consumption (50%) and garbage collector overhead (40% faster) for
14+many operations.
15
16 Bug Fixes
17 *********
18@@ -116,6 +117,14 @@
19 they do occur. This fixes some causes of ``TooManyConcurrentRequests``
20 and similar errors. (Andrew Bennetts, #429747, #243391)
21
22+* Launchpad urls can now be resolved from behind proxies.
23+ (Gordon Tyler, Vincent Ladeuil, #198920)
24+
25+* Reduce the strictness for StaticTuple, instead add a debug flag
26+ ``-Dstatic_tuple`` which will change apis to be strict and raise errors.
27+ This way, most users won't see failures, but developers can improve
28+ internals. (John Arbash Meinel, #471193)
29+
30 * TreeTransform.adjust_path updates the limbo paths of descendants of adjusted
31 files. (Aaron Bentley)
32
33
34=== modified file 'bzrlib/chk_map.py'
35--- bzrlib/chk_map.py 2009-10-23 18:46:03 +0000
36+++ bzrlib/chk_map.py 2009-11-02 18:26:13 +0000
37@@ -50,6 +50,7 @@
38 lru_cache,
39 osutils,
40 registry,
41+ static_tuple,
42 trace,
43 )
44 from bzrlib.static_tuple import StaticTuple
45@@ -720,6 +721,7 @@
46 :param bytes: The bytes of the node.
47 :param key: The key that the serialised node has.
48 """
49+ key = static_tuple.expect_static_tuple(key)
50 return _deserialise_leaf_node(bytes, key,
51 search_key_func=search_key_func)
52
53@@ -1018,9 +1020,7 @@
54 :param key: The key that the serialised node has.
55 :return: An InternalNode instance.
56 """
57- if type(key) is not StaticTuple:
58- raise AssertionError('deserialise should be called with a'
59- ' StaticTuple not %s' % (type(key),))
60+ key = static_tuple.expect_static_tuple(key)
61 return _deserialise_internal_node(bytes, key,
62 search_key_func=search_key_func)
63
64
65=== modified file 'bzrlib/help_topics/en/debug-flags.txt'
66--- bzrlib/help_topics/en/debug-flags.txt 2009-10-07 08:19:19 +0000
67+++ bzrlib/help_topics/en/debug-flags.txt 2009-11-02 18:26:13 +0000
68@@ -30,6 +30,7 @@
69 -Drelock Emit a message every time a branch or repository object is
70 unlocked then relocked the same way.
71 -Dsftp Trace SFTP internals.
72+-Dstatic_tuple Error when a tuple is used where a StaticTuple is expected
73 -Dstream Trace fetch streams.
74 -Dstrict_locks Trace when OS locks are potentially used in a non-portable
75 manner.
76
77=== modified file 'bzrlib/plugins/launchpad/__init__.py'
78--- bzrlib/plugins/launchpad/__init__.py 2009-07-06 13:00:23 +0000
79+++ bzrlib/plugins/launchpad/__init__.py 2009-11-02 18:26:13 +0000
80@@ -262,30 +262,19 @@
81 _register_directory()
82
83
84-def test_suite():
85- """Called by bzrlib to fetch tests for this plugin"""
86- from unittest import TestSuite, TestLoader
87- from bzrlib.plugins.launchpad import (
88- test_account,
89- test_lp_directory,
90- test_lp_login,
91- test_lp_open,
92- test_lp_service,
93- test_register,
94- )
95+def load_tests(basic_tests, module, loader):
96+ testmod_names = [
97+ 'test_account',
98+ 'test_register',
99+ 'test_lp_directory',
100+ 'test_lp_login',
101+ 'test_lp_open',
102+ 'test_lp_service',
103+ ]
104+ basic_tests.addTest(loader.loadTestsFromModuleNames(
105+ ["%s.%s" % (__name__, tmn) for tmn in testmod_names]))
106+ return basic_tests
107
108- loader = TestLoader()
109- suite = TestSuite()
110- for module in [
111- test_account,
112- test_register,
113- test_lp_directory,
114- test_lp_login,
115- test_lp_open,
116- test_lp_service,
117- ]:
118- suite.addTests(loader.loadTestsFromModule(module))
119- return suite
120
121 _launchpad_help = """Integration with Launchpad.net
122
123
124=== modified file 'bzrlib/plugins/launchpad/lp_registration.py'
125--- bzrlib/plugins/launchpad/lp_registration.py 2009-07-04 16:22:16 +0000
126+++ bzrlib/plugins/launchpad/lp_registration.py 2009-11-02 18:26:13 +0000
127@@ -31,6 +31,7 @@
128 errors,
129 __version__ as _bzrlib_version,
130 )
131+from bzrlib.transport.http import _urllib2_wrappers
132
133 # for testing, do
134 '''
135@@ -53,6 +54,29 @@
136 errors.BzrError.__init__(self, url=url)
137
138
139+class XMLRPCTransport(xmlrpclib.Transport):
140+
141+ def __init__(self, scheme, use_datetime=0):
142+ xmlrpclib.Transport.__init__(self, use_datetime=use_datetime)
143+ self._scheme = scheme
144+ self._opener = _urllib2_wrappers.Opener()
145+ self.verbose = 0
146+
147+ def request(self, host, handler, request_body, verbose=0):
148+ self.verbose = verbose
149+ url = self._scheme + "://" + host + handler
150+ request = _urllib2_wrappers.Request("POST", url, request_body)
151+ # FIXME: _urllib2_wrappers will override user-agent with its own
152+ # request.add_header("User-Agent", self.user_agent)
153+ request.add_header("Content-Type", "text/xml")
154+
155+ response = self._opener.open(request)
156+ if response.code != 200:
157+ raise xmlrpclib.ProtocolError(host + handler, response.code,
158+ response.msg, response.info())
159+ return self.parse_response(response)
160+
161+
162 class LaunchpadService(object):
163 """A service to talk to Launchpad via XMLRPC.
164
165@@ -90,10 +114,7 @@
166 self._lp_instance = lp_instance
167 if transport is None:
168 uri_type = urllib.splittype(self.service_url)[0]
169- if uri_type == 'https':
170- transport = xmlrpclib.SafeTransport()
171- else:
172- transport = xmlrpclib.Transport()
173+ transport = XMLRPCTransport(uri_type)
174 transport.user_agent = 'bzr/%s (xmlrpclib/%s)' \
175 % (_bzrlib_version, xmlrpclib.__version__)
176 self.transport = transport
177
178=== modified file 'bzrlib/plugins/launchpad/test_lp_directory.py'
179--- bzrlib/plugins/launchpad/test_lp_directory.py 2009-03-23 14:59:43 +0000
180+++ bzrlib/plugins/launchpad/test_lp_directory.py 2009-11-02 18:26:13 +0000
181@@ -16,10 +16,12 @@
182
183 """Tests for directory lookup through Launchpad.net"""
184
185+import os
186 import xmlrpclib
187
188 from bzrlib import (
189 errors,
190+ tests,
191 )
192 from bzrlib.branch import Branch
193 from bzrlib.directory_service import directories
194@@ -28,10 +30,38 @@
195 TestCaseWithMemoryTransport
196 )
197 from bzrlib.transport import get_transport
198-from bzrlib.plugins.launchpad import _register_directory
199+from bzrlib.plugins.launchpad import (
200+ _register_directory,
201+ lp_registration,
202+ )
203 from bzrlib.plugins.launchpad.lp_directory import (
204 LaunchpadDirectory)
205 from bzrlib.plugins.launchpad.account import get_lp_login
206+from bzrlib.tests import (
207+ http_server,
208+ http_utils,
209+ )
210+
211+
212+def load_tests(standard_tests, module, loader):
213+ result = loader.suiteClass()
214+ t_tests, remaining_tests = tests.split_suite_by_condition(
215+ standard_tests, tests.condition_isinstance((
216+ TestXMLRPCTransport,
217+ )))
218+ transport_scenarios = [
219+ ('http', dict(server_class=PreCannedHTTPServer,)),
220+ ]
221+ if tests.HTTPSServerFeature.available():
222+ transport_scenarios.append(
223+ ('https', dict(server_class=PreCannedHTTPSServer,)),
224+ )
225+ tests.multiply_tests(t_tests, transport_scenarios, result)
226+
227+ # No parametrization for the remaining tests
228+ result.addTests(remaining_tests)
229+
230+ return result
231
232
233 class FakeResolveFactory(object):
234@@ -190,3 +220,116 @@
235 transport = get_transport('lp:///apt')
236 branch = Branch.open_from_transport(transport)
237 self.assertEqual(target_branch.base, branch.base)
238+
239+
240+class PredefinedRequestHandler(http_server.TestingHTTPRequestHandler):
241+ """Request handler for a unique and pre-defined request.
242+
243+ The only thing we care about here is that we receive a connection. But
244+ since we want to dialog with a real http client, we have to send it correct
245+ responses.
246+
247+ We expect to receive a *single* request nothing more (and we won't even
248+ check what request it is), the tests will recognize us from our response.
249+ """
250+
251+ def handle_one_request(self):
252+ tcs = self.server.test_case_server
253+ requestline = self.rfile.readline()
254+ headers = self.MessageClass(self.rfile, 0)
255+ if requestline.startswith('POST'):
256+ # The body should be a single line (or we don't know where it ends
257+ # and we don't want to issue a blocking read)
258+ body = self.rfile.readline()
259+
260+ self.wfile.write(tcs.canned_response)
261+
262+
263+class PreCannedServerMixin(object):
264+
265+ def __init__(self):
266+ super(PreCannedServerMixin, self).__init__(
267+ request_handler=PredefinedRequestHandler)
268+ # Bytes read and written by the server
269+ self.bytes_read = 0
270+ self.bytes_written = 0
271+ self.canned_response = None
272+
273+
274+class PreCannedHTTPServer(PreCannedServerMixin, http_server.HttpServer):
275+ pass
276+
277+
278+if tests.HTTPSServerFeature.available():
279+ from bzrlib.tests import https_server
280+ class PreCannedHTTPSServer(PreCannedServerMixin, https_server.HTTPSServer):
281+ pass
282+
283+
284+class TestXMLRPCTransport(tests.TestCase):
285+
286+ # set by load_tests
287+ server_class = None
288+
289+ def setUp(self):
290+ tests.TestCase.setUp(self)
291+ self.server = self.server_class()
292+ self.server.setUp()
293+ # Ensure we don't clobber env
294+ self._captureVar('BZR_LP_XMLRPC_URL', None)
295+
296+ def tearDown(self):
297+ self.server.tearDown()
298+ tests.TestCase.tearDown(self)
299+
300+ def set_canned_response(self, server, path):
301+ response_format = '''HTTP/1.1 200 OK\r
302+Date: Tue, 11 Jul 2006 04:32:56 GMT\r
303+Server: Apache/2.0.54 (Fedora)\r
304+Last-Modified: Sun, 23 Apr 2006 19:35:20 GMT\r
305+ETag: "56691-23-38e9ae00"\r
306+Accept-Ranges: bytes\r
307+Content-Length: %(length)d\r
308+Connection: close\r
309+Content-Type: text/plain; charset=UTF-8\r
310+\r
311+<?xml version='1.0'?>
312+<methodResponse>
313+<params>
314+<param>
315+<value><struct>
316+<member>
317+<name>urls</name>
318+<value><array><data>
319+<value><string>bzr+ssh://bazaar.launchpad.net/%(path)s</string></value>
320+<value><string>http://bazaar.launchpad.net/%(path)s</string></value>
321+</data></array></value>
322+</member>
323+</struct></value>
324+</param>
325+</params>
326+</methodResponse>
327+'''
328+ length = 334 + 2 * len(path)
329+ server.canned_response = response_format % dict(length=length,
330+ path=path)
331+
332+ def do_request(self, server_url):
333+ os.environ['BZR_LP_XMLRPC_URL'] = self.server.get_url()
334+ service = lp_registration.LaunchpadService()
335+ resolve = lp_registration.ResolveLaunchpadPathRequest('bzr')
336+ result = resolve.submit(service)
337+ return result
338+
339+ def test_direct_request(self):
340+ self.set_canned_response(self.server, '~bzr-pqm/bzr/bzr.dev')
341+ result = self.do_request(self.server.get_url())
342+ urls = result.get('urls', None)
343+ self.assertIsNot(None, urls)
344+ self.assertEquals(
345+ ['bzr+ssh://bazaar.launchpad.net/~bzr-pqm/bzr/bzr.dev',
346+ 'http://bazaar.launchpad.net/~bzr-pqm/bzr/bzr.dev'],
347+ urls)
348+ # FIXME: we need to test with a real proxy, I can't find a way so simulate
349+ # CONNECT without leaving one server hanging the test :-/ Since that maybe
350+ # related to the leaking tests problems, I'll punt for now -- vila 20091030
351
352=== modified file 'bzrlib/static_tuple.py'
353--- bzrlib/static_tuple.py 2009-10-12 20:02:27 +0000
354+++ bzrlib/static_tuple.py 2009-11-02 18:26:13 +0000
355@@ -16,6 +16,8 @@
356
357 """Interface thunk for a StaticTuple implementation."""
358
359+from bzrlib import debug
360+
361 try:
362 from bzrlib._static_tuple_c import StaticTuple
363 except ImportError, e:
364@@ -23,3 +25,18 @@
365 osutils.failed_to_load_extension(e)
366 from bzrlib._static_tuple_py import StaticTuple
367
368+
369+def expect_static_tuple(obj):
370+ """Check if the passed object is a StaticTuple.
371+
372+ Cast it if necessary, but if the 'static_tuple' debug flag is set, raise an
373+ error instead.
374+
375+ As apis are improved, we will probably eventually stop calling this as it
376+ adds overhead we shouldn't need.
377+ """
378+ if 'static_tuple' not in debug.debug_flags:
379+ return StaticTuple.from_sequence(obj)
380+ if type(obj) is not StaticTuple:
381+ raise TypeError('We expected a StaticTuple not a %s' % (type(obj),))
382+ return obj
383
384=== modified file 'bzrlib/tests/test__static_tuple.py'
385--- bzrlib/tests/test__static_tuple.py 2009-10-27 14:07:16 +0000
386+++ bzrlib/tests/test__static_tuple.py 2009-11-02 18:26:13 +0000
387@@ -22,6 +22,7 @@
388
389 from bzrlib import (
390 _static_tuple_py,
391+ debug,
392 errors,
393 osutils,
394 static_tuple,
395@@ -620,3 +621,28 @@
396 return
397 self.assertIs(static_tuple.StaticTuple,
398 self.module.StaticTuple)
399+
400+
401+class TestEnsureStaticTuple(tests.TestCase):
402+
403+ def test_is_static_tuple(self):
404+ st = static_tuple.StaticTuple('foo')
405+ st2 = static_tuple.expect_static_tuple(st)
406+ self.assertIs(st, st2)
407+
408+ def test_is_tuple(self):
409+ t = ('foo',)
410+ st = static_tuple.expect_static_tuple(t)
411+ self.assertIsInstance(st, static_tuple.StaticTuple)
412+ self.assertEqual(t, st)
413+
414+ def test_flagged_is_static_tuple(self):
415+ debug.debug_flags.add('static_tuple')
416+ st = static_tuple.StaticTuple('foo')
417+ st2 = static_tuple.expect_static_tuple(st)
418+ self.assertIs(st, st2)
419+
420+ def test_flagged_is_tuple(self):
421+ debug.debug_flags.add('static_tuple')
422+ t = ('foo',)
423+ self.assertRaises(TypeError, static_tuple.expect_static_tuple, t)
424
425=== modified file 'bzrlib/tests/test_http.py'
426--- bzrlib/tests/test_http.py 2009-09-17 11:54:41 +0000
427+++ bzrlib/tests/test_http.py 2009-11-02 18:26:13 +0000
428@@ -1956,7 +1956,7 @@
429 pass
430
431
432-class TestActivity(tests.TestCase):
433+class TestActivityMixin(object):
434 """Test socket activity reporting.
435
436 We use a special purpose server to control the bytes sent and received and
437@@ -2100,3 +2100,57 @@
438 code, f = t._post('abc def end-of-body\n')
439 self.assertEqual('lalala whatever as long as itsssss\n', f.read())
440 self.assertActivitiesMatch()
441+
442+
443+class TestActivity(tests.TestCase, TestActivityMixin):
444+
445+ def setUp(self):
446+ tests.TestCase.setUp(self)
447+ self.server = self._activity_server(self._protocol_version)
448+ self.server.setUp()
449+ self.activities = {}
450+ def report_activity(t, bytes, direction):
451+ count = self.activities.get(direction, 0)
452+ count += bytes
453+ self.activities[direction] = count
454+
455+ # We override at class level because constructors may propagate the
456+ # bound method and render instance overriding ineffective (an
457+ # alternative would be to define a specific ui factory instead...)
458+ self.orig_report_activity = self._transport._report_activity
459+ self._transport._report_activity = report_activity
460+
461+ def tearDown(self):
462+ self._transport._report_activity = self.orig_report_activity
463+ self.server.tearDown()
464+ tests.TestCase.tearDown(self)
465+
466+
467+class TestNoReportActivity(tests.TestCase, TestActivityMixin):
468+
469+ def setUp(self):
470+ tests.TestCase.setUp(self)
471+ # Unlike TestActivity, we are really testing ReportingFileSocket and
472+ # ReportingSocket, so we don't need all the parametrization. Since
473+ # ReportingFileSocket and ReportingSocket are wrappers, it's easier to
474+ # test them through their use by the transport than directly (that's a
475+ # bit less clean but far more simpler and effective).
476+ self.server = ActivityHTTPServer('HTTP/1.1')
477+ self._transport=_urllib.HttpTransport_urllib
478+
479+ self.server.setUp()
480+
481+ # We override at class level because constructors may propagate the
482+ # bound method and render instance overriding ineffective (an
483+ # alternative would be to define a specific ui factory instead...)
484+ self.orig_report_activity = self._transport._report_activity
485+ self._transport._report_activity = None
486+
487+ def tearDown(self):
488+ self._transport._report_activity = self.orig_report_activity
489+ self.server.tearDown()
490+ tests.TestCase.tearDown(self)
491+
492+ def assertActivitiesMatch(self):
493+ # Nothing to check here
494+ pass
495
496=== modified file 'bzrlib/transport/http/_urllib2_wrappers.py'
497--- bzrlib/transport/http/_urllib2_wrappers.py 2009-08-19 16:33:39 +0000
498+++ bzrlib/transport/http/_urllib2_wrappers.py 2009-11-02 18:26:13 +0000
499@@ -80,10 +80,13 @@
500 self.filesock = filesock
501 self._report_activity = report_activity
502
503+ def report_activity(self, size, direction):
504+ if self._report_activity:
505+ self._report_activity(size, direction)
506
507 def read(self, size=1):
508 s = self.filesock.read(size)
509- self._report_activity(len(s), 'read')
510+ self.report_activity(len(s), 'read')
511 return s
512
513 def readline(self):
514@@ -93,7 +96,7 @@
515 # don't *need* the size parameter we'll stay with readline(self)
516 # -- vila 20090209
517 s = self.filesock.readline()
518- self._report_activity(len(s), 'read')
519+ self.report_activity(len(s), 'read')
520 return s
521
522 def __getattr__(self, name):
523@@ -106,13 +109,17 @@
524 self.sock = sock
525 self._report_activity = report_activity
526
527+ def report_activity(self, size, direction):
528+ if self._report_activity:
529+ self._report_activity(size, direction)
530+
531 def sendall(self, s, *args):
532 self.sock.sendall(s, *args)
533- self._report_activity(len(s), 'write')
534+ self.report_activity(len(s), 'write')
535
536 def recv(self, *args):
537 s = self.sock.recv(*args)
538- self._report_activity(len(s), 'read')
539+ self.report_activity(len(s), 'read')
540 return s
541
542 def makefile(self, mode='r', bufsize=-1):
543@@ -219,8 +226,7 @@
544 # we want to warn. But not below a given thresold.
545 _range_warning_thresold = 1024 * 1024
546
547- def __init__(self,
548- report_activity=None):
549+ def __init__(self, report_activity=None):
550 self._response = None
551 self._report_activity = report_activity
552 self._ranges_received_whole_file = None
553@@ -360,7 +366,13 @@
554
555 def set_proxy(self, proxy, type):
556 """Set the proxy and remember the proxied host."""
557- self.proxied_host = self.get_host()
558+ # We need to set the default port ourselves way before it gets set
559+ # in the HTTP[S]Connection object at build time.
560+ if self.type == 'https':
561+ conn_class = HTTPSConnection
562+ else:
563+ conn_class = HTTPConnection
564+ self.proxied_host = '%s:%s' % (self.get_host(), conn_class.default_port)
565 urllib2.Request.set_proxy(self, proxy, type)
566
567