Augie Fackler <augie@google.com> [Fri, 13 Apr 2018 21:07:18 -0400] rev 37737
httppeer: no matter what Python 3 might think, http headers are bytes
Differential Revision: https://phab.mercurial-scm.org/D3346
Augie Fackler <augie@google.com> [Fri, 13 Apr 2018 21:06:50 -0400] rev 37736
httppeer: fix debug prints to work on Python 3
Differential Revision: https://phab.mercurial-scm.org/D3345
Augie Fackler <augie@google.com> [Fri, 13 Apr 2018 21:04:25 -0400] rev 37735
url: some bytes/str cleanup where we interface with stdlib funcs
Differential Revision: https://phab.mercurial-scm.org/D3344
Augie Fackler <augie@google.com> [Fri, 13 Apr 2018 21:01:17 -0400] rev 37734
hgweb: these strings should be sysstrs, not bytes
Differential Revision: https://phab.mercurial-scm.org/D3343
Augie Fackler <augie@google.com> [Fri, 13 Apr 2018 21:22:05 -0400] rev 37733
tests: port inline extensions in test-http.t to Python 3
Differential Revision: https://phab.mercurial-scm.org/D3342
Jun Wu <quark@fb.com> [Mon, 09 Apr 2018 15:58:30 -0700] rev 37732
patch: implement a new worddiff algorithm
The previous worddiff algorithm has many problems. The major problem is it
does a "similarity check" that selects a subset of matched lines to do
inline diffs. It is a bad idea because:
- The "similarity check" is non-obvious to users. For example, a simple
change from "long long x" to "int64_t x" will fail the similarity check
and won't be diff-ed as expected.
- Selecting "lines" to diff won't work as people expect if there are line
wrapping changes.
- It has a sad time complexity if lines do not match, could be O(N^2)-ish.
There are other problems in implementation details.
- Lines can match across distant hunks (if the next hunk does not have
"-" lines).
- "difflib" is slow.
The solution would be removing the "similarity check", and just diff all
words in a same hunk. So no content will be missed and everything will be
diff-ed as expected. This is similar to what code review tool like
Phabricator does.
This diff implements the word diff algorithm as described above. It also
avoids difflib to be faster.
Note about colors: To be consistent, "changed inserted" parts and "purely
insertion blocks" should have a same color, since they do not exist in the
previous version. Instead of highlighting differences, this patch chooses to
dim common parts. This is also more consistent with Phabricator or GitHub
webpage. That said, the labels are defined in a way that people can still
highlight changed parts and leave purely inserted/deleted hunks use the
"non-highlighted" color.
As one example, running:
hg log -pr
df50b87d8f736aff8dc281f816bddcd6f306930c mercurial/commands.py \
--config experimental.worddiff=1 --color=debug --config diff.unified=0
The previous algorithm outputs:
[diff.file_a|--- a/mercurial/commands.py Fri Mar 09 15:53:41 2018 +0100]
[diff.file_b|+++ b/mercurial/commands.py Sat Mar 10 12:33:19 2018 +0530]
[diff.hunk|@@ -2039,1 +2039,4 @@]
[diff.deleted|-][diff.deleted.highlight|@command('^forget',][diff.deleted| ][diff.deleted.highlight|walkopts,][diff.deleted| _('[OPTION]... FILE...'), inferrepo=True)]
[diff.inserted|+@command(]
[diff.inserted|+ '^forget',]
[diff.inserted|+ walkopts + dryrunopts,]
[diff.inserted|+ ][diff.inserted.highlight| ][diff.inserted| _('[OPTION]... FILE...'), inferrepo=True)]
[diff.hunk|@@ -2074,1 +2077,3 @@]
[diff.deleted|- rejected = cmdutil.forget(ui, repo, m, prefix="",][diff.deleted.highlight| explicitonly=False)[0]]
[diff.inserted|+ dryrun = opts.get(r'dry_run')]
[diff.inserted|+ rejected = cmdutil.forget(ui, repo, m, prefix="",]
[diff.inserted|+ explicitonly=False, dryrun=dryrun)[0]]
The new algorithm outputs:
[diff.file_a|--- a/mercurial/commands.py Fri Mar 09 15:53:41 2018 +0100]
[diff.file_b|+++ b/mercurial/commands.py Sat Mar 10 12:33:19 2018 +0530]
[diff.hunk|@@ -2039,1 +2039,4 @@]
[diff.deleted|-][diff.deleted.unchanged|@command(][diff.deleted.unchanged|'^forget',][diff.deleted.unchanged| ][diff.deleted.changed|walkopts][diff.deleted.unchanged|,][diff.deleted.changed| ][diff.deleted.unchanged|_('[OPTION]... FILE...'), inferrepo=True)]
[diff.inserted|+][diff.inserted.unchanged|@command(]
[diff.inserted|+][diff.inserted.changed| ][diff.inserted.unchanged|'^forget',]
[diff.inserted|+][diff.inserted.changed| walkopts][diff.inserted.unchanged| ][diff.inserted.changed|+ dryrunopts][diff.inserted.unchanged|,]
[diff.inserted|+][diff.inserted.changed| ][diff.inserted.unchanged|_('[OPTION]... FILE...'), inferrepo=True)]
[diff.hunk|@@ -2074,1 +2077,3 @@]
[diff.deleted|-][diff.deleted.unchanged| rejected = cmdutil.forget(ui, repo, m, prefix="",][diff.deleted.changed| ][diff.deleted.unchanged|explicitonly=False][diff.deleted.unchanged|)[0]]
[diff.inserted|+][diff.inserted.changed| dryrun = opts.get(r'dry_run')]
[diff.inserted|+][diff.inserted.unchanged| rejected = cmdutil.forget(ui, repo, m, prefix="",]
[diff.inserted|+][diff.inserted.changed| ][diff.inserted.unchanged|explicitonly=False][diff.inserted.changed|, dryrun=dryrun][diff.inserted.unchanged|)[0]]
Practically, when diffing a 8k line change, the time spent on worddiff
reduces from 4 seconds to 0.14 seconds.
Differential Revision: https://phab.mercurial-scm.org/D3212
Jun Wu <quark@fb.com> [Mon, 19 Mar 2018 04:28:30 -0700] rev 37731
patch: buffer lines for a same hunk
Instead of yielding tokens directly, buffer them if they belong to a same
hunk. This makes it easier for the upcoming new worddiff algorithm to only
focus on the diff hunk, instead of having to worry about other contents.
This breaks how the existing experimental worddiff algorithm works, so the
algorithm was removed, and related tests are disabled for now. The next patch
will add a new worddiff algorithm.
Differential Revision: https://phab.mercurial-scm.org/D3211
Jun Wu <quark@fb.com> [Mon, 19 Mar 2018 04:28:29 -0700] rev 37730
patch: move yielding "\n" to the end of loop
The original logic makes it harder to reason about - it yields the "\n"
character belonging to the last line in the next loop iteration.
The new code is in theory a little bit slower. But is more readable. It
makes the following changes easier to read.
Differential Revision: https://phab.mercurial-scm.org/D3210
Martin von Zweigbergk <martinvonz@google.com> [Mon, 16 Apr 2018 09:39:40 -0700] rev 37729
context: clarify deprecation warning message
I had one developer report that they couldn't find the message. This
patch should make it clear where to find it.
Differential Revision: https://phab.mercurial-scm.org/D3389
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 15 Apr 2018 10:37:29 -0700] rev 37728
wireprotov2: add support for more response types
This adds types to represent error and generator responses from
server commands.
Differential Revision: https://phab.mercurial-scm.org/D3388
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 14 Apr 2018 15:38:11 -0700] rev 37727
wireprotov2: remove support for sending bytes response
We recently declared that all responses must be CBOR. So remove
support for sending a type that isn't CBOR data.
Differential Revision: https://phab.mercurial-scm.org/D3387
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 14 Apr 2018 15:36:12 -0700] rev 37726
wireprotov2: change behavior of error frame
Now that we have a leading CBOR map in command response frames
to indicate overall command result status, we don't need to use
the error response frame to represent command errors. Instead,
we can reserve it for protocol and server level errors. And for the
special case of a command error that occurred after command response
frames were emitted.
The code for error handling still needs a ton of work. But we're
slowly going in the right direction...
Differential Revision: https://phab.mercurial-scm.org/D3386
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 14 Apr 2018 15:19:36 -0700] rev 37725
wireprotov2: change command response protocol to include a leading map
The error handling mechanism for the new wire protocol isn't very
well-defined. This commit takes us a step in the right direction
by introducing a leading CBOR map for command responses. This map
will contain an overall result of the command.
Currently, the map indicates whether the command was overall
successful or if an error occurred. And if an error occurred, that
error is present in the map.
There is still a dedicated error frame. My intent is to use that
for protocol-level errors and for errors that are encountered after
the initial response frame has been sent. This will be clarified in a
later commit.
Differential Revision: https://phab.mercurial-scm.org/D3385
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 14 Apr 2018 14:37:23 -0700] rev 37724
wireprotov2: change frame type and name for command response
There was hole at frame type value 3. And the frame is better
named as a command response.
Differential Revision: https://phab.mercurial-scm.org/D3384
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 14 Apr 2018 12:11:24 -0700] rev 37723
wireprotov2: change frame type value for command data
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.
Differential Revision: https://phab.mercurial-scm.org/D3383
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 14 Apr 2018 12:07:31 -0700] rev 37722
wireprotov2: define response data as CBOR
Previously, response data was defined as a stream of bytes. We had
the option to declare it as CBOR using a frame flag.
We've converged all wire protocol commands exposed on version 2 to
CBOR. I think consistency is important. The overhead to encoding
things with CBOR is minimal. Even a very large bytestring can be
efficiently encoded using an indefinite length bytestring. Now,
there are limitations with consumers not being able to efficiently
stream large CBOR values. But these feel like solvable problems.
This commit removes the "is CBOR" frame flag from command response
frames and defines the frame as always consisting of a stream of
CBOR values.
The framing protocol media type has been bumped to reflect this
BC change.
Differential Revision: https://phab.mercurial-scm.org/D3382
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 14 Apr 2018 11:49:06 -0700] rev 37721
wireprotov2: decode responses to their expected types
Callers of established wire protocol commands expect the
response from that command to be decoded into a data structure.
It's not very useful if callers get back a stream of bytes and
don't know how they should be interpreted - especially since that
stream of bytes varies by wire protocol and even the transport
within that protocol version.
This commit establishes decoding functions for various command
responses so callers of those commands get the response type
they expect.
In theory, this should make the version 2 HTTP peer usable for
various operations. But I haven't tested to confirm.
Differential Revision: https://phab.mercurial-scm.org/D3381
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 14 Apr 2018 11:46:08 -0700] rev 37720
wireprotov2: establish a type for representing command response
It will be desirable to have a higher-level type for representing
command responses. This will allow us to do nicer things.
For now, the instance encapsulates existing logic. It is still
a bit primitive. But we're slowly making things better.
Version 1 protocols have a wrapping layer that decodes the raw
string data into a data structure and that data structure is
sent to the future. Version 2 doesn't yet have this layer and
the future is receiving the raw wire response. Hence why
debugcommands needed to be taught about the response type.
Differential Revision: https://phab.mercurial-scm.org/D3380
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 14 Apr 2018 11:50:19 -0700] rev 37719
wireprotov2: move response handling out of httppeer
And fix some bugs while we're here.
The code for processing response data from the unified framing
protocol is mostly peer agnostic. The peer-specific bits are the
configuration of the client reactor and how I/O is performed. I
initially implemented things in httppeer for expediency.
This commit establishes a module for holding the peer API level
code for the framing based protocol. Inside this module we have
a class to help coordinate higher-level activities, such as managing
response object.
The client handler bits could be rolled into clientreactor. However,
I want clientreactor to be sans I/O and I want it to only be
concerned with protocol-level details, not higher-level concepts
like how protocol events are converted into peer API concepts. I
want clientreactor to receive a frame and then tell the caller what
should probably be done about it. If we start putting things like
future resolution into clientreactor, we'll constrain how the protocol
can be used (e.g. by requiring futures).
The new code is loosely based on what was in httppeer before. I
changed things a bit around response handling. We now buffer the
entire response "body" and then handle it as one atomic unit. This
fixed a bug around decoding CBOR data that spanned multiple frames.
I also fixed an off-by-one bug where we failed to read a single byte
CBOR value at the end of the stream. That's why tests have changed.
The new state of httppeer is much cleaner. It is largely agnostic
about framing protocol implementation details. That's how it should
be: the framing protocol is designed to be largely transport
agnostic. We want peers merely putting bytes on the wire and telling
the framing protocol where to read response data from.
There's still a bit of work to be done here, especially for
representing responses. But at least we're a step closer to having a
higher-level peer interface that can be plugged into the SSH peer
someday.
I initially added this class to wireprotoframing. However, we'll
eventually need version 2 specific functions to convert CBOR responses
into data structures expected by the code calling commands. This
needs to live somewhere. Since that code would be shared across peers,
we need a common module. We have wireprotov1peer for the equivalent
version 1 code. So I decided to establish wireprotov2peer.
Differential Revision: https://phab.mercurial-scm.org/D3379
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 14 Apr 2018 11:49:57 -0700] rev 37718
debugcommands: ability to suppress logging of handshake
The tests for calling wire protocol commands were getting quite
verbose because they included the results of the capabilities
request. Furthermore, it was annoying to have to update several
tests every time the capabilities response changed.
The only tests that really care about the low-level details of
the capabilities requests are those testing the protocol
handshake. And those are mostly not instantiating peer instances
or are contained to limited files.
This commit adds an option to `hg debugwireproto` to suppress logging
of the handshake. The shell helper function to perform HTTP tests
has been updated to use this by default. Lots of excessive test
output has gone away.
Differential Revision: https://phab.mercurial-scm.org/D3378
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 14 Apr 2018 09:57:44 -0700] rev 37717
hg: pass command intents to repo/peer creation (API)
The previous commit introduced a mechanism to declare command intents.
This commit changes the repository and peer instantiation mechanism
so the intents are passed down to each repository and peer type so
they can do with them whatever they please.
Currently, nobody does anything with any intent.
Differential Revision: https://phab.mercurial-scm.org/D3377
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 14 Apr 2018 09:23:48 -0700] rev 37716
registrar: replace "cmdtype" with an intent-based mechanism (API)
Commands perform varied actions and repositories vary in their
capabilities.
Historically, the .hg/requires file has been used to lock out clients
lacking a requirement. But this is a very heavy-handed approach and
is typically reserved for cases where the on-disk storage format
changes and we want to prevent incompatible clients from operating on
a repo.
Outside of the .hg/requires file, we tend to deal with things like
optional, extension-provided features via checking at call sites.
We'll either have checks in core or extensions will monkeypatch
functions in core disabling incompatible features, enabling new
features, etc.
Things are somewhat tolerable today. But once we introduce alternate
storage backends with varying support for repository features and
vastly different modes of behavior, the current model will quickly
grow unwieldy. For example, the implementation of the "simple store"
required a lot of hacks to deal with stripping and verify because
various parts of core assume things are implemented a certain way.
Partial clone will require new ways of modeling file data retrieval,
because we can no longer assume that all file data is already local.
In this new world, some commands might not make any sense for certain
types of repositories.
What we need is a mechanism to affect the construction of repository
(and eventually peer) instances so the requirements/capabilities
needed for the current operation can be taken into account. "Current
operation" can almost certainly be defined by a command. So it makes
sense for commands to declare their intended actions.
This commit introduces the "intents" concept on the command registrar.
"intents" captures a set of strings that declare actions that are
anticipated to be taken, requirements the repository must possess, etc.
These intents will be passed into hg.repo(), which will pass them into
localrepository, where they can be used to influence the object being
created. Some use cases for this include:
* For read-only intents, constructing a repository object that doesn't
expose methods that can mutate the repository. Its VFS instances
don't even allow opening a file with write access.
* For read-only intents, constructing a repository object without
cache invalidation logic. If the repo never changes during its lifetime,
nothing ever needs to be invalidated and we don't need to do expensive
things like verify the changelog's hidden revisions state is accurate
every time we access repo.changelog.
* We can automatically hide commands from `hg help` when the current
repository doesn't provide that command. For example, an alternate
storage backend may not support `hg commit`, so we can hide that
command or anything else that would perform local commits.
We already kind of had an "intents" mechanism on the registrar in the
form of "cmdtype." However, it was never used. And it was limited to
a single value. We really need something that supports multiple
intents. And because intents may be defined by extensions and at this
point are advisory, I think it is best to define them in a set rather
than as separate arguments/attributes on the command.
Differential Revision: https://phab.mercurial-scm.org/D3376
Augie Fackler <augie@google.com> [Sat, 14 Apr 2018 11:20:38 -0400] rev 37715
cleanup: polyfill assertRaisesRegex so we can avoid assertRaisesRegexp
The latter is deprecated on Python 3.7 and causes our tests to fail
due to the warning.
Differential Revision: https://phab.mercurial-scm.org/D3375
Augie Fackler <augie@google.com> [Sat, 14 Apr 2018 11:07:24 -0400] rev 37714
tests: add b prefixes to test-hg-parseurl.py
Now passes on Python 3.
Differential Revision: https://phab.mercurial-scm.org/D3374
Augie Fackler <augie@google.com> [Sat, 14 Apr 2018 11:04:58 -0400] rev 37713
tests: port test-hg-parseurl.py to unittest
Differential Revision: https://phab.mercurial-scm.org/D3373
Augie Fackler <augie@google.com> [Sat, 14 Apr 2018 01:12:55 -0400] rev 37712
hgwebdir: un-bytes the env dict before re-parsing env
Not the most elegant, but it restores
test-subrepo-deep-nested-change.t to passing on Python 3.
Differential Revision: https://phab.mercurial-scm.org/D3367
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 14 Apr 2018 16:36:15 -0700] rev 37711
cborutil: implement support for streaming encoding, bytestring decoding
The vendored cbor2 package is... a bit disappointing.
On the encoding side, it insists that you pass it something with
a write() to send data to. That means if you want to emit data to
a generator, you have to construct an e.g. io.BytesIO(), write()
to it, then get the data back out. There can be non-trivial overhead
involved.
The encoder also doesn't support indefinite types - bytestrings, arrays,
and maps that don't have a known length. Again, this is really
unfortunate because it requires you to buffer the entire source and
destination in memory to encode large things.
On the decoding side, it supports reading indefinite length types.
But it buffers them completely before returning. More sadness.
This commit implements "streaming" encoders for various CBOR types.
Encoding emits a generator of hunks. So you can efficiently stream
encoded data elsewhere.
It also implements support for emitting indefinite length bytestrings,
arrays, and maps.
On the decoding side, we only implement support for decoding an
indefinite length bytestring from a file object. It will emit a
generator of raw chunks from the source.
I didn't want to reinvent so many wheels. But profiling the wire
protocol revealed that the overhead of constructing io.BytesIO()
instances to temporarily hold results has a non-trivial overhead.
We're talking >15% of execution time for operations like
"transfer the fulltexts of all files in a revision." So I can
justify this effort.
Fortunately, CBOR is a relatively straightforward format. And we have
a reference implementation in the repo we can test against.
Differential Revision: https://phab.mercurial-scm.org/D3303
Matt Harbison <matt_harbison@yahoo.com> [Sun, 15 Apr 2018 22:28:03 -0400] rev 37710
configitems: register server.zstdlevel
Somehow, I managed to trigger a devel-warn running `hg serve` outside the test
suite on one of the repos generated by running test-lfs-serve-access.t --keep.
(I'm not hitting it now after doing a `make local`.) The only reference to this
in all of the history is the help text added in
e75463e3179f, and the
translations. (It looks like the string is built dynamically with '%slevel',
which is probably how this was missed. I wonder if this isn't getting routed to
error.log for some reason.)
In any event, server.zliblevel is registered, and that's required to pick up the
documented default.
Martin von Zweigbergk <martinvonz@google.com> [Sat, 14 Apr 2018 00:13:08 -0700] rev 37709
scmutil: make shortesthexnodeidprefix() take a full binary nodeid
The shortest() template function depended on the behavior of
revlog._partialmatch() for these types of inputs:
* non-hex strings
* ambiguous strings
* too long strings
revlog._partialmatch() seems to return the input unchanged in these
cases, but we shouldn't depend on such a low-level function to match
the behavior we want in the user-facing template function. Instead,
let's handle these cases in the template function and always pass a
binary nodeid to _partialmatch().
Differential Revision: https://phab.mercurial-scm.org/D3371
Martin von Zweigbergk <martinvonz@google.com> [Fri, 13 Apr 2018 10:36:03 -0700] rev 37708
scmutil: make shortesthexnodeidprefix() use unfiltered repo
Both callers were doing this, and resolvehexnodeidprefix() was also
working on the unfiltered repo, so it makes more sense to have it all
in one place.
Differential Revision: https://phab.mercurial-scm.org/D3313