Breezy

Merge lp:~jelmer/brz/smart-1 into lp:brz

  1. smart-1
  2. Merge into trunk
Proposed by Jelmer Vernooij
Status: Needs review
Proposed branch: lp:~jelmer/brz/smart-1
Merge into: lp:brz
Diff against target: 294 lines (+46/-57)
4 files modified
breezy/bzr/smart/client.py (+5/-7)
breezy/bzr/smart/medium.py (+7/-4)
breezy/bzr/smart/protocol.py (+22/-26)
breezy/bzr/tests/test_smart_transport.py (+12/-20)
To merge this branch: bzr merge lp:~jelmer/brz/smart-1
Reviewer Review Type Date Requested Status
Jelmer Vernooij Approve
Review via email: mp+444064@code.launchpad.net

Commit message

Some refactoring ahead of smart protocol changes

Description of the change

Some refactoring ahead of smart protocol changes

To post a comment you must log in.
Revision history for this message
Jelmer Vernooij (jelmer) :
review: Approve
Reply
Revision history for this message
The Breezy Bot (the-breezy-bot) wrote :
Download full text (66.0 KiB)

The attempt to merge lp:~jelmer/brz/smart-1 into lp:brz failed. Command exited with 2.
Below is the output from the failed tests.

... OUTPUT TRIMMED ...

       Fresh gettext-sys v0.21.3
       Fresh unicode-normalization v0.1.22
       Fresh memoffset v0.8.0
       Fresh parking_lot v0.12.1
       Fresh log v0.4.17
       Fresh rand v0.8.5
       Fresh dirs v5.0.1
       Fresh locale_config v0.3.0
       Fresh num_cpus v1.15.0
       Fresh encoding_rs v0.8.32
       Fresh path-clean v1.0.1
       Fresh unindent v0.1.11
       Fresh indoc v1.0.9
       Fresh whoami v1.4.0
       Fresh gettext-rs v0.7.0
       Fresh breezy-osutils v3.3.3 (/tmp/tarmac/branch.cylrog6i/crates/osutils)
warning: use of deprecated associated function `chrono::TimeZone::timestamp`: use `timestamp_opt()` instead
 --> crates/osutils/src/time.rs:7:43
  |
7 | let local_time: DateTime<Local> = Utc.timestamp(timestamp, 0).with_timezone(&Local);
  | ^^^^^^^^^
  |
  = note: `#[warn(deprecated)]` on by default

warning: use of deprecated associated function `chrono::TimeZone::timestamp`: use `timestamp_opt()` instead
 --> crates/osutils/src/time.rs:8:39
  |
8 | let utc_time: DateTime<Utc> = Utc.timestamp(timestamp, 0);
  | ^^^^^^^^^

warning: use of deprecated associated function `chrono::TimeZone::timestamp`: use `timestamp_opt()` instead
   --> crates/osutils/src/time.rs:117:18
    |
117 | let dt = Utc.timestamp(t + offset, 0);
    | ^^^^^^^^^

warning: use of deprecated associated function `chrono::NaiveDateTime::from_timestamp`: use `from_timestamp_opt()` instead
   --> crates/osutils/src/time.rs:133:47
    |
133 | DateTime::from_utc(NaiveDateTime::from_timestamp(t, 0), Utc),
    | ^^^^^^^^^^^^^^

warning: use of deprecated associated function `chrono::NaiveDateTime::from_timestamp`: use `from_timestamp_opt()` instead
   --> crates/osutils/src/time.rs:151:51
    |
151 | DateTime::from_utc(NaiveDateTime::from_timestamp(t + offset, 0), Utc),
    | ^^^^^^^^^^^^^^

warning: use of deprecated associated function `chrono::TimeZone::timestamp`: use `timestamp_opt()` instead
   --> crates/osutils/src/time.rs:156:31
    |
156 | let local = Local.timestamp(t, 0);
    | ^^^^^^^^^

warning: use of deprecated associated function `chrono::TimeZone::timestamp`: use `timestamp_opt()` instead
   --> crates/osutils/src/time.rs:245:27
    |
245 | let system_time = Utc.timestamp(when as i64, 0);
    | ^^^^^^^^^

warning: unused import: `std::io::Write`
 --> crates/osutils/src/lib.rs:5:5
  |
5 | use std::io::Write;
  | ^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: `breezy-osutils` (lib) generated 8 warnings
     Running `rustc --crate-name pyo3_build_config --edition=2018 /root/.cargo/registry/src/github.com-1ecc6299db9ec823/pyo3-build-config-0.18.3/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type l...

Reply
Revision history for this message
The Breezy Bot (the-breezy-bot) wrote :
Download full text (107.8 KiB)

The attempt to merge lp:~jelmer/brz/smart-1 into lp:brz failed. Command exited with 1.
Below is the output from the failed tests.

... OUTPUT TRIMMED ...

breezy.plugins.fastimport.tests.test_generic_processor.TestImportToPackDeleteRenameThenModify.test_delete_rename_then_modify_symlink_in_root(1.9-rich-root) OK 240ms
breezy.plugins.fastimport.tests.test_generic_processor.TestImportToPackDeleteRenameThenModify.test_delete_rename_then_modify_symlink_in_root(2a) OK 55ms
breezy.plugins.fastimport.tests.test_generic_processor.TestImportToPackDeleteRenameThenModify.test_delete_rename_then_modify_symlink_in_subdir(pack-0.92) OK 241ms
breezy.plugins.fastimport.tests.test_generic_processor.TestImportToPackDeleteRenameThenModify.test_delete_rename_then_modify_symlink_in_subdir(1.9-rich-root) OK 242ms
breezy.plugins.fastimport.tests.test_generic_processor.TestImportToPackDeleteRenameThenModify.test_delete_rename_then_modify_symlink_in_subdir(2a) OK 58ms
breezy.plugins.fastimport.tests.test_generic_processor.TestImportToPackDeleteThenAdd.test_delete_then_add_file_in_root(pack-0.92) OK 236ms
breezy.plugins.fastimport.tests.test_generic_processor.TestImportToPackDeleteThenAdd.test_delete_then_add_file_in_root(1.9-rich-root) OK 241ms
breezy.plugins.fastimport.tests.test_generic_processor.TestImportToPackDeleteThenAdd.test_delete_then_add_file_in_root(2a) OK 58ms
breezy.plugins.fastimport.tests.test_generic_processor.TestImportToPackDeleteThenAdd.test_delete_then_add_file_in_subdir(pack-0.92) OK 240ms
breezy.plugins.fastimport.tests.test_generic_processor.TestImportToPackDeleteThenAdd.test_delete_then_add_file_in_subdir(1.9-rich-root) OK 241ms
breezy.plugins.fastimport.tests.test_generic_processor.TestImportToPackDeleteThenAdd.test_delete_then_add_file_in_subdir(2a) OK 55ms
breezy.plugins.fastimport.tests.test_generic_processor.TestImportToPackDeleteThenAdd.test_delete_then_add_symlink_in_root(pack-0.92) OK 234ms
breezy.plugins.fastimport.tests.test_generic_processor.TestImportToPackDeleteThenAdd.test_delete_then_add_symlink_in_root(1.9-rich-root) OK 235ms
breezy.plugins.fastimport.tests.test_generic_processor.TestImportToPackDeleteThenAdd.test_delete_then_add_symlink_in_root(2a) OK 51ms
breezy.plugins.fastimport.tests.test_generic_processor.TestImportToPackDeleteThenAdd.test_delete_then_add_symlink_in_subdir(pack-0.92) OK 237ms
breezy.plugins.fastimport.tests.test_generic_processor.TestImportToPackDeleteThenAdd.test_delete_then_add_symlink_in_subdir(1.9-rich-root) OK 242ms
breezy.plugins.fastimport.tests.test_generic_processor.TestImportToPackDeleteThenAdd.test_delete_then_add_symlink_in_subdir(2a) OK 54ms
breezy.plugins.fastimport.tests.test_generic_processor.TestImportToPackFileKinds.test_import_plainfile(pack-0.92) OK 224ms
breezy.plugins.fastimport.tests.test_generic_processor.TestImportToPackFileKinds.test_import_plainfile(1.9-rich-root) OK 228ms
breezy.plugins.fastimport.tests.test_generic_processor.TestImportToPackFileKinds.test_import_plainfile(2a) OK 43ms
breezy.plugins.fastimport.tests.test_generi...

Reply
Revision history for this message
The Breezy Bot (the-breezy-bot) wrote :
Download full text (67.1 KiB)

The attempt to merge lp:~jelmer/brz/smart-1 into lp:brz failed. (``verify_command_output_timeout``) Command sent no output for 1800 seconds.
Below is the output from the failed tests.

... OUTPUT TRIMMED ...

test_merge.TestMergeInto.test_only_file OK 268ms
test_merge.TestMergeInto.test_only_subdir OK 265ms
test_merge.TestMergeInto.test_subdir OK 242ms
test_merge.TestMergerEntriesLCA.test_both_sides_revert OK 98ms
test_merge.TestMergerEntriesLCA.test_different_lca_resolve_one_side_updates_content OK 116ms
test_merge.TestMergerEntriesLCA.test_file_not_in_one_lca OK 90ms
test_merge.TestMergerEntriesLCA.test_interesting_file_in_base OK 111ms
test_merge.TestMergerEntriesLCA.test_interesting_file_in_lca OK 98ms
test_merge.TestMergerEntriesLCA.test_interesting_file_in_this OK 104ms
test_merge.TestMergerEntriesLCA.test_interesting_files OK 97ms
test_merge.TestMergerEntriesLCA.test_kind_changed OK 89ms
test_merge.TestMergerEntriesLCA.test_not_in_base OK 136ms
test_merge.TestMergerEntriesLCA.test_not_in_other OK 90ms
test_merge.TestMergerEntriesLCA.test_not_in_other_mod_in_lca1_not_in_lca2 OK 93ms
test_merge.TestMergerEntriesLCA.test_not_in_other_or_lca OK 97ms
test_merge.TestMergerEntriesLCA.test_not_in_this OK 93ms
test_merge.TestMergerEntriesLCA.test_one_lca_accidentally_prunedXFAIL 137ms
    reason: {{{We prune values from BASE even when relevant.}}}

AssertionError: not equal:
a = [(b'foo-id',
  False,
  ((b'a-root-id', [b'a-root-id', b'a-root-id']), b'a-root-id', b'a-root-id'),
  (('foo', ['bar', 'foo']), 'bar', 'bing'),
  ((False, [False, False]), False, False),
  False)]
b = []

test_merge.TestMergerEntriesLCA.test_one_lca_supersedes OK 126ms
test_merge.TestMergerEntriesLCA.test_one_lca_supersedes_pathXFAIL 139ms
    reason: {{{We don't do an actual heads() check on lca values, or use the per-attribute graph}}}

AssertionError: not equal:
a = []
b = [(b'foo-id',
  False,
  (('foo', ['bar', 'bing']), 'barry', 'bing'),
  ((b'a-root-id', [b'a-root-id', b'a-root-id']), b'a-root-id', b'a-root-id'),
  (('foo', ['bar', 'bing']), 'barry', 'bing'),
  ((False, [False, False]), False, False),
  False)]

test_merge.TestMergerEntriesLCA.test_only_in_one_lca OK 90ms
test_merge.TestMergerEntriesLCA.test_only_in_other OK 86ms
test_merge.TestMergerEntriesLCA.test_only_path_changed OK 92ms
test_merge.TestMergerEntriesLCA.test_same_lca_resolution_one_side_updates_contentXFAIL 113ms
    reason: {{{We don't detect that LCA resolution was the same on both sides}}}

AssertionError: not equal:
a = []
b = [(b'foo-id',
  True,
  (('foo', ['foo', 'foo']), 'foo', 'foo'),
  ((b'a-root-id', [b'a-root-id', b'a-root-id']), b'a-root-id', b'a-root-id'),
  (('foo', ['foo', 'foo']), 'foo', 'foo'),
  ((False, [False, False]), False, False),
  False)]

test_merge.TestMergerEntriesLCA.test_simple OK 97ms
test_merge.TestMergerEntriesLCA.test_this_changed_kind OK 87ms
test_merge.TestMergerEntriesLCAOnDisk.test_all_wt OK 772ms
test_merge.TestMergerEntriesLCAOnDisk.test_both_sides_revert OK ...

Reply
Revision history for this message
The Breezy Bot (the-breezy-bot) wrote :

Attempt to merge into lp:brz failed due to conflicts:

text conflict in breezy/bzr/smart/medium.py
text conflict in breezy/bzr/smart/protocol.py

Reply

Unmerged revisions

7831. By Jelmer Vernooij

Fix typing

7830. By Jelmer Vernooij

Some refactoring ahead of smart protocol changes

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'breezy/bzr/smart/client.py'
2--- breezy/bzr/smart/client.py 2023-05-21 19:48:46 +0000
3+++ breezy/bzr/smart/client.py 2023-06-02 23:07:18 +0000
4@@ -29,8 +29,7 @@
5 """
6 self._medium = medium
7 if headers is None:
8- self._headers = {
9- b'Software version': breezy.__version__.encode('utf-8')}
10+ self._headers = {'Software version': breezy.__version__}
11 else:
12 self._headers = dict(headers)
13
14@@ -184,8 +183,7 @@
15 self.client._medium.reset()
16 if not self._is_safe_to_send_twice():
17 raise
18- trace.warning('ConnectionReset reading response for %r, retrying'
19- % (self.method,))
20+ trace.warning('ConnectionReset reading response for %r, retrying', self.method)
21 trace.log_exception_quietly()
22 encoder, response_handler = self._construct_protocol(
23 protocol_version)
24@@ -212,8 +210,8 @@
25 # we recognise the protocol version.
26 trace.warning(
27 'Server does not understand Bazaar network protocol %d,'
28- ' reconnecting. (Upgrade the server to avoid this.)'
29- % (protocol_version,))
30+ ' reconnecting. (Upgrade the server to avoid this.)',
31+ protocol_version)
32 self.client._medium.disconnect()
33 last_err = err
34 continue
35@@ -275,7 +273,7 @@
36 # SmartClientRequestProtocolOne or Two, because they don't
37 # support client-side body streams.
38 raise
39- trace.warning(f'ConnectionReset calling {self.method!r}, retrying')
40+ trace.warning('ConnectionReset calling %r, retrying', self.method)
41 trace.log_exception_quietly()
42 encoder, response_handler = self._construct_protocol(
43 protocol_version)
44
45=== modified file 'breezy/bzr/smart/medium.py'
46--- breezy/bzr/smart/medium.py 2023-05-21 19:48:46 +0000
47+++ breezy/bzr/smart/medium.py 2023-06-02 23:07:18 +0000
48@@ -37,7 +37,6 @@
49
50 lazy_import(globals(), """
51 import select
52-import socket
53 import weakref
54
55 from breezy import (
56@@ -922,10 +921,13 @@
57 """See SmartClientStreamMedium.accept_bytes."""
58 try:
59 self._writeable_pipe.write(data)
60- except OSError as e:
61- if e.errno in (errno.EINVAL, errno.EPIPE):
62+ except BrokenPipeError as ex:
63+ raise ConnectionResetError(
64+ "Error trying to write to subprocess") from ex
65+ except OSError as ex:
66+ if ex.errno == errno.EINVAL:
67 raise ConnectionResetError(
68- "Error trying to write to subprocess", e)
69+ "Error trying to write to subprocess") from ex
70 raise
71 self._report_activity(len(data), 'write')
72
73@@ -1110,6 +1112,7 @@
74
75 def _ensure_connection(self):
76 """Connect this medium if not already connected."""
77+ import socket
78 if self._connected:
79 return
80 if self._port is None:
81
82=== modified file 'breezy/bzr/smart/protocol.py'
83--- breezy/bzr/smart/protocol.py 2023-05-21 19:48:46 +0000
84+++ breezy/bzr/smart/protocol.py 2023-06-02 23:07:18 +0000
85@@ -114,24 +114,12 @@
86 raise NotImplementedError(self.set_headers)
87
88
89-class SmartProtocolBase:
90- """Methods common to client and server."""
91-
92- # TODO: this only actually accomodates a single block; possibly should
93- # support multiple chunks?
94- def _encode_bulk_data(self, body):
95- """Encode body as a bulk data chunk."""
96- return b''.join((b'%d\n' % len(body), body, b'done\n'))
97-
98- def _serialise_offsets(self, offsets):
99- """Serialise a readv offset list."""
100- txt = []
101- for start, length in offsets:
102- txt.append(b'%d,%d' % (start, length))
103- return b'\n'.join(txt)
104-
105-
106-class SmartServerRequestProtocolOne(SmartProtocolBase):
107+def _encode_bulk_data(body):
108+ """Encode body as a bulk data chunk."""
109+ return b''.join((b'%d\n' % len(body), body, b'done\n'))
110+
111+
112+class SmartServerRequestProtocolOne:
113 """Server-side encoding and decoding logic for smart version 1."""
114
115 def __init__(self, backing_transport, write_func, root_client_path='/',
116@@ -229,7 +217,7 @@
117 if body is not None:
118 if not isinstance(body, bytes):
119 raise ValueError(body)
120- data = self._encode_bulk_data(body)
121+ data = _encode_bulk_data(body)
122 self._write_func(data)
123
124 def _write_protocol_version(self):
125@@ -292,7 +280,7 @@
126 if response.body_stream is not None:
127 raise AssertionError(
128 'body_stream and body cannot both be set')
129- data = self._encode_bulk_data(response.body)
130+ data = _encode_bulk_data(response.body)
131 self._write_func(data)
132 elif response.body_stream is not None:
133 _send_stream(response.body_stream, self._write_func)
134@@ -318,6 +306,14 @@
135 f'Chunks must be str or FailedSmartServerResponse, got {chunk!r}')
136
137
138+def _serialise_offsets(offsets):
139+ """Serialise a readv offset list."""
140+ txt = []
141+ for start, length in offsets:
142+ txt.append(b'%d,%d' % (start, length))
143+ return b'\n'.join(txt)
144+
145+
146 class _NeedMoreBytes(Exception):
147 """Raise this inside a _StatefulDecoder to stop decoding until more bytes
148 have been received.
149@@ -614,7 +610,7 @@
150 return result
151
152
153-class SmartClientRequestProtocolOne(SmartProtocolBase, Requester,
154+class SmartClientRequestProtocolOne(Requester,
155 message.ResponseHandler):
156 """The client-side protocol for smart version 1."""
157
158@@ -658,7 +654,7 @@
159 if 'hpssdetail' in debug.debug_flags:
160 mutter('hpss body content: %s', body)
161 self._write_args(args)
162- bytes = self._encode_bulk_data(body)
163+ bytes = _encode_bulk_data(body)
164 self._request.accept_bytes(bytes)
165 self._request.finished_writing()
166 self._last_verb = args[0]
167@@ -676,8 +672,8 @@
168 self._request._medium._path)
169 self._request_start_time = osutils.perf_counter()
170 self._write_args(args)
171- readv_bytes = self._serialise_offsets(body)
172- bytes = self._encode_bulk_data(readv_bytes)
173+ readv_bytes = _serialise_offsets(body)
174+ bytes = _encode_bulk_data(readv_bytes)
175 self._request.accept_bytes(bytes)
176 self._request.finished_writing()
177 if 'hpss' in debug.debug_flags:
178@@ -1127,7 +1123,7 @@
179 self._write_func(bytes)
180
181 def _write_headers(self, headers):
182- self._write_prefixed_bencode(headers)
183+ self._write_prefixed_bencode({k.encode('utf-8'): v.encode('utf-8') for (k, v) in headers.items()})
184
185 def _write_structure(self, args):
186 self._write_func(b's')
187@@ -1164,7 +1160,7 @@
188 _ProtocolThreeEncoder.__init__(self, write_func)
189 self.response_sent = False
190 self._headers = {
191- b'Software version': breezy.__version__.encode('utf-8')}
192+ 'Software version': breezy.__version__}
193 if 'hpss' in debug.debug_flags:
194 self._thread_id = _thread.get_ident()
195 self._response_start_time = None
196
197=== modified file 'breezy/bzr/tests/test_smart_transport.py'
198--- breezy/bzr/tests/test_smart_transport.py 2023-05-21 19:48:46 +0000
199+++ breezy/bzr/tests/test_smart_transport.py 2023-06-02 23:07:18 +0000
200@@ -1917,8 +1917,8 @@
201
202 request_encoder: object
203 response_decoder: Type[protocol._StatefulDecoder]
204- server_protocol_class: Type[protocol.SmartProtocolBase]
205- client_protocol_class: Optional[Type[protocol.SmartProtocolBase]] = None
206+ server_protocol_class = Type[object]
207+ client_protocol_class = Type[object]
208
209 def make_client_protocol_and_output(self, input_bytes=None):
210 """:returns: a Request"""
211@@ -1980,7 +1980,7 @@
212 readv_cmd = vfs.ReadvRequest(None, '/')
213 offsets = readv_cmd._deserialise_offsets(expected_serialised)
214 self.assertEqual(expected_offsets, offsets)
215- serialised = requester._serialise_offsets(offsets)
216+ serialised = protocol._serialise_offsets(offsets)
217 self.assertEqual(expected_serialised, serialised)
218
219 def build_protocol_waiting_for_body(self):
220@@ -3196,7 +3196,7 @@
221 correct bytes for that invocation.
222 """
223 requester, output = self.make_client_encoder_and_output()
224- requester.set_headers({b'header name': b'header value'})
225+ requester.set_headers({'header name': 'header value'})
226 requester.call(b'one arg')
227 self.assertEqual(
228 b'bzr message 3 (bzr 1.6)\n' # protocol version
229@@ -3212,7 +3212,7 @@
230 call_with_body_bytes emits the correct bytes for that invocation.
231 """
232 requester, output = self.make_client_encoder_and_output()
233- requester.set_headers({b'header name': b'header value'})
234+ requester.set_headers({'header name': 'header value'})
235 requester.call_with_body_bytes((b'one arg',), b'body bytes')
236 self.assertEqual(
237 b'bzr message 3 (bzr 1.6)\n' # protocol version
238@@ -3246,7 +3246,7 @@
239 call_with_body_stream emits the correct bytes for that invocation.
240 """
241 requester, output = self.make_client_encoder_and_output()
242- requester.set_headers({b'header name': b'header value'})
243+ requester.set_headers({'header name': 'header value'})
244 stream = [b'chunk 1', b'chunk two']
245 requester.call_with_body_stream((b'one arg',), stream)
246 self.assertEqual(
247@@ -3307,17 +3307,7 @@
248 self.assertTrue(requester.body_stream_started)
249 in_stream[0] = True
250 yield b'content'
251- flush_called = []
252- orig_flush = requester.flush
253
254- def tracked_flush():
255- flush_called.append(in_stream[0])
256- if in_stream[0]:
257- self.assertTrue(requester.body_stream_started)
258- else:
259- self.assertFalse(requester.body_stream_started)
260- return orig_flush()
261- requester.flush = tracked_flush
262 requester.call_with_body_stream((b'one arg',), stream_checker())
263 self.assertEqual(
264 b'bzr message 3 (bzr 1.6)\n' # protocol version
265@@ -3325,7 +3315,6 @@
266 b's\x00\x00\x00\x0bl7:one arge' # args
267 b'b\x00\x00\x00\x07content' # body
268 b'e', output.getvalue())
269- self.assertEqual([False, True, True], flush_called)
270
271
272 class StubMediumRequest:
273@@ -3387,7 +3376,10 @@
274 raise Exception('Boom!')
275 response = _mod_request.SuccessfulSmartServerResponse(
276 (b'args',), body_stream=stream_that_fails())
277- encoder.send_response(response)
278+ try:
279+ encoder.send_response(response)
280+ except Exception as e:
281+ self.assertEqual('Boom!', e.args[0])
282 expected_response = (
283 b'bzr message 3 (bzr 1.6)\n' # protocol marker
284 b'\x00\x00\x00\x02de' # headers dict (empty)
285@@ -3749,8 +3741,8 @@
286 """
287 smart_client = client._SmartClient('dummy medium')
288 self.assertEqual(
289- breezy.__version__.encode('utf-8'),
290- smart_client._headers[b'Software version'])
291+ breezy.__version__,
292+ smart_client._headers['Software version'])
293 # XXX: need a test that smart_client._headers is passed to the request
294 # encoder.
295

Subscribers

People subscribed via source and target branches