This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3383

wireprotov2: change frame type value for command data
ClosedPublic

Authored by indygreg on Apr 14 2018, 8:21 PM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHGe8fba6d578f0: wireprotov2: change frame type value for command data
Summary

When we dropped the dedicated command argument frame type, this left
a hole in our frame type numbering. Let's start plugging that hole.

The command data frame is now type value 2 instead of 3.

There was limited test fallout because a) we do a good job of using
the constants to refer to frame types b) not many tests are sending
command data frames.

Bumping the media type will be performed in a later commit, once all
type value adjustment has been performed.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

indygreg created this revision.Apr 14 2018, 8:21 PM
durin42 accepted this revision.Apr 16 2018, 6:52 PM
This revision is now accepted and ready to land.Apr 16 2018, 6:52 PM
Closed by commit rHGe8fba6d578f0: wireprotov2: change frame type value for command data (authored by indygreg). · Explain WhyApr 16 2018, 7:10 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 8326

mercurial/help/internals/wireprotocol.txt

expected to follow the final command request frame for this request. expected to follow the final command request frame for this request.
``0x01`` MUST be set on the initial command request frame for a ``0x01`` MUST be set on the initial command request frame for a
``Request ID``. ``Request ID``.
``0x01`` or ``0x02`` MUST be set to indicate this frame's role in ``0x01`` or ``0x02`` MUST be set to indicate this frame's role in
a series of command request frames. a series of command request frames.
If command data frames are to be sent, ``0x10`` MUST be set on ALL If command data frames are to be sent, ``0x08`` MUST be set on ALL
command request frames. command request frames.
Command Data (``0x03``) Command Data (``0x02``)
----------------------- -----------------------
This frame contains raw data for a command. This frame contains raw data for a command.
Most commands can be executed by specifying arguments. However, Most commands can be executed by specifying arguments. However,
arguments have an upper bound to their length. For commands that arguments have an upper bound to their length. For commands that
accept data that is beyond this length or whose length isn't known accept data that is beyond this length or whose length isn't known
when the command is initially sent, they will need to stream when the command is initially sent, they will need to stream

mercurial/wireprotoframing.py

STREAM_FLAGS = { STREAM_FLAGS = {
b'stream-begin': STREAM_FLAG_BEGIN_STREAM, b'stream-begin': STREAM_FLAG_BEGIN_STREAM,
b'stream-end': STREAM_FLAG_END_STREAM, b'stream-end': STREAM_FLAG_END_STREAM,
b'encoded': STREAM_FLAG_ENCODING_APPLIED, b'encoded': STREAM_FLAG_ENCODING_APPLIED,
} }
FRAME_TYPE_COMMAND_REQUEST = 0x01 FRAME_TYPE_COMMAND_REQUEST = 0x01
FRAME_TYPE_COMMAND_DATA = 0x03 FRAME_TYPE_COMMAND_DATA = 0x02
FRAME_TYPE_BYTES_RESPONSE = 0x04 FRAME_TYPE_BYTES_RESPONSE = 0x04
FRAME_TYPE_ERROR_RESPONSE = 0x05 FRAME_TYPE_ERROR_RESPONSE = 0x05
FRAME_TYPE_TEXT_OUTPUT = 0x06 FRAME_TYPE_TEXT_OUTPUT = 0x06
FRAME_TYPE_PROGRESS = 0x07 FRAME_TYPE_PROGRESS = 0x07
FRAME_TYPE_STREAM_SETTINGS = 0x08 FRAME_TYPE_STREAM_SETTINGS = 0x08
FRAME_TYPES = { FRAME_TYPES = {
b'command-request': FRAME_TYPE_COMMAND_REQUEST, b'command-request': FRAME_TYPE_COMMAND_REQUEST,

tests/test-wireproto-serverreactor.py

}) })
def testunexpectedcommanddata(self): def testunexpectedcommanddata(self):
"""Command data frame when not running a command is an error.""" """Command data frame when not running a command is an error."""
result = self._sendsingleframe(makereactor(), result = self._sendsingleframe(makereactor(),
ffs(b'1 1 stream-begin command-data 0 ignored')) ffs(b'1 1 stream-begin command-data 0 ignored'))
self.assertaction(result, b'error') self.assertaction(result, b'error')
self.assertEqual(result[1], { self.assertEqual(result[1], {
b'message': b'expected command request frame; got 3', b'message': b'expected command request frame; got 2',
}) })
def testunexpectedcommanddatareceiving(self): def testunexpectedcommanddatareceiving(self):
"""Same as above except the command is receiving.""" """Same as above except the command is receiving."""
results = list(sendframes(makereactor(), [ results = list(sendframes(makereactor(), [
ffs(b'1 1 stream-begin command-request new|more ' ffs(b'1 1 stream-begin command-request new|more '
b"cbor:{b'name': b'ignored'}"), b"cbor:{b'name': b'ignored'}"),
ffs(b'1 1 0 command-data eos ignored'), ffs(b'1 1 0 command-data eos ignored'),